score: Critical fix for thread dispatching

The changes in _Thread_Dispatch() of commits dad36c52b8 and d4dc7c8196 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().
2026-09-22 16:25:14 +08:00 · 2012-10-07 14:40:49 +02:00
parent cb7b57e174
commit 9b83a66546
7 changed files with 255 additions and 5 deletions
@@ -55,8 +55,40 @@ 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.
     */
@@ -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();
 }
@@ -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
@@ -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
@@ -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@.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
@@ -0,0 +1,173 @@
+/*
+ * Copyright (c) 2012 embedded brains GmbH.  All rights reserved.
+ *
+ *  embedded brains GmbH
+ *  Obere Lagerstr. 30
+ *  82178 Puchheim
+ *  Germany
+ *  <rtems@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>
@@ -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.
@@ -0,0 +1,4 @@
+*** TEST INTERRUPT CRITICAL SECTION 18 ***
+
+Support - rtems_timer_create - creating timer 1
+*** END OF TEST INTERRUPT CRITICAL SECTION 18 ***