[rtems commit] score: Critical fix for thread dispatching

Sebastian Huber sebh at rtems.org
Sun Oct 7 12:40:51 UTC 2012


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

Author:    Sebastian Huber <sebastian.huber at embedded-brains.de>
Date:      Tue Oct  2 16:13:00 2012 +0200

score: Critical fix for thread dispatching

The changes in _Thread_Dispatch() of commits
dad36c52b8be5d7b46bc7af85655055db7208652 and
d4dc7c8196355f08044e67a3f5c1e19485f17ff1 introduced a severe bug which
destroys the real-time properties of RTEMS completely.

Consider the following scenario.  We have three tasks L (lowest
priority), M (middle priority), and H (highest priority).  Now let a
thread dispatch from M to L happen.  An interrupt occurs in
_Thread_Dispatch() here:

void _Thread_Dispatch( void )
{
  [...]

post_switch:

  _ISR_Enable( level );

  <-- INTERRUPT
  <-- AFTER INTERRUPT

  _Thread_Unnest_dispatch();

  _API_extensions_Run_postswitch();
}

The interrupt event makes task H ready.  The interrupt code will see
_Thread_Dispatch_disable_level > 0 and thus doesn't perform a
_Thread_Dispatch().  Now we return to position "AFTER INTERRUPT".  This
means task L executes now although task H is ready!  Task H will execute
once someone calls _Thread_Dispatch().

---

 cpukit/score/src/threaddispatch.c                  |   47 +++++-
 testsuites/sptests/Makefile.am                     |    2 +-
 testsuites/sptests/configure.ac                    |    1 +
 testsuites/sptests/spintrcritical18/Makefile.am    |   22 +++
 testsuites/sptests/spintrcritical18/init.c         |  173 ++++++++++++++++++++
 .../sptests/spintrcritical18/spintrcritical18.doc  |   11 ++
 .../sptests/spintrcritical18/spintrcritical18.scn  |    4 +
 7 files changed, 255 insertions(+), 5 deletions(-)

diff --git a/cpukit/score/src/threaddispatch.c b/cpukit/score/src/threaddispatch.c
index 672a099..8a77d1b 100644
--- a/cpukit/score/src/threaddispatch.c
+++ b/cpukit/score/src/threaddispatch.c
@@ -55,9 +55,41 @@ void _Thread_Dispatch( void )
   Thread_Control   *heir;
   ISR_Level         level;
 
-  _Thread_Disable_dispatch();
   #if defined(RTEMS_SMP)
     /*
+     * WARNING: The SMP sequence has severe defects regarding the real-time
+     * performance.
+     *
+     * Consider the following scenario.  We have three tasks L (lowest
+     * priority), M (middle priority), and H (highest priority).  Now let a
+     * thread dispatch from M to L happen.  An interrupt occurs in
+     * _Thread_Dispatch() here:
+     *
+     * void _Thread_Dispatch( void )
+     * {
+     *   [...]
+     *
+     * post_switch:
+     *
+     *   _ISR_Enable( level );
+     *
+     *   <-- INTERRUPT
+     *   <-- AFTER INTERRUPT
+     *
+     *   _Thread_Unnest_dispatch();
+     *
+     *   _API_extensions_Run_postswitch();
+     * }
+     *
+     * The interrupt event makes task H ready.  The interrupt code will see
+     * _Thread_Dispatch_disable_level > 0 and thus doesn't perform a
+     * _Thread_Dispatch().  Now we return to position "AFTER INTERRUPT".  This
+     * means task L executes now although task H is ready!  Task H will execute
+     * once someone calls _Thread_Dispatch().
+     */
+    _Thread_Disable_dispatch();
+
+    /*
      *  If necessary, send dispatch request to other cores.
      */
     _SMP_Request_other_cores_to_dispatch();
@@ -69,8 +101,10 @@ void _Thread_Dispatch( void )
   executing   = _Thread_Executing;
   _ISR_Disable( level );
   while ( _Thread_Dispatch_necessary == true ) {
-
     heir = _Thread_Heir;
+    #ifndef RTEMS_SMP
+      _Thread_Dispatch_set_disable_level( 1 );
+    #endif
     _Thread_Dispatch_necessary = false;
     _Thread_Executing = heir;
 
@@ -167,10 +201,15 @@ void _Thread_Dispatch( void )
   }
 
 post_switch:
+  #ifndef RTEMS_SMP
+    _Thread_Dispatch_set_disable_level( 0 );
+  #endif
 
   _ISR_Enable( level );
 
-  _Thread_Unnest_dispatch();
- 
+  #ifdef RTEMS_SMP
+    _Thread_Unnest_dispatch();
+  #endif
+
   _API_extensions_Run_postswitch();
 }
diff --git a/testsuites/sptests/Makefile.am b/testsuites/sptests/Makefile.am
index d791175..479cdf6 100644
--- a/testsuites/sptests/Makefile.am
+++ b/testsuites/sptests/Makefile.am
@@ -24,7 +24,7 @@ SUBDIRS = \
     spintrcritical05 spintrcritical06 spintrcritical07 spintrcritical08 \
     spintrcritical09 spintrcritical10 spintrcritical11 spintrcritical12 \
     spintrcritical13 spintrcritical14 spintrcritical15 spintrcritical16 \
-    spintrcritical17 spmkdir spmountmgr01 spheapprot \
+    spintrcritical17 spintrcritical18 spmkdir spmountmgr01 spheapprot \
     spsimplesched01 spsimplesched02 spsimplesched03 spnsext01 \
     spedfsched01 spedfsched02 spedfsched03 \
     spcbssched01 spcbssched02 spcbssched03 spqreslib sptimespec01
diff --git a/testsuites/sptests/configure.ac b/testsuites/sptests/configure.ac
index d568c11..33a0755 100644
--- a/testsuites/sptests/configure.ac
+++ b/testsuites/sptests/configure.ac
@@ -27,6 +27,7 @@ AC_CHECK_SIZEOF([time_t])
 
 # Explicitly list all Makefiles here
 AC_CONFIG_FILES([Makefile
+spintrcritical18/Makefile
 sp01/Makefile
 sp02/Makefile
 sp03/Makefile
diff --git a/testsuites/sptests/spintrcritical18/Makefile.am b/testsuites/sptests/spintrcritical18/Makefile.am
new file mode 100644
index 0000000..32938b3
--- /dev/null
+++ b/testsuites/sptests/spintrcritical18/Makefile.am
@@ -0,0 +1,22 @@
+rtems_tests_PROGRAMS = spintrcritical18
+spintrcritical18_SOURCES = init.c
+spintrcritical18_SOURCES += ../spintrcritical_support/intrcritical.h
+spintrcritical18_SOURCES += ../spintrcritical_support/intrcritical.c
+
+dist_rtems_tests_DATA = spintrcritical18.scn spintrcritical18.doc
+
+include $(RTEMS_ROOT)/make/custom/@RTEMS_BSP at .cfg
+include $(top_srcdir)/../automake/compile.am
+include $(top_srcdir)/../automake/leaf.am
+
+AM_CPPFLAGS += -I$(top_srcdir)/../support/include
+AM_CPPFLAGS += -I$(top_srcdir)/spintrcritical_support
+
+LINK_OBJS = $(spintrcritical18_OBJECTS)
+LINK_LIBS = $(spintrcritical18_LDLIBS)
+
+spintrcritical18$(EXEEXT): $(spintrcritical18_OBJECTS) $(spintrcritical18_DEPENDENCIES)
+	@rm -f spintrcritical18$(EXEEXT)
+	$(make-exe)
+
+include $(top_srcdir)/../automake/local.am
diff --git a/testsuites/sptests/spintrcritical18/init.c b/testsuites/sptests/spintrcritical18/init.c
new file mode 100644
index 0000000..311eae8
--- /dev/null
+++ b/testsuites/sptests/spintrcritical18/init.c
@@ -0,0 +1,173 @@
+/*
+ * Copyright (c) 2012 embedded brains GmbH.  All rights reserved.
+ *
+ *  embedded brains GmbH
+ *  Obere Lagerstr. 30
+ *  82178 Puchheim
+ *  Germany
+ *  <rtems at embedded-brains.de>
+ *
+ * The license and distribution terms for this file may be
+ * found in the file LICENSE in this distribution or at
+ * http://www.rtems.com/license/LICENSE.
+ */
+
+#ifdef HAVE_CONFIG_H
+  #include "config.h"
+#endif
+
+#include <tmacros.h>
+#include <intrcritical.h>
+
+#define TEST_NAME "18"
+
+#define WAKE_UP RTEMS_EVENT_0
+
+#define PRIORITY_LOW 3
+
+#define PRIORITY_MIDDLE 2
+
+#define PRIORITY_HIGH 1
+
+#define ASSERT_SC(sc) rtems_test_assert( (sc) == RTEMS_SUCCESSFUL )
+
+typedef struct {
+  rtems_id middle_priority_task;
+  rtems_id high_priority_task;
+  bool high_priority_task_activated;
+} test_context;
+
+static test_context global_ctx;
+
+static void wake_up(rtems_id task)
+{
+  rtems_status_code sc;
+
+  sc = rtems_event_send( task, WAKE_UP );
+  ASSERT_SC( sc );
+}
+
+static void wait_for_wake_up( void )
+{
+  rtems_status_code sc;
+  rtems_event_set events;
+
+  sc = rtems_event_receive(
+    WAKE_UP,
+    RTEMS_EVENT_ALL | RTEMS_WAIT,
+    RTEMS_NO_TIMEOUT,
+    &events
+  );
+  ASSERT_SC( sc );
+  rtems_test_assert( events == WAKE_UP );
+}
+
+static void active_high_priority_task( rtems_id timer, void *arg )
+{
+  /* The arg is NULL */
+  test_context *ctx = &global_ctx;
+
+  rtems_test_assert( !ctx->high_priority_task_activated );
+  ctx->high_priority_task_activated = true;
+  wake_up( ctx->high_priority_task );
+}
+
+static void middle_priority_task( rtems_task_argument arg )
+{
+  test_context *ctx = (test_context *) arg;
+
+  while ( true ) {
+    wait_for_wake_up();
+
+    rtems_test_assert( !ctx->high_priority_task_activated );
+  }
+}
+
+static void high_priority_task( rtems_task_argument arg )
+{
+  test_context *ctx = (test_context *) arg;
+
+  while ( true ) {
+    wait_for_wake_up();
+
+    rtems_test_assert( ctx->high_priority_task_activated );
+    ctx->high_priority_task_activated = false;
+  }
+}
+
+static void Init( rtems_task_argument ignored )
+{
+  test_context *ctx = &global_ctx;
+  rtems_status_code sc;
+  int resets = 0;
+
+  puts( "\n\n*** TEST INTERRUPT CRITICAL SECTION " TEST_NAME " ***\n" );
+
+  sc = rtems_task_create(
+    rtems_build_name( 'H', 'I', 'G', 'H' ),
+    PRIORITY_HIGH,
+    RTEMS_MINIMUM_STACK_SIZE,
+    RTEMS_DEFAULT_MODES,
+    RTEMS_DEFAULT_ATTRIBUTES,
+    &ctx->high_priority_task
+  );
+  ASSERT_SC(sc);
+
+  sc = rtems_task_start(
+    ctx->high_priority_task,
+    high_priority_task,
+    (rtems_task_argument) ctx
+  );
+  ASSERT_SC(sc);
+
+  sc = rtems_task_create(
+    rtems_build_name( 'M', 'I', 'D', 'L' ),
+    PRIORITY_MIDDLE,
+    RTEMS_MINIMUM_STACK_SIZE,
+    RTEMS_DEFAULT_MODES,
+    RTEMS_DEFAULT_ATTRIBUTES,
+    &ctx->middle_priority_task
+  );
+  ASSERT_SC(sc);
+
+  sc = rtems_task_start(
+    ctx->middle_priority_task,
+    middle_priority_task,
+    (rtems_task_argument) ctx
+  );
+  ASSERT_SC(sc);
+
+  interrupt_critical_section_test_support_initialize(
+    active_high_priority_task
+  );
+
+  while ( resets < 3 ) {
+    if ( interrupt_critical_section_test_support_delay() ) {
+      ++resets;
+    }
+
+    wake_up( ctx->middle_priority_task );
+  }
+
+  puts( "*** END OF TEST INTERRUPT CRITICAL SECTION " TEST_NAME " ***" );
+
+  rtems_test_exit( 0 );
+}
+
+#define CONFIGURE_APPLICATION_NEEDS_CONSOLE_DRIVER
+#define CONFIGURE_APPLICATION_NEEDS_CLOCK_DRIVER
+
+#define CONFIGURE_MICROSECONDS_PER_TICK 1000
+
+#define CONFIGURE_MAXIMUM_TASKS 3
+#define CONFIGURE_MAXIMUM_TIMERS 1
+
+#define CONFIGURE_INIT_TASK_PRIORITY PRIORITY_LOW
+#define CONFIGURE_INIT_TASK_ATTRIBUTES RTEMS_DEFAULT_ATTRIBUTES
+#define CONFIGURE_INIT_TASK_INITIAL_MODES RTEMS_DEFAULT_MODES
+
+#define CONFIGURE_RTEMS_INIT_TASKS_TABLE
+
+#define CONFIGURE_INIT
+
+#include <rtems/confdefs.h>
diff --git a/testsuites/sptests/spintrcritical18/spintrcritical18.doc b/testsuites/sptests/spintrcritical18/spintrcritical18.doc
new file mode 100644
index 0000000..1d9404b
--- /dev/null
+++ b/testsuites/sptests/spintrcritical18/spintrcritical18.doc
@@ -0,0 +1,11 @@
+This file describes the directives and concepts tested by this test set.
+
+test set name: spintrcritical18
+
+directives:
+
+  _Thread_Dispatch()
+
+concepts:
+
+  Ensure that thread dispatching works if interrupts happen in the meantime.
diff --git a/testsuites/sptests/spintrcritical18/spintrcritical18.scn b/testsuites/sptests/spintrcritical18/spintrcritical18.scn
new file mode 100644
index 0000000..4cf1d93
--- /dev/null
+++ b/testsuites/sptests/spintrcritical18/spintrcritical18.scn
@@ -0,0 +1,4 @@
+*** TEST INTERRUPT CRITICAL SECTION 18 ***
+
+Support - rtems_timer_create - creating timer 1
+*** END OF TEST INTERRUPT CRITICAL SECTION 18 ***




More information about the vc mailing list