From b84a51c8a48b1bba6cf5ecea4a226f47e39ef22b Mon Sep 17 00:00:00 2001 From: Sebastian Huber Date: Tue, 10 Nov 2015 17:23:12 +0100 Subject: 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. --- cpukit/rtems/src/eventseize.c | 6 +++++ cpukit/rtems/src/eventsurrender.c | 55 +++++++++++++++++++++++---------------- cpukit/score/src/threadtimeout.c | 47 +++++++++++++++++++-------------- 3 files changed, 67 insertions(+), 41 deletions(-) diff --git a/cpukit/rtems/src/eventseize.c b/cpukit/rtems/src/eventseize.c index a9290b38e8..9c41b777f9 100644 --- a/cpukit/rtems/src/eventseize.c +++ b/cpukit/rtems/src/eventseize.c @@ -100,6 +100,12 @@ void _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( executing, intend_to_block, diff --git a/cpukit/rtems/src/eventsurrender.c b/cpukit/rtems/src/eventsurrender.c index 156586023d..6a14467b7d 100644 --- a/cpukit/rtems/src/eventsurrender.c +++ b/cpukit/rtems/src/eventsurrender.c @@ -34,6 +34,20 @@ static void _Event_Satisfy( *(rtems_event_set *) the_thread->Wait.return_argument = seized_events; } +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, rtems_event_set pending_events, @@ -59,46 +73,43 @@ void _Event_Surrender( ISR_lock_Context *lock_context ) { - 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 ); _Event_sets_Post( event_in, &event->pending_events ); 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; + + _Event_Satisfy( the_thread, event, pending_events, seized_events ); - intend_to_block = wait_class | THREAD_WAIT_STATE_INTEND_TO_BLOCK; - blocked = wait_class | THREAD_WAIT_STATE_BLOCKED; + /* 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 { unblock = false; diff --git a/cpukit/score/src/threadtimeout.c b/cpukit/score/src/threadtimeout.c index 0e04998b26..45c2292ffa 100644 --- a/cpukit/score/src/threadtimeout.c +++ b/cpukit/score/src/threadtimeout.c @@ -39,35 +39,44 @@ void _Thread_Timeout( Objects_Id id, void *arg ) void *thread_lock; 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; the_thread = arg; thread_lock = _Thread_Lock_acquire( the_thread, &lock_context ); 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; } -- cgit v1.2.3