[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