From 7f4ee2b4ae39928ab5f449048e562ef6b2c5d17d Mon Sep 17 00:00:00 2001 From: Sebastian Huber Date: Fri, 22 Apr 2016 14:37:13 +0200 Subject: posix: Avoid Giant lock for condition variables 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/condbroadcast.c | 6 - cpukit/posix/src/conddestroy.c | 55 +++---- cpukit/posix/src/condget.c | 62 ++++---- cpukit/posix/src/condinit.c | 12 +- cpukit/posix/src/condsignal.c | 6 - cpukit/posix/src/condsignalsupp.c | 60 ++++---- cpukit/posix/src/condtimedwait.c | 7 +- cpukit/posix/src/condwait.c | 6 - cpukit/posix/src/condwaitsupp.c | 188 +++++++++++++---------- cpukit/score/include/rtems/score/coremuteximpl.h | 8 + 13 files changed, 241 insertions(+), 234 deletions(-) diff --git a/cpukit/posix/include/rtems/posix/cond.h b/cpukit/posix/include/rtems/posix/cond.h index 1839279591..4fa7de7525 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 b079a43233..b17886948e 100644 --- a/cpukit/posix/include/rtems/posix/condimpl.h +++ b/cpukit/posix/include/rtems/posix/condimpl.h @@ -20,7 +20,8 @@ #include #include #include -#include + +#include #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 fb30d5810e..6d21e96f6e 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/condbroadcast.c b/cpukit/posix/src/condbroadcast.c index db73837d09..3a19d7ebb7 100644 --- a/cpukit/posix/src/condbroadcast.c +++ b/cpukit/posix/src/condbroadcast.c @@ -18,13 +18,7 @@ #include "config.h" #endif -#include -#include - -#include -#include #include -#include /** * 11.4.3 Broadcasting and Signaling a Condition, P1003.1c/Draft 10, p. 101 diff --git a/cpukit/posix/src/conddestroy.c b/cpukit/posix/src/conddestroy.c index c6696f506c..d47c6b2bed 100644 --- a/cpukit/posix/src/conddestroy.c +++ b/cpukit/posix/src/conddestroy.c @@ -18,13 +18,7 @@ #include "config.h" #endif -#include -#include - -#include -#include #include -#include /** * 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 6ebba730a3..e3cf59c4a5 100644 --- a/cpukit/posix/src/condget.c +++ b/cpukit/posix/src/condget.c @@ -11,45 +11,49 @@ #include "config.h" #endif -#include -#include - -#include -#include #include -#include -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 f4aeae657d..dde400f58d 100644 --- a/cpukit/posix/src/condinit.c +++ b/cpukit/posix/src/condinit.c @@ -18,13 +18,7 @@ #include "config.h" #endif -#include -#include - -#include -#include #include -#include /** * 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/condsignal.c b/cpukit/posix/src/condsignal.c index 6fd743fd6b..6f826c26b9 100644 --- a/cpukit/posix/src/condsignal.c +++ b/cpukit/posix/src/condsignal.c @@ -18,13 +18,7 @@ #include "config.h" #endif -#include -#include - -#include -#include #include -#include /** * 11.4.3 Broadcasting and Signaling a Condition, P1003.1c/Draft 10, p. 101 diff --git a/cpukit/posix/src/condsignalsupp.c b/cpukit/posix/src/condsignalsupp.c index 9c67ddc030..c625b3a18f 100644 --- a/cpukit/posix/src/condsignalsupp.c +++ b/cpukit/posix/src/condsignalsupp.c @@ -18,13 +18,7 @@ #include "config.h" #endif -#include -#include - -#include -#include #include -#include /* * _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/condtimedwait.c b/cpukit/posix/src/condtimedwait.c index 3c978e41b8..1a2f5ff542 100644 --- a/cpukit/posix/src/condtimedwait.c +++ b/cpukit/posix/src/condtimedwait.c @@ -18,13 +18,8 @@ #include "config.h" #endif -#include -#include - -#include -#include #include -#include +#include /* * 11.4.4 Waiting on a Condition, P1003.1c/Draft 10, p. 105 diff --git a/cpukit/posix/src/condwait.c b/cpukit/posix/src/condwait.c index 787c91fadf..22004f8dbd 100644 --- a/cpukit/posix/src/condwait.c +++ b/cpukit/posix/src/condwait.c @@ -18,13 +18,7 @@ #include "config.h" #endif -#include -#include - -#include -#include #include -#include /* * 11.4.4 Waiting on a Condition, P1003.1c/Draft 10, p. 105 diff --git a/cpukit/posix/src/condwaitsupp.c b/cpukit/posix/src/condwaitsupp.c index c907f16cdd..b578a5f76f 100644 --- a/cpukit/posix/src/condwaitsupp.c +++ b/cpukit/posix/src/condwaitsupp.c @@ -18,15 +18,11 @@ #include "config.h" #endif -#include -#include - -#include -#include -#include -#include #include #include +#include +#include +#include THREAD_WAIT_QUEUE_OBJECT_ASSERT( POSIX_Condition_variables_Control, @@ -41,88 +37,110 @@ 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 ); + _Thread_Dispatch_enable( cpu_self ); + 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 49404ce6c0..4b144e8836 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. * -- cgit v1.2.3