From 91e7b0c5ae837c055fd67d77d8db26bfb648261b Mon Sep 17 00:00:00 2001 From: Sebastian Huber Date: Thu, 27 Mar 2014 09:04:47 +0100 Subject: score: PR2172: _Thread_queue_Extract() 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 040e16bc07..9d248121ac 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 9563f44a1d..0a9c9d4932 100644 --- a/cpukit/score/src/threadqextract.c +++ b/cpukit/score/src/threadqextract.c @@ -21,9 +21,10 @@ #include -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 6ddd001cce..80fc7f3964 100644 --- a/cpukit/score/src/threadqextractfifo.c +++ b/cpukit/score/src/threadqextractfifo.c @@ -25,8 +25,9 @@ #include #include -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 724652ae8a..2f2555be33 100644 --- a/cpukit/score/src/threadqextractpriority.c +++ b/cpukit/score/src/threadqextractpriority.c @@ -24,8 +24,9 @@ #include #include -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 feb09210e5..616901900d 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 4eed7b3afe..ee15b3dec7 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 ffaf3e946e..233f3df4b6 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? */ } -- cgit v1.2.3