[rtems commit] smp: Add and use _Assert_Owner_of_giant()

Sebastian Huber sebh at rtems.org
Fri Aug 30 09:10:59 UTC 2013


Module:    rtems
Branch:    master
Commit:    bf30999cc697325eda3afd1833b78a7e64255fc4
Changeset: http://git.rtems.org/rtems/commit/?id=bf30999cc697325eda3afd1833b78a7e64255fc4

Author:    Sebastian Huber <sebastian.huber at embedded-brains.de>
Date:      Fri Aug 23 16:15:50 2013 +0200

smp: Add and use _Assert_Owner_of_giant()

Add and use _ISR_Disable_without_giant() and
_ISR_Enable_without_giant() if RTEMS_SMP is defined.

On single processor systems the ISR disable/enable was the big hammer
which ensured system-wide mutual exclusion.  On SMP configurations this
no longer works since other processors do not care about disabled
interrupts on this processor and continue to execute freely.

On SMP in addition to ISR disable/enable an SMP lock must be used.
Currently we have only the Giant lock so we can check easily that ISR
disable/enable is used only in the right context.

---

 cpukit/rtems/src/taskmode.c                       |    4 +-
 cpukit/score/include/rtems/score/assert.h         |    9 ++
 cpukit/score/include/rtems/score/isr.h            |    4 +-
 cpukit/score/include/rtems/score/isrlevel.h       |   20 ++++
 cpukit/score/include/rtems/score/thread.h         |    4 +-
 cpukit/score/include/rtems/score/threaddispatch.h |    4 +-
 cpukit/score/src/smp.c                            |    2 +-
 cpukit/score/src/threaddispatch.c                 |    4 +-
 cpukit/score/src/threaddispatchdisablelevel.c     |   34 +++++--
 testsuites/sptests/sp37/init.c                    |   14 +++
 testsuites/tmtests/tm26/task1.c                   |  110 ++++++++++++++++-----
 testsuites/tmtests/tm27/task1.c                   |    4 +-
 12 files changed, 165 insertions(+), 48 deletions(-)

diff --git a/cpukit/rtems/src/taskmode.c b/cpukit/rtems/src/taskmode.c
index 606366b..8650ccc 100644
--- a/cpukit/rtems/src/taskmode.c
+++ b/cpukit/rtems/src/taskmode.c
@@ -39,7 +39,7 @@ static void _RTEMS_Tasks_Dispatch_if_necessary(
 #if defined( RTEMS_SMP )
     ISR_Level level;
 
-    _ISR_Disable( level );
+    _ISR_Disable_without_giant( level );
 #endif
 
     if ( !_Thread_Is_heir( executing ) && executing->is_preemptible ) {
@@ -48,7 +48,7 @@ static void _RTEMS_Tasks_Dispatch_if_necessary(
     }
 
 #if defined( RTEMS_SMP )
-    _ISR_Enable( level );
+    _ISR_Enable_without_giant( level );
 #endif
 
     if ( dispatch_necessary ) {
diff --git a/cpukit/score/include/rtems/score/assert.h b/cpukit/score/include/rtems/score/assert.h
index 174676e..4856eae 100644
--- a/cpukit/score/include/rtems/score/assert.h
+++ b/cpukit/score/include/rtems/score/assert.h
@@ -50,6 +50,15 @@ extern "C" {
   #define _Assert_Thread_dispatching_repressed() ( ( void ) 0 )
 #endif
 
+/**
+ * @brief Asserts that current thread of execution owns the giant lock.
+ */
+#if defined( RTEMS_DEBUG ) && defined( RTEMS_SMP )
+  void _Assert_Owner_of_giant( void );
+#else
+  #define _Assert_Owner_of_giant() ( ( void ) 0 )
+#endif
+
 #ifdef __cplusplus
 }
 #endif /* __cplusplus */
diff --git a/cpukit/score/include/rtems/score/isr.h b/cpukit/score/include/rtems/score/isr.h
index 4d4a5f3..fa0052f 100644
--- a/cpukit/score/include/rtems/score/isr.h
+++ b/cpukit/score/include/rtems/score/isr.h
@@ -163,13 +163,13 @@ RTEMS_INLINE_ROUTINE uint32_t _ISR_Get_nest_level( void )
   #if defined( RTEMS_SMP )
     ISR_Level level;
 
-    _ISR_Disable( level );
+    _ISR_Disable_without_giant( level );
   #endif
 
   isr_nest_level = _ISR_Nest_level;
 
   #if defined( RTEMS_SMP )
-    _ISR_Enable( level );
+    _ISR_Enable_without_giant( level );
   #endif
 
   return isr_nest_level;
diff --git a/cpukit/score/include/rtems/score/isrlevel.h b/cpukit/score/include/rtems/score/isrlevel.h
index 35d3d03..3da67fd 100644
--- a/cpukit/score/include/rtems/score/isrlevel.h
+++ b/cpukit/score/include/rtems/score/isrlevel.h
@@ -20,6 +20,7 @@
 #define _RTEMS_SCORE_ISR_LEVEL_h
 
 #include <rtems/score/cpu.h>
+#include <rtems/score/assert.h>
 
 #ifdef __cplusplus
 extern "C" {
@@ -57,6 +58,7 @@ typedef uint32_t   ISR_Level;
 #define _ISR_Disable( _level ) \
   do { \
     _CPU_ISR_Disable( _level ); \
+    _Assert_Owner_of_giant(); \
     RTEMS_COMPILER_MEMORY_BARRIER(); \
   } while (0)
 
@@ -74,6 +76,7 @@ typedef uint32_t   ISR_Level;
 #define _ISR_Enable( _level ) \
   do { \
     RTEMS_COMPILER_MEMORY_BARRIER(); \
+    _Assert_Owner_of_giant(); \
     _CPU_ISR_Enable( _level ); \
   } while (0)
 
@@ -99,6 +102,7 @@ typedef uint32_t   ISR_Level;
 #define _ISR_Flash( _level ) \
   do { \
     RTEMS_COMPILER_MEMORY_BARRIER(); \
+    _Assert_Owner_of_giant(); \
     _CPU_ISR_Flash( _level ); \
     RTEMS_COMPILER_MEMORY_BARRIER(); \
   } while (0)
@@ -132,6 +136,22 @@ typedef uint32_t   ISR_Level;
     RTEMS_COMPILER_MEMORY_BARRIER();  \
   } while (0)
 
+#if defined( RTEMS_SMP )
+
+#define _ISR_Disable_without_giant( _level ) \
+  do { \
+    _CPU_ISR_Disable( _level ); \
+    RTEMS_COMPILER_MEMORY_BARRIER(); \
+  } while (0)
+
+#define _ISR_Enable_without_giant( _level ) \
+  do { \
+    RTEMS_COMPILER_MEMORY_BARRIER(); \
+    _CPU_ISR_Enable( _level ); \
+  } while (0)
+
+#endif /* defined( RTEMS_SMP ) */
+
 /**@}*/
 
 #ifdef __cplusplus
diff --git a/cpukit/score/include/rtems/score/thread.h b/cpukit/score/include/rtems/score/thread.h
index 77f1d7a..33b5244 100644
--- a/cpukit/score/include/rtems/score/thread.h
+++ b/cpukit/score/include/rtems/score/thread.h
@@ -495,13 +495,13 @@ RTEMS_INLINE_ROUTINE Thread_Control *_Thread_Get_executing( void )
   #if defined( RTEMS_SMP )
     ISR_Level level;
 
-    _ISR_Disable( level );
+    _ISR_Disable_without_giant( level );
   #endif
 
   executing = _Thread_Executing;
 
   #if defined( RTEMS_SMP )
-    _ISR_Enable( level );
+    _ISR_Enable_without_giant( level );
   #endif
 
   return executing;
diff --git a/cpukit/score/include/rtems/score/threaddispatch.h b/cpukit/score/include/rtems/score/threaddispatch.h
index 9cd1287..2211662 100644
--- a/cpukit/score/include/rtems/score/threaddispatch.h
+++ b/cpukit/score/include/rtems/score/threaddispatch.h
@@ -54,13 +54,13 @@ RTEMS_INLINE_ROUTINE bool _Thread_Dispatch_is_enabled(void)
 #if defined(RTEMS_SMP)
   ISR_Level level;
 
-  _ISR_Disable( level );
+  _ISR_Disable_without_giant( level );
 #endif
 
   enabled = _Thread_Dispatch_disable_level == 0;
 
 #if defined(RTEMS_SMP)
-  _ISR_Enable( level );
+  _ISR_Enable_without_giant( level );
 #endif
 
   return enabled;
diff --git a/cpukit/score/src/smp.c b/cpukit/score/src/smp.c
index 6020b94..2f8a488 100644
--- a/cpukit/score/src/smp.c
+++ b/cpukit/score/src/smp.c
@@ -75,7 +75,7 @@ void rtems_smp_process_interrupt( void )
     #endif
 
     if ( ( message & RTEMS_BSP_SMP_SHUTDOWN ) != 0 ) {
-      _ISR_Disable( level );
+      _ISR_Disable_without_giant( level );
 
       _Thread_Dispatch_set_disable_level( 0 );
 
diff --git a/cpukit/score/src/threaddispatch.c b/cpukit/score/src/threaddispatch.c
index 3b5fb42..0c8298f 100644
--- a/cpukit/score/src/threaddispatch.c
+++ b/cpukit/score/src/threaddispatch.c
@@ -35,7 +35,7 @@ void _Thread_Dispatch( void )
   ISR_Level         level;
 
 #if defined( RTEMS_SMP )
-  _ISR_Disable( level );
+  _ISR_Disable_without_giant( level );
 #endif
 
   per_cpu = _Per_CPU_Get();
@@ -43,7 +43,7 @@ void _Thread_Dispatch( void )
   per_cpu->thread_dispatch_disable_level = 1;
 
 #if defined( RTEMS_SMP )
-  _ISR_Enable( level );
+  _ISR_Enable_without_giant( level );
 #endif
 
   /*
diff --git a/cpukit/score/src/threaddispatchdisablelevel.c b/cpukit/score/src/threaddispatchdisablelevel.c
index 3075534..a3dec99 100644
--- a/cpukit/score/src/threaddispatchdisablelevel.c
+++ b/cpukit/score/src/threaddispatchdisablelevel.c
@@ -17,6 +17,7 @@
 
 #include <rtems/score/threaddispatch.h>
 #include <rtems/score/assert.h>
+#include <rtems/score/sysstate.h>
 
 #define NO_OWNER_CPU 0xffffffffU
 
@@ -63,7 +64,7 @@ uint32_t _Thread_Dispatch_increment_disable_level( void )
   uint32_t disable_level;
   Per_CPU_Control *self_cpu;
 
-  _ISR_Disable( isr_level );
+  _ISR_Disable_without_giant( isr_level );
 
   /*
    * We must obtain the processor ID after interrupts are disabled to prevent
@@ -78,7 +79,7 @@ uint32_t _Thread_Dispatch_increment_disable_level( void )
   ++disable_level;
   self_cpu->thread_dispatch_disable_level = disable_level;
 
-  _ISR_Enable( isr_level );
+  _ISR_Enable_without_giant( isr_level );
 
   return disable_level;
 }
@@ -89,7 +90,7 @@ uint32_t _Thread_Dispatch_decrement_disable_level( void )
   uint32_t disable_level;
   Per_CPU_Control *self_cpu;
 
-  _ISR_Disable( isr_level );
+  _ISR_Disable_without_giant( isr_level );
 
   self_cpu = _Per_CPU_Get();
   disable_level = self_cpu->thread_dispatch_disable_level;
@@ -97,8 +98,9 @@ uint32_t _Thread_Dispatch_decrement_disable_level( void )
   self_cpu->thread_dispatch_disable_level = disable_level;
 
   _Giant_Do_release();
+  _Assert( disable_level != 0 || _Giant.owner_cpu == NO_OWNER_CPU );
 
-  _ISR_Enable( isr_level );
+  _ISR_Enable_without_giant( isr_level );
 
   return disable_level;
 }
@@ -118,9 +120,9 @@ uint32_t _Thread_Dispatch_set_disable_level(uint32_t value)
   ISR_Level isr_level;
   uint32_t disable_level;
 
-  _ISR_Disable( isr_level );
+  _ISR_Disable_without_giant( isr_level );
   disable_level = _Thread_Dispatch_disable_level;
-  _ISR_Enable( isr_level );
+  _ISR_Enable_without_giant( isr_level );
 
   /*
    * If we need the dispatch level to go higher 
@@ -147,18 +149,30 @@ void _Giant_Acquire( void )
 {
   ISR_Level isr_level;
 
-  _ISR_Disable( isr_level );
+  _ISR_Disable_without_giant( isr_level );
   _Assert( _Thread_Dispatch_disable_level != 0 );
   _Giant_Do_acquire( _SMP_Get_current_processor() );
-  _ISR_Enable( isr_level );
+  _ISR_Enable_without_giant( isr_level );
 }
 
 void _Giant_Release( void )
 {
   ISR_Level isr_level;
 
-  _ISR_Disable( isr_level );
+  _ISR_Disable_without_giant( isr_level );
   _Assert( _Thread_Dispatch_disable_level != 0 );
   _Giant_Do_release();
-  _ISR_Enable( isr_level );
+  _ISR_Enable_without_giant( isr_level );
 }
+
+#if defined( RTEMS_DEBUG )
+void _Assert_Owner_of_giant( void )
+{
+  Giant_Control *giant = &_Giant;
+
+  _Assert(
+    giant->owner_cpu == _SMP_Get_current_processor()
+      || !_System_state_Is_up( _System_state_Get() )
+  );
+}
+#endif
diff --git a/testsuites/sptests/sp37/init.c b/testsuites/sptests/sp37/init.c
index efb92b8..b6ab157 100644
--- a/testsuites/sptests/sp37/init.c
+++ b/testsuites/sptests/sp37/init.c
@@ -229,13 +229,19 @@ void test_interrupt_inline(void)
   }
 
   puts( "interrupt disable (use inline)" );
+  _Thread_Disable_dispatch();
   rtems_interrupt_disable( level );
+  _Thread_Enable_dispatch();
 
   puts( "interrupt flash (use inline)" );
+  _Thread_Disable_dispatch();
   rtems_interrupt_flash( level );
+  _Thread_Enable_dispatch();
 
   puts( "interrupt enable (use inline)" );
+  _Thread_Disable_dispatch();
   rtems_interrupt_enable( level );
+  _Thread_Enable_dispatch();
 
   puts( "interrupt level mode (use inline)" );
   level_mode_body = rtems_interrupt_level_body( level );
@@ -463,16 +469,24 @@ rtems_task Init(
   }
 
   puts( "interrupt disable (use body)" );
+  _Thread_Disable_dispatch();
   level = rtems_interrupt_disable();
+  _Thread_Enable_dispatch();
 
   puts( "interrupt disable (use body)" );
+  _Thread_Disable_dispatch();
   level = rtems_interrupt_disable();
+  _Thread_Enable_dispatch();
 
   puts( "interrupt flash (use body)" );
+  _Thread_Disable_dispatch();
   rtems_interrupt_flash( level );
+  _Thread_Enable_dispatch();
 
   puts( "interrupt enable (use body)" );
+  _Thread_Disable_dispatch();
   rtems_interrupt_enable( level );
+  _Thread_Enable_dispatch();
 
   puts( "interrupt level mode (use body)" );
   level_mode_body = rtems_interrupt_level_body( level );
diff --git a/testsuites/tmtests/tm26/task1.c b/testsuites/tmtests/tm26/task1.c
index fbe0d2f..5a51f4c 100644
--- a/testsuites/tmtests/tm26/task1.c
+++ b/testsuites/tmtests/tm26/task1.c
@@ -21,6 +21,10 @@
 
 #include <rtems/rtems/semimpl.h>
 
+#if defined( RTEMS_SMP ) && defined( RTEMS_DEBUG )
+  #define PREVENT_SMP_ASSERT_FAILURES
+#endif
+
 /* TEST DATA */
 rtems_id Semaphore_id;
 
@@ -84,56 +88,56 @@ void complete_test( void );
 
 static void set_thread_dispatch_necessary( bool dispatch_necessary )
 {
-#if defined( RTEMS_SMP )
-  rtems_interrupt_level level;
+#if defined( PREVENT_SMP_ASSERT_FAILURES )
+  ISR_Level level;
 
-  rtems_interrupt_disable( level );
+  _ISR_Disable_without_giant( level );
 #endif
 
   _Thread_Dispatch_necessary = dispatch_necessary;
 
-#if defined( RTEMS_SMP )
-  rtems_interrupt_enable( level );
+#if defined( PREVENT_SMP_ASSERT_FAILURES )
+  _ISR_Enable_without_giant( level );
 #endif
 }
 
 static void set_thread_heir( Thread_Control *thread )
 {
-#if defined( RTEMS_SMP )
-  rtems_interrupt_level level;
+#if defined( PREVENT_SMP_ASSERT_FAILURES )
+  ISR_Level level;
 
-  rtems_interrupt_disable( level );
+  _ISR_Disable_without_giant( level );
 #endif
 
   _Thread_Heir = thread;
 
-#if defined( RTEMS_SMP )
-  rtems_interrupt_enable( level );
+#if defined( PREVENT_SMP_ASSERT_FAILURES )
+  _ISR_Enable_without_giant( level );
 #endif
 }
 
 static void set_thread_executing( Thread_Control *thread )
 {
-#if defined( RTEMS_SMP )
-  rtems_interrupt_level level;
+#if defined( PREVENT_SMP_ASSERT_FAILURES )
+  ISR_Level level;
 
-  rtems_interrupt_disable( level );
+  _ISR_Disable_without_giant( level );
 #endif
 
   _Thread_Executing = thread;
 
-#if defined( RTEMS_SMP )
-  rtems_interrupt_enable( level );
+#if defined( PREVENT_SMP_ASSERT_FAILURES )
+  _ISR_Enable_without_giant( level );
 #endif
 }
 
 static void thread_disable_dispatch( void )
 {
-#if defined( RTEMS_SMP )
+#if defined( PREVENT_SMP_ASSERT_FAILURES )
   Per_CPU_Control *self_cpu;
-  rtems_interrupt_level level;
+  ISR_Level level;
 
-  rtems_interrupt_disable( level );
+  _ISR_Disable_without_giant( level );
   ( void ) level;
 
   self_cpu = _Per_CPU_Get();
@@ -145,6 +149,58 @@ static void thread_disable_dispatch( void )
 #endif
 }
 
+static void thread_set_state( Thread_Control *thread, States_Control state )
+{
+#if defined( PREVENT_SMP_ASSERT_FAILURES )
+  _Thread_Disable_dispatch();
+#endif
+
+  _Thread_Set_state( thread, state );
+
+#if defined( PREVENT_SMP_ASSERT_FAILURES )
+  _Thread_Unnest_dispatch();
+#endif
+}
+
+static void thread_resume( Thread_Control *thread )
+{
+#if defined( PREVENT_SMP_ASSERT_FAILURES )
+  _Thread_Disable_dispatch();
+#endif
+
+  _Thread_Resume( thread );
+
+#if defined( PREVENT_SMP_ASSERT_FAILURES )
+  _Thread_Unnest_dispatch();
+#endif
+}
+
+static void thread_unblock( Thread_Control *thread )
+{
+#if defined( PREVENT_SMP_ASSERT_FAILURES )
+  _Thread_Disable_dispatch();
+#endif
+
+  _Thread_Unblock( thread );
+
+#if defined( PREVENT_SMP_ASSERT_FAILURES )
+  _Thread_Unnest_dispatch();
+#endif
+}
+
+static void thread_ready( Thread_Control *thread )
+{
+#if defined( PREVENT_SMP_ASSERT_FAILURES )
+  _Thread_Disable_dispatch();
+#endif
+
+  _Thread_Ready( thread );
+
+#if defined( PREVENT_SMP_ASSERT_FAILURES )
+  _Thread_Unnest_dispatch();
+#endif
+}
+
 rtems_task null_task(
   rtems_task_argument argument
 )
@@ -273,6 +329,8 @@ rtems_task High_task(
 {
   rtems_interrupt_level level;
 
+  _Thread_Disable_dispatch();
+
   benchmark_timer_initialize();
     rtems_interrupt_disable( level );
   isr_disable_time = benchmark_timer_read();
@@ -285,6 +343,8 @@ rtems_task High_task(
     rtems_interrupt_enable( level );
   isr_enable_time = benchmark_timer_read();
 
+  _Thread_Enable_dispatch();
+
   benchmark_timer_initialize();
     _Thread_Disable_dispatch();
   thread_disable_dispatch_time = benchmark_timer_read();
@@ -294,7 +354,7 @@ rtems_task High_task(
   thread_enable_dispatch_time = benchmark_timer_read();
 
   benchmark_timer_initialize();
-    _Thread_Set_state( _Thread_Get_executing(), STATES_SUSPENDED );
+    thread_set_state( _Thread_Get_executing(), STATES_SUSPENDED );
   thread_set_state_time = benchmark_timer_read();
 
   set_thread_dispatch_necessary( true );
@@ -311,7 +371,7 @@ rtems_task Middle_task(
 
   thread_dispatch_no_fp_time = benchmark_timer_read();
 
-  _Thread_Set_state( _Thread_Get_executing(), STATES_SUSPENDED );
+  thread_set_state( _Thread_Get_executing(), STATES_SUSPENDED );
 
   Middle_tcb   = _Thread_Get_executing();
 
@@ -478,19 +538,19 @@ void complete_test( void )
   rtems_id          task_id;
 
   benchmark_timer_initialize();
-    _Thread_Resume( Middle_tcb );
+    thread_resume( Middle_tcb );
   thread_resume_time = benchmark_timer_read();
 
-  _Thread_Set_state( Middle_tcb, STATES_WAITING_FOR_MESSAGE );
+  thread_set_state( Middle_tcb, STATES_WAITING_FOR_MESSAGE );
 
   benchmark_timer_initialize();
-    _Thread_Unblock( Middle_tcb );
+    thread_unblock( Middle_tcb );
   thread_unblock_time = benchmark_timer_read();
 
-  _Thread_Set_state( Middle_tcb, STATES_WAITING_FOR_MESSAGE );
+  thread_set_state( Middle_tcb, STATES_WAITING_FOR_MESSAGE );
 
   benchmark_timer_initialize();
-    _Thread_Ready( Middle_tcb );
+    thread_ready( Middle_tcb );
   thread_ready_time = benchmark_timer_read();
 
   benchmark_timer_initialize();
diff --git a/testsuites/tmtests/tm27/task1.c b/testsuites/tmtests/tm27/task1.c
index fac889c..7b6726d 100644
--- a/testsuites/tmtests/tm27/task1.c
+++ b/testsuites/tmtests/tm27/task1.c
@@ -184,7 +184,7 @@ rtems_task Task_1(
   _Thread_Dispatch_set_disable_level( 0 );
 
 #if defined(RTEMS_SMP)
-  rtems_interrupt_disable(level);
+  _ISR_Disable_without_giant(level);
 #endif
 
   ready_queues      = (Chain_Control *) _Scheduler.information;
@@ -194,7 +194,7 @@ rtems_task Task_1(
   _Thread_Dispatch_necessary = 1;
 
 #if defined(RTEMS_SMP)
-  rtems_interrupt_enable(level);
+  _ISR_Enable_without_giant(level);
 #endif
 
   Interrupt_occurred = 0;




More information about the vc mailing list