[PATCH 1/8] score: Update _Thread_Heir only if necessary
Sebastian Huber
sebastian.huber at embedded-brains.de
Wed Mar 4 15:07:01 UTC 2015
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 | 50 ++++-------
cpukit/score/include/rtems/score/schedulerimpl.h | 15 ++--
cpukit/score/src/schedulercbsunblock.c | 8 +-
cpukit/score/src/scheduleredfchangepriority.c | 2 +
cpukit/score/src/scheduleredfunblock.c | 8 +-
cpukit/score/src/schedulerprioritychangepriority.c | 2 +
cpukit/score/src/schedulerpriorityunblock.c | 8 +-
cpukit/score/src/schedulerpriorityyield.c | 12 +--
cpukit/score/src/schedulersimplechangepriority.c | 2 +
cpukit/score/src/schedulersimpleunblock.c | 8 +-
cpukit/score/src/threadchangepriority.c | 9 +-
testsuites/sptests/sp04/system.h | 1 +
testsuites/sptests/sp04/task1.c | 99 ++++++++++++++++++++++
testsuites/tmtests/tm04/task1.c | 3 +
14 files changed, 152 insertions(+), 75 deletions(-)
diff --git a/cpukit/rtems/src/taskmode.c b/cpukit/rtems/src/taskmode.c
index eeb3e0f..1056c6b 100644
--- a/cpukit/rtems/src/taskmode.c
+++ b/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;
}
diff --git a/cpukit/score/include/rtems/score/schedulerimpl.h b/cpukit/score/include/rtems/score/schedulerimpl.h
index 31ae6d1..7bf4038 100644
--- a/cpukit/score/include/rtems/score/schedulerimpl.h
+++ b/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_Heir = heir;
+ Thread_Control *heir = _Thread_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(
diff --git a/cpukit/score/src/schedulercbsunblock.c b/cpukit/score/src/schedulercbsunblock.c
index 688253c..bd27aff 100644
--- a/cpukit/score/src/schedulercbsunblock.c
+++ b/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 == 0
+ );
}
SCHEDULER_RETURN_VOID_OR_NULL;
diff --git a/cpukit/score/src/scheduleredfchangepriority.c b/cpukit/score/src/scheduleredfchangepriority.c
index 3eabc83..61e0481 100644
--- a/cpukit/score/src/scheduleredfchangepriority.c
+++ b/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;
}
diff --git a/cpukit/score/src/scheduleredfunblock.c b/cpukit/score/src/scheduleredfunblock.c
index 308a691..4218b2d 100644
--- a/cpukit/score/src/scheduleredfunblock.c
+++ b/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 == 0
+ );
}
SCHEDULER_RETURN_VOID_OR_NULL;
diff --git a/cpukit/score/src/schedulerprioritychangepriority.c b/cpukit/score/src/schedulerprioritychangepriority.c
index 06c5f0f..f883e02 100644
--- a/cpukit/score/src/schedulerprioritychangepriority.c
+++ b/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;
}
diff --git a/cpukit/score/src/schedulerpriorityunblock.c b/cpukit/score/src/schedulerpriorityunblock.c
index 06d29f3..ec85f58 100644
--- a/cpukit/score/src/schedulerpriorityunblock.c
+++ b/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 == 0
+ );
}
SCHEDULER_RETURN_VOID_OR_NULL;
diff --git a/cpukit/score/src/schedulerpriorityyield.c b/cpukit/score/src/schedulerpriorityyield.c
index 2ee2d03..5dab094 100644
--- a/cpukit/score/src/schedulerpriorityyield.c
+++ b/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;
}
diff --git a/cpukit/score/src/schedulersimplechangepriority.c b/cpukit/score/src/schedulersimplechangepriority.c
index b8638ad..9b94b3a 100644
--- a/cpukit/score/src/schedulersimplechangepriority.c
+++ b/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;
}
diff --git a/cpukit/score/src/schedulersimpleunblock.c b/cpukit/score/src/schedulersimpleunblock.c
index 6f9b2f7..e65a39b 100644
--- a/cpukit/score/src/schedulersimpleunblock.c
+++ b/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 == 0
+ );
}
SCHEDULER_RETURN_VOID_OR_NULL;
diff --git a/cpukit/score/src/threadchangepriority.c b/cpukit/score/src/threadchangepriority.c
index ca2c587..d61dfb8 100644
--- a/cpukit/score/src/threadchangepriority.c
+++ b/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 );
diff --git a/testsuites/sptests/sp04/system.h b/testsuites/sptests/sp04/system.h
index b770ff2..2181b8c 100644
--- a/testsuites/sptests/sp04/system.h
+++ b/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>
diff --git a/testsuites/sptests/sp04/task1.c b/testsuites/sptests/sp04/task1.c
index 364e09c..eac1bf6 100644
--- a/testsuites/sptests/sp04/task1.c
+++ b/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);
}
diff --git a/testsuites/tmtests/tm04/task1.c b/testsuites/tmtests/tm04/task1.c
index 9701e73..7be2afd 100644
--- a/testsuites/tmtests/tm04/task1.c
+++ b/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(
--
1.8.4.5
More information about the devel
mailing list