[PATCH] score: PR2172: _Thread_queue_Extract()

Sebastian Huber sebastian.huber at embedded-brains.de
Mon Mar 31 09:42:36 UTC 2014


Add _Thread_queue_Extract_with_return_code().  On SMP this sequence in
_Thread_queue_Process_timeout() was broken:

[...]
      /*
       * After we enable interrupts here, a lot may happen in the
       * meantime, e.g. nested interrupts may release the resource that
       * times out here.  So we enter _Thread_queue_Extract()
       * speculatively.  Inside this function we check the actual status
       * under ISR disable protection.  This ensures that exactly one
       * executing context performs the extract operation (other parties
       * may call _Thread_queue_Dequeue()).  If this context won, then
       * we have a timeout.
       *
       * We can use the_thread_queue pointer here even if
       * the_thread->Wait.queue is already set to NULL since the extract
       * operation will only use the thread queue discipline to select
       * the right extract operation.  The timeout status is set during
       * thread queue initialization.
       */
      we_did_it = _Thread_queue_Extract( the_thread_queue, the_thread );
      if ( we_did_it ) {
        the_thread->Wait.return_code = the_thread_queue->timeout_status;
      }
[...]

In case _Thread_queue_Extract() successfully extracted a thread, then
this thread may start execution on a remote processor immediately and
read the the_thread->Wait.return_code before we update it here with the
timeout status.  Thus it observes a successful operation even if it
timed out.
---
 cpukit/score/include/rtems/score/threadqimpl.h |   23 ++++++++++++++---------
 cpukit/score/src/threadqextract.c              |   21 +++++++++++++++++----
 cpukit/score/src/threadqextractfifo.c          |   10 +++++-----
 cpukit/score/src/threadqextractpriority.c      |   12 +++++++-----
 cpukit/score/src/threadqprocesstimeout.c       |   11 +++++------
 cpukit/score/src/threadqrequeue.c              |    6 +++++-
 testsuites/sptests/spthreadq01/init.c          |    4 +---
 7 files changed, 54 insertions(+), 33 deletions(-)

diff --git a/cpukit/score/include/rtems/score/threadqimpl.h b/cpukit/score/include/rtems/score/threadqimpl.h
index 040e16b..9d24812 100644
--- a/cpukit/score/include/rtems/score/threadqimpl.h
+++ b/cpukit/score/include/rtems/score/threadqimpl.h
@@ -135,15 +135,18 @@ void _Thread_queue_Requeue(
  *
  *  @param[in] the_thread_queue is the pointer to the ThreadQ header
  *  @param[in] the_thread is the pointer to a thread control block that is to be removed
- *
- *  @retval true The extract operation was performed by the executing context.
- *  @retval false Otherwise.
  */
-bool _Thread_queue_Extract(
+void _Thread_queue_Extract(
   Thread_queue_Control *the_thread_queue,
   Thread_Control       *the_thread
 );
 
+void _Thread_queue_Extract_with_return_code(
+  Thread_queue_Control *the_thread_queue,
+  Thread_Control       *the_thread,
+  uint32_t              return_code
+);
+
 /**
  *  @brief Extracts the_thread from the_thread_queue.
  *
@@ -265,8 +268,9 @@ Thread_blocking_operation_States _Thread_queue_Enqueue_priority (
  *  @retval true The extract operation was performed by the executing context.
  *  @retval false Otherwise.
  */
-bool _Thread_queue_Extract_priority_helper(
+void _Thread_queue_Extract_priority_helper(
   Thread_Control       *the_thread,
+  uint32_t              return_code,
   bool                  requeuing
 );
 
@@ -276,8 +280,8 @@ bool _Thread_queue_Extract_priority_helper(
  * This macro wraps the underlying call and hides the requeuing argument.
  */
 
-#define _Thread_queue_Extract_priority( _the_thread ) \
-  _Thread_queue_Extract_priority_helper( _the_thread, false )
+#define _Thread_queue_Extract_priority( _the_thread, _return_code ) \
+  _Thread_queue_Extract_priority_helper( _the_thread, _return_code, false )
 /**
  *  @brief Get highest priority thread on the_thread_queue.
  *
@@ -337,8 +341,9 @@ Thread_blocking_operation_States _Thread_queue_Enqueue_fifo (
  *  This routine removes the_thread from the_thread_queue
  *  and cancels any timeouts associated with this blocking.
  */
-bool _Thread_queue_Extract_fifo(
-  Thread_Control       *the_thread
+void _Thread_queue_Extract_fifo(
+  Thread_Control       *the_thread,
+  uint32_t              return_code
 );
 
 /**
diff --git a/cpukit/score/src/threadqextract.c b/cpukit/score/src/threadqextract.c
index 9563f44..0a9c9d4 100644
--- a/cpukit/score/src/threadqextract.c
+++ b/cpukit/score/src/threadqextract.c
@@ -21,9 +21,10 @@
 
 #include <rtems/score/threadqimpl.h>
 
-bool _Thread_queue_Extract(
+void _Thread_queue_Extract_with_return_code(
   Thread_queue_Control *the_thread_queue,
-  Thread_Control       *the_thread
+  Thread_Control       *the_thread,
+  uint32_t              return_code
 )
 {
   /*
@@ -31,8 +32,20 @@ bool _Thread_queue_Extract(
    * is a macro and the underlying methods do not have the same signature.
    */
   if  ( the_thread_queue->discipline == THREAD_QUEUE_DISCIPLINE_PRIORITY )
-    return _Thread_queue_Extract_priority( the_thread );
+    return _Thread_queue_Extract_priority( the_thread, return_code );
   else /* must be THREAD_QUEUE_DISCIPLINE_FIFO */
-    return _Thread_queue_Extract_fifo( the_thread );
+    return _Thread_queue_Extract_fifo( the_thread, return_code );
+
+}
 
+void _Thread_queue_Extract(
+  Thread_queue_Control *the_thread_queue,
+  Thread_Control       *the_thread
+)
+{
+  _Thread_queue_Extract_with_return_code(
+    the_thread_queue,
+    the_thread,
+    the_thread->Wait.return_code
+  );
 }
diff --git a/cpukit/score/src/threadqextractfifo.c b/cpukit/score/src/threadqextractfifo.c
index 6ddd001..80fc7f3 100644
--- a/cpukit/score/src/threadqextractfifo.c
+++ b/cpukit/score/src/threadqextractfifo.c
@@ -25,8 +25,9 @@
 #include <rtems/score/threadimpl.h>
 #include <rtems/score/watchdogimpl.h>
 
-bool _Thread_queue_Extract_fifo(
-  Thread_Control       *the_thread
+void _Thread_queue_Extract_fifo(
+  Thread_Control       *the_thread,
+  uint32_t              return_code
 )
 {
   ISR_Level level;
@@ -35,12 +36,13 @@ bool _Thread_queue_Extract_fifo(
 
   if ( !_States_Is_waiting_on_thread_queue( the_thread->current_state ) ) {
     _ISR_Enable( level );
-    return false;
+    return;
   }
 
   _Chain_Extract_unprotected( &the_thread->Object.Node );
 
   the_thread->Wait.queue = NULL;
+  the_thread->Wait.return_code = return_code;
 
   if ( !_Watchdog_Is_active( &the_thread->Timer ) ) {
     _ISR_Enable( level );
@@ -56,6 +58,4 @@ bool _Thread_queue_Extract_fifo(
   if ( !_Objects_Is_local_id( the_thread->Object.id ) )
     _Thread_MP_Free_proxy( the_thread );
 #endif
-
-  return true;
 }
diff --git a/cpukit/score/src/threadqextractpriority.c b/cpukit/score/src/threadqextractpriority.c
index 724652a..2f2555b 100644
--- a/cpukit/score/src/threadqextractpriority.c
+++ b/cpukit/score/src/threadqextractpriority.c
@@ -24,8 +24,9 @@
 #include <rtems/score/threadimpl.h>
 #include <rtems/score/watchdogimpl.h>
 
-bool _Thread_queue_Extract_priority_helper(
+void _Thread_queue_Extract_priority_helper(
   Thread_Control       *the_thread,
+  uint32_t              return_code,
   bool                  requeuing
 )
 {
@@ -44,7 +45,7 @@ bool _Thread_queue_Extract_priority_helper(
   _ISR_Disable( level );
   if ( !_States_Is_waiting_on_thread_queue( the_thread->current_state ) ) {
     _ISR_Enable( level );
-    return false;
+    return;
   }
 
   /*
@@ -86,9 +87,12 @@ bool _Thread_queue_Extract_priority_helper(
 
   if ( requeuing ) {
     _ISR_Enable( level );
-    return true;
+    return;
   }
 
+  the_thread->Wait.queue = NULL;
+  the_thread->Wait.return_code = return_code;
+
   if ( !_Watchdog_Is_active( &the_thread->Timer ) ) {
     _ISR_Enable( level );
   } else {
@@ -102,6 +106,4 @@ bool _Thread_queue_Extract_priority_helper(
   if ( !_Objects_Is_local_id( the_thread->Object.id ) )
     _Thread_MP_Free_proxy( the_thread );
 #endif
-
-  return true;
 }
diff --git a/cpukit/score/src/threadqprocesstimeout.c b/cpukit/score/src/threadqprocesstimeout.c
index feb0921..6169019 100644
--- a/cpukit/score/src/threadqprocesstimeout.c
+++ b/cpukit/score/src/threadqprocesstimeout.c
@@ -51,8 +51,6 @@ void _Thread_queue_Process_timeout(
       }
       _ISR_Enable( level );
     } else {
-      bool we_did_it;
-
       _ISR_Enable( level );
 
       /*
@@ -70,10 +68,11 @@ void _Thread_queue_Process_timeout(
        * right extract operation.  The timeout status is set during thread
        * queue initialization.
        */
-      we_did_it = _Thread_queue_Extract( the_thread_queue, the_thread );
-      if ( we_did_it ) {
-        the_thread->Wait.return_code = the_thread_queue->timeout_status;
-      }
+      _Thread_queue_Extract_with_return_code(
+        the_thread_queue,
+        the_thread,
+        the_thread_queue->timeout_status
+      );
     }
   } else {
     _ISR_Enable( level );
diff --git a/cpukit/score/src/threadqrequeue.c b/cpukit/score/src/threadqrequeue.c
index 4eed7b3..ee15b3d 100644
--- a/cpukit/score/src/threadqrequeue.c
+++ b/cpukit/score/src/threadqrequeue.c
@@ -45,7 +45,11 @@ void _Thread_queue_Requeue(
     _ISR_Disable( level );
     if ( _States_Is_waiting_on_thread_queue( the_thread->current_state ) ) {
       _Thread_queue_Enter_critical_section( tq );
-      _Thread_queue_Extract_priority_helper( the_thread, true );
+      _Thread_queue_Extract_priority_helper(
+        the_thread,
+        the_thread->Wait.return_code,
+        true
+      );
       (void) _Thread_queue_Enqueue_priority( tq, the_thread, &level_ignored );
     }
     _ISR_Enable( level );
diff --git a/testsuites/sptests/spthreadq01/init.c b/testsuites/sptests/spthreadq01/init.c
index ffaf3e9..233f3df 100644
--- a/testsuites/sptests/spthreadq01/init.c
+++ b/testsuites/sptests/spthreadq01/init.c
@@ -31,16 +31,14 @@ void threadq_first_empty(
 )
 {
   Thread_queue_Control tq;
-  bool we_did_it;
 
   printf( "Init - initialize thread queue for %s\n", discipline_string );
   _Thread_queue_Initialize( &tq, discipline, 0x01, 3 );
 
   puts( "Init - _Thread_queue_Extract - thread not blocked on a thread queue" );
   _Thread_Disable_dispatch();
-  we_did_it = _Thread_queue_Extract( &tq, _Thread_Executing );
+  _Thread_queue_Extract( &tq, _Thread_Executing );
   _Thread_Enable_dispatch();
-  rtems_test_assert( !we_did_it );
   /* is there more to check? */
 }
 
-- 
1.7.7




More information about the devel mailing list