[PATCH 3/6] score: Use an ISR lock for Per_CPU_Control::Lock
Sebastian Huber
sebastian.huber at embedded-brains.de
Thu Apr 11 13:45:37 UTC 2019
The use of a hand crafted lock for Per_CPU_Control::Lock was necessary
at some point in the SMP support development, but it is no longer
justified.
---
cpukit/include/rtems/score/percpu.h | 161 +++++++++++++-------------
cpukit/include/rtems/score/schedulersmpimpl.h | 12 +-
cpukit/include/rtems/score/threadimpl.h | 6 +-
cpukit/include/rtems/score/userextimpl.h | 5 +-
cpukit/posix/src/pspinlock.c | 11 +-
cpukit/posix/src/pspinunlock.c | 13 ++-
cpukit/score/src/schedulersmp.c | 13 ++-
cpukit/score/src/smp.c | 6 +-
cpukit/score/src/threaddispatch.c | 17 +--
cpukit/score/src/userextaddset.c | 6 +-
cpukit/score/src/userextremoveset.c | 6 +-
11 files changed, 133 insertions(+), 123 deletions(-)
diff --git a/cpukit/include/rtems/score/percpu.h b/cpukit/include/rtems/score/percpu.h
index 8e33aaee4a..d7232c632f 100644
--- a/cpukit/include/rtems/score/percpu.h
+++ b/cpukit/include/rtems/score/percpu.h
@@ -28,7 +28,6 @@
#include <rtems/score/chain.h>
#include <rtems/score/isrlock.h>
#include <rtems/score/smp.h>
- #include <rtems/score/smplock.h>
#include <rtems/score/timestamp.h>
#include <rtems/score/watchdog.h>
#endif
@@ -333,7 +332,7 @@ typedef struct Per_CPU_Control {
*
* It is volatile since interrupts may alter this flag.
*
- * This field is not protected by a lock and must be accessed only by this
+ * This member is not protected by a lock and must be accessed only by this
* processor. Code (e.g. scheduler and post-switch action requests) running
* on another processors must use an inter-processor interrupt to set the
* thread dispatch necessary indicator to true.
@@ -352,7 +351,8 @@ typedef struct Per_CPU_Control {
/**
* @brief This is the thread executing on this processor.
*
- * This field is not protected by a lock. The only writer is this processor.
+ * This member is not protected by a lock. The only writer is this
+ * processor.
*
* On SMP configurations a thread may be registered as executing on more than
* one processor in case a thread migration is in progress. On SMP
@@ -364,10 +364,10 @@ typedef struct Per_CPU_Control {
/**
* @brief This is the heir thread for this processor.
*
- * This field is not protected by a lock. The only writer after multitasking
- * start is the scheduler owning this processor. It is assumed that stores
- * to pointers are atomic on all supported SMP architectures. The CPU port
- * specific code (inter-processor interrupt handling and
+ * This member is not protected by a lock. The only writer after
+ * multitasking start is the scheduler owning this processor. It is assumed
+ * that stores to pointers are atomic on all supported SMP architectures.
+ * The CPU port specific code (inter-processor interrupt handling and
* _CPU_SMP_Send_interrupt()) must guarantee that this processor observes the
* last value written.
*
@@ -418,39 +418,31 @@ typedef struct Per_CPU_Control {
#if defined( RTEMS_SMP )
/**
- * @brief This lock protects some parts of the low-level thread dispatching.
+ * @brief This lock protects some members of this structure.
+ */
+ ISR_lock_Control Lock;
+
+ /**
+ * @brief Lock context used to acquire all per-CPU locks.
*
- * We must use a ticket lock here since we cannot transport a local context
- * through the context switch.
+ * This member is protected by the Per_CPU_Control::Lock lock.
*
- * @see _Thread_Dispatch().
+ * @see _Per_CPU_Acquire_all().
*/
- SMP_ticket_lock_Control Lock;
-
- #if defined( RTEMS_PROFILING )
- /**
- * @brief Lock statistics for the per-CPU lock.
- */
- SMP_lock_Stats Lock_stats;
-
- /**
- * @brief Lock statistics context for the per-CPU lock.
- */
- SMP_lock_Stats_context Lock_stats_context;
- #endif
+ ISR_lock_Context Lock_context;
/**
* @brief Chain of threads in need for help.
*
- * This field is protected by the Per_CPU_Control::Lock lock.
+ * This member is protected by the Per_CPU_Control::Lock lock.
*/
Chain_Control Threads_in_need_for_help;
/**
* @brief Bit field for SMP messages.
*
- * This bit field is not protected locks. Atomic operations are used to
- * set and get the message bits.
+ * This member is not protected locks. Atomic operations are used to set
+ * and get the message bits.
*/
Atomic_Ulong message;
@@ -488,7 +480,7 @@ typedef struct Per_CPU_Control {
/**
* @brief Indicates the current state of the CPU.
*
- * This field is protected by the _Per_CPU_State_lock lock.
+ * This member is protected by the _Per_CPU_State_lock lock.
*
* @see _Per_CPU_State_change().
*/
@@ -539,62 +531,11 @@ typedef struct {
*/
extern Per_CPU_Control_envelope _Per_CPU_Information[] CPU_STRUCTURE_ALIGNMENT;
-#if defined( RTEMS_SMP )
-#define _Per_CPU_Acquire( cpu ) \
- _SMP_ticket_lock_Acquire( \
- &( cpu )->Lock, \
- &( cpu )->Lock_stats, \
- &( cpu )->Lock_stats_context \
- )
-#else
-#define _Per_CPU_Acquire( cpu ) \
- do { \
- (void) ( cpu ); \
- } while ( 0 )
-#endif
-
-#if defined( RTEMS_SMP )
-#define _Per_CPU_Release( cpu ) \
- _SMP_ticket_lock_Release( \
- &( cpu )->Lock, \
- &( cpu )->Lock_stats_context \
- )
-#else
-#define _Per_CPU_Release( cpu ) \
- do { \
- (void) ( cpu ); \
- } while ( 0 )
-#endif
+#define _Per_CPU_Acquire( cpu, lock_context ) \
+ _ISR_lock_Acquire( &( cpu )->Lock, lock_context )
-#if defined( RTEMS_SMP )
-#define _Per_CPU_Acquire_all( isr_cookie ) \
- do { \
- uint32_t ncpus = _SMP_Get_processor_maximum(); \
- uint32_t cpu; \
- _ISR_Local_disable( isr_cookie ); \
- for ( cpu = 0 ; cpu < ncpus ; ++cpu ) { \
- _Per_CPU_Acquire( _Per_CPU_Get_by_index( cpu ) ); \
- } \
- } while ( 0 )
-#else
-#define _Per_CPU_Acquire_all( isr_cookie ) \
- _ISR_Local_disable( isr_cookie )
-#endif
-
-#if defined( RTEMS_SMP )
-#define _Per_CPU_Release_all( isr_cookie ) \
- do { \
- uint32_t ncpus = _SMP_Get_processor_maximum(); \
- uint32_t cpu; \
- for ( cpu = 0 ; cpu < ncpus ; ++cpu ) { \
- _Per_CPU_Release( _Per_CPU_Get_by_index( cpu ) ); \
- } \
- _ISR_Local_enable( isr_cookie ); \
- } while ( 0 )
-#else
-#define _Per_CPU_Release_all( isr_cookie ) \
- _ISR_Local_enable( isr_cookie )
-#endif
+#define _Per_CPU_Release( cpu, lock_context ) \
+ _ISR_lock_Release( &( cpu )->Lock, lock_context )
/*
* If we get the current processor index in a context which allows thread
@@ -671,6 +612,60 @@ static inline bool _Per_CPU_Is_boot_processor(
#endif
}
+RTEMS_INLINE_ROUTINE void _Per_CPU_Acquire_all(
+ ISR_lock_Context *lock_context
+)
+{
+#if defined(RTEMS_SMP)
+ uint32_t cpu_max;
+ uint32_t cpu_index;
+ Per_CPU_Control *previous_cpu;
+
+ cpu_max = _SMP_Get_processor_maximum();
+ previous_cpu = _Per_CPU_Get_by_index( 0 );
+
+ _ISR_lock_ISR_disable( lock_context );
+ _Per_CPU_Acquire( previous_cpu, lock_context );
+
+ for ( cpu_index = 1 ; cpu_index < cpu_max ; ++cpu_index ) {
+ Per_CPU_Control *cpu;
+
+ cpu = _Per_CPU_Get_by_index( cpu_index );
+ _Per_CPU_Acquire( cpu, &previous_cpu->Lock_context );
+ previous_cpu = cpu;
+ }
+#else
+ _ISR_lock_ISR_disable( lock_context )
+#endif
+}
+
+RTEMS_INLINE_ROUTINE void _Per_CPU_Release_all(
+ ISR_lock_Context *lock_context
+)
+{
+#if defined(RTEMS_SMP)
+ uint32_t cpu_max;
+ uint32_t cpu_index;
+ Per_CPU_Control *cpu;
+
+ cpu_max = _SMP_Get_processor_maximum();
+ cpu = _Per_CPU_Get_by_index( cpu_max - 1 );
+
+ for ( cpu_index = cpu_max - 1 ; cpu_index > 0 ; --cpu_index ) {
+ Per_CPU_Control *previous_cpu;
+
+ previous_cpu = _Per_CPU_Get_by_index( cpu_index - 1 );
+ _Per_CPU_Release( cpu, &previous_cpu->Lock_context );
+ cpu = previous_cpu;
+ }
+
+ _Per_CPU_Release( cpu, lock_context );
+ _ISR_lock_ISR_enable( lock_context );
+#else
+ _ISR_lock_ISR_enable( lock_context )
+#endif
+}
+
#if defined( RTEMS_SMP )
/**
diff --git a/cpukit/include/rtems/score/schedulersmpimpl.h b/cpukit/include/rtems/score/schedulersmpimpl.h
index 0f4805d7f5..f7b5fcc1e2 100644
--- a/cpukit/include/rtems/score/schedulersmpimpl.h
+++ b/cpukit/include/rtems/score/schedulersmpimpl.h
@@ -566,13 +566,13 @@ static inline Thread_Control *_Scheduler_SMP_Preempt(
)
{
Thread_Control *victim_thread;
- ISR_lock_Context lock_context;
+ ISR_lock_Context scheduler_lock_context;
Per_CPU_Control *victim_cpu;
victim_thread = _Scheduler_Node_get_user( victim );
_Scheduler_SMP_Node_change_state( victim, SCHEDULER_SMP_NODE_READY );
- _Thread_Scheduler_acquire_critical( victim_thread, &lock_context );
+ _Thread_Scheduler_acquire_critical( victim_thread, &scheduler_lock_context );
victim_cpu = _Thread_Get_CPU( victim_thread );
@@ -580,16 +580,18 @@ static inline Thread_Control *_Scheduler_SMP_Preempt(
_Scheduler_Thread_change_state( victim_thread, THREAD_SCHEDULER_READY );
if ( victim_thread->Scheduler.helping_nodes > 0 ) {
- _Per_CPU_Acquire( victim_cpu );
+ ISR_lock_Context per_cpu_lock_context;
+
+ _Per_CPU_Acquire( victim_cpu, &per_cpu_lock_context );
_Chain_Append_unprotected(
&victim_cpu->Threads_in_need_for_help,
&victim_thread->Scheduler.Help_node
);
- _Per_CPU_Release( victim_cpu );
+ _Per_CPU_Release( victim_cpu, &per_cpu_lock_context );
}
}
- _Thread_Scheduler_release_critical( victim_thread, &lock_context );
+ _Thread_Scheduler_release_critical( victim_thread, &scheduler_lock_context );
_Scheduler_SMP_Allocate_processor(
context,
diff --git a/cpukit/include/rtems/score/threadimpl.h b/cpukit/include/rtems/score/threadimpl.h
index 2232d57dd0..75725aeb48 100644
--- a/cpukit/include/rtems/score/threadimpl.h
+++ b/cpukit/include/rtems/score/threadimpl.h
@@ -990,14 +990,16 @@ RTEMS_INLINE_ROUTINE void _Thread_Scheduler_cancel_need_for_help(
Per_CPU_Control *cpu
)
{
- _Per_CPU_Acquire( cpu );
+ ISR_lock_Context lock_context;
+
+ _Per_CPU_Acquire( cpu, &lock_context );
if ( !_Chain_Is_node_off_chain( &the_thread->Scheduler.Help_node ) ) {
_Chain_Extract_unprotected( &the_thread->Scheduler.Help_node );
_Chain_Set_off_chain( &the_thread->Scheduler.Help_node );
}
- _Per_CPU_Release( cpu );
+ _Per_CPU_Release( cpu, &lock_context );
}
#endif
diff --git a/cpukit/include/rtems/score/userextimpl.h b/cpukit/include/rtems/score/userextimpl.h
index b781773863..d311e220b3 100644
--- a/cpukit/include/rtems/score/userextimpl.h
+++ b/cpukit/include/rtems/score/userextimpl.h
@@ -255,6 +255,7 @@ static inline void _User_extensions_Thread_switch(
const Chain_Control *chain;
const Chain_Node *tail;
const Chain_Node *node;
+ ISR_lock_Context lock_context;
chain = &_User_extensions_Switches_list;
tail = _Chain_Immutable_tail( chain );
@@ -271,7 +272,7 @@ static inline void _User_extensions_Thread_switch(
#if defined(RTEMS_SMP)
_ISR_Local_disable( level );
#endif
- _Per_CPU_Acquire( cpu_self );
+ _Per_CPU_Acquire( cpu_self, &lock_context );
#if defined(RTEMS_SMP)
node = _Chain_Immutable_first( chain );
@@ -286,7 +287,7 @@ static inline void _User_extensions_Thread_switch(
node = _Chain_Immutable_next( node );
}
- _Per_CPU_Release( cpu_self );
+ _Per_CPU_Release( cpu_self, &lock_context );
#if defined(RTEMS_SMP)
_ISR_Local_enable( level );
#endif
diff --git a/cpukit/posix/src/pspinlock.c b/cpukit/posix/src/pspinlock.c
index 9bacd367b6..f505768426 100644
--- a/cpukit/posix/src/pspinlock.c
+++ b/cpukit/posix/src/pspinlock.c
@@ -58,20 +58,17 @@ int pthread_spin_lock( pthread_spinlock_t *spinlock )
POSIX_Spinlock_Control *the_spinlock;
ISR_Level level;
#if defined(RTEMS_SMP) && defined(RTEMS_PROFILING)
- Per_CPU_Control *cpu_self;
+ SMP_lock_Stats unused_stats;
+ SMP_lock_Stats_context unused_context;
#endif
the_spinlock = _POSIX_Spinlock_Get( spinlock );
_ISR_Local_disable( level );
#if defined(RTEMS_SMP)
-#if defined(RTEMS_PROFILING)
- /* The lock statistics are incorrect in case of nested pthread spinlocks */
- cpu_self = _Per_CPU_Get();
-#endif
_SMP_ticket_lock_Acquire(
&the_spinlock->Lock,
- &cpu_self->Lock_stats,
- &cpu_self->Lock_stats_context
+ &unused_stats,
+ &unused_context
);
#endif
the_spinlock->interrupt_state = level;
diff --git a/cpukit/posix/src/pspinunlock.c b/cpukit/posix/src/pspinunlock.c
index 74baa7cad7..bcbb2108ef 100644
--- a/cpukit/posix/src/pspinunlock.c
+++ b/cpukit/posix/src/pspinunlock.c
@@ -28,13 +28,24 @@ int pthread_spin_unlock( pthread_spinlock_t *lock )
{
POSIX_Spinlock_Control *the_spinlock;
ISR_Level level;
+#if defined(RTEMS_SMP) && defined(RTEMS_PROFILING)
+ SMP_lock_Stats unused_stats;
+ SMP_lock_Stats_context unused_context;
+#endif
the_spinlock = _POSIX_Spinlock_Get( lock );
level = the_spinlock->interrupt_state;
#if defined(RTEMS_SMP)
+#if defined(RTEMS_PROFILING)
+ /* This is a hack to get around the lock profiling statistics */
+ unused_stats.total_section_time = 0;
+ unused_stats.max_section_time = 0;
+ unused_context.stats = &unused_stats;
+ unused_context.acquire_instant = 0;
+#endif
_SMP_ticket_lock_Release(
&the_spinlock->Lock,
- &_Per_CPU_Get()->Lock_stats_context
+ &unused_context
);
#endif
_ISR_Local_enable( level );
diff --git a/cpukit/score/src/schedulersmp.c b/cpukit/score/src/schedulersmp.c
index d68ac4fc8b..a498bda90a 100644
--- a/cpukit/score/src/schedulersmp.c
+++ b/cpukit/score/src/schedulersmp.c
@@ -14,25 +14,26 @@
void _Scheduler_Request_ask_for_help( Thread_Control *the_thread )
{
- ISR_lock_Context lock_context;
+ ISR_lock_Context scheduler_lock_context;
- _Thread_Scheduler_acquire_critical( the_thread, &lock_context );
+ _Thread_Scheduler_acquire_critical( the_thread, &scheduler_lock_context );
if ( _Chain_Is_node_off_chain( &the_thread->Scheduler.Help_node ) ) {
- Per_CPU_Control *cpu;
+ Per_CPU_Control *cpu;
+ ISR_lock_Context per_cpu_lock_context;
cpu = _Thread_Get_CPU( the_thread );
- _Per_CPU_Acquire( cpu );
+ _Per_CPU_Acquire( cpu, &per_cpu_lock_context );
_Chain_Append_unprotected(
&cpu->Threads_in_need_for_help,
&the_thread->Scheduler.Help_node
);
- _Per_CPU_Release( cpu );
+ _Per_CPU_Release( cpu, &per_cpu_lock_context );
_Thread_Dispatch_request( _Per_CPU_Get(), cpu );
}
- _Thread_Scheduler_release_critical( the_thread, &lock_context );
+ _Thread_Scheduler_release_critical( the_thread, &scheduler_lock_context );
}
diff --git a/cpukit/score/src/smp.c b/cpukit/score/src/smp.c
index bfc23d5ce3..ef791fd026 100644
--- a/cpukit/score/src/smp.c
+++ b/cpukit/score/src/smp.c
@@ -114,11 +114,11 @@ void _SMP_Handler_initialize( void )
cpu_config_max = rtems_configuration_get_maximum_processors();
for ( cpu_index = 0 ; cpu_index < cpu_config_max; ++cpu_index ) {
- Per_CPU_Control *cpu = _Per_CPU_Get_by_index( cpu_index );
+ Per_CPU_Control *cpu;
+ cpu = _Per_CPU_Get_by_index( cpu_index );
+ _ISR_lock_Set_name( &cpu->Lock, "Per-CPU" );
_ISR_lock_Set_name( &cpu->Watchdog.Lock, "Watchdog" );
- _SMP_ticket_lock_Initialize( &cpu->Lock );
- _SMP_lock_Stats_initialize( &cpu->Lock_stats, "Per-CPU" );
_Chain_Initialize_empty( &cpu->Threads_in_need_for_help );
}
diff --git a/cpukit/score/src/threaddispatch.c b/cpukit/score/src/threaddispatch.c
index 55dc437eaf..65951cccfe 100644
--- a/cpukit/score/src/threaddispatch.c
+++ b/cpukit/score/src/threaddispatch.c
@@ -158,27 +158,30 @@ static ISR_Level _Thread_Preemption_intervention(
)
{
#if defined(RTEMS_SMP)
+ ISR_lock_Context lock_context;
+
level = _Thread_Check_pinning( executing, cpu_self, level );
- _Per_CPU_Acquire( cpu_self );
+ _Per_CPU_Acquire( cpu_self, &lock_context );
while ( !_Chain_Is_empty( &cpu_self->Threads_in_need_for_help ) ) {
- Chain_Node *node;
- Thread_Control *the_thread;
- ISR_lock_Context lock_context;
+ Chain_Node *node;
+ Thread_Control *the_thread;
node = _Chain_Get_first_unprotected( &cpu_self->Threads_in_need_for_help );
_Chain_Set_off_chain( node );
the_thread = THREAD_OF_SCHEDULER_HELP_NODE( node );
- _Per_CPU_Release( cpu_self );
+ _Per_CPU_Release( cpu_self, &lock_context );
+
_Thread_State_acquire( the_thread, &lock_context );
_Thread_Ask_for_help( the_thread );
_Thread_State_release( the_thread, &lock_context );
- _Per_CPU_Acquire( cpu_self );
+
+ _Per_CPU_Acquire( cpu_self, &lock_context );
}
- _Per_CPU_Release( cpu_self );
+ _Per_CPU_Release( cpu_self, &lock_context );
#else
(void) cpu_self;
#endif
diff --git a/cpukit/score/src/userextaddset.c b/cpukit/score/src/userextaddset.c
index 585712e043..2b13dfad62 100644
--- a/cpukit/score/src/userextaddset.c
+++ b/cpukit/score/src/userextaddset.c
@@ -41,17 +41,15 @@ void _User_extensions_Add_set(
*/
if ( the_extension->Callouts.thread_switch != NULL ) {
- ISR_Level level;
-
the_extension->Switch.thread_switch =
the_extension->Callouts.thread_switch;
- _Per_CPU_Acquire_all( level );
+ _Per_CPU_Acquire_all( &lock_context );
_Chain_Initialize_node( &the_extension->Switch.Node );
_Chain_Append_unprotected(
&_User_extensions_Switches_list,
&the_extension->Switch.Node
);
- _Per_CPU_Release_all( level );
+ _Per_CPU_Release_all( &lock_context );
}
}
diff --git a/cpukit/score/src/userextremoveset.c b/cpukit/score/src/userextremoveset.c
index adaf049677..4eb13f0c55 100644
--- a/cpukit/score/src/userextremoveset.c
+++ b/cpukit/score/src/userextremoveset.c
@@ -41,10 +41,10 @@ void _User_extensions_Remove_set (
*/
if ( the_extension->Callouts.thread_switch != NULL ) {
- ISR_Level level;
+ ISR_lock_Context lock_context;
- _Per_CPU_Acquire_all( level );
+ _Per_CPU_Acquire_all( &lock_context );
_Chain_Extract_unprotected( &the_extension->Switch.Node );
- _Per_CPU_Release_all( level );
+ _Per_CPU_Release_all( &lock_context );
}
}
--
2.16.4
More information about the devel
mailing list