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

[Sebastian Huber]

On 06/13/2014 06:13 PM, Gedare Bloom wrote:
> 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?

The cost is not that high, on RISC machines normally one opcode.  I am 
not sure which alternative is better.

>
>> 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.

Yes, you are right, this is bad style.  I tend to #ifdef it away for 
non-SMP.

[...]

-- 
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax     : +49 89 189 47 41-09
E-Mail  : sebastian.huber at embedded-brains.de
PGP     : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.