[PATCH] score: Simplify _Thread_Change_priority()

[Sebastian Huber]

On 05/14/2014 05:40 PM, Joel Sherrill wrote:
>
>
> On 5/14/2014 10:32 AM, Sebastian Huber wrote:
>> The function to change a thread priority was too complex.  Simplify it
>> with a new scheduler operation.  This increases the average case
>> performance due to the simplified logic.  The interrupt disabled
>> critical section is a bit prolonged since now the extract, update and
>> enqueue steps are executed atomically.  This should however not impact
>> the worst-case interrupt latency since at least for the Deterministic
>> Priority Scheduler this sequence can be carried out with a wee bit of
>> instructions and no loops.
> I tend to agree with your analysis on performance. It won't have much 
> impact
> on the Simple since the insert still dominates. We added an extract. 
> Big deal.
>> Add _Scheduler_Change_priority() to replace
>>    - _Thread_Set_transient(),
>>    - _Scheduler_Extract(),
>>    - _Scheduler_Enqueue(), and
>>    - _Scheduler_Enqueue_first().
> Do you remain "replace the sequence"?

Ok, I will update this.

>> Delete STATES_TRANSIENT, _States_Is_transient() and
>> _Thread_Set_transient() since this state is now superfluous.
> I am using essentially the same sequence to change affinity.
> Set STATES_TRANSIENT, set the new affinity, remove STATES_TRANSIENT.
>
> Are you proposing that I change to using another scheduler operation
> to do that?

Which function do you implement?  Can't you use STATES_MIGRATING, like here:

RTEMS_INLINE_ROUTINE void _Scheduler_Set(
   const Scheduler_Control *scheduler,
   Thread_Control          *the_thread
)
{
#if defined(RTEMS_SMP)
   const Scheduler_Control *current_scheduler = _Scheduler_Get( 
the_thread );

   if ( current_scheduler != scheduler ) {
     _Thread_Set_state( the_thread, STATES_MIGRATING );
     _Scheduler_Free( _Scheduler_Get( the_thread ), the_thread );
     the_thread->scheduler = scheduler;
     _Scheduler_Allocate( scheduler, the_thread );
     _Scheduler_Update( scheduler, the_thread );
     _Thread_Clear_state( the_thread, STATES_MIGRATING );
   }
#else
   (void) scheduler;
#endif
}

Currently this function is only allowed at thread level.  Here the Giant 
lock takes care that we don't do stupid things.

>> With this change it is possible to get rid of the
>> SCHEDULER_SMP_NODE_IN_THE_AIR state.  This considerably simplifies the
>> implementation of the new SMP locking protocols.
> Agreed. I am concerned about locking while changing affinity. Is the
> scheduler sufficiently locked that I can scan the executing list? 

The scheduler operations are currently protected by the Giant lock and 
ISR disabled.

These transient states are very problematic if we want to introduce fine 
grained locking some time in the future.  It can lead to an explosion of 
states.

I am not sure if you can use the existing SMP scheduler support to 
implement arbitrary thread affinities within a scheduler instance. The 
situation improved a bit with this patch however:

http://git.rtems.org/rtems/commit/?id=38b59a6d3052654e356ae16b4a243c362312acce

You may have to modify _Scheduler_SMP_Allocate_processor() and use a 
more sophisticated _Scheduler_SMP_Is_processor_owned_by_us().

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