score: Update _Thread_Heir only if necessary

Previously, the _Thread_Heir was updated unconditionally in case a new
heir was determined.  The _Thread_Dispatch_necessary was only updated in
case the executing thread was preemptible or an internal thread was
unblocked.  Change this to update the _Thread_Heir and
_Thread_Dispatch_necessary only in case the currently selected heir
thread is preemptible or a dispatch is forced.  Move the schedule
decision into the change priority operation and use the schedule
operation only in rtems_task_mode() in case preemption is enabled or an
ASR dispatch is necessary.  This is a behaviour change.  Previously, the
RTEMS_NO_PREEMPT also prevented signal delivery in certain cases (not
always).  Now, signal delivery is no longer influenced by
RTEMS_NO_PREEMPT.  Since the currently selected heir thread is used to
determine if a new heir is chosen, non-preemptible heir threads
currently not executing now prevent a new heir.  This may have an
application impact, see change test tm04.  Document this change in sp04.

Update #2273.

cpukit/rtems/src/taskmode.c

@@ -21,42 +21,10 @@
 #include <rtems/rtems/tasks.h>
 #include <rtems/rtems/asrimpl.h>
 #include <rtems/rtems/modesimpl.h>
+#include <rtems/score/schedulerimpl.h>
 #include <rtems/score/threadimpl.h>
 #include <rtems/config.h>
 
-static void _RTEMS_Tasks_Dispatch_if_necessary(
-  Thread_Control *executing,
-  bool            needs_asr_dispatching
-)
-{
-  if ( _Thread_Dispatch_is_enabled() ) {
-    bool dispatch_necessary = needs_asr_dispatching;
-
-    /*
-     * FIXME: This locking approach is brittle.  It only works since the
-     * current simple SMP scheduler has no support for the non-preempt mode.
-     */
-#if defined( RTEMS_SMP )
-    ISR_Level level;
-
-    _ISR_Disable_without_giant( level );
-#endif
-
-    if ( !_Thread_Is_heir( executing ) && executing->is_preemptible ) {
-      dispatch_necessary = true;
-      _Thread_Dispatch_necessary = dispatch_necessary;
-    }
-
-#if defined( RTEMS_SMP )
-    _ISR_Enable_without_giant( level );
-#endif
-
-    if ( dispatch_necessary ) {
-      _Thread_Dispatch();
-    }
-  }
-}
-
 rtems_status_code rtems_task_mode(
   rtems_mode  mode_set,
   rtems_mode  mask,
@@ -66,6 +34,7 @@ rtems_status_code rtems_task_mode(
   Thread_Control     *executing;
   RTEMS_API_Control  *api;
   ASR_Information    *asr;
+  bool                preempt_enabled;
   bool                needs_asr_dispatching;
   rtems_mode          old_mode;
 
@@ -91,6 +60,7 @@ rtems_status_code rtems_task_mode(
   /*
    *  These are generic thread scheduling characteristics.
    */
+  preempt_enabled = false;
   if ( mask & RTEMS_PREEMPT_MASK ) {
 #if defined( RTEMS_SMP )
     if ( rtems_configuration_is_smp_enabled() &&
@@ -98,8 +68,10 @@ rtems_status_code rtems_task_mode(
       return RTEMS_NOT_IMPLEMENTED;
     }
 #endif
+    bool is_preempt_enabled = _Modes_Is_preempt( mode_set );
 
-    executing->is_preemptible = _Modes_Is_preempt( mode_set );
+    preempt_enabled = !executing->is_preemptible && is_preempt_enabled;
+    executing->is_preemptible = is_preempt_enabled;
   }
 
   if ( mask & RTEMS_TIMESLICE_MASK ) {
@@ -137,7 +109,15 @@ rtems_status_code rtems_task_mode(
     }
   }
 
-  _RTEMS_Tasks_Dispatch_if_necessary( executing, needs_asr_dispatching );
+  if ( preempt_enabled || needs_asr_dispatching ) {
+    ISR_Level level;
+
+    _Thread_Disable_dispatch();
+    _ISR_Disable( level );
+    _Scheduler_Schedule( executing );
+    _ISR_Enable( level );
+    _Thread_Enable_dispatch();
+  }
 
   return RTEMS_SUCCESSFUL;
 }

cpukit/score/include/rtems/score/schedulerimpl.h

@@ -314,6 +314,9 @@ RTEMS_INLINE_ROUTINE void _Scheduler_Unblock( Thread_Control *the_thread )
  * must ensure that the priority value actually changed and is not equal to the
  * current priority value.
  *
+ * The operation must update the heir and thread dispatch necessary variables
+ * in case the set of scheduled threads changes.
+ *
  * @param[in] the_thread The thread changing its priority.
  * @param[in] new_priority The new thread priority.
  * @param[in] prepend_it In case this is true, then enqueue the thread as the
@@ -630,16 +633,16 @@ bool _Scheduler_Set_affinity(
 #endif /* defined(__RTEMS_HAVE_SYS_CPUSET_H__) */
 
 RTEMS_INLINE_ROUTINE void _Scheduler_Update_heir(
-  Thread_Control *heir,
-  bool force_dispatch
+  Thread_Control *new_heir,
+  bool            force_dispatch
 )
 {
-  Thread_Control *executing = _Thread_Executing;
+  Thread_Control *heir = _Thread_Heir;
 
-  _Thread_Heir = heir;
-
-  if ( executing != heir && ( force_dispatch || executing->is_preemptible ) )
+  if ( heir != new_heir && ( heir->is_preemptible || force_dispatch ) ) {
+    _Thread_Heir = new_heir;
     _Thread_Dispatch_necessary = true;
+  }
 }
 
 RTEMS_INLINE_ROUTINE void _Scheduler_Generic_block(

cpukit/score/src/schedulercbsunblock.c

@@ -79,10 +79,10 @@ Scheduler_Void_or_thread _Scheduler_CBS_Unblock(
        _Thread_Heir->current_priority
     )
   ) {
-    _Thread_Heir = the_thread;
-    if ( _Thread_Executing->is_preemptible ||
-         the_thread->current_priority == 0 )
-      _Thread_Dispatch_necessary = true;
+    _Scheduler_Update_heir(
+      the_thread,
+      the_thread->current_priority == PRIORITY_PSEUDO_ISR
+    );
   }
 
   SCHEDULER_RETURN_VOID_OR_NULL;

cpukit/score/src/scheduleredfchangepriority.c

@@ -39,5 +39,7 @@ Scheduler_Void_or_thread _Scheduler_EDF_Change_priority(
     false
   );
 
+  _Scheduler_EDF_Schedule_body( scheduler, the_thread, false );
+
   SCHEDULER_RETURN_VOID_OR_NULL;
 }

cpukit/score/src/scheduleredfunblock.c

@@ -46,10 +46,10 @@ Scheduler_Void_or_thread _Scheduler_EDF_Unblock(
          scheduler,
          _Thread_Heir->current_priority,
          the_thread->current_priority )) {
-    _Thread_Heir = the_thread;
-    if ( _Thread_Executing->is_preemptible ||
-         the_thread->current_priority == 0 )
-      _Thread_Dispatch_necessary = true;
+    _Scheduler_Update_heir(
+      the_thread,
+      the_thread->current_priority == PRIORITY_PSEUDO_ISR
+    );
   }
 
   SCHEDULER_RETURN_VOID_OR_NULL;

cpukit/score/src/schedulerprioritychangepriority.c

@@ -59,5 +59,7 @@ Scheduler_Void_or_thread _Scheduler_priority_Change_priority(
     );
   }
 
+  _Scheduler_priority_Schedule_body( scheduler, the_thread, false );
+
   SCHEDULER_RETURN_VOID_OR_NULL;
 }

cpukit/score/src/schedulerpriorityunblock.c

@@ -52,10 +52,10 @@ Scheduler_Void_or_thread _Scheduler_priority_Unblock (
    *    a pseudo-ISR system task, we need to do a context switch.
    */
   if ( the_thread->current_priority < _Thread_Heir->current_priority ) {
-    _Thread_Heir = the_thread;
-    if ( _Thread_Executing->is_preemptible ||
-        the_thread->current_priority == 0 )
-      _Thread_Dispatch_necessary = true;
+    _Scheduler_Update_heir(
+      the_thread,
+      the_thread->current_priority == PRIORITY_PSEUDO_ISR
+    );
   }
 
   SCHEDULER_RETURN_VOID_OR_NULL;

cpukit/score/src/schedulerpriorityyield.c

@@ -29,20 +29,12 @@ Scheduler_Void_or_thread _Scheduler_priority_Yield(
   Scheduler_priority_Node *node = _Scheduler_priority_Thread_get_node( the_thread );
   Chain_Control *ready_chain = node->Ready_queue.ready_chain;
 
-  (void) scheduler;
-
   if ( !_Chain_Has_only_one_node( ready_chain ) ) {
     _Chain_Extract_unprotected( &the_thread->Object.Node );
     _Chain_Append_unprotected( ready_chain, &the_thread->Object.Node );
-
-    if ( _Thread_Is_heir( the_thread ) ) {
-      _Thread_Heir = (Thread_Control *) _Chain_First( ready_chain );
-    }
-
-    _Thread_Dispatch_necessary = true;
-  } else if ( !_Thread_Is_heir( the_thread ) ) {
-    _Thread_Dispatch_necessary = true;
   }
 
+  _Scheduler_priority_Schedule_body( scheduler, the_thread, true );
+
   SCHEDULER_RETURN_VOID_OR_NULL;
 }

cpukit/score/src/schedulersimplechangepriority.c

@@ -39,5 +39,7 @@ Scheduler_Void_or_thread _Scheduler_simple_Change_priority(
     _Scheduler_simple_Insert_priority_fifo( &context->Ready, the_thread );
   }
 
+  _Scheduler_simple_Schedule_body( scheduler, the_thread, false );
+
   SCHEDULER_RETURN_VOID_OR_NULL;
 }

cpukit/score/src/schedulersimpleunblock.c

@@ -44,10 +44,10 @@ Scheduler_Void_or_thread _Scheduler_simple_Unblock(
    *    a pseudo-ISR system task, we need to do a context switch.
    */
   if ( the_thread->current_priority < _Thread_Heir->current_priority ) {
-    _Thread_Heir = the_thread;
-    if ( _Thread_Executing->is_preemptible ||
-        the_thread->current_priority == 0 )
-      _Thread_Dispatch_necessary = true;
+    _Scheduler_Update_heir(
+      the_thread,
+      the_thread->current_priority == PRIORITY_PSEUDO_ISR
+    );
   }
 
   SCHEDULER_RETURN_VOID_OR_NULL;

cpukit/score/src/threadchangepriority.c

@@ -46,17 +46,10 @@ void _Thread_Change_priority(
         new_priority,
         prepend_it
       );
-
-      _ISR_Flash( level );
-
-      /*
-       *  We altered the set of thread priorities.  So let's figure out
-       *  who is the heir and if we need to switch to them.
-       */
-      _Scheduler_Schedule( the_thread );
     } else {
       _Scheduler_Update_priority( the_thread, new_priority );
     }
+
     _ISR_Enable( level );
 
     _Thread_queue_Requeue( the_thread->Wait.queue, the_thread );

testsuites/sptests/sp04/system.h

@@ -50,6 +50,7 @@ void Task_switch(
 
 #define CONFIGURE_EXTRA_TASK_STACKS         (3 * RTEMS_MINIMUM_STACK_SIZE)
 #define CONFIGURE_MAXIMUM_TASKS             4
+#define CONFIGURE_MAXIMUM_TIMERS            1
 
 #include <rtems/confdefs.h>
 

testsuites/sptests/sp04/task1.c

@@ -34,6 +34,103 @@ showTaskSwitches (void)
   }
 }
 
+static int test_no_preempt_step;
+
+static rtems_id high_task_id;
+
+static rtems_id low_task_id;
+
+static void high_task( rtems_task_argument arg )
+{
+  rtems_status_code sc;
+
+  rtems_test_assert( test_no_preempt_step == 2 );
+  test_no_preempt_step = 3;
+
+  sc = rtems_event_transient_send( Task_id[ 1 ] );
+  rtems_test_assert( sc == RTEMS_SUCCESSFUL );
+
+  rtems_task_suspend(RTEMS_SELF);
+  rtems_test_assert(0);
+}
+
+static void low_task( rtems_task_argument arg )
+{
+  rtems_test_assert( test_no_preempt_step == 1 );
+  test_no_preempt_step = 2;
+
+  rtems_task_suspend(RTEMS_SELF);
+  rtems_test_assert(0);
+}
+
+static void no_preempt_timer( rtems_id id, void *arg )
+{
+  rtems_status_code sc;
+
+  rtems_test_assert( test_no_preempt_step == 0 );
+  test_no_preempt_step = 1;
+
+  sc = rtems_task_start( low_task_id, low_task, 0 );
+  rtems_test_assert( sc == RTEMS_SUCCESSFUL );
+
+  sc = rtems_task_start( high_task_id, high_task, 0 );
+  rtems_test_assert( sc == RTEMS_SUCCESSFUL );
+}
+
+static void test_no_preempt( void )
+{
+  rtems_status_code sc;
+  rtems_id id;
+
+  rtems_test_assert( test_no_preempt_step == 0 );
+
+  sc = rtems_task_delete( Task_id[ 2 ] );
+  rtems_test_assert( sc == RTEMS_SUCCESSFUL );
+
+  sc = rtems_task_delete( Task_id[ 3 ] );
+  rtems_test_assert( sc == RTEMS_SUCCESSFUL );
+
+  sc = rtems_task_create(
+    rtems_build_name( 'H', 'I', 'G', 'H' ),
+    1,
+    RTEMS_MINIMUM_STACK_SIZE,
+    RTEMS_DEFAULT_MODES,
+    RTEMS_DEFAULT_ATTRIBUTES,
+    &high_task_id
+  );
+  rtems_test_assert( sc == RTEMS_SUCCESSFUL );
+
+  sc = rtems_task_create(
+    rtems_build_name( 'L', 'O', 'W', ' ' ),
+    2,
+    RTEMS_MINIMUM_STACK_SIZE,
+    RTEMS_NO_PREEMPT,
+    RTEMS_DEFAULT_ATTRIBUTES,
+    &low_task_id
+  );
+  rtems_test_assert( sc == RTEMS_SUCCESSFUL );
+
+  sc = rtems_timer_create( rtems_build_name( 'N', 'O', 'P', 'R' ), &id );
+  rtems_test_assert( sc == RTEMS_SUCCESSFUL );
+
+  sc = rtems_timer_fire_after( id, 1, no_preempt_timer, NULL );
+  rtems_test_assert( sc == RTEMS_SUCCESSFUL );
+
+  sc = rtems_event_transient_receive( RTEMS_WAIT, RTEMS_NO_TIMEOUT );
+  rtems_test_assert( sc == RTEMS_SUCCESSFUL );
+
+  sc = rtems_timer_delete( id );
+  rtems_test_assert( sc == RTEMS_SUCCESSFUL );
+
+  sc = rtems_task_delete( high_task_id );
+  rtems_test_assert( sc == RTEMS_SUCCESSFUL );
+
+  sc = rtems_task_delete( low_task_id );
+  rtems_test_assert( sc == RTEMS_SUCCESSFUL );
+
+  rtems_test_assert( test_no_preempt_step == 3 );
+}
+
 rtems_task Task_1(
   rtems_task_argument argument
 )
@@ -117,6 +214,8 @@ rtems_task Task_1(
       status = rtems_extension_delete( Extension_id[1] );
       directive_failed( status, "rtems_extension_delete" );
 
+      test_no_preempt();
+
       TEST_END();
       rtems_test_exit (0);
     }

testsuites/tmtests/tm04/task1.c

@@ -343,6 +343,7 @@ rtems_task Low_tasks(
 {
   rtems_id          id;
   rtems_status_code status;
+  rtems_mode        prev;
 
   task_count--;
 
@@ -379,6 +380,8 @@ rtems_task Low_tasks(
     RTEMS_DEFAULT_OPTIONS,
     RTEMS_NO_TIMEOUT
   );
+
+  rtems_task_mode(RTEMS_PREEMPT, RTEMS_PREEMPT_MASK, &prev);
 }
 
 rtems_task Restart_task(