[PATCH 6/6] score: Need for help indicator for scheduler ops

[Gedare Bloom]

On Fri, Jun 13, 2014 at 10:37 AM, Sebastian Huber
<sebastian.huber at embedded-brains.de> wrote:
> Return the displaced thread for the following scheduler operations
>
> - unblock,
> - change priority, and
> - yield.
>
> A displaced thread is a thread that encounters a scheduler state change
> from scheduled to ready.  Such a displaced thread can ask threads which
> depend on resources owned by the displaced thread for help.
>
> In case the unblock operation returns no displaced thread, then this
> indicates that the unblocked thread is now in the ready state and has no
> processor allocated.  Such a thread can ask now also for help.
> ---
>  cpukit/score/include/rtems/score/scheduler.h       |   27 ++-
>  cpukit/score/include/rtems/score/schedulercbs.h    |    2 +-
>  cpukit/score/include/rtems/score/scheduleredf.h    |    6 +-
>  cpukit/score/include/rtems/score/schedulerimpl.h   |   33 ++
>  .../score/include/rtems/score/schedulerpriority.h  |    6 +-
>  .../rtems/score/schedulerpriorityaffinitysmp.h     |    4 +-
>  .../include/rtems/score/schedulerprioritysmp.h     |    6 +-
>  cpukit/score/include/rtems/score/schedulersimple.h |    6 +-
>  .../score/include/rtems/score/schedulersimplesmp.h |    6 +-
>  .../score/include/rtems/score/schedulersmpimpl.h   |   46 ++-
>  cpukit/score/src/schedulercbsunblock.c             |    4 +-
>  cpukit/score/src/scheduleredfchangepriority.c      |    4 +-
>  cpukit/score/src/scheduleredfunblock.c             |    4 +-
>  cpukit/score/src/scheduleredfyield.c               |    4 +-
>  cpukit/score/src/schedulerpriorityaffinitysmp.c    |   39 ++-
>  cpukit/score/src/schedulerprioritychangepriority.c |    4 +-
>  cpukit/score/src/schedulerprioritysmp.c            |   34 +-
>  cpukit/score/src/schedulerpriorityunblock.c        |    4 +-
>  cpukit/score/src/schedulerpriorityyield.c          |    4 +-
>  cpukit/score/src/schedulersimplechangepriority.c   |    4 +-
>  cpukit/score/src/schedulersimplesmp.c              |   34 +-
>  cpukit/score/src/schedulersimpleunblock.c          |    4 +-
>  cpukit/score/src/schedulersimpleyield.c            |    4 +-
>  testsuites/smptests/smpscheduler03/init.c          |  385 +++++++++++++++++++-
>  24 files changed, 562 insertions(+), 112 deletions(-)
>
> diff --git a/cpukit/score/include/rtems/score/scheduler.h b/cpukit/score/include/rtems/score/scheduler.h
> index d9d21d3..d2436ff 100644
> --- a/cpukit/score/include/rtems/score/scheduler.h
> +++ b/cpukit/score/include/rtems/score/scheduler.h
> @@ -44,6 +44,16 @@ typedef struct Scheduler_Control Scheduler_Control;
>
>  typedef struct Scheduler_Node Scheduler_Node;
>
> +#if defined(RTEMS_SMP)
> +  typedef Thread_Control * Scheduler_Void_or_thread;
> +
> +  #define SCHEDULER_RETURN_VOID_OR_NULL return NULL
> +#else
> +  typedef void Scheduler_Void_or_thread;
> +
> +  #define SCHEDULER_RETURN_VOID_OR_NULL return
> +#endif
> +
What's the cost to always return a thread even if it is NULL for the
uniprocessor case? It seems abstracting the return value is probably
more trouble for understanding the code than it is worth for saving a
few cycles for these schedule operations?

> diff --git a/cpukit/score/include/rtems/score/schedulerimpl.h b/cpukit/score/include/rtems/score/schedulerimpl.h
> index f3284eb..0f7150f 100644
> --- a/cpukit/score/include/rtems/score/schedulerimpl.h
> +++ b/cpukit/score/include/rtems/score/schedulerimpl.h
> @@ -107,6 +107,11 @@ RTEMS_INLINE_ROUTINE void _Scheduler_Schedule( Thread_Control *the_thread )
>    ( *scheduler->Operations.schedule )( scheduler, the_thread );
>  }
>
> +RTEMS_INLINE_ROUTINE void _Scheduler_Ask_for_help( Thread_Control *needs_help )
> +{
> +  (void) needs_help;
> +}
> +
>  /**
>   * @brief Scheduler yield with a particular thread.
>   *
> @@ -118,8 +123,14 @@ RTEMS_INLINE_ROUTINE void _Scheduler_Schedule( Thread_Control *the_thread )
>  RTEMS_INLINE_ROUTINE void _Scheduler_Yield( Thread_Control *the_thread )
>  {
>    const Scheduler_Control *scheduler = _Scheduler_Get( the_thread );
> +  Thread_Control *needs_help;
>
> +#if defined(RTEMS_SMP)
> +  needs_help =
> +#endif
>    ( *scheduler->Operations.yield )( scheduler, the_thread );
> +
> +  _Scheduler_Ask_for_help( needs_help );
For non-SMP this passes an uninitialized pointer to a function. This
seems like a poor design choice. I'd either wrap the call with
#ifdef(SMP) or make sure there is an ifdef in the function that gets
called. Of course right now that function does nothing and does not
use the pointer, but it should be explicit that the non-SMP case will
never do anything with this pointer.

Alternately, if you change the scheduler function to return NULL for
the non-SMP case, this is no problem if you get rid of the ifdef.

>  }
>
>  /**
> @@ -152,8 +163,24 @@ RTEMS_INLINE_ROUTINE void _Scheduler_Block( Thread_Control *the_thread )
>  RTEMS_INLINE_ROUTINE void _Scheduler_Unblock( Thread_Control *the_thread )
>  {
>    const Scheduler_Control *scheduler = _Scheduler_Get( the_thread );
> +#if defined(RTEMS_SMP)
> +  Thread_Control *displaced;
>
> +  displaced =
> +#endif
>    ( *scheduler->Operations.unblock )( scheduler, the_thread );
> +
> +#if defined(RTEMS_SMP)
> +  if ( displaced != NULL ) {
> +    _Scheduler_Ask_for_help( displaced );
> +  } else {
> +    /*
> +     * There is no displaced thread due to this operation, thus the thread is
> +     * now in the ready state and has no processor allocated.
> +     */
> +    _Scheduler_Ask_for_help( the_thread );
> +  }
> +#endif
>  }
>
>  /**
> @@ -176,13 +203,19 @@ RTEMS_INLINE_ROUTINE void _Scheduler_Change_priority(
>  )
>  {
>    const Scheduler_Control *scheduler = _Scheduler_Get( the_thread );
> +  Thread_Control *displaced;
>
> +#if defined(RTEMS_SMP)
> +  displaced =
> +#endif
>    ( *scheduler->Operations.change_priority )(
>      scheduler,
>      the_thread,
>      new_priority,
>      prepend_it
>    );
> +
> +  _Scheduler_Ask_for_help( displaced );
ditto.

-Gedare