score: Fine grained locking for mutexes

Update #2273.

cpukit/libnetworking/rtems/rtems_glue.c

@@ -378,9 +378,6 @@ rtems_bsdnet_semaphore_obtain (void)
 #ifdef RTEMS_FAST_MUTEX
 	ISR_lock_Context lock_context;
 	Thread_Control *executing;
-#ifdef RTEMS_SMP
-	_Thread_Disable_dispatch();
-#endif
 	_ISR_lock_ISR_disable(&lock_context);
 	if (!the_networkSemaphore)
 		rtems_panic ("rtems-net: network sema obtain: network not initialised\n");
@@ -393,9 +390,6 @@ rtems_bsdnet_semaphore_obtain (void)
 		0,		/* forever */
 		&lock_context
 		);
-#ifdef RTEMS_SMP
-	_Thread_Enable_dispatch();
-#endif
 	if (executing->Wait.return_code)
 		rtems_panic ("rtems-net: can't obtain network sema: %d\n",
                  executing->Wait.return_code);
@@ -416,18 +410,19 @@ void
 rtems_bsdnet_semaphore_release (void)
 {
 #ifdef RTEMS_FAST_MUTEX
-	int i;
+        ISR_lock_Context lock_context;
+	CORE_mutex_Status status;
 
-	_Thread_Disable_dispatch();
 	if (!the_networkSemaphore)
 		rtems_panic ("rtems-net: network sema obtain: network not initialised\n");
-	i = _CORE_mutex_Surrender (
+        _ISR_lock_ISR_disable(&lock_context);
+	status = _CORE_mutex_Surrender (
 		&the_networkSemaphore->Core_control.mutex,
 		networkSemaphore,
-		NULL
+		NULL,
+                &lock_context
 		);
-	_Thread_Enable_dispatch();
+	if (status != CORE_MUTEX_STATUS_SUCCESSFUL)
-	if (i)
 		rtems_panic ("rtems-net: can't release network sema: %i\n");
 #else
 	rtems_status_code sc;

cpukit/posix/src/mutexlocksupp.c

@@ -54,9 +54,6 @@ int _POSIX_Mutex_Lock_support(
   switch ( location ) {
 
     case OBJECTS_LOCAL:
-#if defined(RTEMS_SMP)
-      _Thread_Disable_dispatch();
-#endif
       executing = _Thread_Executing;
       _CORE_mutex_Seize(
         &the_mutex->Mutex,
@@ -66,10 +63,6 @@ int _POSIX_Mutex_Lock_support(
         timeout,
         &lock_context
       );
-#if defined(RTEMS_SMP)
-      _Thread_Enable_dispatch();
-#endif
-      _Objects_Put_for_get_isr_disable( &the_mutex->Object );
       return _POSIX_Mutex_Translate_core_mutex_return_code(
         (CORE_mutex_Status) executing->Wait.return_code
       );

cpukit/posix/src/mutexsetprioceiling.c

@@ -41,6 +41,7 @@ int pthread_mutex_setprioceiling(
   register POSIX_Mutex_Control *the_mutex;
   Objects_Locations             location;
   Priority_Control              the_priority;
+  ISR_lock_Context              lock_context;
 
   if ( !old_ceiling )
     return EINVAL;
@@ -64,7 +65,11 @@ int pthread_mutex_setprioceiling(
    *  NOTE: This makes it easier to get 100% binary coverage since the
    *        bad Id case is handled by the switch.
    */
-  the_mutex = _POSIX_Mutex_Get( mutex, &location );
+  the_mutex = _POSIX_Mutex_Get_interrupt_disable(
+    mutex,
+    &location,
+    &lock_context
+  );
   switch ( location ) {
 
     case OBJECTS_LOCAL:
@@ -78,9 +83,9 @@ int pthread_mutex_setprioceiling(
       _CORE_mutex_Surrender(
         &the_mutex->Mutex,
         the_mutex->Object.id,
-        NULL
+        NULL,
+        &lock_context
       );
-      _Objects_Put( &the_mutex->Object );
 
       return 0;
 

cpukit/posix/src/mutexunlock.c

@@ -41,17 +41,22 @@ int pthread_mutex_unlock(
   register POSIX_Mutex_Control *the_mutex;
   Objects_Locations             location;
   CORE_mutex_Status             status;
+  ISR_lock_Context              lock_context;
 
-  the_mutex = _POSIX_Mutex_Get( mutex, &location );
+  the_mutex = _POSIX_Mutex_Get_interrupt_disable(
+    mutex,
+    &location,
+    &lock_context
+  );
   switch ( location ) {
 
     case OBJECTS_LOCAL:
       status = _CORE_mutex_Surrender(
         &the_mutex->Mutex,
         the_mutex->Object.id,
-        NULL
+        NULL,
+        &lock_context
       );
-      _Objects_Put( &the_mutex->Object );
       return _POSIX_Mutex_Translate_core_mutex_return_code( status );
 
 #if defined(RTEMS_MULTIPROCESSING)

cpukit/rtems/src/semobtain.c

@@ -73,9 +73,6 @@ rtems_status_code rtems_semaphore_obtain(
       } else
 #endif
       if ( !_Attributes_Is_counting_semaphore( attribute_set ) ) {
-#if defined(RTEMS_SMP)
-        _Thread_Disable_dispatch();
-#endif
         _CORE_mutex_Seize(
           &the_semaphore->Core_control.mutex,
           executing,
@@ -84,10 +81,6 @@ rtems_status_code rtems_semaphore_obtain(
           timeout,
           &lock_context
         );
-#if defined(RTEMS_SMP)
-        _Thread_Enable_dispatch();
-#endif
-        _Objects_Put_for_get_isr_disable( &the_semaphore->Object );
         return _Semaphore_Translate_core_mutex_return_code(
                   executing->Wait.return_code );
       }

cpukit/rtems/src/semrelease.c

@@ -86,14 +86,12 @@ rtems_status_code rtems_semaphore_release(
       } else
 #endif
       if ( !_Attributes_Is_counting_semaphore( attribute_set ) ) {
-        _Thread_Disable_dispatch();
-        _ISR_lock_ISR_enable( &lock_context );
         mutex_status = _CORE_mutex_Surrender(
           &the_semaphore->Core_control.mutex,
           id,
-          MUTEX_MP_SUPPORT
+          MUTEX_MP_SUPPORT,
+          &lock_context
         );
-        _Thread_Enable_dispatch();
         return _Semaphore_Translate_core_mutex_return_code( mutex_status );
       } else {
         semaphore_status = _CORE_semaphore_Surrender(

cpukit/score/include/rtems/score/coremuteximpl.h

@@ -210,17 +210,10 @@ void _CORE_mutex_Seize_interrupt_blocking(
  *
  *  @retval this method returns true if dispatch is in an unsafe state.
  */
-#ifdef RTEMS_SMP
+#define _CORE_mutex_Check_dispatch_for_seize(_wait) \
-  #define _CORE_mutex_Check_dispatch_for_seize(_wait) \
-      (_Thread_Dispatch_get_disable_level() != 1 \
-        && (_wait) \
-        && (_System_state_Get() >= SYSTEM_STATE_UP))
-#else
-  #define _CORE_mutex_Check_dispatch_for_seize(_wait) \
   (!_Thread_Dispatch_is_enabled() \
     && (_wait) \
     && (_System_state_Get() >= SYSTEM_STATE_UP))
-#endif
 
 /**
  *  @brief Attempt to obtain the mutex.
@@ -265,9 +258,10 @@ RTEMS_INLINE_ROUTINE void _CORE_mutex_Seize_body(
       INTERNAL_ERROR_MUTEX_OBTAIN_FROM_BAD_STATE
     );
   }
+  _Thread_queue_Acquire_critical( &the_mutex->Wait_queue, lock_context );
   if ( _CORE_mutex_Seize_interrupt_trylock( the_mutex, executing, lock_context ) ) {
     if ( !wait ) {
-      _ISR_lock_ISR_enable( lock_context );
+      _Thread_queue_Release( &the_mutex->Wait_queue, lock_context );
       executing->Wait.return_code =
         CORE_MUTEX_STATUS_UNSATISFIED_NOWAIT;
     } else {
@@ -320,13 +314,15 @@ RTEMS_INLINE_ROUTINE void _CORE_mutex_Seize_body(
  *  @param[in] id is the id of the RTEMS Object associated with this mutex
  *  @param[in] api_mutex_mp_support is the routine that will be called when
  *         unblocking a remote mutex
+ *  @param[in] lock_context is the interrupt level
  *
  *  @retval an indication of whether the routine succeeded or failed
  */
 CORE_mutex_Status _CORE_mutex_Surrender(
   CORE_mutex_Control                *the_mutex,
   Objects_Id                         id,
-  CORE_mutex_API_mp_support_callout  api_mutex_mp_support
+  CORE_mutex_API_mp_support_callout  api_mutex_mp_support,
+  ISR_lock_Context                  *lock_context
 );
 
 /**
@@ -472,7 +468,7 @@ RTEMS_INLINE_ROUTINE int _CORE_mutex_Seize_interrupt_trylock_body(
     }
 
     if ( !_CORE_mutex_Is_priority_ceiling( &the_mutex->Attributes ) ) {
-      _ISR_lock_ISR_enable( lock_context );
+      _Thread_queue_Release( &the_mutex->Wait_queue, lock_context );
       return 0;
     } /* else must be CORE_MUTEX_DISCIPLINES_PRIORITY_CEILING
        *
@@ -486,19 +482,21 @@ RTEMS_INLINE_ROUTINE int _CORE_mutex_Seize_interrupt_trylock_body(
       ceiling = the_mutex->Attributes.priority_ceiling;
       current = executing->current_priority;
       if ( current == ceiling ) {
-        _ISR_lock_ISR_enable( lock_context );
+        _Thread_queue_Release( &the_mutex->Wait_queue, lock_context );
         return 0;
       }
 
       if ( current > ceiling ) {
-        _Thread_Disable_dispatch();
+        Per_CPU_Control *cpu_self;
-        _ISR_lock_ISR_enable( lock_context );
+
+        cpu_self = _Thread_Dispatch_disable_critical();
+        _Thread_queue_Release( &the_mutex->Wait_queue, lock_context );
         _Thread_Change_priority(
           executing,
           ceiling,
           false
         );
-        _Thread_Enable_dispatch();
+        _Thread_Dispatch_enable( cpu_self );
         return 0;
       }
       /* if ( current < ceiling ) */ {
@@ -506,7 +504,7 @@ RTEMS_INLINE_ROUTINE int _CORE_mutex_Seize_interrupt_trylock_body(
         the_mutex->holder = NULL;
         the_mutex->nest_count = 0;     /* undo locking above */
         executing->resource_count--;   /* undo locking above */
-        _ISR_lock_ISR_enable( lock_context );
+        _Thread_queue_Release( &the_mutex->Wait_queue, lock_context );
         return 0;
       }
     }
@@ -522,12 +520,12 @@ RTEMS_INLINE_ROUTINE int _CORE_mutex_Seize_interrupt_trylock_body(
     switch ( the_mutex->Attributes.lock_nesting_behavior ) {
       case CORE_MUTEX_NESTING_ACQUIRES:
         the_mutex->nest_count++;
-        _ISR_lock_ISR_enable( lock_context );
+        _Thread_queue_Release( &the_mutex->Wait_queue, lock_context );
         return 0;
       #if defined(RTEMS_POSIX_API)
         case CORE_MUTEX_NESTING_IS_ERROR:
           executing->Wait.return_code = CORE_MUTEX_STATUS_NESTING_NOT_ALLOWED;
-          _ISR_lock_ISR_enable( lock_context );
+          _Thread_queue_Release( &the_mutex->Wait_queue, lock_context );
           return 0;
       #endif
       case CORE_MUTEX_NESTING_BLOCKS:

cpukit/score/src/apimutexlock.c

@@ -30,10 +30,6 @@ void _API_Mutex_Lock( API_Mutex_Control *the_mutex )
 
   previous_thread_life_protection = _Thread_Set_life_protection( true );
 
-  #if defined(RTEMS_SMP)
-    _Thread_Disable_dispatch();
-  #endif
-
   _ISR_lock_ISR_disable( &lock_context );
 
   _CORE_mutex_Seize(
@@ -49,8 +45,4 @@ void _API_Mutex_Lock( API_Mutex_Control *the_mutex )
     the_mutex->previous_thread_life_protection =
       previous_thread_life_protection;
   }
-
-  #if defined(RTEMS_SMP)
-    _Thread_Enable_dispatch();
-  #endif
 }

cpukit/score/src/apimutexunlock.c

@@ -24,18 +24,21 @@
 
 void _API_Mutex_Unlock( API_Mutex_Control *the_mutex )
 {
+  ISR_lock_Context lock_context;
   bool             previous_thread_life_protection;
   bool             restore_thread_life_protection;
 
-  _Thread_Disable_dispatch();
-
   previous_thread_life_protection =
     the_mutex->previous_thread_life_protection;
   restore_thread_life_protection = the_mutex->Mutex.nest_count == 1;
 
-  _CORE_mutex_Surrender( &the_mutex->Mutex, the_mutex->Object.id, NULL );
+  _ISR_lock_ISR_disable( &lock_context );
-
+  _CORE_mutex_Surrender(
-  _Thread_Enable_dispatch();
+    &the_mutex->Mutex,
+    the_mutex->Object.id,
+    NULL,
+    &lock_context
+  );
 
   if ( restore_thread_life_protection ) {
     _Thread_Set_life_protection( previous_thread_life_protection );

cpukit/score/src/coremutex.c

@@ -48,13 +48,14 @@ CORE_mutex_Status _CORE_mutex_Initialize(
     if (  is_priority_ceiling ||
          _CORE_mutex_Is_inherit_priority( &the_mutex->Attributes ) ) {
       Priority_Control ceiling = the_mutex->Attributes.priority_ceiling;
+      Per_CPU_Control *cpu_self;
 
       /*
        * The mutex initialization is only protected by the allocator lock in
        * general.  Disable thread dispatching before the priority check to
        * prevent interference with priority inheritance.
        */
-      _Thread_Disable_dispatch();
+      cpu_self = _Thread_Dispatch_disable();
 
       if ( is_priority_ceiling && executing->current_priority < ceiling ) {
         _Thread_Enable_dispatch();
@@ -73,7 +74,7 @@ CORE_mutex_Status _CORE_mutex_Initialize(
         _Thread_Change_priority( executing, ceiling, false );
       }
 
-      _Thread_Enable_dispatch();
+      _Thread_Dispatch_enable( cpu_self );
     }
   } else {
     the_mutex->nest_count = 0;

cpukit/score/src/coremutexseize.c

@@ -53,17 +53,28 @@ void _CORE_mutex_Seize_interrupt_blocking(
   ISR_lock_Context    *lock_context
 )
 {
-  _Thread_Disable_dispatch();
+#if !defined(RTEMS_SMP)
+  /*
+   * We must disable thread dispatching here since we enable the interrupts for
+   * priority inheritance mutexes.
+   */
+  _Thread_Dispatch_disable();
+#endif
 
   if ( _CORE_mutex_Is_inherit_priority( &the_mutex->Attributes ) ) {
     Thread_Control *holder = the_mutex->holder;
 
+#if !defined(RTEMS_SMP)
     /*
-     * To enable interrupts here works only since we own the Giant lock and
+     * To enable interrupts here works only since exactly one executing thread
-     * only threads are allowed to seize and surrender mutexes with the
+     * exists and only threads are allowed to seize and surrender mutexes with
-     * priority inheritance protocol.
+     * the priority inheritance protocol.  On SMP configurations more than one
+     * executing thread may exist, so here we must not release the lock, since
+     * otherwise the current holder may be no longer the holder of the mutex
+     * once we released the lock.
      */
-    _ISR_lock_ISR_enable( lock_context );
+    _Thread_queue_Release( &the_mutex->Wait_queue, lock_context );
+#endif
 
     _Scheduler_Change_priority_if_higher(
       _Scheduler_Get( holder ),
@@ -72,10 +83,11 @@ void _CORE_mutex_Seize_interrupt_blocking(
       false
     );
 
-    _ISR_lock_ISR_disable( lock_context );
+#if !defined(RTEMS_SMP)
+    _Thread_queue_Acquire( &the_mutex->Wait_queue, lock_context );
+#endif
   }
 
-  _Thread_queue_Acquire_critical( &the_mutex->Wait_queue, lock_context );
   _Thread_queue_Enqueue_critical(
     &the_mutex->Wait_queue,
     executing,
@@ -85,6 +97,8 @@ void _CORE_mutex_Seize_interrupt_blocking(
     lock_context
   );
 
-  _Thread_Enable_dispatch();
+#if !defined(RTEMS_SMP)
+  _Thread_Dispatch_enable( _Per_CPU_Get() );
+#endif
 }
 

cpukit/score/src/coremutexsurrender.c

@@ -89,11 +89,12 @@ CORE_mutex_Status _CORE_mutex_Surrender(
   CORE_mutex_Control                *the_mutex,
 #if defined(RTEMS_MULTIPROCESSING)
   Objects_Id                         id,
-  CORE_mutex_API_mp_support_callout  api_mutex_mp_support
+  CORE_mutex_API_mp_support_callout  api_mutex_mp_support,
 #else
   Objects_Id                         id __attribute__((unused)),
-  CORE_mutex_API_mp_support_callout  api_mutex_mp_support __attribute__((unused))
+  CORE_mutex_API_mp_support_callout  api_mutex_mp_support __attribute__((unused)),
 #endif
+  ISR_lock_Context                  *lock_context
 )
 {
   Thread_Control *the_thread;
@@ -110,14 +111,20 @@ CORE_mutex_Status _CORE_mutex_Surrender(
    */
 
   if ( the_mutex->Attributes.only_owner_release ) {
-    if ( !_Thread_Is_executing( holder ) )
+    if ( !_Thread_Is_executing( holder ) ) {
+      _ISR_lock_ISR_enable( lock_context );
       return CORE_MUTEX_STATUS_NOT_OWNER_OF_RESOURCE;
     }
+  }
+
+  _Thread_queue_Acquire_critical( &the_mutex->Wait_queue, lock_context );
 
   /* XXX already unlocked -- not right status */
 
-  if ( !the_mutex->nest_count )
+  if ( !the_mutex->nest_count ) {
+    _Thread_queue_Release( &the_mutex->Wait_queue, lock_context );
     return CORE_MUTEX_STATUS_SUCCESSFUL;
+  }
 
   the_mutex->nest_count--;
 
@@ -130,10 +137,12 @@ CORE_mutex_Status _CORE_mutex_Surrender(
     #if defined(RTEMS_DEBUG)
       switch ( the_mutex->Attributes.lock_nesting_behavior ) {
         case CORE_MUTEX_NESTING_ACQUIRES:
+          _Thread_queue_Release( &the_mutex->Wait_queue, lock_context );
           return CORE_MUTEX_STATUS_SUCCESSFUL;
         #if defined(RTEMS_POSIX_API)
           case CORE_MUTEX_NESTING_IS_ERROR:
             /* should never occur */
+            _Thread_queue_Release( &the_mutex->Wait_queue, lock_context );
             return CORE_MUTEX_STATUS_NESTING_NOT_ALLOWED;
         #endif
         case CORE_MUTEX_NESTING_BLOCKS:
@@ -141,6 +150,7 @@ CORE_mutex_Status _CORE_mutex_Surrender(
           break;
       }
     #else
+      _Thread_queue_Release( &the_mutex->Wait_queue, lock_context );
       /* must be CORE_MUTEX_NESTING_ACQUIRES or we wouldn't be here */
       return CORE_MUTEX_STATUS_SUCCESSFUL;
     #endif
@@ -155,20 +165,12 @@ CORE_mutex_Status _CORE_mutex_Surrender(
     CORE_mutex_Status pop_status =
       _CORE_mutex_Pop_priority( the_mutex, holder );
 
-    if ( pop_status != CORE_MUTEX_STATUS_SUCCESSFUL )
+    if ( pop_status != CORE_MUTEX_STATUS_SUCCESSFUL ) {
+      _Thread_queue_Release( &the_mutex->Wait_queue, lock_context );
       return pop_status;
+    }
 
     holder->resource_count--;
-
-    /*
-     *  Whether or not someone is waiting for the mutex, an
-     *  inherited priority must be lowered if this is the last
-     *  mutex (i.e. resource) this task has.
-     */
-    if ( !_Thread_Owns_resources( holder ) &&
-         holder->real_priority != holder->current_priority ) {
-      _Thread_Change_priority( holder, holder->real_priority, true );
-    }
   }
   the_mutex->holder = NULL;
 
@@ -176,20 +178,21 @@ CORE_mutex_Status _CORE_mutex_Surrender(
    *  Now we check if another thread was waiting for this mutex.  If so,
    *  transfer the mutex to that thread.
    */
-  if ( ( the_thread = _Thread_queue_Dequeue( &the_mutex->Wait_queue ) ) ) {
+  if ( ( the_thread = _Thread_queue_First_locked( &the_mutex->Wait_queue ) ) ) {
+    /*
+     * We must extract the thread now since this will restore its default
+     * thread lock.  This is necessary to avoid a deadlock in the
+     * _Thread_Change_priority() below due to a recursive thread queue lock
+     * acquire.
+     */
+    _Thread_queue_Extract_locked( &the_mutex->Wait_queue, the_thread );
 
 #if defined(RTEMS_MULTIPROCESSING)
-    if ( !_Objects_Is_local_id( the_thread->Object.id ) ) {
+    _Thread_Dispatch_disable();
 
-      the_mutex->holder     = NULL;
+    if ( _Objects_Is_local_id( the_thread->Object.id ) )
-      the_mutex->nest_count = 1;
-
-      ( *api_mutex_mp_support)( the_thread, id );
-
-    } else
 #endif
     {
-
       the_mutex->holder     = the_thread;
       the_mutex->nest_count = 1;
 
@@ -215,6 +218,41 @@ CORE_mutex_Status _CORE_mutex_Surrender(
           break;
       }
     }
+
+    _Thread_queue_Unblock_critical(
+      &the_mutex->Wait_queue,
+      the_thread,
+      lock_context
+    );
+
+#if defined(RTEMS_MULTIPROCESSING)
+    if ( !_Objects_Is_local_id( the_thread->Object.id ) ) {
+
+      the_mutex->holder     = NULL;
+      the_mutex->nest_count = 1;
+
+      ( *api_mutex_mp_support)( the_thread, id );
+
+    }
+
+    _Thread_Dispatch_enable( _Per_CPU_Get() );
+#endif
+  } else {
+    _Thread_queue_Release( &the_mutex->Wait_queue, lock_context );
+  }
+
+  /*
+   *  Whether or not someone is waiting for the mutex, an
+   *  inherited priority must be lowered if this is the last
+   *  mutex (i.e. resource) this task has.
+   */
+  if ( !_Thread_Owns_resources( holder ) &&
+       holder->real_priority != holder->current_priority ) {
+    Per_CPU_Control *cpu_self;
+
+    cpu_self = _Thread_Dispatch_disable();
+    _Thread_Change_priority( holder, holder->real_priority, true );
+    _Thread_Dispatch_enable( cpu_self );
   }
 
   return CORE_MUTEX_STATUS_SUCCESSFUL;

cpukit/score/src/debugisownerofallocator.c

@@ -18,8 +18,7 @@
 
 #include <rtems/score/assert.h>
 #include <rtems/score/apimutex.h>
-#include <rtems/score/thread.h>
+#include <rtems/score/threadimpl.h>
-#include <rtems/score/threaddispatch.h>
 
 #if defined( RTEMS_DEBUG )
   bool _Debug_Is_owner_of_allocator( void )
@@ -27,20 +26,12 @@
     API_Mutex_Control *mutex = _RTEMS_Allocator_Mutex;
     bool owner;
 
-    /*
-     * We have to synchronize with the _CORE_mutex_Surrender() operation,
-     * otherwise we may observe an outdated mutex holder.
-     */
-    _Thread_Disable_dispatch();
-
     if ( mutex != NULL ) {
-      owner = mutex->Mutex.holder == _Thread_Executing;
+      owner = mutex->Mutex.holder == _Thread_Get_executing();
     } else {
       owner = false;
     }
 
-    _Thread_Enable_dispatch();
-
     return owner;
   }
 #endif