[PATCH] score: PR2181: Add _Thread_Yield()
Gedare Bloom
gedare at rtems.org
Thu Jun 12 14:50:07 UTC 2014
Good.
On Thu, Jun 12, 2014 at 10:19 AM, Sebastian Huber
<sebastian.huber at embedded-brains.de> wrote:
> The _Scheduler_Yield() was called by the executing thread with thread
> dispatching disabled and interrupts enabled. The rtems_task_suspend()
> is explicitly allowed in ISRs:
>
> http://rtems.org/onlinedocs/doc-current/share/rtems/html/c_user/Interrupt-Manager-Directives-Allowed-from-an-ISR.html#Interrupt-Manager-Directives-Allowed-from-an-ISR
>
> Unlike the other scheduler operations the locking was performed inside
> the operation. This lead to the following race condition. Suppose a
> ISR suspends the executing thread right before the yield scheduler
> operation. Now the executing thread is not longer in the set of ready
> threads. The typical scheduler operations did not check the thread
> state and will now extract the thread again and enqueue it. This
> corrupted data structures.
>
> Add _Thread_Yield() and do the scheduler yield operation with interrupts
> disabled. This has a negligible effect on the interrupt latency.
> ---
> cpukit/posix/src/nanosleep.c | 3 +-
> cpukit/posix/src/sched_yield.c | 7 +--
> cpukit/rtems/src/taskwakeafter.c | 3 +-
> cpukit/score/Makefile.am | 1 +
> .../score/include/rtems/score/schedulersmpimpl.h | 21 ++++++++++
> cpukit/score/include/rtems/score/threadimpl.h | 2 +
> cpukit/score/src/schedulerdefaulttick.c | 4 +-
> cpukit/score/src/scheduleredfyield.c | 7 ---
> cpukit/score/src/schedulerprioritysmp.c | 14 +++---
> cpukit/score/src/schedulerpriorityyield.c | 23 ++++-------
> cpukit/score/src/schedulersimplesmp.c | 14 +++---
> cpukit/score/src/schedulersimpleyield.c | 15 +------
> cpukit/score/src/threadyield.c | 41 ++++++++++++++++++++
> 13 files changed, 98 insertions(+), 57 deletions(-)
> create mode 100644 cpukit/score/src/threadyield.c
>
> diff --git a/cpukit/posix/src/nanosleep.c b/cpukit/posix/src/nanosleep.c
> index ebaef33..54902b9 100644
> --- a/cpukit/posix/src/nanosleep.c
> +++ b/cpukit/posix/src/nanosleep.c
> @@ -22,7 +22,6 @@
> #include <errno.h>
>
> #include <rtems/seterr.h>
> -#include <rtems/score/schedulerimpl.h>
> #include <rtems/score/threadimpl.h>
> #include <rtems/score/timespec.h>
> #include <rtems/score/watchdogimpl.h>
> @@ -65,7 +64,7 @@ int nanosleep(
> if ( !ticks ) {
> _Thread_Disable_dispatch();
> executing = _Thread_Executing;
> - _Scheduler_Yield( _Scheduler_Get( executing ), executing );
> + _Thread_Yield( executing );
> _Thread_Enable_dispatch();
> if ( rmtp ) {
> rmtp->tv_sec = 0;
> diff --git a/cpukit/posix/src/sched_yield.c b/cpukit/posix/src/sched_yield.c
> index 5293b19..e398fbf 100644
> --- a/cpukit/posix/src/sched_yield.c
> +++ b/cpukit/posix/src/sched_yield.c
> @@ -21,16 +21,13 @@
> #include <sched.h>
>
> #include <rtems/score/percpu.h>
> -#include <rtems/score/schedulerimpl.h>
> #include <rtems/score/threaddispatch.h>
> +#include <rtems/score/threadimpl.h>
>
> int sched_yield( void )
> {
> - Thread_Control *executing;
> -
> _Thread_Disable_dispatch();
> - executing = _Thread_Executing;
> - _Scheduler_Yield( _Scheduler_Get( executing ), executing );
> + _Thread_Yield( _Thread_Executing );
> _Thread_Enable_dispatch();
> return 0;
> }
> diff --git a/cpukit/rtems/src/taskwakeafter.c b/cpukit/rtems/src/taskwakeafter.c
> index 367b43a..88de6e5 100644
> --- a/cpukit/rtems/src/taskwakeafter.c
> +++ b/cpukit/rtems/src/taskwakeafter.c
> @@ -20,7 +20,6 @@
>
> #include <rtems/rtems/tasks.h>
> #include <rtems/score/threadimpl.h>
> -#include <rtems/score/schedulerimpl.h>
> #include <rtems/score/watchdogimpl.h>
>
> rtems_status_code rtems_task_wake_after(
> @@ -37,7 +36,7 @@ rtems_status_code rtems_task_wake_after(
> executing = _Thread_Executing;
>
> if ( ticks == 0 ) {
> - _Scheduler_Yield( _Scheduler_Get( executing ), executing );
> + _Thread_Yield( executing );
> } else {
> _Thread_Set_state( executing, STATES_DELAYING );
> _Watchdog_Initialize(
> diff --git a/cpukit/score/Makefile.am b/cpukit/score/Makefile.am
> index 2fc31c5..6caefb5 100644
> --- a/cpukit/score/Makefile.am
> +++ b/cpukit/score/Makefile.am
> @@ -287,6 +287,7 @@ libscore_a_SOURCES += src/thread.c src/threadchangepriority.c \
> src/threadstackallocate.c src/threadstackfree.c src/threadstart.c \
> src/threadstartmultitasking.c src/iterateoverthreads.c \
> src/threadblockingoperationcancel.c
> +libscore_a_SOURCES += src/threadyield.c
>
> if HAS_SMP
> libscore_a_SOURCES += src/smpbarrierwait.c
> diff --git a/cpukit/score/include/rtems/score/schedulersmpimpl.h b/cpukit/score/include/rtems/score/schedulersmpimpl.h
> index b4126b5..9d74434 100644
> --- a/cpukit/score/include/rtems/score/schedulersmpimpl.h
> +++ b/cpukit/score/include/rtems/score/schedulersmpimpl.h
> @@ -672,6 +672,27 @@ static inline void _Scheduler_SMP_Change_priority(
> }
> }
>
> +static inline void _Scheduler_SMP_Yield(
> + Scheduler_Context *context,
> + Thread_Control *thread,
> + Scheduler_SMP_Extract extract_from_ready,
> + Scheduler_SMP_Enqueue enqueue_fifo,
> + Scheduler_SMP_Enqueue enqueue_scheduled_fifo
> +)
> +{
> + Scheduler_SMP_Node *node = _Scheduler_SMP_Node_get( thread );
> +
> + if ( node->state == SCHEDULER_SMP_NODE_SCHEDULED ) {
> + _Scheduler_SMP_Extract_from_scheduled( thread );
> +
> + ( *enqueue_scheduled_fifo )( context, thread );
> + } else {
> + ( *extract_from_ready )( context, thread );
> +
> + ( *enqueue_fifo )( context, thread );
> + }
> +}
> +
> static inline void _Scheduler_SMP_Insert_scheduled_lifo(
> Scheduler_Context *context,
> Thread_Control *thread
> diff --git a/cpukit/score/include/rtems/score/threadimpl.h b/cpukit/score/include/rtems/score/threadimpl.h
> index 7e6681b..e90346c 100644
> --- a/cpukit/score/include/rtems/score/threadimpl.h
> +++ b/cpukit/score/include/rtems/score/threadimpl.h
> @@ -186,6 +186,8 @@ bool _Thread_Restart(
> Thread_Entry_numeric_type numeric_argument
> );
>
> +void _Thread_Yield( Thread_Control *executing );
> +
> bool _Thread_Set_life_protection( bool protect );
>
> void _Thread_Life_action_handler(
> diff --git a/cpukit/score/src/schedulerdefaulttick.c b/cpukit/score/src/schedulerdefaulttick.c
> index 98cd05e..db67ca1 100644
> --- a/cpukit/score/src/schedulerdefaulttick.c
> +++ b/cpukit/score/src/schedulerdefaulttick.c
> @@ -29,6 +29,8 @@ void _Scheduler_default_Tick(
> Thread_Control *executing
> )
> {
> + (void) scheduler;
> +
> #ifdef __RTEMS_USE_TICKS_FOR_STATISTICS__
> /*
> * Increment the number of ticks this thread has been executing
> @@ -69,7 +71,7 @@ void _Scheduler_default_Tick(
> * currently executing thread is placed at the rear of the
> * FIFO for this priority and a new heir is selected.
> */
> - _Scheduler_Yield( scheduler, executing );
> + _Thread_Yield( executing );
> executing->cpu_time_budget =
> rtems_configuration_get_ticks_per_timeslice();
> }
> diff --git a/cpukit/score/src/scheduleredfyield.c b/cpukit/score/src/scheduleredfyield.c
> index 9eb0782..d43448b 100644
> --- a/cpukit/score/src/scheduleredfyield.c
> +++ b/cpukit/score/src/scheduleredfyield.c
> @@ -29,9 +29,6 @@ void _Scheduler_EDF_Yield(
> Scheduler_EDF_Context *context =
> _Scheduler_EDF_Get_context( scheduler );
> Scheduler_EDF_Node *node = _Scheduler_EDF_Node_get( the_thread );
> - ISR_Level level;
> -
> - _ISR_Disable( level );
>
> /*
> * The RBTree has more than one node, enqueue behind the tasks
> @@ -40,9 +37,5 @@ void _Scheduler_EDF_Yield(
> _RBTree_Extract( &context->Ready, &node->Node );
> _RBTree_Insert( &context->Ready, &node->Node );
>
> - _ISR_Flash( level );
> -
> _Scheduler_EDF_Schedule_body( scheduler, the_thread, false );
> -
> - _ISR_Enable( level );
> }
> diff --git a/cpukit/score/src/schedulerprioritysmp.c b/cpukit/score/src/schedulerprioritysmp.c
> index 96b1689..bcbd26d 100644
> --- a/cpukit/score/src/schedulerprioritysmp.c
> +++ b/cpukit/score/src/schedulerprioritysmp.c
> @@ -237,12 +237,12 @@ void _Scheduler_priority_SMP_Yield(
> )
> {
> Scheduler_Context *context = _Scheduler_Get_context( scheduler );
> - ISR_Level level;
>
> - _ISR_Disable( level );
> -
> - _Scheduler_SMP_Extract_from_scheduled( thread );
> - _Scheduler_priority_SMP_Enqueue_scheduled_fifo( context, thread );
> -
> - _ISR_Enable( level );
> + return _Scheduler_SMP_Yield(
> + context,
> + thread,
> + _Scheduler_priority_SMP_Extract_from_ready,
> + _Scheduler_priority_SMP_Enqueue_fifo,
> + _Scheduler_priority_SMP_Enqueue_scheduled_fifo
> + );
> }
> diff --git a/cpukit/score/src/schedulerpriorityyield.c b/cpukit/score/src/schedulerpriorityyield.c
> index f2aeada..60bab39 100644
> --- a/cpukit/score/src/schedulerpriorityyield.c
> +++ b/cpukit/score/src/schedulerpriorityyield.c
> @@ -19,7 +19,6 @@
> #endif
>
> #include <rtems/score/schedulerpriorityimpl.h>
> -#include <rtems/score/isr.h>
> #include <rtems/score/threadimpl.h>
>
> void _Scheduler_priority_Yield(
> @@ -29,23 +28,19 @@ void _Scheduler_priority_Yield(
> {
> Scheduler_priority_Node *node = _Scheduler_priority_Node_get( the_thread );
> Chain_Control *ready_chain = node->Ready_queue.ready_chain;
> - ISR_Level level;
>
> (void) scheduler;
>
> - _ISR_Disable( level );
> - 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 ( !_Chain_Has_only_one_node( ready_chain ) ) {
> + _Chain_Extract_unprotected( &the_thread->Object.Node );
> + _Chain_Append_unprotected( ready_chain, &the_thread->Object.Node );
>
> - _ISR_Flash( level );
> -
> - if ( _Thread_Is_heir( the_thread ) )
> - _Thread_Heir = (Thread_Control *) _Chain_First( ready_chain );
> - _Thread_Dispatch_necessary = true;
> + if ( _Thread_Is_heir( the_thread ) ) {
> + _Thread_Heir = (Thread_Control *) _Chain_First( ready_chain );
> }
> - else if ( !_Thread_Is_heir( the_thread ) )
> - _Thread_Dispatch_necessary = true;
>
> - _ISR_Enable( level );
> + _Thread_Dispatch_necessary = true;
> + } else if ( !_Thread_Is_heir( the_thread ) ) {
> + _Thread_Dispatch_necessary = true;
> + }
> }
> diff --git a/cpukit/score/src/schedulersimplesmp.c b/cpukit/score/src/schedulersimplesmp.c
> index eb260ef..37458d6 100644
> --- a/cpukit/score/src/schedulersimplesmp.c
> +++ b/cpukit/score/src/schedulersimplesmp.c
> @@ -302,12 +302,12 @@ void _Scheduler_simple_SMP_Yield(
> )
> {
> Scheduler_Context *context = _Scheduler_Get_context( scheduler );
> - ISR_Level level;
>
> - _ISR_Disable( level );
> -
> - _Scheduler_SMP_Extract_from_scheduled( thread );
> - _Scheduler_simple_SMP_Enqueue_scheduled_fifo( context, thread );
> -
> - _ISR_Enable( level );
> + return _Scheduler_SMP_Yield(
> + context,
> + thread,
> + _Scheduler_simple_SMP_Extract_from_ready,
> + _Scheduler_simple_SMP_Enqueue_fifo,
> + _Scheduler_simple_SMP_Enqueue_scheduled_fifo
> + );
> }
> diff --git a/cpukit/score/src/schedulersimpleyield.c b/cpukit/score/src/schedulersimpleyield.c
> index 65578d0..b807530 100644
> --- a/cpukit/score/src/schedulersimpleyield.c
> +++ b/cpukit/score/src/schedulersimpleyield.c
> @@ -19,7 +19,6 @@
> #endif
>
> #include <rtems/score/schedulersimpleimpl.h>
> -#include <rtems/score/isr.h>
>
> void _Scheduler_simple_Yield(
> const Scheduler_Control *scheduler,
> @@ -28,16 +27,8 @@ void _Scheduler_simple_Yield(
> {
> Scheduler_simple_Context *context =
> _Scheduler_simple_Get_context( scheduler );
> - ISR_Level level;
>
> - _ISR_Disable( level );
> -
> - _Chain_Extract_unprotected( &the_thread->Object.Node );
> - _Scheduler_simple_Insert_priority_fifo( &context->Ready, the_thread );
> -
> - _ISR_Flash( level );
> -
> - _Scheduler_simple_Schedule_body( scheduler, the_thread, false );
> -
> - _ISR_Enable( level );
> + _Chain_Extract_unprotected( &the_thread->Object.Node );
> + _Scheduler_simple_Insert_priority_fifo( &context->Ready, the_thread );
> + _Scheduler_simple_Schedule_body( scheduler, the_thread, false );
> }
> diff --git a/cpukit/score/src/threadyield.c b/cpukit/score/src/threadyield.c
> new file mode 100644
> index 0000000..b49e2b3
> --- /dev/null
> +++ b/cpukit/score/src/threadyield.c
> @@ -0,0 +1,41 @@
> +/**
> + * @file
> + *
> + * @brief Thread Yield
> + *
> + * @ingroup ScoreThread
> + */
> +
> +/*
> + * Copyright (c) 2014 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.org/license/LICENSE.
> + */
> +
> +#if HAVE_CONFIG_H
> + #include "config.h"
> +#endif
> +
> +#include <rtems/score/threadimpl.h>
> +#include <rtems/score/schedulerimpl.h>
> +
> +void _Thread_Yield( Thread_Control *executing )
> +{
> + ISR_Level level;
> +
> + _ISR_Disable( level );
> +
> + if ( _States_Is_ready( executing->current_state ) ) {
> + _Scheduler_Yield( _Scheduler_Get( executing ), executing );
> + }
> +
> + _ISR_Enable( level );
> +}
> --
> 1.7.7
>
> _______________________________________________
> devel mailing list
> devel at rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
More information about the devel
mailing list