[PATCH v2] score: Fix _Thread_Change_priority()

Sebastian Huber sebastian.huber at embedded-brains.de
Wed Mar 18 20:20:26 UTC 2015


Atomically update the current priority of a thread and the wait queue.
Serialize the scheduler update in a separate critical section with a
generation number.

New test sptests/spintrcritical23.

Close #2310.
---
 cpukit/score/include/rtems/score/thread.h          |   9 ++
 cpukit/score/src/threadchangepriority.c            |  35 ++--
 cpukit/score/src/threadinitialize.c                |   1 +
 testsuites/sptests/Makefile.am                     |   1 +
 testsuites/sptests/configure.ac                    |   1 +
 testsuites/sptests/spintrcritical23/Makefile.am    |  22 +++
 testsuites/sptests/spintrcritical23/init.c         | 177 +++++++++++++++++++++
 .../sptests/spintrcritical23/spintrcritical23.doc  |  12 ++
 .../sptests/spintrcritical23/spintrcritical23.scn  |   2 +
 9 files changed, 246 insertions(+), 14 deletions(-)
 create mode 100644 testsuites/sptests/spintrcritical23/Makefile.am
 create mode 100644 testsuites/sptests/spintrcritical23/init.c
 create mode 100644 testsuites/sptests/spintrcritical23/spintrcritical23.doc
 create mode 100644 testsuites/sptests/spintrcritical23/spintrcritical23.scn

diff --git a/cpukit/score/include/rtems/score/thread.h b/cpukit/score/include/rtems/score/thread.h
index cea88f4..11f60f3 100644
--- a/cpukit/score/include/rtems/score/thread.h
+++ b/cpukit/score/include/rtems/score/thread.h
@@ -582,6 +582,15 @@ struct Thread_Control_struct {
   Priority_Control         current_priority;
   /** This field is the base priority of this thread. */
   Priority_Control         real_priority;
+
+  /**
+   * @brief Generation of the current priority value.
+   *
+   * It is used in _Thread_Change_priority() to serialize the update of
+   * priority related data structures.
+   */
+  uint32_t                 priority_generation;
+
   /** This field is the number of mutexes currently held by this thread. */
   uint32_t                 resource_count;
   /** This field is the blocking information for this thread. */
diff --git a/cpukit/score/src/threadchangepriority.c b/cpukit/score/src/threadchangepriority.c
index d61dfb8..6ee65f5 100644
--- a/cpukit/score/src/threadchangepriority.c
+++ b/cpukit/score/src/threadchangepriority.c
@@ -29,29 +29,36 @@ void _Thread_Change_priority(
   bool              prepend_it
 )
 {
+  ISR_Level level;
+
+  _ISR_Disable( level );
+
   /*
    *  Do not bother recomputing all the priority related information if
    *  we are not REALLY changing priority.
    */
   if ( the_thread->current_priority != new_priority ) {
-    ISR_Level level;
-
-    _ISR_Disable( level );
+    uint32_t my_generation = the_thread->priority_generation + 1;
 
     the_thread->current_priority = new_priority;
+    the_thread->priority_generation = my_generation;
 
-    if ( _States_Is_ready( the_thread->current_state ) ) {
-      _Scheduler_Change_priority(
-        the_thread,
-        new_priority,
-        prepend_it
-      );
-    } else {
-      _Scheduler_Update_priority( the_thread, new_priority );
-    }
+    _Thread_queue_Requeue( the_thread->Wait.queue, the_thread );
 
-    _ISR_Enable( level );
+    _ISR_Flash( level );
 
-    _Thread_queue_Requeue( the_thread->Wait.queue, the_thread );
+    if ( the_thread->priority_generation == my_generation ) {
+      if ( _States_Is_ready( the_thread->current_state ) ) {
+        _Scheduler_Change_priority(
+          the_thread,
+          new_priority,
+          prepend_it
+        );
+      } else {
+        _Scheduler_Update_priority( the_thread, new_priority );
+      }
+    }
   }
+
+  _ISR_Enable( level );
 }
diff --git a/cpukit/score/src/threadinitialize.c b/cpukit/score/src/threadinitialize.c
index 934fea9..993d95f 100644
--- a/cpukit/score/src/threadinitialize.c
+++ b/cpukit/score/src/threadinitialize.c
@@ -198,6 +198,7 @@ bool _Thread_Initialize(
   the_thread->Wait.queue              = NULL;
   the_thread->resource_count          = 0;
   the_thread->real_priority           = priority;
+  the_thread->priority_generation     = 0;
   the_thread->Start.initial_priority  = priority;
 
   _Thread_Wait_flags_set( the_thread, THREAD_WAIT_FLAGS_INITIAL );
diff --git a/testsuites/sptests/Makefile.am b/testsuites/sptests/Makefile.am
index 0d1e687..9025ff3 100644
--- a/testsuites/sptests/Makefile.am
+++ b/testsuites/sptests/Makefile.am
@@ -37,6 +37,7 @@ if HAS_SMP
 else
 _SUBDIRS += sp29
 endif
+_SUBDIRS += spintrcritical23
 _SUBDIRS += spatomic01
 _SUBDIRS += spintrcritical22
 _SUBDIRS += spsem03
diff --git a/testsuites/sptests/configure.ac b/testsuites/sptests/configure.ac
index eef901b..ae3c763 100644
--- a/testsuites/sptests/configure.ac
+++ b/testsuites/sptests/configure.ac
@@ -40,6 +40,7 @@ AM_CONDITIONAL(HAS_SMP,test "$rtems_cv_RTEMS_SMP" = "yes")
 
 # Explicitly list all Makefiles here
 AC_CONFIG_FILES([Makefile
+spintrcritical23/Makefile
 spatomic01/Makefile
 spglobalcon01/Makefile
 spintrcritical22/Makefile
diff --git a/testsuites/sptests/spintrcritical23/Makefile.am b/testsuites/sptests/spintrcritical23/Makefile.am
new file mode 100644
index 0000000..6baaf7b
--- /dev/null
+++ b/testsuites/sptests/spintrcritical23/Makefile.am
@@ -0,0 +1,22 @@
+rtems_tests_PROGRAMS = spintrcritical23
+spintrcritical23_SOURCES = init.c
+spintrcritical23_SOURCES += ../spintrcritical_support/intrcritical.h
+spintrcritical23_SOURCES += ../spintrcritical_support/intrcritical.c
+
+dist_rtems_tests_DATA = spintrcritical23.scn spintrcritical23.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 = $(spintrcritical23_OBJECTS)
+LINK_LIBS = $(spintrcritical23_LDLIBS)
+
+spintrcritical23$(EXEEXT): $(spintrcritical23_OBJECTS) $(spintrcritical23_DEPENDENCIES)
+	@rm -f spintrcritical23$(EXEEXT)
+	$(make-exe)
+
+include $(top_srcdir)/../automake/local.am
diff --git a/testsuites/sptests/spintrcritical23/init.c b/testsuites/sptests/spintrcritical23/init.c
new file mode 100644
index 0000000..01cb379
--- /dev/null
+++ b/testsuites/sptests/spintrcritical23/init.c
@@ -0,0 +1,177 @@
+/*
+ * Copyright (c) 2015 embedded brains GmbH.  All rights reserved.
+ *
+ *  embedded brains GmbH
+ *  Dornierstr. 4
+ *  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>
+
+#include <string.h>
+
+#include <rtems.h>
+#include <rtems/score/schedulerpriority.h>
+#include <rtems/score/threadimpl.h>
+
+const char rtems_test_name[] = "SPINTRCRITICAL 23";
+
+typedef struct {
+  RTEMS_INTERRUPT_LOCK_MEMBER(lock)
+  rtems_id task_id;
+  Thread_Control *tcb;
+  rtems_task_priority next_priority;
+  uint32_t priority_generation;
+  Scheduler_priority_Node scheduler_node;
+  bool done;
+} test_context;
+
+static test_context ctx_instance;
+
+static Thread_Control *get_tcb(rtems_id id)
+{
+  Objects_Locations location;
+  Thread_Control *tcb;
+
+  tcb = _Thread_Get(id, &location);
+  _Objects_Put(&tcb->Object);
+
+  rtems_test_assert(tcb != NULL && location == OBJECTS_LOCAL);
+
+  return tcb;
+}
+
+static bool scheduler_node_unchanged(const test_context *ctx)
+{
+   return memcmp(
+     &ctx->scheduler_node,
+     ctx->tcb->Scheduler.node,
+     sizeof(ctx->scheduler_node)
+   ) == 0;
+}
+
+static void change_priority(rtems_id timer, void *arg)
+{
+  /* The arg is NULL */
+  test_context *ctx = &ctx_instance;
+  rtems_interrupt_lock_context lock_context;
+  rtems_task_priority my_priority;
+
+  rtems_interrupt_lock_acquire(&ctx->lock, &lock_context);
+  if (
+    ctx->priority_generation != ctx->tcb->priority_generation
+      && scheduler_node_unchanged(ctx)
+  ) {
+    ctx->done = true;
+  }
+  my_priority = ctx->next_priority;
+  rtems_interrupt_lock_release(&ctx->lock, &lock_context);
+
+  if (ctx->done) {
+    rtems_status_code sc;
+
+    sc = rtems_task_set_priority(
+      ctx->task_id,
+      my_priority,
+      &my_priority
+    );
+    rtems_test_assert(sc == RTEMS_SUCCESSFUL);
+  }
+}
+
+static bool test_body(void *arg)
+{
+  test_context *ctx = arg;
+  rtems_status_code sc;
+  rtems_interrupt_lock_context lock_context;
+  rtems_task_priority my_priority;
+  rtems_task_priority last_priority;
+
+  rtems_interrupt_lock_acquire(&ctx->lock, &lock_context);
+  my_priority = 1 + (ctx->next_priority + 1) % 3;
+  ctx->next_priority = 1 + (my_priority + 1) % 3;
+  ctx->priority_generation = ctx->tcb->priority_generation;
+  memcpy(
+    &ctx->scheduler_node,
+    ctx->tcb->Scheduler.node,
+    sizeof(ctx->scheduler_node)
+  );
+  rtems_interrupt_lock_release(&ctx->lock, &lock_context);
+
+  sc = rtems_task_set_priority(
+    ctx->task_id,
+    my_priority,
+    &last_priority
+  );
+  rtems_test_assert(sc == RTEMS_SUCCESSFUL);
+
+  if (ctx->done) {
+    rtems_task_priority next_priority;
+
+    sc = rtems_task_set_priority(
+      ctx->task_id,
+      RTEMS_CURRENT_PRIORITY,
+      &next_priority
+    );
+    rtems_test_assert(sc == RTEMS_SUCCESSFUL);
+    rtems_test_assert(next_priority == ctx->next_priority);
+    rtems_test_assert(next_priority != my_priority);
+  }
+
+  return ctx->done;
+}
+
+static void Init(rtems_task_argument arg)
+{
+  test_context *ctx = &ctx_instance;
+  rtems_status_code sc;
+
+  TEST_BEGIN();
+
+  sc = rtems_task_create(
+    rtems_build_name('T', 'E', 'S', 'T'),
+    1,
+    RTEMS_MINIMUM_STACK_SIZE,
+    RTEMS_DEFAULT_MODES,
+    RTEMS_DEFAULT_ATTRIBUTES,
+    &ctx->task_id
+  );
+  rtems_test_assert(sc == RTEMS_SUCCESSFUL);
+
+  rtems_interrupt_lock_initialize(&ctx->lock, "Test");
+  ctx->tcb = get_tcb(ctx->task_id);
+
+  interrupt_critical_section_test(test_body, ctx, change_priority);
+  rtems_test_assert(ctx->done);
+
+  TEST_END();
+  rtems_test_exit(0);
+}
+
+#define CONFIGURE_APPLICATION_NEEDS_CLOCK_DRIVER
+#define CONFIGURE_APPLICATION_NEEDS_CONSOLE_DRIVER
+
+#define CONFIGURE_MICROSECONDS_PER_TICK 1000
+
+#define CONFIGURE_MAXIMUM_TASKS 2
+#define CONFIGURE_MAXIMUM_TIMERS 1
+#define CONFIGURE_MAXIMUM_USER_EXTENSIONS 1
+
+#define CONFIGURE_INITIAL_EXTENSIONS RTEMS_TEST_INITIAL_EXTENSION
+
+#define CONFIGURE_RTEMS_INIT_TASKS_TABLE
+
+#define CONFIGURE_INIT
+
+#include <rtems/confdefs.h>
diff --git a/testsuites/sptests/spintrcritical23/spintrcritical23.doc b/testsuites/sptests/spintrcritical23/spintrcritical23.doc
new file mode 100644
index 0000000..89e5281
--- /dev/null
+++ b/testsuites/sptests/spintrcritical23/spintrcritical23.doc
@@ -0,0 +1,12 @@
+This file describes the directives and concepts tested by this test set.
+
+test set name: spintrcritical23
+
+directives:
+
+  - _Thread_Change_priority()
+
+concepts:
+
+  - Ensure that priority updates work if carried out in interrupt context while
+    a priority change at task level is in progress.
diff --git a/testsuites/sptests/spintrcritical23/spintrcritical23.scn b/testsuites/sptests/spintrcritical23/spintrcritical23.scn
new file mode 100644
index 0000000..cea3292
--- /dev/null
+++ b/testsuites/sptests/spintrcritical23/spintrcritical23.scn
@@ -0,0 +1,2 @@
+*** BEGIN OF TEST SPINTRCRITICAL 23 ***
+*** END OF TEST SPINTRCRITICAL 23 ***
-- 
2.1.4


More information about the devel mailing list