Rework the condition variables support for VxWorks

This change reworks the condition variables support for VxWorks
to address the very legit points raised by Rasmus in

 https://gcc.gnu.org/pipermail/gcc/2020-May/232524.html

While some of the issues were taken care of by the use of semFlush,
a few others were indeed calling for adjustments.

We first considered resorting to the condvarLib library available
in VxWorks7. Unfortunately, it is vx7 only and we wanted something working
for at least vx 6.9 as well. It also turned out requiring the use of
recursive mutexes for condVarWait, which seemed unnecessarily constraining.

Instead, this change corrects the sequencing logic in a few places and
leverages the semExchange API to ensure the key atomicity requirement on
cond_wait operations.

2020-10-14  Alexandre Oliva  <oliva@adacore.com>

libgcc/
	* config/gthr-vxworks-thread.c: Include stdlib.h.
	(tls_delete_hook): Prototype it.
	(__gthread_cond_signal): Return early if no waiters.  Consume
	signal in case the semaphore got full.  Use semInfoGet instead
	of kernel-mode-only semInfo.
	(__gthread_cond_timedwait): Use semExchange.  Always take the
	mutex again before returning.
	* config/gthr-vxworks-cond.c (__ghtread_cond_wait): Likewise.

diff --git a/libgcc/config/gthr-vxworks-cond.c b/libgcc/config/gthr-vxworks-cond.c
index d65d07aebc3593f4e2b2136c7dfd3db19263d391..65f0a6af183aa690c77eacddda88d955aec5abf4 100644 (file)
--- a/libgcc/config/gthr-vxworks-cond.c
+++ b/libgcc/config/gthr-vxworks-cond.c
@@ -66,13 +66,11 @@ __gthread_cond_wait (__gthread_cond_t *cond,
   if (!mutex)
     return ERROR;
 
-  __RETURN_ERRNO_IF_NOT_OK (semGive (*mutex));
-
-  __RETURN_ERRNO_IF_NOT_OK (semTake (*cond, WAIT_FOREVER));
+  int ret = __CHECK_RESULT (semExchange (*mutex, *cond, WAIT_FOREVER));
 
   __RETURN_ERRNO_IF_NOT_OK (semTake (*mutex, WAIT_FOREVER));
 
-  return OK;
+  return ret;
 }
 
 int

diff --git a/libgcc/config/gthr-vxworks-thread.c b/libgcc/config/gthr-vxworks-thread.c
index 8544b03dd18c35c145659ba0f61a169bb0c99e47..8e26d855931bf3c3a11e18288d8f0d6b91438282 100644 (file)
--- a/libgcc/config/gthr-vxworks-thread.c
+++ b/libgcc/config/gthr-vxworks-thread.c
@@ -29,6 +29,7 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 
 #include "gthr.h"
 #include <taskLib.h>
+#include <stdlib.h>
 
 #define __TIMESPEC_TO_NSEC(timespec) \
   ((long long)timespec.tv_sec * 1000000000 + (long long)timespec.tv_nsec)
@@ -38,7 +39,7 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
     / 1000000000)
 
 #ifdef __RTP__
-  void tls_delete_hook ();
+  void tls_delete_hook (void);
   #define __CALL_DELETE_HOOK(tcb) tls_delete_hook()
 #else
   /* In kernel mode, we need to pass the TCB to task_delete_hook. The TCB is
@@ -47,17 +48,55 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
   #define __CALL_DELETE_HOOK(tcb) tls_delete_hook((WIND_TCB *) ((tcb)->task_id))
 #endif
 
-/* -------------------- Timed Condition Variables --------------------- */
-
 int
 __gthread_cond_signal (__gthread_cond_t *cond)
 {
   if (!cond)
     return ERROR;
 
-  return __CHECK_RESULT (semGive (*cond));
+  /* If nobody is waiting, skip the semGive altogether: no one can get
+     in line while we hold the mutex associated with *COND.  We could
+     skip this test altogether, but it's presumed cheaper than going
+     through the give and take below, and that a signal without a
+     waiter occurs often enough for the test to be worth it.  */
+  SEM_INFO info;
+  memset (&info, 0, sizeof (info));
+  __RETURN_ERRNO_IF_NOT_OK (semInfoGet (*cond, &info));
+  if (info.numTasks == 0)
+    return OK;
+
+  int ret = __CHECK_RESULT (semGive (*cond));
+
+  /* It might be the case, however, that when we called semInfo, there
+     was a waiter just about to timeout, and by the time we called
+     semGive, it had already timed out, so our semGive would leave the
+     *cond semaphore full, so the next caller of wait would pass
+     through.  We don't want that.  So, make sure we leave the
+     semaphore empty.  Despite the window in which the semaphore will
+     be full, this works because:
+
+     - we're holding the mutex, so nobody else can semGive, and any
+       pending semTakes are actually within semExchange.  there might
+       be others blocked to acquire the mutex, but those are not
+       relevant for the analysis.
+
+     - if there was another non-timed out waiter, semGive will wake it
+       up immediately instead of leaving the semaphore full, so the
+       semTake below will time out, and the semantics are as expected
+
+     - otherwise, if all waiters timed out before the semGive (or if
+       there weren't any to begin with), our semGive completed leaving
+       the semaphore full, and our semTake below will consume it
+       before any other waiter has a change to reach the semExchange,
+       because we're holding the mutex.  */
+  if (ret == OK)
+    semTake (*cond, NO_WAIT);
+
+  return ret;
 }
 
+/* -------------------- Timed Condition Variables --------------------- */
+
 int
 __gthread_cond_timedwait (__gthread_cond_t *cond,
                          __gthread_mutex_t *mutex,
@@ -93,13 +132,11 @@ __gthread_cond_timedwait (__gthread_cond_t *cond,
   if (waiting_ticks > INT_MAX)
     waiting_ticks = INT_MAX;
 
-  __RETURN_ERRNO_IF_NOT_OK (semGive (*mutex));
-
-  __RETURN_ERRNO_IF_NOT_OK (semTake (*cond, waiting_ticks));
+  int ret = __CHECK_RESULT (semExchange (*mutex, *cond, waiting_ticks));
 
   __RETURN_ERRNO_IF_NOT_OK (semTake (*mutex, WAIT_FOREVER));
 
-  return OK;
+  return ret;
 }
 
 /* --------------------------- Timed Mutexes ------------------------------ */