From b8a5abf3fafa9df7cc0354c0ada6192c38e78354 Mon Sep 17 00:00:00 2001 From: Sebastian Huber Date: Thu, 26 Feb 2015 10:33:36 +0100 Subject: 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 | 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 eeb3e0fc7c..1056c6b11e 100644 --- a/cpukit/rtems/src/taskmode.c +++ b/cpukit/rtems/src/taskmode.c @@ -21,42 +21,10 @@ #include #include #include +#include #include #include -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 31ae6d184d..7bf4038b5e 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 688253c279..56b35de8b9 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 == PRIORITY_PSEUDO_ISR + ); } SCHEDULER_RETURN_VOID_OR_NULL; diff --git a/cpukit/score/src/scheduleredfchangepriority.c b/cpukit/score/src/scheduleredfchangepriority.c index 3eabc83878..61e0481d03 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 308a691e64..977534214c 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 == PRIORITY_PSEUDO_ISR + ); } SCHEDULER_RETURN_VOID_OR_NULL; diff --git a/cpukit/score/src/schedulerprioritychangepriority.c b/cpukit/score/src/schedulerprioritychangepriority.c index 06c5f0f7c6..f883e02d43 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 06d29f3b91..a912ebfbe2 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 == PRIORITY_PSEUDO_ISR + ); } SCHEDULER_RETURN_VOID_OR_NULL; diff --git a/cpukit/score/src/schedulerpriorityyield.c b/cpukit/score/src/schedulerpriorityyield.c index 2ee2d03057..5dab094f46 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 b8638ad28c..9b94b3ab26 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 6f9b2f719f..a020f74767 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 == PRIORITY_PSEUDO_ISR + ); } SCHEDULER_RETURN_VOID_OR_NULL; diff --git a/cpukit/score/src/threadchangepriority.c b/cpukit/score/src/threadchangepriority.c index ca2c5871af..d61dfb859a 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 b770ff214a..2181b8c9f0 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 diff --git a/testsuites/sptests/sp04/task1.c b/testsuites/sptests/sp04/task1.c index 364e09c63e..eac1bf6d51 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 9701e739d9..7be2afd7c1 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( -- cgit v1.2.3