[PATCH] posix: Avoid Giant lock for condition variables
Sebastian Huber
sebastian.huber at embedded-brains.de
Mon Apr 25 06:19:52 UTC 2016
Update #2555.
---
cpukit/posix/include/rtems/posix/cond.h | 3 +-
cpukit/posix/include/rtems/posix/condimpl.h | 52 +++++--
cpukit/posix/include/rtems/posix/muteximpl.h | 10 ++
cpukit/posix/src/conddestroy.c | 55 +++----
cpukit/posix/src/condget.c | 62 ++++----
cpukit/posix/src/condinit.c | 12 +-
cpukit/posix/src/condsignalsupp.c | 60 ++++----
cpukit/posix/src/condwaitsupp.c | 187 ++++++++++++-----------
cpukit/score/include/rtems/score/coremuteximpl.h | 8 +
9 files changed, 239 insertions(+), 210 deletions(-)
diff --git a/cpukit/posix/include/rtems/posix/cond.h b/cpukit/posix/include/rtems/posix/cond.h
index 1839279..4fa7de7 100644
--- a/cpukit/posix/include/rtems/posix/cond.h
+++ b/cpukit/posix/include/rtems/posix/cond.h
@@ -43,8 +43,7 @@ extern "C" {
typedef struct {
Objects_Control Object;
Thread_queue_Control Wait_queue;
- int process_shared;
- pthread_mutex_t Mutex;
+ pthread_mutex_t mutex;
} POSIX_Condition_variables_Control;
#ifdef __cplusplus
diff --git a/cpukit/posix/include/rtems/posix/condimpl.h b/cpukit/posix/include/rtems/posix/condimpl.h
index b079a43..b178869 100644
--- a/cpukit/posix/include/rtems/posix/condimpl.h
+++ b/cpukit/posix/include/rtems/posix/condimpl.h
@@ -20,7 +20,8 @@
#include <rtems/posix/cond.h>
#include <rtems/score/objectimpl.h>
#include <rtems/score/threadqimpl.h>
-#include <rtems/score/watchdog.h>
+
+#include <errno.h>
#ifdef __cplusplus
extern "C" {
@@ -45,6 +46,37 @@ extern Objects_Information _POSIX_Condition_variables_Information;
*/
extern const pthread_condattr_t _POSIX_Condition_variables_Default_attributes;
+RTEMS_INLINE_ROUTINE void _POSIX_Condition_variables_Initialize(
+ POSIX_Condition_variables_Control *the_cond
+)
+{
+ _Thread_queue_Initialize( &the_cond->Wait_queue );
+ the_cond->mutex = POSIX_CONDITION_VARIABLES_NO_MUTEX;
+}
+
+RTEMS_INLINE_ROUTINE void _POSIX_Condition_variables_Destroy(
+ POSIX_Condition_variables_Control *the_cond
+)
+{
+ _Thread_queue_Destroy( &the_cond->Wait_queue );
+}
+
+RTEMS_INLINE_ROUTINE void _POSIX_Condition_variables_Acquire_critical(
+ POSIX_Condition_variables_Control *the_cond,
+ ISR_lock_Context *lock_context
+)
+{
+ _Thread_queue_Acquire_critical( &the_cond->Wait_queue, lock_context );
+}
+
+RTEMS_INLINE_ROUTINE void _POSIX_Condition_variables_Release(
+ POSIX_Condition_variables_Control *the_cond,
+ ISR_lock_Context *lock_context
+)
+{
+ _Thread_queue_Release( &the_cond->Wait_queue, lock_context );
+}
+
/**
* @brief POSIX Condition Variable Allocate
*
@@ -68,27 +100,15 @@ RTEMS_INLINE_ROUTINE void _POSIX_Condition_variables_Free (
POSIX_Condition_variables_Control *the_condition_variable
)
{
- _Thread_queue_Destroy( &the_condition_variable->Wait_queue );
_Objects_Free(
&_POSIX_Condition_variables_Information,
&the_condition_variable->Object
);
}
-/**
- * @brief POSIX Condition Variable Get
- *
- * This function maps condition variable IDs to condition variable control
- * blocks. If ID corresponds to a local condition variable, then it returns
- * the_condition variable control pointer which maps to ID and location
- * is set to OBJECTS_LOCAL. if the condition variable ID is global and
- * resides on a remote node, then location is set to OBJECTS_REMOTE,
- * and the_condition variable is undefined. Otherwise, location is set
- * to OBJECTS_ERROR and the_condition variable is undefined.
- */
-POSIX_Condition_variables_Control *_POSIX_Condition_variables_Get (
- pthread_cond_t *cond,
- Objects_Locations *location
+POSIX_Condition_variables_Control *_POSIX_Condition_variables_Get(
+ pthread_cond_t *cond,
+ ISR_lock_Context *lock_context
);
/**
diff --git a/cpukit/posix/include/rtems/posix/muteximpl.h b/cpukit/posix/include/rtems/posix/muteximpl.h
index fb30d58..6d21e96 100644
--- a/cpukit/posix/include/rtems/posix/muteximpl.h
+++ b/cpukit/posix/include/rtems/posix/muteximpl.h
@@ -131,6 +131,16 @@ POSIX_Mutex_Control *_POSIX_Mutex_Get_interrupt_disable(
ISR_lock_Context *lock_context
);
+RTEMS_INLINE_ROUTINE POSIX_Mutex_Control *_POSIX_Mutex_Get_no_protection(
+ const pthread_mutex_t *mutex
+)
+{
+ return (POSIX_Mutex_Control *) _Objects_Get_no_protection(
+ (Objects_Id) *mutex,
+ &_POSIX_Mutex_Information
+ );
+}
+
#ifdef __cplusplus
}
#endif
diff --git a/cpukit/posix/src/conddestroy.c b/cpukit/posix/src/conddestroy.c
index c6696f5..d47c6b2 100644
--- a/cpukit/posix/src/conddestroy.c
+++ b/cpukit/posix/src/conddestroy.c
@@ -18,13 +18,7 @@
#include "config.h"
#endif
-#include <pthread.h>
-#include <errno.h>
-
-#include <rtems/system.h>
-#include <rtems/score/watchdog.h>
#include <rtems/posix/condimpl.h>
-#include <rtems/posix/muteximpl.h>
/**
* 11.4.2 Initializing and Destroying a Condition Variable,
@@ -35,42 +29,31 @@ int pthread_cond_destroy(
)
{
POSIX_Condition_variables_Control *the_cond;
- Objects_Locations location;
+ ISR_lock_Context lock_context;
_Objects_Allocator_lock();
- the_cond = _POSIX_Condition_variables_Get( cond, &location );
- switch ( location ) {
-
- case OBJECTS_LOCAL:
+ the_cond = _POSIX_Condition_variables_Get( cond, &lock_context );
- if (
- _Thread_queue_First(
- &the_cond->Wait_queue,
- POSIX_CONDITION_VARIABLES_TQ_OPERATIONS
- )
- ) {
- _Objects_Put( &the_cond->Object );
- _Objects_Allocator_unlock();
- return EBUSY;
- }
+ if ( the_cond == NULL ) {
+ _Objects_Allocator_unlock();
+ return EINVAL;
+ }
- _Objects_Close(
- &_POSIX_Condition_variables_Information,
- &the_cond->Object
- );
- _Objects_Put( &the_cond->Object );
- _POSIX_Condition_variables_Free( the_cond );
- _Objects_Allocator_unlock();
- return 0;
+ _POSIX_Condition_variables_Acquire_critical( the_cond, &lock_context );
-#if defined(RTEMS_MULTIPROCESSING)
- case OBJECTS_REMOTE:
-#endif
- case OBJECTS_ERROR:
- break;
+ if ( !_Thread_queue_Is_empty( &the_cond->Wait_queue.Queue ) ) {
+ _POSIX_Condition_variables_Release( the_cond, &lock_context );
+ _Objects_Allocator_unlock();
+ return EBUSY;
}
+ _Objects_Close(
+ &_POSIX_Condition_variables_Information,
+ &the_cond->Object
+ );
+ _POSIX_Condition_variables_Release( the_cond, &lock_context );
+ _POSIX_Condition_variables_Destroy( the_cond );
+ _POSIX_Condition_variables_Free( the_cond );
_Objects_Allocator_unlock();
-
- return EINVAL;
+ return 0;
}
diff --git a/cpukit/posix/src/condget.c b/cpukit/posix/src/condget.c
index 6ebba73..e3cf59c 100644
--- a/cpukit/posix/src/condget.c
+++ b/cpukit/posix/src/condget.c
@@ -11,45 +11,49 @@
#include "config.h"
#endif
-#include <pthread.h>
-#include <errno.h>
-
-#include <rtems/system.h>
-#include <rtems/score/watchdog.h>
#include <rtems/posix/condimpl.h>
-#include <rtems/posix/muteximpl.h>
-POSIX_Condition_variables_Control *_POSIX_Condition_variables_Get (
- pthread_cond_t *cond,
- Objects_Locations *location
+static bool _POSIX_Condition_variables_Check_id_and_auto_init(
+ pthread_cond_t *cond
)
{
- int status;
-
- if ( !cond ) {
- *location = OBJECTS_ERROR;
- return (POSIX_Condition_variables_Control *) 0;
+ if ( cond == NULL ) {
+ return false;
}
if ( *cond == PTHREAD_COND_INITIALIZER ) {
- /*
- * Do an "auto-create" here.
- */
-
- status = pthread_cond_init( cond, 0 );
- if ( status ) {
- *location = OBJECTS_ERROR;
- return (POSIX_Condition_variables_Control *) 0;
+ int eno;
+
+ _Once_Lock();
+
+ if ( *cond == PTHREAD_COND_INITIALIZER ) {
+ eno = pthread_cond_init( cond, NULL );
+ } else {
+ eno = 0;
+ }
+
+ _Once_Unlock();
+
+ if ( eno != 0 ) {
+ return false;
}
}
- /*
- * Now call Objects_Get()
- */
- return (POSIX_Condition_variables_Control *)_Objects_Get(
- &_POSIX_Condition_variables_Information,
+ return true;
+}
+
+POSIX_Condition_variables_Control *_POSIX_Condition_variables_Get(
+ pthread_cond_t *cond,
+ ISR_lock_Context *lock_context
+)
+{
+ if ( !_POSIX_Condition_variables_Check_id_and_auto_init( cond ) ) {
+ return NULL;
+ }
+
+ return (POSIX_Condition_variables_Control *) _Objects_Get_local(
(Objects_Id) *cond,
- location
+ &_POSIX_Condition_variables_Information,
+ lock_context
);
}
-
diff --git a/cpukit/posix/src/condinit.c b/cpukit/posix/src/condinit.c
index f4aeae6..dde400f 100644
--- a/cpukit/posix/src/condinit.c
+++ b/cpukit/posix/src/condinit.c
@@ -18,13 +18,7 @@
#include "config.h"
#endif
-#include <pthread.h>
-#include <errno.h>
-
-#include <rtems/system.h>
-#include <rtems/score/watchdog.h>
#include <rtems/posix/condimpl.h>
-#include <rtems/posix/muteximpl.h>
/**
* 11.4.2 Initializing and Destroying a Condition Variable,
@@ -57,11 +51,7 @@ int pthread_cond_init(
return ENOMEM;
}
- the_cond->process_shared = the_attr->process_shared;
-
- the_cond->Mutex = POSIX_CONDITION_VARIABLES_NO_MUTEX;
-
- _Thread_queue_Initialize( &the_cond->Wait_queue );
+ _POSIX_Condition_variables_Initialize( the_cond );
_Objects_Open_u32(
&_POSIX_Condition_variables_Information,
diff --git a/cpukit/posix/src/condsignalsupp.c b/cpukit/posix/src/condsignalsupp.c
index 9c67ddc..c625b3a 100644
--- a/cpukit/posix/src/condsignalsupp.c
+++ b/cpukit/posix/src/condsignalsupp.c
@@ -18,13 +18,7 @@
#include "config.h"
#endif
-#include <pthread.h>
-#include <errno.h>
-
-#include <rtems/system.h>
-#include <rtems/score/watchdog.h>
#include <rtems/posix/condimpl.h>
-#include <rtems/posix/muteximpl.h>
/*
* _POSIX_Condition_variables_Signal_support
@@ -38,35 +32,39 @@ int _POSIX_Condition_variables_Signal_support(
bool is_broadcast
)
{
- POSIX_Condition_variables_Control *the_cond;
- Objects_Locations location;
- Thread_Control *the_thread;
+ Thread_Control *the_thread;
- the_cond = _POSIX_Condition_variables_Get( cond, &location );
- switch ( location ) {
+ do {
+ POSIX_Condition_variables_Control *the_cond;
+ ISR_lock_Context lock_context;
- case OBJECTS_LOCAL:
- do {
- the_thread = _Thread_queue_Dequeue(
- &the_cond->Wait_queue,
- POSIX_CONDITION_VARIABLES_TQ_OPERATIONS,
- NULL,
- 0
- );
- if ( !the_thread )
- the_cond->Mutex = POSIX_CONDITION_VARIABLES_NO_MUTEX;
- } while ( is_broadcast && the_thread );
+ the_cond = _POSIX_Condition_variables_Get( cond, &lock_context );
- _Objects_Put( &the_cond->Object );
+ if ( the_cond == NULL ) {
+ return EINVAL;
+ }
- return 0;
+ _POSIX_Condition_variables_Acquire_critical( the_cond, &lock_context );
-#if defined(RTEMS_MULTIPROCESSING)
- case OBJECTS_REMOTE:
-#endif
- case OBJECTS_ERROR:
- break;
- }
+ the_thread = _Thread_queue_First_locked(
+ &the_cond->Wait_queue,
+ POSIX_CONDITION_VARIABLES_TQ_OPERATIONS
+ );
+
+ if ( the_thread != NULL ) {
+ _Thread_queue_Extract_critical(
+ &the_cond->Wait_queue.Queue,
+ POSIX_CONDITION_VARIABLES_TQ_OPERATIONS,
+ the_thread,
+ NULL,
+ 0,
+ &lock_context
+ );
+ } else {
+ the_cond->mutex = POSIX_CONDITION_VARIABLES_NO_MUTEX;
+ _POSIX_Condition_variables_Release( the_cond, &lock_context );
+ }
+ } while ( is_broadcast && the_thread != NULL );
- return EINVAL;
+ return 0;
}
diff --git a/cpukit/posix/src/condwaitsupp.c b/cpukit/posix/src/condwaitsupp.c
index c907f16..aa320e8 100644
--- a/cpukit/posix/src/condwaitsupp.c
+++ b/cpukit/posix/src/condwaitsupp.c
@@ -18,15 +18,11 @@
#include "config.h"
#endif
-#include <pthread.h>
-#include <errno.h>
-
-#include <rtems/system.h>
-#include <rtems/score/watchdog.h>
-#include <rtems/score/statesimpl.h>
-#include <rtems/score/threadimpl.h>
#include <rtems/posix/condimpl.h>
#include <rtems/posix/muteximpl.h>
+#include <rtems/score/assert.h>
+#include <rtems/score/statesimpl.h>
+#include <rtems/score/threadimpl.h>
THREAD_WAIT_QUEUE_OBJECT_ASSERT(
POSIX_Condition_variables_Control,
@@ -41,88 +37,109 @@ int _POSIX_Condition_variables_Wait_support(
)
{
POSIX_Condition_variables_Control *the_cond;
- Objects_Locations location;
+ POSIX_Mutex_Control *the_mutex;
+ ISR_lock_Context lock_context;
int status;
int mutex_status;
+ CORE_mutex_Status core_mutex_status;
+ Per_CPU_Control *cpu_self;
Thread_Control *executing;
- the_cond = _POSIX_Condition_variables_Get( cond, &location );
- switch ( location ) {
-
- case OBJECTS_LOCAL:
-
- if ( the_cond->Mutex && ( the_cond->Mutex != *mutex ) ) {
- _Objects_Put( &the_cond->Object );
- return EINVAL;
- }
-
-
- mutex_status = pthread_mutex_unlock( mutex );
- /*
- * Historically, we ignored the return code since the behavior
- * is undefined by POSIX. But GNU/Linux returns EPERM in this
- * case, so we follow their lead.
- */
- if ( mutex_status ) {
- _Objects_Put( &the_cond->Object );
- return EPERM;
- }
-
- if ( !already_timedout ) {
- the_cond->Mutex = *mutex;
-
- executing = _Thread_Executing;
- executing->Wait.return_code = 0;
-
- _Thread_queue_Enqueue(
- &the_cond->Wait_queue,
- POSIX_CONDITION_VARIABLES_TQ_OPERATIONS,
- executing,
- STATES_WAITING_FOR_CONDITION_VARIABLE
- | STATES_INTERRUPTIBLE_BY_SIGNAL,
- timeout,
- ETIMEDOUT
- );
-
- _Objects_Put( &the_cond->Object );
-
- /*
- * Switch ourself out because we blocked as a result of the
- * _Thread_queue_Enqueue.
- */
-
- /*
- * If the thread is interrupted, while in the thread queue, by
- * a POSIX signal, then pthread_cond_wait returns spuriously,
- * according to the POSIX standard. It means that pthread_cond_wait
- * returns a success status, except for the fact that it was not
- * woken up a pthread_cond_signal or a pthread_cond_broadcast.
- */
- status = executing->Wait.return_code;
- if ( status == EINTR )
- status = 0;
-
- } else {
- _Objects_Put( &the_cond->Object );
- status = ETIMEDOUT;
- }
-
- /*
- * When we get here the dispatch disable level is 0.
- */
-
- mutex_status = pthread_mutex_lock( mutex );
- if ( mutex_status )
- return EINVAL;
-
- return status;
-
-#if defined(RTEMS_MULTIPROCESSING)
- case OBJECTS_REMOTE:
-#endif
- case OBJECTS_ERROR:
- break;
+ if ( mutex == NULL ) {
+ return EINVAL;
+ }
+
+ the_cond = _POSIX_Condition_variables_Get( cond, &lock_context );
+
+ if ( the_cond == NULL ) {
+ return EINVAL;
+ }
+
+ _POSIX_Condition_variables_Acquire_critical( the_cond, &lock_context );
+
+ if (
+ the_cond->mutex != POSIX_CONDITION_VARIABLES_NO_MUTEX
+ && the_cond->mutex != *mutex
+ ) {
+ _POSIX_Condition_variables_Release( the_cond, &lock_context );
+ return EINVAL;
+ }
+
+ the_cond->mutex = *mutex;
+
+ cpu_self = _Thread_Dispatch_disable_critical( &lock_context );
+ executing = _Per_CPU_Get_executing( cpu_self );
+
+ /*
+ * Historically, we ignored the unlock status since the behavior
+ * is undefined by POSIX. But GNU/Linux returns EPERM in this
+ * case, so we follow their lead.
+ */
+
+ the_mutex = _POSIX_Mutex_Get_no_protection( mutex );
+ if (
+ the_mutex == NULL
+ || !_CORE_mutex_Is_owner( &the_mutex->Mutex, executing )
+ ) {
+ _POSIX_Condition_variables_Release( the_cond, &lock_context );
+ return EPERM;
+ }
+
+ if ( !already_timedout ) {
+ executing->Wait.return_code = 0;
+ _Thread_queue_Enqueue_critical(
+ &the_cond->Wait_queue.Queue,
+ POSIX_CONDITION_VARIABLES_TQ_OPERATIONS,
+ executing,
+ STATES_WAITING_FOR_CONDITION_VARIABLE,
+ timeout,
+ ETIMEDOUT,
+ &lock_context
+ );
+ } else {
+ _POSIX_Condition_variables_Release( the_cond, &lock_context );
+ executing->Wait.return_code = ETIMEDOUT;
+ }
+
+ _ISR_lock_ISR_disable( &lock_context );
+ core_mutex_status = _CORE_mutex_Surrender(
+ &the_mutex->Mutex,
+ NULL,
+ 0,
+ &lock_context
+ );
+ _Assert( core_mutex_status == CORE_MUTEX_STATUS_SUCCESSFUL );
+ (void) core_mutex_status;
+
+ /*
+ * Switch ourself out because we blocked as a result of the
+ * _Thread_queue_Enqueue_critical().
+ */
+
+ _Thread_Dispatch_enable( cpu_self );
+
+ status = (int) executing->Wait.return_code;
+
+ /*
+ * If the thread is interrupted, while in the thread queue, by
+ * a POSIX signal, then pthread_cond_wait returns spuriously,
+ * according to the POSIX standard. It means that pthread_cond_wait
+ * returns a success status, except for the fact that it was not
+ * woken up a pthread_cond_signal() or a pthread_cond_broadcast().
+ */
+
+ if ( status == EINTR ) {
+ status = 0;
+ }
+
+ /*
+ * When we get here the dispatch disable level is 0.
+ */
+
+ mutex_status = pthread_mutex_lock( mutex );
+ if ( mutex_status != 0 ) {
+ return EINVAL;
}
- return EINVAL;
+ return status;
}
diff --git a/cpukit/score/include/rtems/score/coremuteximpl.h b/cpukit/score/include/rtems/score/coremuteximpl.h
index 49404ce..4b144e8 100644
--- a/cpukit/score/include/rtems/score/coremuteximpl.h
+++ b/cpukit/score/include/rtems/score/coremuteximpl.h
@@ -385,6 +385,14 @@ RTEMS_INLINE_ROUTINE bool _CORE_mutex_Is_locked(
return the_mutex->holder != NULL;
}
+RTEMS_INLINE_ROUTINE bool _CORE_mutex_Is_owner(
+ const CORE_mutex_Control *the_mutex,
+ const Thread_Control *the_thread
+)
+{
+ return the_mutex->holder == the_thread;
+}
+
/**
* @brief Does core mutex use FIFO blocking.
*
--
1.8.4.5
More information about the devel
mailing list