[PATCH 12/20] posix: Avoid Giant lock for mutexes

Sebastian Huber sebastian.huber at embedded-brains.de
Tue Apr 19 13:12:40 UTC 2016


Delete _POSIX_Mutex_Get().  Use _POSIX_Mutex_Get_interrupt_disable()
instead.

Update #2555.
---
 cpukit/posix/include/rtems/posix/muteximpl.h | 14 -------
 cpukit/posix/src/mutexdestroy.c              | 56 +++++++++++-----------------
 cpukit/posix/src/mutexget.c                  | 24 +-----------
 cpukit/posix/src/mutexgetprioceiling.c       | 40 ++++++++------------
 cpukit/posix/src/mutexinit.c                 |  6 ---
 cpukit/posix/src/mutexsetprioceiling.c       |  1 +
 testsuites/psxtests/psx09/init.c             | 46 +++++++++++++++++++++++
 testsuites/psxtests/psx09/psx09.doc          |  1 +
 testsuites/psxtests/psx09/system.h           |  2 +-
 9 files changed, 89 insertions(+), 101 deletions(-)

diff --git a/cpukit/posix/include/rtems/posix/muteximpl.h b/cpukit/posix/include/rtems/posix/muteximpl.h
index 1bd74cc..fb30d58 100644
--- a/cpukit/posix/include/rtems/posix/muteximpl.h
+++ b/cpukit/posix/include/rtems/posix/muteximpl.h
@@ -66,7 +66,6 @@ RTEMS_INLINE_ROUTINE void _POSIX_Mutex_Free(
   POSIX_Mutex_Control *the_mutex
 )
 {
-  _CORE_mutex_Destroy( &the_mutex->Mutex );
   _Objects_Free( &_POSIX_Mutex_Information, &the_mutex->Object );
 }
 
@@ -120,19 +119,6 @@ RTEMS_INLINE_ROUTINE int _POSIX_Mutex_Translate_core_mutex_return_code(
 }
 
 /**
- *  @brief POSIX Mutex Get (Thread Dispatch Disable)
- *
- *  A support routine which translates the mutex id into a local pointer.
- *  As a side-effect, it may create the mutex.
- *
- *  @note This version of the method uses a dispatching critical section.
- */
-POSIX_Mutex_Control *_POSIX_Mutex_Get (
-  pthread_mutex_t   *mutex,
-  Objects_Locations *location
-);
-
-/**
  *  @brief POSIX Mutex Get (Interrupt Disable)
  *
  *  A support routine which translates the mutex id into a local pointer.
diff --git a/cpukit/posix/src/mutexdestroy.c b/cpukit/posix/src/mutexdestroy.c
index fb5aee6..bebb306 100644
--- a/cpukit/posix/src/mutexdestroy.c
+++ b/cpukit/posix/src/mutexdestroy.c
@@ -18,14 +18,7 @@
 #include "config.h"
 #endif
 
-#include <errno.h>
-#include <pthread.h>
-
-#include <rtems/system.h>
-#include <rtems/score/coremuteximpl.h>
-#include <rtems/score/watchdog.h>
 #include <rtems/posix/muteximpl.h>
-#include <rtems/posix/priorityimpl.h>
 
 /*
  *  11.3.2 Initializing and Destroying a Mutex, P1003.1c/Draft 10, p. 87
@@ -35,41 +28,36 @@ int pthread_mutex_destroy(
   pthread_mutex_t           *mutex
 )
 {
-  register POSIX_Mutex_Control *the_mutex;
-  Objects_Locations             location;
+  POSIX_Mutex_Control *the_mutex;
+  ISR_lock_Context     lock_context;
+  int                  eno;
 
   _Objects_Allocator_lock();
-  the_mutex = _POSIX_Mutex_Get( mutex, &location );
-  switch ( location ) {
 
-    case OBJECTS_LOCAL:
-       /*
-        * XXX: There is an error for the mutex being locked
-        *  or being in use by a condition variable.
-        */
+  the_mutex = _POSIX_Mutex_Get_interrupt_disable( mutex, &lock_context );
+
+  if ( the_mutex != NULL ) {
+    _CORE_mutex_Acquire_critical( &the_mutex->Mutex, &lock_context );
 
-      if ( _CORE_mutex_Is_locked( &the_mutex->Mutex ) ) {
-        _Objects_Put( &the_mutex->Object );
-        _Objects_Allocator_unlock();
-        return EBUSY;
-      }
+    /*
+     * XXX: There is an error for the mutex being locked
+     *  or being in use by a condition variable.
+     */
 
+    if ( !_CORE_mutex_Is_locked( &the_mutex->Mutex ) ) {
       _Objects_Close( &_POSIX_Mutex_Information, &the_mutex->Object );
-      _CORE_mutex_Flush( &the_mutex->Mutex, EINVAL, NULL, 0 );
-      _Objects_Put( &the_mutex->Object );
+      _CORE_mutex_Release( &the_mutex->Mutex, &lock_context );
+      _CORE_mutex_Destroy( &the_mutex->Mutex );
       _POSIX_Mutex_Free( the_mutex );
-      _Objects_Allocator_unlock();
-
-      return 0;
-
-#if defined(RTEMS_MULTIPROCESSING)
-    case OBJECTS_REMOTE:
-#endif
-    case OBJECTS_ERROR:
-      break;
+      eno = 0;
+    } else {
+      _CORE_mutex_Release( &the_mutex->Mutex, &lock_context );
+      eno = EBUSY;
+    }
+  } else {
+    eno = EINVAL;
   }
 
   _Objects_Allocator_unlock();
-
-  return EINVAL;
+  return eno;
 }
diff --git a/cpukit/posix/src/mutexget.c b/cpukit/posix/src/mutexget.c
index 9ad226a..9d34ecb 100644
--- a/cpukit/posix/src/mutexget.c
+++ b/cpukit/posix/src/mutexget.c
@@ -21,14 +21,9 @@
 #include <rtems/posix/muteximpl.h>
 #include <rtems/score/apimutex.h>
 
-static bool _POSIX_Mutex_Check_id_and_auto_init(
-  pthread_mutex_t   *mutex,
-  Objects_Locations *location
-)
+static bool _POSIX_Mutex_Check_id_and_auto_init( pthread_mutex_t *mutex )
 {
   if ( mutex == NULL ) {
-    *location = OBJECTS_ERROR;
-
     return false;
   }
 
@@ -46,8 +41,6 @@ static bool _POSIX_Mutex_Check_id_and_auto_init(
     _Once_Unlock();
 
     if ( eno != 0 ) {
-      *location = OBJECTS_ERROR;
-
       return false;
     }
   }
@@ -55,19 +48,6 @@ static bool _POSIX_Mutex_Check_id_and_auto_init(
   return true;
 }
 
-POSIX_Mutex_Control *_POSIX_Mutex_Get (
-  pthread_mutex_t   *mutex,
-  Objects_Locations *location
-)
-{
-  if ( !_POSIX_Mutex_Check_id_and_auto_init( mutex, location ) ) {
-    return NULL;
-  }
-
-  return (POSIX_Mutex_Control *)
-    _Objects_Get( &_POSIX_Mutex_Information, (Objects_Id) *mutex, location );
-}
-
 POSIX_Mutex_Control *_POSIX_Mutex_Get_interrupt_disable(
   pthread_mutex_t  *mutex,
   ISR_lock_Context *lock_context
@@ -75,7 +55,7 @@ POSIX_Mutex_Control *_POSIX_Mutex_Get_interrupt_disable(
 {
   Objects_Locations location;
 
-  if ( !_POSIX_Mutex_Check_id_and_auto_init( mutex, &location ) ) {
+  if ( !_POSIX_Mutex_Check_id_and_auto_init( mutex ) ) {
     return NULL;
   }
 
diff --git a/cpukit/posix/src/mutexgetprioceiling.c b/cpukit/posix/src/mutexgetprioceiling.c
index 129f661..e66e983 100644
--- a/cpukit/posix/src/mutexgetprioceiling.c
+++ b/cpukit/posix/src/mutexgetprioceiling.c
@@ -18,12 +18,6 @@
 #include "config.h"
 #endif
 
-#include <errno.h>
-#include <pthread.h>
-
-#include <rtems/system.h>
-#include <rtems/score/coremuteximpl.h>
-#include <rtems/score/watchdog.h>
 #include <rtems/posix/muteximpl.h>
 #include <rtems/posix/priorityimpl.h>
 
@@ -36,28 +30,26 @@ int pthread_mutex_getprioceiling(
   int               *prioceiling
 )
 {
-  register POSIX_Mutex_Control *the_mutex;
-  Objects_Locations             location;
+  POSIX_Mutex_Control *the_mutex;
+  ISR_lock_Context     lock_context;
 
-  if ( !prioceiling )
+  if ( prioceiling == NULL ) {
     return EINVAL;
+  }
 
-  the_mutex = _POSIX_Mutex_Get( mutex, &location );
-  switch ( location ) {
-
-    case OBJECTS_LOCAL:
-      *prioceiling = _POSIX_Priority_From_core(
-        the_mutex->Mutex.Attributes.priority_ceiling
-      );
-      _Objects_Put( &the_mutex->Object );
-      return 0;
+  the_mutex = _POSIX_Mutex_Get_interrupt_disable( mutex, &lock_context );
 
-#if defined(RTEMS_MULTIPROCESSING)
-    case OBJECTS_REMOTE:
-#endif
-    case OBJECTS_ERROR:
-      break;
+  if ( the_mutex == NULL ) {
+    return EINVAL;
   }
 
-  return EINVAL;
+  _CORE_mutex_Acquire_critical( &the_mutex->Mutex, &lock_context );
+
+  *prioceiling = _POSIX_Priority_From_core(
+    the_mutex->Mutex.Attributes.priority_ceiling
+  );
+
+  _CORE_mutex_Release( &the_mutex->Mutex, &lock_context );
+
+  return 0;
 }
diff --git a/cpukit/posix/src/mutexinit.c b/cpukit/posix/src/mutexinit.c
index db7f6b8..1888699 100644
--- a/cpukit/posix/src/mutexinit.c
+++ b/cpukit/posix/src/mutexinit.c
@@ -18,12 +18,6 @@
 #include "config.h"
 #endif
 
-#include <errno.h>
-#include <pthread.h>
-
-#include <rtems/system.h>
-#include <rtems/score/coremuteximpl.h>
-#include <rtems/score/watchdog.h>
 #include <rtems/posix/muteximpl.h>
 #include <rtems/posix/priorityimpl.h>
 
diff --git a/cpukit/posix/src/mutexsetprioceiling.c b/cpukit/posix/src/mutexsetprioceiling.c
index 1f9f516..5d50f14 100644
--- a/cpukit/posix/src/mutexsetprioceiling.c
+++ b/cpukit/posix/src/mutexsetprioceiling.c
@@ -19,6 +19,7 @@
 #endif
 
 #include <rtems/posix/muteximpl.h>
+#include <rtems/posix/priorityimpl.h>
 
 /*
  *  13.6.2 Change the Priority Ceiling of a Mutex, P1003.1c/Draft 10, p. 131
diff --git a/testsuites/psxtests/psx09/init.c b/testsuites/psxtests/psx09/init.c
index 15c1e48..64bb4af 100644
--- a/testsuites/psxtests/psx09/init.c
+++ b/testsuites/psxtests/psx09/init.c
@@ -49,6 +49,50 @@ void print_schedparam(
 #endif
 }
 
+static void *mutex_lock_task(void *arg)
+{
+  pthread_mutex_t *mtx;
+  int              eno;
+
+  mtx = arg;
+
+  eno = pthread_mutex_lock( mtx );
+  rtems_test_assert( eno == 0 );
+
+  sched_yield();
+
+  eno = pthread_mutex_unlock( mtx );
+  rtems_test_assert( eno == 0 );
+
+  return NULL;
+}
+
+static void test_destroy_locked_mutex(void)
+{
+  pthread_mutex_t mtx;
+  pthread_t       th;
+  int             eno;
+
+  eno = pthread_mutex_init( &mtx, NULL );
+  rtems_test_assert( eno == 0 );
+
+  eno = pthread_create( &th, NULL, mutex_lock_task, &mtx );
+  rtems_test_assert( eno == 0 );
+
+  sched_yield();
+
+  eno = pthread_mutex_destroy( &mtx );
+  rtems_test_assert( eno == EBUSY );
+
+  sched_yield();
+
+  eno = pthread_mutex_destroy( &mtx );
+  rtems_test_assert( eno == 0 );
+
+  eno = pthread_join( th, NULL );
+  rtems_test_assert( eno == 0 );
+}
+
 void *POSIX_Init(
   void *argument
 )
@@ -65,6 +109,8 @@ void *POSIX_Init(
 
   TEST_BEGIN();
 
+  test_destroy_locked_mutex();
+
   /* set the time of day, and print our buffer in multiple ways */
 
   set_time( TM_FRIDAY, TM_MAY, 24, 96, 11, 5, 0 );
diff --git a/testsuites/psxtests/psx09/psx09.doc b/testsuites/psxtests/psx09/psx09.doc
index ed5f83b..b90367b 100644
--- a/testsuites/psxtests/psx09/psx09.doc
+++ b/testsuites/psxtests/psx09/psx09.doc
@@ -23,3 +23,4 @@ concepts:
 + thread priority no longer gets adjusted after obtaining mutex
 + thread priority gets locked at the ceiling level
 + unlocks mutex and thread priority is set to low priority successfully
++ lock returns proper status if deleted during lock operation
diff --git a/testsuites/psxtests/psx09/system.h b/testsuites/psxtests/psx09/system.h
index 1c71e7f..a5aabeb 100644
--- a/testsuites/psxtests/psx09/system.h
+++ b/testsuites/psxtests/psx09/system.h
@@ -34,7 +34,7 @@ void *Task_2(
 
 #define CONFIGURE_INITIAL_EXTENSIONS RTEMS_TEST_INITIAL_EXTENSION
 
-#define CONFIGURE_MAXIMUM_POSIX_THREADS   1
+#define CONFIGURE_MAXIMUM_POSIX_THREADS   2
 #define CONFIGURE_MAXIMUM_POSIX_KEYS     10
 #define CONFIGURE_MAXIMUM_POSIX_MUTEXES  10
 
-- 
1.8.4.5




More information about the devel mailing list