[PATCH] score: Fix thread deletion on SMP

Sebastian Huber sebastian.huber at embedded-brains.de
Wed Jun 18 13:23:27 UTC 2014


All blocking operations using _Thread_Get() in the corresponding release
operation must check for thread termination after
_Thread_Disable_dispatch() on SMP.
---
 cpukit/rtems/src/eventreceive.c                    |    7 +
 cpukit/rtems/src/systemeventreceive.c              |    7 +
 cpukit/rtems/src/tasksuspend.c                     |    6 +
 testsuites/smptests/smpthreadlife01/init.c         |  169 ++++++++++++++++++--
 .../smptests/smpthreadlife01/smpthreadlife01.doc   |    6 +
 5 files changed, 182 insertions(+), 13 deletions(-)

diff --git a/cpukit/rtems/src/eventreceive.c b/cpukit/rtems/src/eventreceive.c
index 49fdb2a..0ff350f 100644
--- a/cpukit/rtems/src/eventreceive.c
+++ b/cpukit/rtems/src/eventreceive.c
@@ -22,6 +22,7 @@
 #include <rtems/rtems/tasks.h>
 #include <rtems/score/statesimpl.h>
 #include <rtems/score/threaddispatch.h>
+#include <rtems/score/threadimpl.h>
 
 rtems_status_code rtems_event_receive(
   rtems_event_set  event_in,
@@ -39,6 +40,12 @@ rtems_status_code rtems_event_receive(
 
     if ( !_Event_sets_Is_empty( event_in ) ) {
       _Thread_Disable_dispatch();
+#if defined(RTEMS_SMP)
+      if ( _Thread_Is_life_terminating( executing->Life.state ) ) {
+        _Thread_Enable_dispatch();
+        _Assert_Not_reached();
+      }
+#endif
       _Event_Seize(
         event_in,
         option_set,
diff --git a/cpukit/rtems/src/systemeventreceive.c b/cpukit/rtems/src/systemeventreceive.c
index c24e19a..c38962d 100644
--- a/cpukit/rtems/src/systemeventreceive.c
+++ b/cpukit/rtems/src/systemeventreceive.c
@@ -28,6 +28,7 @@
 #include <rtems/rtems/tasks.h>
 #include <rtems/score/statesimpl.h>
 #include <rtems/score/threaddispatch.h>
+#include <rtems/score/threadimpl.h>
 
 rtems_status_code rtems_event_system_receive(
   rtems_event_set  event_in,
@@ -45,6 +46,12 @@ rtems_status_code rtems_event_system_receive(
 
     if ( !_Event_sets_Is_empty( event_in ) ) {
       _Thread_Disable_dispatch();
+#if defined(RTEMS_SMP)
+      if ( _Thread_Is_life_terminating( executing->Life.state ) ) {
+        _Thread_Enable_dispatch();
+        _Assert_Not_reached();
+      }
+#endif
       _Event_Seize(
         event_in,
         option_set,
diff --git a/cpukit/rtems/src/tasksuspend.c b/cpukit/rtems/src/tasksuspend.c
index f8c43a7..24f60fd 100644
--- a/cpukit/rtems/src/tasksuspend.c
+++ b/cpukit/rtems/src/tasksuspend.c
@@ -32,6 +32,12 @@ rtems_status_code rtems_task_suspend(
   switch ( location ) {
 
     case OBJECTS_LOCAL:
+#if defined(RTEMS_SMP)
+      if ( _Thread_Is_life_terminating( the_thread->Life.state ) ) {
+        _Objects_Put( &the_thread->Object );
+        _Assert_Not_reached();
+      }
+#endif
       if ( !_States_Is_suspended( the_thread->current_state ) ) {
         _Thread_Suspend( the_thread );
         _Objects_Put( &the_thread->Object );
diff --git a/testsuites/smptests/smpthreadlife01/init.c b/testsuites/smptests/smpthreadlife01/init.c
index 65d8709..2d0cce5 100644
--- a/testsuites/smptests/smpthreadlife01/init.c
+++ b/testsuites/smptests/smpthreadlife01/init.c
@@ -45,6 +45,11 @@ static test_context test_instance = {
   .worker_barrier_state = SMP_BARRIER_STATE_INITIALIZER
 };
 
+static void barrier(test_context *ctx, SMP_barrier_State *state)
+{
+  _SMP_barrier_Wait(&ctx->barrier, state, CPU_COUNT);
+}
+
 static void restart_extension(
   Thread_Control *executing,
   Thread_Control *restarted
@@ -74,7 +79,10 @@ static void switch_extension(Thread_Control *executing, Thread_Control *heir)
 
   if (ctx->delay_switch_for_executing == executing) {
     ctx->delay_switch_for_executing = NULL;
-    _SMP_barrier_Wait(&ctx->barrier, &ctx->worker_barrier_state, CPU_COUNT);
+
+    /* (A) */
+    barrier(ctx, &ctx->worker_barrier_state);
+
     rtems_counter_delay_nanoseconds(100000000);
 
     /* Avoid bad profiling statisitics */
@@ -90,7 +98,8 @@ static void worker_task(rtems_task_argument arg)
 
   ctx->worker_arg = arg;
 
-  _SMP_barrier_Wait(&ctx->barrier, &ctx->worker_barrier_state, CPU_COUNT);
+  /* (B) */
+  barrier(ctx, &ctx->worker_barrier_state);
 
   while (true) {
     /* Do nothing */
@@ -120,7 +129,8 @@ static void test_restart(void)
   sc = rtems_task_start(id, worker_task, 0);
   rtems_test_assert(sc == RTEMS_SUCCESSFUL);
 
-  _SMP_barrier_Wait(&ctx->barrier, &ctx->main_barrier_state, CPU_COUNT);
+  /* (B) */
+  barrier(ctx, &ctx->main_barrier_state);
 
   for (arg = 1; arg < 23; ++arg) {
     ctx->main_arg = arg;
@@ -129,7 +139,8 @@ static void test_restart(void)
     sc = rtems_task_restart(id, arg);
     rtems_test_assert(sc == RTEMS_SUCCESSFUL);
 
-    _SMP_barrier_Wait(&ctx->barrier, &ctx->main_barrier_state, CPU_COUNT);
+    /* (B) */
+    barrier(ctx, &ctx->main_barrier_state);
 
     rtems_test_assert(ctx->worker_arg == arg);
   }
@@ -168,7 +179,8 @@ static void test_delete(void)
     sc = rtems_task_start(id, worker_task, arg);
     rtems_test_assert(sc == RTEMS_SUCCESSFUL);
 
-    _SMP_barrier_Wait(&ctx->barrier, &ctx->main_barrier_state, CPU_COUNT);
+    /* (B) */
+    barrier(ctx, &ctx->main_barrier_state);
 
     rtems_test_assert(ctx->worker_arg == arg);
     rtems_test_assert(!ctx->terminated);
@@ -190,7 +202,8 @@ static void delay_ipi_task(rtems_task_argument variant)
   _ISR_Disable_without_giant(level);
   (void) level;
 
-  _SMP_barrier_Wait(&ctx->barrier, &ctx->worker_barrier_state, CPU_COUNT);
+  /* (C) */
+  barrier(ctx, &ctx->worker_barrier_state);
 
   /*
    * Interrupts are disabled, so the inter-processor interrupt deleting us will
@@ -237,7 +250,8 @@ static void test_set_life_protection(rtems_task_argument variant)
   sc = rtems_task_start(id, delay_ipi_task, variant);
   rtems_test_assert(sc == RTEMS_SUCCESSFUL);
 
-  _SMP_barrier_Wait(&ctx->barrier, &ctx->main_barrier_state, CPU_COUNT);
+  /* (C) */
+  barrier(ctx, &ctx->main_barrier_state);
 
   sc = rtems_task_delete(id);
   rtems_test_assert(sc == RTEMS_SUCCESSFUL);
@@ -256,7 +270,8 @@ static void delay_switch_task(rtems_task_argument arg)
 
   ctx->delay_switch_for_executing = _Thread_Get_executing();
 
-  _SMP_barrier_Wait(&ctx->barrier, &ctx->worker_barrier_state, CPU_COUNT);
+  /* (D) */
+  barrier(ctx, &ctx->worker_barrier_state);
 
   sc = rtems_task_delete(RTEMS_SELF);
   rtems_test_assert(sc == RTEMS_SUCCESSFUL);
@@ -284,11 +299,11 @@ static void test_wait_for_execution_stop(void)
   sc = rtems_task_start(id, delay_switch_task, 0);
   rtems_test_assert(sc == RTEMS_SUCCESSFUL);
 
-  /* Wait for delay switch task */
-  _SMP_barrier_Wait(&ctx->barrier, &ctx->main_barrier_state, CPU_COUNT);
+  /* (D) */
+  barrier(ctx, &ctx->main_barrier_state);
 
-  /* Wait for delay switch extension */
-  _SMP_barrier_Wait(&ctx->barrier, &ctx->main_barrier_state, CPU_COUNT);
+  /* (A) */
+  barrier(ctx, &ctx->main_barrier_state);
 
   sc = rtems_task_create(
     rtems_build_name('W', 'A', 'I', 'T'),
@@ -306,6 +321,131 @@ static void test_wait_for_execution_stop(void)
   rtems_test_assert(rtems_resource_snapshot_check(&snapshot));
 }
 
+static void suspend_task(rtems_task_argument arg)
+{
+  test_context *ctx = &test_instance;
+  ISR_Level level;
+
+  _ISR_Disable_without_giant(level);
+  (void) level;
+
+  /* (E) */
+  barrier(ctx, &ctx->worker_barrier_state);
+
+  /* (F) */
+  barrier(ctx, &ctx->worker_barrier_state);
+
+  rtems_task_suspend(RTEMS_SELF);
+  rtems_test_assert(0);
+}
+
+static void event_task(rtems_task_argument arg)
+{
+  test_context *ctx = &test_instance;
+  ISR_Level level;
+  rtems_event_set events;
+
+  _ISR_Disable_without_giant(level);
+  (void) level;
+
+  /* (E) */
+  barrier(ctx, &ctx->worker_barrier_state);
+
+  /* (F) */
+  barrier(ctx, &ctx->worker_barrier_state);
+
+  rtems_event_receive(
+    RTEMS_EVENT_0,
+    RTEMS_EVENT_ALL | RTEMS_WAIT,
+    RTEMS_NO_TIMEOUT,
+    &events
+  );
+  rtems_test_assert(0);
+}
+
+static void system_event_task(rtems_task_argument arg)
+{
+  test_context *ctx = &test_instance;
+  ISR_Level level;
+  rtems_event_set events;
+
+  _ISR_Disable_without_giant(level);
+  (void) level;
+
+  /* (E) */
+  barrier(ctx, &ctx->worker_barrier_state);
+
+  /* (F) */
+  barrier(ctx, &ctx->worker_barrier_state);
+
+  rtems_event_system_receive(
+    RTEMS_EVENT_0,
+    RTEMS_EVENT_ALL | RTEMS_WAIT,
+    RTEMS_NO_TIMEOUT,
+    &events
+  );
+  rtems_test_assert(0);
+}
+
+static void help_task(rtems_task_argument arg)
+{
+  test_context *ctx = &test_instance;
+
+  /* (F) */
+  barrier(ctx, &ctx->main_barrier_state);
+
+  rtems_task_suspend(RTEMS_SELF);
+  rtems_test_assert(0);
+}
+
+static void test_operation_with_delete_in_progress(rtems_task_entry entry)
+{
+  test_context *ctx = &test_instance;
+  rtems_status_code sc;
+  rtems_id worker_id;
+  rtems_id help_id;
+  rtems_resource_snapshot snapshot;
+
+  rtems_resource_snapshot_take(&snapshot);
+
+  sc = rtems_task_create(
+    rtems_build_name('W', 'O', 'R', 'K'),
+    1,
+    RTEMS_MINIMUM_STACK_SIZE,
+    RTEMS_DEFAULT_MODES,
+    RTEMS_DEFAULT_ATTRIBUTES,
+    &worker_id
+  );
+  rtems_test_assert(sc == RTEMS_SUCCESSFUL);
+
+  sc = rtems_task_start(worker_id, entry, 0);
+  rtems_test_assert(sc == RTEMS_SUCCESSFUL);
+
+  /* (E) */
+  barrier(ctx, &ctx->main_barrier_state);
+
+  sc = rtems_task_create(
+    rtems_build_name('H', 'E', 'L', 'P'),
+    2,
+    RTEMS_MINIMUM_STACK_SIZE,
+    RTEMS_DEFAULT_MODES,
+    RTEMS_DEFAULT_ATTRIBUTES,
+    &help_id
+  );
+  rtems_test_assert(sc == RTEMS_SUCCESSFUL);
+
+  sc = rtems_task_start(help_id, help_task, 0);
+  rtems_test_assert(sc == RTEMS_SUCCESSFUL);
+
+  sc = rtems_task_delete(worker_id);
+  rtems_test_assert(sc == RTEMS_SUCCESSFUL);
+
+  sc = rtems_task_delete(help_id);
+  rtems_test_assert(sc == RTEMS_SUCCESSFUL);
+
+  rtems_test_assert(rtems_resource_snapshot_check(&snapshot));
+}
+
 static void Init(rtems_task_argument arg)
 {
   TEST_BEGIN();
@@ -316,6 +456,9 @@ static void Init(rtems_task_argument arg)
     test_set_life_protection(0);
     test_set_life_protection(1);
     test_wait_for_execution_stop();
+    test_operation_with_delete_in_progress(suspend_task);
+    test_operation_with_delete_in_progress(event_task);
+    test_operation_with_delete_in_progress(system_event_task);
   }
 
   TEST_END();
@@ -329,7 +472,7 @@ static void Init(rtems_task_argument arg)
 
 #define CONFIGURE_SMP_MAXIMUM_PROCESSORS CPU_COUNT
 
-#define CONFIGURE_MAXIMUM_TASKS CPU_COUNT
+#define CONFIGURE_MAXIMUM_TASKS (CPU_COUNT + 1)
 
 #define CONFIGURE_INITIAL_EXTENSIONS \
   { \
diff --git a/testsuites/smptests/smpthreadlife01/smpthreadlife01.doc b/testsuites/smptests/smpthreadlife01/smpthreadlife01.doc
index 54fb858..818284d 100644
--- a/testsuites/smptests/smpthreadlife01/smpthreadlife01.doc
+++ b/testsuites/smptests/smpthreadlife01/smpthreadlife01.doc
@@ -7,6 +7,9 @@ directives:
   - rtems_task_restart()
   - _Thread_Set_life_protection()
   - _Thread_Wait_for_execution_stop()
+  - rtems_task_suspend()
+  - rtems_event_receive()
+  - rtems_event_system_receive()
 
 concepts:
 
@@ -14,3 +17,6 @@ concepts:
   - Ensure that a _Thread_Set_life_protection() works on SMP in case interrupt
     processing is delayed.
   - Ensure that _Thread_Wait_for_execution_stop() works on SMP.
+  - Ensure that rtems_task_suspend(), rtems_event_receive() and
+    rtems_event_system_receive() aborts in case a thread deletion is in
+    progress on SMP.
-- 
1.7.7



More information about the devel mailing list