score: Avoid atomic fences for thread wait flags

The use of atomic fences is brittle and may break due to changes in
different areas which is hard to manage.

7 files changed, 37 insertions, 34 deletions

diff --git a/cpukit/rtems/src/eventseize.c b/cpukit/rtems/src/eventseize.c
index c91d308438..7f5903df89 100644
--- a/cpukit/rtems/src/eventseize.c
+++ b/cpukit/rtems/src/eventseize.c
@@ -91,13 +91,7 @@ rtems_status_code _Event_Seize(
 
   _Thread_Set_state( executing, block_state );
 
-  /*
-   * See _Event_Surrender() and _Thread_Timeout(), corresponding atomic
-   * variable is Thread_Control::Wait::flags.
-   */
-  _Atomic_Fence( ATOMIC_ORDER_ACQUIRE );
-
-  success = _Thread_Wait_flags_try_change(
+  success = _Thread_Wait_flags_try_change_acquire(
     executing,
     intend_to_block,
     wait_class | THREAD_WAIT_STATE_BLOCKED
diff --git a/cpukit/rtems/src/eventsurrender.c b/cpukit/rtems/src/eventsurrender.c
index 576670b0b7..a9bef5916c 100644
--- a/cpukit/rtems/src/eventsurrender.c
+++ b/cpukit/rtems/src/eventsurrender.c
@@ -91,11 +91,8 @@ rtems_status_code _Event_Surrender(
 
     _Event_Satisfy( the_thread, event, pending_events, seized_events );
 
-    /* See _Event_Seize() */
-    _Atomic_Fence( ATOMIC_ORDER_RELEASE );
-
     ready_again = wait_class | THREAD_WAIT_STATE_READY_AGAIN;
-    success = _Thread_Wait_flags_try_change_critical(
+    success = _Thread_Wait_flags_try_change_release(
       the_thread,
       wait_class | THREAD_WAIT_STATE_INTEND_TO_BLOCK,
       ready_again
diff --git a/cpukit/rtems/src/ratemonperiod.c b/cpukit/rtems/src/ratemonperiod.c
index 771f9c15ab..303fe174e5 100644
--- a/cpukit/rtems/src/ratemonperiod.c
+++ b/cpukit/rtems/src/ratemonperiod.c
@@ -221,7 +221,7 @@ static rtems_status_code _Rate_monotonic_Block_while_active(
 
   _Thread_Set_state( executing, STATES_WAITING_FOR_PERIOD );
 
-  success = _Thread_Wait_flags_try_change(
+  success = _Thread_Wait_flags_try_change_acquire(
     executing,
     RATE_MONOTONIC_INTEND_TO_BLOCK,
     RATE_MONOTONIC_BLOCKED
diff --git a/cpukit/rtems/src/ratemontimeout.c b/cpukit/rtems/src/ratemontimeout.c
index 78c78e2a7f..bd38153b82 100644
--- a/cpukit/rtems/src/ratemontimeout.c
+++ b/cpukit/rtems/src/ratemontimeout.c
@@ -43,7 +43,7 @@ void _Rate_monotonic_Timeout( Watchdog_Control *the_watchdog )
 
     owner->Wait.return_argument = NULL;
 
-    success = _Thread_Wait_flags_try_change_critical(
+    success = _Thread_Wait_flags_try_change_release(
       owner,
       RATE_MONOTONIC_INTEND_TO_BLOCK,
       RATE_MONOTONIC_READY_AGAIN
diff --git a/cpukit/score/include/rtems/score/threadimpl.h b/cpukit/score/include/rtems/score/threadimpl.h
index a4e746957a..b4d2f4f13e 100644
--- a/cpukit/score/include/rtems/score/threadimpl.h
+++ b/cpukit/score/include/rtems/score/threadimpl.h
@@ -1311,8 +1311,10 @@ RTEMS_INLINE_ROUTINE Thread_Wait_flags _Thread_Wait_flags_get(
 }
 
 /**
- * @brief Tries to change the thread wait flags inside a critical section
- * (interrupts disabled).
+ * @brief Tries to change the thread wait flags with release semantics in case
+ * of success.
+ *
+ * Must be called inside a critical section (interrupts disabled).
  *
  * In case the wait flags are equal to the expected wait flags, then the wait
  * flags are set to the desired wait flags.
@@ -1324,22 +1326,24 @@ RTEMS_INLINE_ROUTINE Thread_Wait_flags _Thread_Wait_flags_get(
  * @retval true The wait flags were equal to the expected wait flags.
  * @retval false Otherwise.
  */
-RTEMS_INLINE_ROUTINE bool _Thread_Wait_flags_try_change_critical(
+RTEMS_INLINE_ROUTINE bool _Thread_Wait_flags_try_change_release(
   Thread_Control    *the_thread,
   Thread_Wait_flags  expected_flags,
   Thread_Wait_flags  desired_flags
 )
 {
+  _Assert( _ISR_Get_level() != 0 );
+
 #if defined(RTEMS_SMP)
   return _Atomic_Compare_exchange_uint(
     &the_thread->Wait.flags,
     &expected_flags,
     desired_flags,
-    ATOMIC_ORDER_RELAXED,
+    ATOMIC_ORDER_RELEASE,
     ATOMIC_ORDER_RELAXED
   );
 #else
-  bool success = the_thread->Wait.flags == expected_flags;
+  bool success = ( the_thread->Wait.flags == expected_flags );
 
   if ( success ) {
     the_thread->Wait.flags = desired_flags;
@@ -1350,30 +1354,44 @@ RTEMS_INLINE_ROUTINE bool _Thread_Wait_flags_try_change_critical(
 }
 
 /**
- * @brief Tries to change the thread wait flags.
+ * @brief Tries to change the thread wait flags with acquire semantics.
+ *
+ * In case the wait flags are equal to the expected wait flags, then the wait
+ * flags are set to the desired wait flags.
+ *
+ * @param[in] the_thread The thread.
+ * @param[in] expected_flags The expected wait flags.
+ * @param[in] desired_flags The desired wait flags.
  *
- * @see _Thread_Wait_flags_try_change_critical().
+ * @retval true The wait flags were equal to the expected wait flags.
+ * @retval false Otherwise.
  */
-RTEMS_INLINE_ROUTINE bool _Thread_Wait_flags_try_change(
+RTEMS_INLINE_ROUTINE bool _Thread_Wait_flags_try_change_acquire(
   Thread_Control    *the_thread,
   Thread_Wait_flags  expected_flags,
   Thread_Wait_flags  desired_flags
 )
 {
   bool success;
-#if !defined(RTEMS_SMP)
+#if defined(RTEMS_SMP)
+  return _Atomic_Compare_exchange_uint(
+    &the_thread->Wait.flags,
+    &expected_flags,
+    desired_flags,
+    ATOMIC_ORDER_ACQUIRE,
+    ATOMIC_ORDER_ACQUIRE
+  );
+#else
   ISR_Level level;
 
   _ISR_Local_disable( level );
-#endif
 
-  success = _Thread_Wait_flags_try_change_critical(
+  success = _Thread_Wait_flags_try_change_release(
     the_thread,
     expected_flags,
     desired_flags
   );
 
-#if !defined(RTEMS_SMP)
   _ISR_Local_enable( level );
 #endif
 
diff --git a/cpukit/score/src/threadqenqueue.c b/cpukit/score/src/threadqenqueue.c
index f014fc5781..3be7d58f16 100644
--- a/cpukit/score/src/threadqenqueue.c
+++ b/cpukit/score/src/threadqenqueue.c
@@ -101,7 +101,7 @@ void _Thread_queue_Enqueue_critical(
    * as long as we are in the THREAD_QUEUE_INTEND_TO_BLOCK thread wait state,
    * thus we have to cancel the blocking operation ourself if necessary.
    */
-  success = _Thread_Wait_flags_try_change(
+  success = _Thread_Wait_flags_try_change_acquire(
     the_thread,
     THREAD_QUEUE_INTEND_TO_BLOCK,
     THREAD_QUEUE_BLOCKED
@@ -144,7 +144,7 @@ bool _Thread_queue_Do_extract_locked(
    * We must update the wait flags under protection of the current thread lock,
    * otherwise a _Thread_Timeout() running on another processor may interfere.
    */
-  success = _Thread_Wait_flags_try_change_critical(
+  success = _Thread_Wait_flags_try_change_release(
     the_thread,
     THREAD_QUEUE_INTEND_TO_BLOCK,
     THREAD_QUEUE_READY_AGAIN
diff --git a/cpukit/score/src/threadtimeout.c b/cpukit/score/src/threadtimeout.c
index 9b5cfa637d..a2ba61f9e7 100644
--- a/cpukit/score/src/threadtimeout.c
+++ b/cpukit/score/src/threadtimeout.c
@@ -54,15 +54,9 @@ void _Thread_Timeout( Watchdog_Control *watchdog )
 
     _Thread_Do_timeout( the_thread );
 
-    /*
-     * This fence is only necessary for the events, see _Event_Seize().  The
-     * thread queues use the thread lock for synchronization.
-     */
-    _Atomic_Fence( ATOMIC_ORDER_RELEASE );
-
     wait_class = wait_flags & THREAD_WAIT_CLASS_MASK;
     ready_again = wait_class | THREAD_WAIT_STATE_READY_AGAIN;
-    success = _Thread_Wait_flags_try_change_critical(
+    success = _Thread_Wait_flags_try_change_release(
       the_thread,
       wait_class | THREAD_WAIT_STATE_INTEND_TO_BLOCK,
       ready_again