[PATCH] score: Add Thread_queue_Deadlock_status

Sebastian Huber sebastian.huber at embedded-brains.de
Fri Sep 24 18:21:48 UTC 2021


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        | 63 ++++++++++++++----------
 cpukit/score/src/threadqops.c            |  4 +-
 4 files changed, 80 insertions(+), 41 deletions(-)

diff --git a/cpukit/include/rtems/score/threadqimpl.h b/cpukit/include/rtems/score/threadqimpl.h
index 22e0c7f069..d4fa8b8316 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 detected a deadlock.
+   */
+  THREAD_QUEUE_DEADLOCK_DETECTED,
+
+  /**
+   * @brief The operation did not detect a deadlock.
+   */
+  THREAD_QUEUE_NO_DEADLOCK
+} 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.
+ *
+ * A 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..ee5c6fff5e 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,7 +174,7 @@ static void _Thread_queue_Link_remove( Thread_queue_Link *link )
 #if !defined(RTEMS_SMP)
 static
 #endif
-void _Thread_queue_Path_release_critical(
+void _Thread_queue_Path_release(
   Thread_queue_Context *queue_context
 )
 {
@@ -260,7 +259,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
@@ -284,11 +283,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 +310,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 +334,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 +354,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 +395,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 +409,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 +426,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 +475,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 +498,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 a42876cf09..f09cf77df1 100644
--- a/cpukit/score/src/threadqops.c
+++ b/cpukit/score/src/threadqops.c
@@ -1197,7 +1197,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(
@@ -1210,7 +1210,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
 }
 
-- 
2.31.1



More information about the devel mailing list