From 61ef22bbeb75d0c4447a6d3a794466a9ed74162a Mon Sep 17 00:00:00 2001 From: Sebastian Huber Date: Fri, 24 Sep 2021 20:15:52 +0200 Subject: score: Add Thread_queue_Deadlock_status Replace the boolen return value with the new enum Thread_queue_Deadlock_status. This improves the code readability. Improve documentation. Shorten function names. --- cpukit/include/rtems/score/threadqimpl.h | 50 ++++++++++++++++----- cpukit/score/src/threadchangepriority.c | 4 +- cpukit/score/src/threadqenqueue.c | 76 ++++++++++++++++++-------------- cpukit/score/src/threadqops.c | 4 +- 4 files changed, 86 insertions(+), 48 deletions(-) (limited to 'cpukit') diff --git a/cpukit/include/rtems/score/threadqimpl.h b/cpukit/include/rtems/score/threadqimpl.h index 22e0c7f069..7e6f2665be 100644 --- a/cpukit/include/rtems/score/threadqimpl.h +++ b/cpukit/include/rtems/score/threadqimpl.h @@ -1339,28 +1339,56 @@ void _Thread_queue_Unblock_proxy( #endif /** - * @brief Acquires the thread queue path in a critical section. + * @brief This is a status code to indicate if a deadlock was detected or not. + */ +typedef enum { + /** + * @brief The operation did not detect a deadlock. + */ + THREAD_QUEUE_NO_DEADLOCK, + + /** + * @brief The operation detected a deadlock. + */ + THREAD_QUEUE_DEADLOCK_DETECTED +} Thread_queue_Deadlock_status; + +#if defined(RTEMS_SMP) +/** + * @brief Acquires the thread queue path. * - * @param queue The thread queue queue. - * @param the_thread The thread for the operation. - * @param queue_context The thread queue context. + * The caller must own the thread queue lock. + * + * An acquired thread queue path must be released by calling + * _Thread_queue_Path_release() with the same thread queue context. + * + * @param queue is the thread queue queue. + * + * @param the_thread is the thread for the operation. + * + * @param queue_context is the thread queue context. * - * @retval true The operation was successful. - * @retval false The operation failed. + * @retval THREAD_QUEUE_NO_DEADLOCK No deadlock was detected. + * + * @retval THREAD_QUEUE_DEADLOCK_DETECTED A deadlock was detected while + * acquiring the thread queue path. The thread queue path must still be + * released by _Thread_queue_Path_release() in this case. */ -#if defined(RTEMS_SMP) -bool _Thread_queue_Path_acquire_critical( +Thread_queue_Deadlock_status _Thread_queue_Path_acquire( Thread_queue_Queue *queue, Thread_Control *the_thread, Thread_queue_Context *queue_context ); /** - * @brief Releases the thread queue path in a critical section. + * @brief Releases the thread queue path. * - * @param queue_context The thread queue context. + * The caller must have acquired the thread queue path with a corresponding + * _Thread_queue_Path_acquire(). + * + * @param queue_context is the thread queue context. */ -void _Thread_queue_Path_release_critical( +void _Thread_queue_Path_release( Thread_queue_Context *queue_context ); #endif diff --git a/cpukit/score/src/threadchangepriority.c b/cpukit/score/src/threadchangepriority.c index 613d0cd7af..bd4fef279b 100644 --- a/cpukit/score/src/threadchangepriority.c +++ b/cpukit/score/src/threadchangepriority.c @@ -278,11 +278,11 @@ static void _Thread_Priority_apply( if ( !_Priority_Actions_is_empty( &queue_context->Priority.Actions ) ) { #if defined(RTEMS_SMP) - _Thread_queue_Path_acquire_critical( queue, the_thread, queue_context ); + (void) _Thread_queue_Path_acquire( queue, the_thread, queue_context ); #endif _Thread_Priority_perform_actions( queue->owner, queue_context ); #if defined(RTEMS_SMP) - _Thread_queue_Path_release_critical( queue_context ); + _Thread_queue_Path_release( queue_context ); #endif } } diff --git a/cpukit/score/src/threadqenqueue.c b/cpukit/score/src/threadqenqueue.c index 1b8b82eab9..b3bf5512ac 100644 --- a/cpukit/score/src/threadqenqueue.c +++ b/cpukit/score/src/threadqenqueue.c @@ -8,10 +8,9 @@ * _Thread_queue_Do_dequeue(), _Thread_queue_Enqueue(), * _Thread_queue_Enqueue_do_nothing_extra(), _Thread_queue_Enqueue_sticky(), * _Thread_queue_Extract(), _Thread_queue_Extract_locked(), - * _Thread_queue_Path_acquire_critical(), - * _Thread_queue_Path_release_critical(), + * _Thread_queue_Path_acquire(), _Thread_queue_Path_release(), * _Thread_queue_Resume(),_Thread_queue_Surrender(), - * _Thread_queue_Surrender_sticky(). + * _Thread_queue_Surrender_no_priority(), _Thread_queue_Surrender_sticky(). */ /* @@ -113,7 +112,7 @@ static Thread_queue_Link *_Thread_queue_Link_find( ); } -static bool _Thread_queue_Link_add( +static Thread_queue_Deadlock_status _Thread_queue_Link_add( Thread_queue_Link *link, Thread_queue_Queue *source, Thread_queue_Queue *target @@ -144,7 +143,7 @@ static bool _Thread_queue_Link_add( if ( recursive_target == source ) { _ISR_lock_Release( &links->Lock, &lock_context ); - return false; + return THREAD_QUEUE_DEADLOCK_DETECTED; } } @@ -156,7 +155,7 @@ static bool _Thread_queue_Link_add( ); _ISR_lock_Release( &links->Lock, &lock_context ); - return true; + return THREAD_QUEUE_NO_DEADLOCK; } static void _Thread_queue_Link_remove( Thread_queue_Link *link ) @@ -175,9 +174,7 @@ static void _Thread_queue_Link_remove( Thread_queue_Link *link ) #if !defined(RTEMS_SMP) static #endif -void _Thread_queue_Path_release_critical( - Thread_queue_Context *queue_context -) +void _Thread_queue_Path_release( Thread_queue_Context *queue_context ) { #if defined(RTEMS_SMP) Chain_Node *head; @@ -260,7 +257,7 @@ static void _Thread_queue_Path_append_deadlock_thread( #if !defined(RTEMS_SMP) static #endif -bool _Thread_queue_Path_acquire_critical( +Thread_queue_Deadlock_status _Thread_queue_Path_acquire( Thread_queue_Queue *queue, Thread_Control *the_thread, Thread_queue_Context *queue_context @@ -272,11 +269,12 @@ bool _Thread_queue_Path_acquire_critical( Thread_queue_Queue *target; /* - * For an overview please look at the non-SMP part below. We basically do - * the same on SMP configurations. The fact that we may have more than one - * executing thread and each thread queue has its own SMP lock makes the task - * a bit more difficult. We have to avoid deadlocks at SMP lock level, since - * this would result in an unrecoverable deadlock of the overall system. + * For an overview please look at the non-SMP part below. In SMP + * configurations, we basically do the same. The fact that we may have more + * than one executing thread and each thread queue has its own SMP lock makes + * the procedure a bit more difficult. We have to avoid deadlocks at SMP + * lock level, since this would result in an unrecoverable deadlock of the + * overall system. */ _Chain_Initialize_empty( &queue_context->Path.Links ); @@ -284,11 +282,11 @@ bool _Thread_queue_Path_acquire_critical( owner = queue->owner; if ( owner == NULL ) { - return true; + return THREAD_QUEUE_NO_DEADLOCK; } if ( owner == the_thread ) { - return false; + return THREAD_QUEUE_DEADLOCK_DETECTED; } _Chain_Initialize_node( @@ -311,7 +309,11 @@ bool _Thread_queue_Path_acquire_critical( link->Lock_context.Wait.queue = target; if ( target != NULL ) { - if ( _Thread_queue_Link_add( link, queue, target ) ) { + Thread_queue_Deadlock_status deadlock_status; + + deadlock_status = _Thread_queue_Link_add( link, queue, target ); + + if ( deadlock_status == THREAD_QUEUE_NO_DEADLOCK ) { _Thread_queue_Gate_add( &owner->Wait.Lock.Pending_requests, &link->Lock_context.Wait.Gate @@ -331,15 +333,15 @@ bool _Thread_queue_Path_acquire_critical( ); _Thread_Wait_remove_request_locked( owner, &link->Lock_context ); _Assert( owner->Wait.queue == NULL ); - return true; + return THREAD_QUEUE_NO_DEADLOCK; } } else { link->Lock_context.Wait.queue = NULL; _Thread_queue_Path_append_deadlock_thread( owner, queue_context ); - return false; + return THREAD_QUEUE_DEADLOCK_DETECTED; } } else { - return true; + return THREAD_QUEUE_NO_DEADLOCK; } link = &owner->Wait.Link; @@ -351,18 +353,18 @@ bool _Thread_queue_Path_acquire_critical( owner = queue->owner; if ( owner == NULL ) { - return true; + return THREAD_QUEUE_NO_DEADLOCK; } if ( owner == the_thread ) { - return false; + return THREAD_QUEUE_DEADLOCK_DETECTED; } queue = owner->Wait.queue; } while ( queue != NULL ); #endif - return true; + return THREAD_QUEUE_NO_DEADLOCK; } void _Thread_queue_Enqueue_do_nothing_extra( @@ -392,8 +394,9 @@ void _Thread_queue_Enqueue( Thread_queue_Context *queue_context ) { - Per_CPU_Control *cpu_self; - bool success; + Thread_queue_Deadlock_status deadlock_status; + Per_CPU_Control *cpu_self; + bool success; _Assert( queue_context->enqueue_callout != NULL ); @@ -405,8 +408,11 @@ void _Thread_queue_Enqueue( _Thread_Wait_claim( the_thread, queue ); - if ( !_Thread_queue_Path_acquire_critical( queue, the_thread, queue_context ) ) { - _Thread_queue_Path_release_critical( queue_context ); + deadlock_status = + _Thread_queue_Path_acquire( queue, the_thread, queue_context ); + + if ( deadlock_status == THREAD_QUEUE_DEADLOCK_DETECTED ) { + _Thread_queue_Path_release( queue_context ); _Thread_Wait_restore_default( the_thread ); _Thread_queue_Queue_release( queue, &queue_context->Lock_context.Lock_context ); _Thread_Wait_tranquilize( the_thread ); @@ -419,7 +425,7 @@ void _Thread_queue_Enqueue( _Thread_Wait_claim_finalize( the_thread, operations ); ( *operations->enqueue )( queue, the_thread, queue_context ); - _Thread_queue_Path_release_critical( queue_context ); + _Thread_queue_Path_release( queue_context ); the_thread->Wait.return_code = STATUS_SUCCESSFUL; _Thread_Wait_flags_set( the_thread, THREAD_QUEUE_INTEND_TO_BLOCK ); @@ -468,14 +474,18 @@ Status_Control _Thread_queue_Enqueue_sticky( Thread_queue_Context *queue_context ) { - Per_CPU_Control *cpu_self; + Thread_queue_Deadlock_status deadlock_status; + Per_CPU_Control *cpu_self; _Assert( queue_context->enqueue_callout != NULL ); _Thread_Wait_claim( the_thread, queue ); - if ( !_Thread_queue_Path_acquire_critical( queue, the_thread, queue_context ) ) { - _Thread_queue_Path_release_critical( queue_context ); + deadlock_status = + _Thread_queue_Path_acquire( queue, the_thread, queue_context ); + + if ( deadlock_status == THREAD_QUEUE_DEADLOCK_DETECTED ) { + _Thread_queue_Path_release( queue_context ); _Thread_Wait_restore_default( the_thread ); _Thread_queue_Queue_release( queue, &queue_context->Lock_context.Lock_context ); _Thread_Wait_tranquilize( the_thread ); @@ -487,7 +497,7 @@ Status_Control _Thread_queue_Enqueue_sticky( _Thread_Wait_claim_finalize( the_thread, operations ); ( *operations->enqueue )( queue, the_thread, queue_context ); - _Thread_queue_Path_release_critical( queue_context ); + _Thread_queue_Path_release( queue_context ); the_thread->Wait.return_code = STATUS_SUCCESSFUL; _Thread_Wait_flags_set( the_thread, THREAD_QUEUE_INTEND_TO_BLOCK ); diff --git a/cpukit/score/src/threadqops.c b/cpukit/score/src/threadqops.c index 972af21265..ce3a8f6a73 100644 --- a/cpukit/score/src/threadqops.c +++ b/cpukit/score/src/threadqops.c @@ -1186,7 +1186,7 @@ static void _Thread_queue_Priority_inherit_extract( * resolves the deadlock. Thread T1 and T2 can the complete their * operations. */ - _Thread_queue_Path_acquire_critical( queue, the_thread, queue_context ); + (void) _Thread_queue_Path_acquire( queue, the_thread, queue_context ); #endif _Thread_queue_Queue_extract( @@ -1199,7 +1199,7 @@ static void _Thread_queue_Priority_inherit_extract( ); #if defined(RTEMS_SMP) - _Thread_queue_Path_release_critical( queue_context ); + _Thread_queue_Path_release( queue_context ); #endif } -- cgit v1.2.3