From 5a5fb3b9d6d99d6751d129458217f1a3b5b85ff8 Mon Sep 17 00:00:00 2001 From: Sebastian Huber Date: Fri, 18 Mar 2016 14:03:01 +0100 Subject: score: Avoid Giant lock for CORE spinlock Use an ISR lock to protect the spinlock state. Remove empty attributes. Update #2555. --- cpukit/score/Makefile.am | 2 +- cpukit/score/include/rtems/score/corespinlock.h | 30 +++++------ .../score/include/rtems/score/corespinlockimpl.h | 55 +++++++++++-------- cpukit/score/src/corespinlock.c | 37 ------------- cpukit/score/src/corespinlockrelease.c | 19 +++---- cpukit/score/src/corespinlockwait.c | 63 +++++++++------------- 6 files changed, 80 insertions(+), 126 deletions(-) delete mode 100644 cpukit/score/src/corespinlock.c (limited to 'cpukit/score') diff --git a/cpukit/score/Makefile.am b/cpukit/score/Makefile.am index b6824ad446..6ff4e02adf 100644 --- a/cpukit/score/Makefile.am +++ b/cpukit/score/Makefile.am @@ -194,7 +194,7 @@ libscore_a_SOURCES += src/coresem.c ## CORE_SPINLOCK_C_FILES if HAS_PTHREADS -libscore_a_SOURCES += src/corespinlock.c src/corespinlockrelease.c \ +libscore_a_SOURCES += src/corespinlockrelease.c \ src/corespinlockwait.c endif diff --git a/cpukit/score/include/rtems/score/corespinlock.h b/cpukit/score/include/rtems/score/corespinlock.h index ca50eed5e2..1666538bf6 100644 --- a/cpukit/score/include/rtems/score/corespinlock.h +++ b/cpukit/score/include/rtems/score/corespinlock.h @@ -19,7 +19,8 @@ #ifndef _RTEMS_SCORE_CORESPINLOCK_H #define _RTEMS_SCORE_CORESPINLOCK_H -#include +#include +#include #ifdef __cplusplus extern "C" { @@ -35,38 +36,35 @@ extern "C" { */ /**@{*/ -/** - * The following defines the control block used to manage the - * attributes of each spinlock. - */ -typedef struct { - /** This element indicates XXX - */ - uint32_t XXX; -} CORE_spinlock_Attributes; - /** * The following defines the control block used to manage each * spinlock. */ typedef struct { - /** XXX may not be needed */ - CORE_spinlock_Attributes Attributes; + /** + * @brief Lock to protect the other fields. + * + * This implementation is a bit stupid. However, test cases in the Linux + * Test Project do things like sleep() and printf() while owning a + * pthread_spinlock_t, e.g. + * testcases/open_posix_testsuite/conformance/interfaces/pthread_spin_lock/1-2.c + */ + ISR_LOCK_MEMBER( Lock ) /** This field is the lock. */ - volatile uint32_t lock; + uint32_t lock; /** This field is a count of the current number of threads using * this spinlock. It includes the thread holding the lock as well * as those waiting. */ - volatile uint32_t users; + uint32_t users; /** This field is the Id of the thread holding the lock. It may or may * not be the thread which acquired it. */ - volatile Objects_Id holder; + Thread_Control *holder; } CORE_spinlock_Control; /**@}*/ diff --git a/cpukit/score/include/rtems/score/corespinlockimpl.h b/cpukit/score/include/rtems/score/corespinlockimpl.h index fe6f9b67f5..189bddb243 100644 --- a/cpukit/score/include/rtems/score/corespinlockimpl.h +++ b/cpukit/score/include/rtems/score/corespinlockimpl.h @@ -22,6 +22,8 @@ #include #include +#include + #ifdef __cplusplus extern "C" { #endif @@ -78,12 +80,29 @@ typedef enum { * This routine initializes the spinlock based on the parameters passed. * * @param[in] the_spinlock is the spinlock control block to initialize - * @param[in] the_spinlock_attributes define the behavior of this instance */ -void _CORE_spinlock_Initialize( - CORE_spinlock_Control *the_spinlock, - CORE_spinlock_Attributes *the_spinlock_attributes -); +RTEMS_INLINE_ROUTINE void _CORE_spinlock_Initialize( + CORE_spinlock_Control *the_spinlock +) +{ + memset( the_spinlock, 0, sizeof( *the_spinlock ) ); +} + +RTEMS_INLINE_ROUTINE void _CORE_spinlock_Acquire_critical( + CORE_spinlock_Control *the_spinlock, + ISR_lock_Context *lock_context +) +{ + _ISR_lock_Acquire( &the_spinlock->Lock, lock_context ); +} + +RTEMS_INLINE_ROUTINE void _CORE_spinlock_Release( + CORE_spinlock_Control *the_spinlock, + ISR_lock_Context *lock_context +) +{ + _ISR_lock_Release_and_ISR_enable( &the_spinlock->Lock, lock_context ); +} /** * @brief Wait for spinlock. @@ -100,10 +119,11 @@ void _CORE_spinlock_Initialize( * @retval A status is returned which indicates the success or failure of * this operation. */ -CORE_spinlock_Status _CORE_spinlock_Wait( - CORE_spinlock_Control *the_spinlock, - bool wait, - Watchdog_Interval timeout +CORE_spinlock_Status _CORE_spinlock_Seize( + CORE_spinlock_Control *the_spinlock, + bool wait, + Watchdog_Interval timeout, + ISR_lock_Context *lock_context ); /** @@ -114,22 +134,11 @@ CORE_spinlock_Status _CORE_spinlock_Wait( * * @param[in] the_spinlock is the spinlock to surrender */ -CORE_spinlock_Status _CORE_spinlock_Release( - CORE_spinlock_Control *the_spinlock +CORE_spinlock_Status _CORE_spinlock_Surrender( + CORE_spinlock_Control *the_spinlock, + ISR_lock_Context *lock_context ); -/** - * This method is used to initialize core spinlock attributes. - * - * @param[in] the_attributes pointer to the attributes to initialize. - */ -RTEMS_INLINE_ROUTINE void _CORE_spinlock_Initialize_attributes( - CORE_spinlock_Attributes *the_attributes -) -{ - the_attributes->XXX = 0; -} - /** * This method is used to determine if the spinlock is available or not. * diff --git a/cpukit/score/src/corespinlock.c b/cpukit/score/src/corespinlock.c deleted file mode 100644 index b84eb2e14e..0000000000 --- a/cpukit/score/src/corespinlock.c +++ /dev/null @@ -1,37 +0,0 @@ -/** - * @file - * - * @brief Initialize a Spinlock - * - * @ingroup ScoreSpinlock - */ - -/* - * COPYRIGHT (c) 1989-2006. - * On-Line Applications Research Corporation (OAR). - * - * The license and distribution terms for this file may be - * found in the file LICENSE in this distribution or at - * http://www.rtems.org/license/LICENSE. - */ - -#if HAVE_CONFIG_H -#include "config.h" -#endif - -#include -#include -#include - -void _CORE_spinlock_Initialize( - CORE_spinlock_Control *the_spinlock, - CORE_spinlock_Attributes *the_spinlock_attributes -) -{ - - the_spinlock->Attributes = *the_spinlock_attributes; - - the_spinlock->lock = 0; - the_spinlock->users = 0; - the_spinlock->holder = 0; -} diff --git a/cpukit/score/src/corespinlockrelease.c b/cpukit/score/src/corespinlockrelease.c index c10337a123..358d352955 100644 --- a/cpukit/score/src/corespinlockrelease.c +++ b/cpukit/score/src/corespinlockrelease.c @@ -20,30 +20,27 @@ #include #include -#include -#include -CORE_spinlock_Status _CORE_spinlock_Release( - CORE_spinlock_Control *the_spinlock +CORE_spinlock_Status _CORE_spinlock_Surrender( + CORE_spinlock_Control *the_spinlock, + ISR_lock_Context *lock_context ) { - ISR_Level level; - - _ISR_Disable( level ); + _CORE_spinlock_Acquire_critical( the_spinlock, lock_context ); /* * It must locked before it can be unlocked. */ if ( the_spinlock->lock == CORE_SPINLOCK_UNLOCKED ) { - _ISR_Enable( level ); + _CORE_spinlock_Release( the_spinlock, lock_context ); return CORE_SPINLOCK_NOT_LOCKED; } /* * It must locked by the current thread before it can be unlocked. */ - if ( the_spinlock->holder != _Thread_Executing->Object.id ) { - _ISR_Enable( level ); + if ( the_spinlock->holder != _Thread_Executing ) { + _CORE_spinlock_Release( the_spinlock, lock_context ); return CORE_SPINLOCK_NOT_HOLDER; } @@ -54,6 +51,6 @@ CORE_spinlock_Status _CORE_spinlock_Release( the_spinlock->lock = CORE_SPINLOCK_UNLOCKED; the_spinlock->holder = 0; - _ISR_Enable( level ); + _CORE_spinlock_Release( the_spinlock, lock_context ); return CORE_SPINLOCK_SUCCESSFUL; } diff --git a/cpukit/score/src/corespinlockwait.c b/cpukit/score/src/corespinlockwait.c index 1f102962ac..cc939c2344 100644 --- a/cpukit/score/src/corespinlockwait.c +++ b/cpukit/score/src/corespinlockwait.c @@ -18,48 +18,36 @@ #include "config.h" #endif -#include #include -#include -#include +#include -/* - * _CORE_spinlock_Wait - * - * This function waits for the spinlock to become available. Optionally, - * a limit may be placed on the duration of the spin. - * - * Input parameters: - * the_spinlock - the spinlock control block to initialize - * wait - true if willing to wait - * timeout - the maximum number of ticks to spin (0 is forever) - * - * Output parameters: NONE - */ - -CORE_spinlock_Status _CORE_spinlock_Wait( - CORE_spinlock_Control *the_spinlock, - bool wait, - Watchdog_Interval timeout +CORE_spinlock_Status _CORE_spinlock_Seize( + CORE_spinlock_Control *the_spinlock, + bool wait, + Watchdog_Interval timeout, + ISR_lock_Context *lock_context ) { - ISR_Level level; + Thread_Control *executing; + #if defined(FUNCTIONALITY_NOT_CURRENTLY_USED_BY_ANY_API) Watchdog_Interval limit = _Watchdog_Ticks_since_boot + timeout; #endif - _ISR_Disable( level ); - if ( (the_spinlock->lock == CORE_SPINLOCK_LOCKED) && - (the_spinlock->holder == _Thread_Executing->Object.id) ) { - _ISR_Enable( level ); + executing = _Thread_Executing; + + _CORE_spinlock_Acquire_critical( the_spinlock, lock_context ); + if ( the_spinlock->lock == CORE_SPINLOCK_LOCKED && + the_spinlock->holder == executing ) { + _CORE_spinlock_Release( the_spinlock, lock_context ); return CORE_SPINLOCK_HOLDER_RELOCKING; } the_spinlock->users += 1; for ( ;; ) { if ( the_spinlock->lock == CORE_SPINLOCK_UNLOCKED ) { the_spinlock->lock = CORE_SPINLOCK_LOCKED; - the_spinlock->holder = _Thread_Executing->Object.id; - _ISR_Enable( level ); + the_spinlock->holder = executing; + _CORE_spinlock_Release( the_spinlock, lock_context ); return CORE_SPINLOCK_SUCCESSFUL; } @@ -68,7 +56,7 @@ CORE_spinlock_Status _CORE_spinlock_Wait( */ if ( !wait ) { the_spinlock->users -= 1; - _ISR_Enable( level ); + _CORE_spinlock_Release( the_spinlock, lock_context ); return CORE_SPINLOCK_UNAVAILABLE; } @@ -78,7 +66,7 @@ CORE_spinlock_Status _CORE_spinlock_Wait( */ if ( timeout && (limit <= _Watchdog_Ticks_since_boot) ) { the_spinlock->users -= 1; - _ISR_Enable( level ); + _CORE_spinlock_Release( the_spinlock, lock_context ); return CORE_SPINLOCK_TIMEOUT; } #endif @@ -100,16 +88,15 @@ CORE_spinlock_Status _CORE_spinlock_Wait( * safe from deletion. */ - _ISR_Enable( level ); - /* An ISR could occur here */ - - _Thread_Enable_dispatch(); - /* Another thread could get dispatched here */ + _CORE_spinlock_Release( the_spinlock, lock_context ); - /* Reenter the critical sections so we can attempt the lock again. */ - _Thread_Disable_dispatch(); + /* + * An ISR could occur here. Another thread could get dispatched here. + * Reenter the critical sections so we can attempt the lock again. + */ - _ISR_Disable( level ); + _ISR_lock_ISR_disable( lock_context ); + _CORE_spinlock_Acquire_critical( the_spinlock, lock_context ); } } -- cgit v1.2.3