Changeset 72857659 in rtems

Timestamp:

11/10/15 16:23:12 (8 years ago)

Author:

Sebastian Huber <sebastian.huber@…>

Branches:

4.11

Children:

2d7aad73

Parents:

0c9bf40b

git-author:

Sebastian Huber <sebastian.huber@…> (11/10/15 16:23:12)

git-committer:

Sebastian Huber <sebastian.huber@…> (11/17/15 06:47:48)

Message:

score: Fix race condition on SMP

We must ensure that the Thread_Control::Wait information update is
visible to the target thread before we update its wait flags, otherwise
we may return out of date events or a wrong status.

Close #2471.

Location:

cpukit

Files:

• rtems/src/eventseize.c (modified) (1 diff)
• rtems/src/eventsurrender.c (modified) (3 diffs)
• score/src/threadtimeout.c (modified) (2 diffs)

cpukit/rtems/src/eventseize.c

@@ -101 +101 @@
   _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(
     executing,

cpukit/rtems/src/eventsurrender.c

@@ -35 +35 @@
 }
 
+static bool _Event_Is_blocking_on_event(
+  const Thread_Control *the_thread,
+  Thread_Wait_flags     wait_class
+)
+{
+  Thread_Wait_flags wait_flags;
+  Thread_Wait_flags wait_mask;
+
+  wait_flags = _Thread_Wait_flags_get( the_thread );
+  wait_mask = THREAD_WAIT_CLASS_MASK | THREAD_WAIT_STATE_READY_AGAIN;
+
+  return ( wait_flags & wait_mask ) == wait_class;
+}
+
 static bool _Event_Is_satisfied(
   const Thread_Control *the_thread,
@@ -60 +74 @@
 )
 {
-  rtems_event_set   pending_events;
-  rtems_event_set   seized_events;
-  Thread_Wait_flags wait_flags;
-  bool              unblock;
+  rtems_event_set pending_events;
+  rtems_event_set seized_events;
+  bool            unblock;
 
   _Thread_Lock_acquire_default_critical( the_thread, lock_context );
@@ -70 +83 @@
   pending_events = event->pending_events;
 
-  wait_flags = _Thread_Wait_flags_get( the_thread );
-
   if (
-    ( wait_flags & THREAD_WAIT_CLASS_MASK ) == wait_class
+    _Event_Is_blocking_on_event( the_thread, wait_class )
       && _Event_Is_satisfied( the_thread, pending_events, &seized_events )
   ) {
-    Thread_Wait_flags intend_to_block;
-    Thread_Wait_flags blocked;
-    bool success;
+    Thread_Wait_flags ready_again;
+    bool              success;
 
-    intend_to_block = wait_class | THREAD_WAIT_STATE_INTEND_TO_BLOCK;
-    blocked = wait_class | THREAD_WAIT_STATE_BLOCKED;
+    _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(
       the_thread,
-      intend_to_block,
-      wait_class | THREAD_WAIT_STATE_READY_AGAIN
+      wait_class | THREAD_WAIT_STATE_INTEND_TO_BLOCK,
+      ready_again
     );
+
     if ( success ) {
-      _Event_Satisfy( the_thread, event, pending_events, seized_events );
       unblock = false;
-    } else if ( _Thread_Wait_flags_get( the_thread ) == blocked ) {
-      _Event_Satisfy( the_thread, event, pending_events, seized_events );
-      _Thread_Wait_flags_set(
-        the_thread,
-        wait_class | THREAD_WAIT_STATE_READY_AGAIN
+    } else {
+      _Assert(
+        _Thread_Wait_flags_get( the_thread )
+          == wait_class | THREAD_WAIT_STATE_BLOCKED
       );
+      _Thread_Wait_flags_set( the_thread, ready_again );
       unblock = true;
-    } else {
-      unblock = false;
     }
   } else {

cpukit/score/src/threadtimeout.c

@@ -40 +40 @@
   ISR_lock_Context   lock_context;
   Thread_Wait_flags  wait_flags;
-  Thread_Wait_flags  wait_class;
-  Thread_Wait_flags  intend_to_block;
-  Thread_Wait_flags  blocked;
-  bool               success;
   bool               unblock;
 
@@ -50 +46 @@
 
   wait_flags = _Thread_Wait_flags_get( the_thread );
-  wait_class = wait_flags & THREAD_WAIT_CLASS_MASK;
-  intend_to_block = wait_class | THREAD_WAIT_STATE_INTEND_TO_BLOCK;
-  blocked = wait_class | THREAD_WAIT_STATE_BLOCKED;
-  success = _Thread_Wait_flags_try_change_critical(
-    the_thread,
-    intend_to_block,
-    wait_class | THREAD_WAIT_STATE_READY_AGAIN
-  );
 
-  if ( success ) {
+  if ( ( wait_flags & THREAD_WAIT_STATE_READY_AGAIN ) == 0 ) {
+    Thread_Wait_flags wait_class;
+    Thread_Wait_flags ready_again;
+    bool              success;
+
     _Thread_Do_timeout( the_thread );
-    unblock = false;
-  } else if ( _Thread_Wait_flags_get( the_thread ) == blocked ) {
-    _Thread_Wait_flags_set(
+
+    /*
+     * 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(
       the_thread,
-      wait_class | THREAD_WAIT_STATE_READY_AGAIN
+      wait_class | THREAD_WAIT_STATE_INTEND_TO_BLOCK,
+      ready_again
     );
-    _Thread_Do_timeout( the_thread );
-    unblock = true;
+
+    if ( success ) {
+      unblock = false;
+    } else {
+      _Assert(
+        _Thread_Wait_flags_get( the_thread )
+          == wait_class | THREAD_WAIT_STATE_BLOCKED
+      );
+      _Thread_Wait_flags_set( the_thread, ready_again );
+      unblock = true;
+    }
   } else {
     unblock = false;