Mirrors_Framework/rtems
1
0
Fork 0
mirror of https://gitlab.rtems.org/rtems/rtos/rtems.git synced 2026-09-30 20:24:04 +08:00
Code Issues Packages Projects Releases Wiki Activity

score: Fix and simplify thread wait locks

Browse Source 
There was a subtile race condition in _Thread_queue_Do_extract_locked().
It must first update the thread wait flags and then restore the default
thread wait state.  In the previous implementation this could lead under
rare timing conditions to an ineffective _Thread_Wait_tranquilize()
resulting to a corrupt system state.

Update #2556.
This commit is contained in:
Sebastian Huber
2016-08-03 13:57:30 +02:00
parent 3b3552bf01
commit ff2e6c647d
8 changed files with 139 additions and 269 deletions
Download Patch File Download Diff File Expand all files Collapse all files
cpukit/score/include/rtems/score/thread.h
+11
View File
@@ -326,6 +326,17 @@ typedef struct {
* case the thread is enqueued on a thread queue.
*/
Chain_Control Pending_requests;
/**
* @brief Tranquilizer gate used by _Thread_Wait_tranquilize().
*
* This gate is closed by _Thread_Wait_claim(). In case there are no
* pending requests during a _Thread_Wait_restore_default(), then this gate
* is opened immediately, otherwise it is placed on the pending request
* chain and opened by _Thread_Wait_remove_request_locked() as the last
* gate on the chain to signal overall request completion.
*/
Thread_queue_Gate Tranquilizer;
} Lock;
/**
cpukit/score/include/rtems/score/threadimpl.h
+72 -63
View File
File diff suppressed because it is too large Load Diff
cpukit/score/include/rtems/score/threadq.h
+4 -25
View File
@@ -164,27 +164,9 @@ typedef struct {
Thread_queue_Gate Gate;
/**
* @brief The thread queue lock in case the thread is blocked on a thread
* queue at thread wait lock acquire time.
*/
SMP_ticket_lock_Control *queue_lock;
/**
* @brief The thread queue after thread wait lock acquire.
*
* In case the thread queue is NULL and the thread queue lock is non-NULL
* in this context, then we have a stale thread queue. This happens in
* case the thread wait default is restored while we wait on the thread
* queue lock, e.g. during a mutex ownership transfer.
*
* @see _Thread_Wait_restore_default().
* @brief The thread queue in case the thread is blocked on a thread queue.
*/
Thread_queue_Queue *queue;
/**
* @brief The thread queue operations after thread wait lock acquire.
*/
const Thread_queue_Operations *operations;
} Wait;
#endif
} Thread_queue_Context;
@@ -349,19 +331,16 @@ struct Thread_queue_Queue {
/**
* @brief Thread queue priority change operation.
*
* @param[in] queue The actual thread queue.
* @param[in] the_thread The thread.
* @param[in] new_priority The new priority value.
* @param[in] prepend_it In case this is true, then the thread is prepended to
* its priority group in its scheduler instance, otherwise it is appended.
* @param[in] queue The actual thread queue.
*
* @see Thread_queue_Operations.
*/
typedef void ( *Thread_queue_Priority_change_operation )(
Thread_queue_Queue *queue,
Thread_Control *the_thread,
Priority_Control new_priority,
bool prepend_it,
Thread_queue_Queue *queue
Priority_Control new_priority
);
/**
cpukit/score/include/rtems/score/threadqimpl.h
+6 -106
View File
@@ -234,115 +234,19 @@ RTEMS_INLINE_ROUTINE void _Thread_queue_Context_set_MP_callout(
} while ( 0 )
#endif
/**
* @brief Gets the thread wait queue of the thread queue context.
*
* On SMP configurations, the value is stored in the thread queue context,
* otherwise in the thread itself.
*
* @param queue_context The thread queue context.
* @param the_thread The thread.
*/
#if defined(RTEMS_SMP)
#define _Thread_queue_Context_get_queue( queue_context, the_thread ) \
( queue_context )->Wait.queue
#else
#define _Thread_queue_Context_get_queue( queue_context, the_thread ) \
( the_thread )->Wait.queue
#endif
RTEMS_INLINE_ROUTINE void _Thread_queue_Gate_close(
Thread_queue_Gate *gate
)
{
_Atomic_Store_uint( &gate->go_ahead, 0, ATOMIC_ORDER_RELAXED );
}
/**
* @brief Gets the thread wait operations of the thread queue context.
*
* On SMP configurations, the value is stored in the thread queue context,
* otherwise in the thread itself.
*
* @param queue_context The thread queue context.
* @param the_thread The thread.
*/
#if defined(RTEMS_SMP)
#define _Thread_queue_Context_get_operations( queue_context, the_thread ) \
( queue_context )->Wait.operations
#else
#define _Thread_queue_Context_get_operations( queue_context, the_thread ) \
( the_thread )->Wait.operations
#endif
/**
* @brief Thread priority change by means of the thread queue context.
*
* On SMP configurations, the used data is stored in the thread queue context,
* otherwise in the thread itself.
*
* @param queue_context The thread queue context.
* @param the_thread The thread.
* @param new_priority The new thread priority.
* @param prepend_it Prepend it to its priority group or not.
*/
#if defined(RTEMS_SMP)
#define _Thread_queue_Context_priority_change( \
queue_context, \
the_thread, \
new_priority, \
prepend_it \
) \
( *( queue_context )->Wait.operations->priority_change )( \
the_thread, \
new_priority, \
prepend_it, \
( queue_context )->Wait.queue \
)
#else
#define _Thread_queue_Context_priority_change( \
queue_context, \
the_thread, \
new_priority, \
prepend_it \
) \
( *( the_thread )->Wait.operations->priority_change )( \
the_thread, \
new_priority, \
prepend_it, \
( the_thread )->Wait.queue \
)
#endif
/**
* @brief Thread queue extract by means of the thread queue context.
*
* On SMP configurations, the used data is stored in the thread queue context,
* otherwise in the thread itself.
*
* @param queue_context The thread queue context.
* @param the_thread The thread.
*/
#if defined(RTEMS_SMP)
#define _Thread_queue_Context_extract( \
queue_context, \
the_thread \
) \
( *( queue_context )->Wait.operations->extract )( \
( queue_context )->Wait.queue, \
the_thread \
)
#else
#define _Thread_queue_Context_extract( \
queue_context, \
the_thread \
) \
( *( the_thread )->Wait.operations->extract )( \
( the_thread )->Wait.queue, \
the_thread \
)
#endif
#if defined(RTEMS_SMP)
RTEMS_INLINE_ROUTINE void _Thread_queue_Gate_add(
Chain_Control *chain,
Thread_queue_Gate *gate
)
{
_Atomic_Store_uint( &gate->go_ahead, 0, ATOMIC_ORDER_RELAXED );
_Chain_Append_unprotected( chain, &gate->Node );
}
@@ -1087,10 +991,6 @@ extern const Thread_queue_Operations _Thread_queue_Operations_priority;
extern const Thread_queue_Operations _Thread_queue_Operations_priority_inherit;
#if defined(RTEMS_SMP)
extern const Thread_queue_Operations _Thread_queue_Operations_stale_queue;
#endif
/**@}*/
#ifdef __cplusplus
cpukit/score/src/threadchangepriority.c
+5 -4
View File
@@ -46,11 +46,12 @@ static Thread_Control *_Thread_Apply_priority_locked(
* we are not REALLY changing priority.
*/
if ( ( *filter )( the_thread, &new_priority, arg ) ) {
_Thread_queue_Context_priority_change(
queue_context,
_Scheduler_Thread_set_priority( the_thread, new_priority, prepend_it );
( *the_thread->Wait.operations->priority_change )(
the_thread->Wait.queue,
the_thread,
new_priority,
prepend_it
new_priority
);
} else {
the_thread = NULL;
cpukit/score/src/threadinitialize.c
+1 -1
View File
@@ -186,7 +186,7 @@ bool _Thread_Initialize(
&the_thread->Wait.Lock.Default,
"Thread Wait Default Lock"
);
_Chain_Initialize_empty( &the_thread->Wait.Lock.Pending_requests );
_Thread_queue_Gate_open( &the_thread->Wait.Lock.Tranquilizer );
_SMP_lock_Stats_initialize( &the_thread->Potpourri_stats, "Thread Potpourri" );
#endif
cpukit/score/src/threadqenqueue.c
+24 -20
View File
@@ -176,7 +176,7 @@ static void _Thread_queue_Path_release( Thread_queue_Path *path )
link = RTEMS_CONTAINER_OF( node, Thread_queue_Link, Path_node );
if ( link->Queue_context.Wait.queue_lock != NULL ) {
if ( link->Queue_context.Wait.queue != NULL ) {
_Thread_queue_Link_remove( link );
}
@@ -213,8 +213,6 @@ static bool _Thread_queue_Path_acquire(
*/
_Chain_Initialize_empty( &path->Links );
_Chain_Initialize_node( &path->Start.Path_node );
_Thread_queue_Context_initialize( &path->Start.Queue_context );
owner = queue->owner;
@@ -226,6 +224,8 @@ static bool _Thread_queue_Path_acquire(
return false;
}
_Chain_Initialize_node( &path->Start.Path_node );
_Thread_queue_Context_initialize( &path->Start.Queue_context );
link = &path->Start;
do {
@@ -239,34 +239,36 @@ static bool _Thread_queue_Path_acquire(
target = owner->Wait.queue;
link->Queue_context.Wait.queue = target;
link->Queue_context.Wait.operations = owner->Wait.operations;
if ( target != NULL ) {
if ( _Thread_queue_Link_add( link, queue, target ) ) {
link->Queue_context.Wait.queue_lock = &target->Lock;
_Chain_Append_unprotected(
_Thread_queue_Gate_add(
&owner->Wait.Lock.Pending_requests,
&link->Queue_context.Wait.Gate.Node
&link->Queue_context.Wait.Gate
);
_Thread_Wait_release_default_critical(
owner,
&link->Queue_context.Lock_context
);
_Thread_Wait_acquire_queue_critical(
&target->Lock,
&link->Queue_context
);
_Thread_Wait_acquire_queue_critical( target, &link->Queue_context );
if ( link->Queue_context.Wait.queue == NULL ) {
_Thread_queue_Link_remove( link );
_Thread_Wait_release_queue_critical( target, &link->Queue_context );
_Thread_Wait_acquire_default_critical(
owner,
&link->Queue_context.Lock_context
);
_Thread_Wait_remove_request_locked( owner, &link->Queue_context );
_Assert( owner->Wait.queue == NULL );
return true;
}
} else {
link->Queue_context.Wait.queue_lock = NULL;
link->Queue_context.Wait.queue = NULL;
_Thread_queue_Path_release( path );
return false;
}
} else {
link->Queue_context.Wait.queue_lock = NULL;
return true;
}
@@ -330,6 +332,7 @@ void _Thread_queue_Enqueue_critical(
if ( !_Thread_queue_Path_acquire( the_thread, queue, &path ) ) {
_Thread_Wait_restore_default( the_thread );
_Thread_queue_Queue_release( queue, &queue_context->Lock_context );
_Thread_Wait_tranquilize( the_thread );
( *queue_context->deadlock_callout )( the_thread );
return;
}
@@ -504,14 +507,15 @@ void _Thread_queue_Extract_critical(
void _Thread_queue_Extract( Thread_Control *the_thread )
{
Thread_queue_Context queue_context;
Thread_queue_Context queue_context;
Thread_queue_Queue *queue;
_Thread_queue_Context_initialize( &queue_context );
_Thread_Wait_acquire( the_thread, &queue_context );
if (
_Thread_queue_Context_get_queue( &queue_context, the_thread ) != NULL
) {
queue = the_thread->Wait.queue;
if ( queue != NULL ) {
bool unblock;
_Thread_Wait_remove_request( the_thread, &queue_context );
@@ -520,14 +524,14 @@ void _Thread_queue_Extract( Thread_Control *the_thread )
_Thread_queue_MP_callout_do_nothing
);
unblock = _Thread_queue_Extract_locked(
_Thread_queue_Context_get_queue( &queue_context, the_thread ),
_Thread_queue_Context_get_operations( &queue_context, the_thread ),
queue,
the_thread->Wait.operations,
the_thread,
&queue_context.Lock_context
);
_Thread_queue_Unblock_critical(
unblock,
_Thread_queue_Context_get_queue( &queue_context, the_thread ),
queue,
the_thread,
&queue_context.Lock_context
);
cpukit/score/src/threadqops.c
+16 -50
View File
@@ -22,50 +22,25 @@
#include <rtems/score/rbtreeimpl.h>
#include <rtems/score/schedulerimpl.h>
static void _Thread_queue_Default_priority_change(
static void _Thread_queue_Do_nothing_priority_change(
Thread_queue_Queue *queue,
Thread_Control *the_thread,
Priority_Control new_priority,
bool prepend_it,
Thread_queue_Queue *queue
Priority_Control new_priority
)
{
(void) queue;
_Scheduler_Thread_set_priority( the_thread, new_priority, prepend_it );
(void) the_thread;
(void) new_priority;
}
static void _Thread_queue_Do_nothing_extract(
Thread_queue_Queue *queue,
Thread_Control *the_thread
Thread_Control *the_thread
)
{
/* Do nothing */
}
#if defined(RTEMS_SMP)
static void _Thread_queue_Stale_queue_priority_change(
Thread_Control *the_thread,
Priority_Control new_priority,
bool prepend_it,
Thread_queue_Queue *queue
)
{
ISR_lock_Context lock_context;
(void) queue;
/*
* This operation is used to change the priority in case we have a thread
* queue context with a stale thread queue. We own the thread queue lock of
* the former thread queue. In addition, we need the thread wait default
* lock, see _Thread_Wait_restore_default().
*/
_Thread_Wait_acquire_default_critical( the_thread, &lock_context );
_Scheduler_Thread_set_priority( the_thread, new_priority, prepend_it );
_Thread_Wait_release_default_critical( the_thread, &lock_context );
(void) the_thread;
}
#endif
static Thread_queue_Heads *_Thread_queue_Queue_enqueue(
Thread_queue_Queue *queue,
@@ -216,10 +191,9 @@ static bool _Thread_queue_Priority_less(
}
static void _Thread_queue_Priority_priority_change(
Thread_queue_Queue *queue,
Thread_Control *the_thread,
Priority_Control new_priority,
bool prepend_it,
Thread_queue_Queue *queue
Priority_Control new_priority
)
{
Thread_queue_Heads *heads = queue->heads;
@@ -227,8 +201,6 @@ static void _Thread_queue_Priority_priority_change(
_Assert( heads != NULL );
_Scheduler_Thread_set_priority( the_thread, new_priority, prepend_it );
priority_queue = _Thread_queue_Priority_queue( heads, the_thread );
_RBTree_Extract(
@@ -389,11 +361,12 @@ static void _Thread_queue_Priority_inherit_enqueue(
owner->priority_restore_hint = true;
_Atomic_Fence( ATOMIC_ORDER_ACQ_REL );
_Thread_queue_Context_priority_change(
&path->Start.Queue_context,
_Scheduler_Thread_set_priority( owner, priority, false );
( *owner->Wait.operations->priority_change )(
owner->Wait.queue,
owner,
priority,
false
priority
);
} else {
path->update_priority = NULL;
@@ -424,7 +397,7 @@ void _Thread_queue_Boost_priority(
#endif
const Thread_queue_Operations _Thread_queue_Operations_default = {
.priority_change = _Thread_queue_Default_priority_change,
.priority_change = _Thread_queue_Do_nothing_priority_change,
.extract = _Thread_queue_Do_nothing_extract
/*
* The default operations are only used in _Thread_Change_priority() and
@@ -434,7 +407,7 @@ const Thread_queue_Operations _Thread_queue_Operations_default = {
};
const Thread_queue_Operations _Thread_queue_Operations_FIFO = {
.priority_change = _Thread_queue_Default_priority_change,
.priority_change = _Thread_queue_Do_nothing_priority_change,
.enqueue = _Thread_queue_FIFO_enqueue,
.extract = _Thread_queue_FIFO_extract,
.first = _Thread_queue_FIFO_first
@@ -453,10 +426,3 @@ const Thread_queue_Operations _Thread_queue_Operations_priority_inherit = {
.extract = _Thread_queue_Priority_extract,
.first = _Thread_queue_Priority_first
};
#if defined(RTEMS_SMP)
const Thread_queue_Operations _Thread_queue_Operations_stale_queue = {
.priority_change = _Thread_queue_Stale_queue_priority_change,
.extract = _Thread_queue_Do_nothing_extract
};
#endif