[PATCH 1/8] score: Update _Thread_Heir only if necessary

[Sebastian Huber]

On 04/03/15 16:44, Gedare Bloom wrote:
> On Wed, Mar 4, 2015 at 10:42 AM, Joel Sherrill
> <joel.sherrill at oarcorp.com> wrote:
>>
>> On 3/4/2015 9:07 AM, Sebastian Huber wrote:
>>> Previously, the _Thread_Heir was updated unconditionally in case a new
>>> heir was determined.  The _Thread_Dispatch_necessary was only updated in
>>> case the executing thread was preemptible or an internal thread was
>>> unblocked.  Change this to update the _Thread_Heir and
>>> _Thread_Dispatch_necessary only in case the currently selected heir
>>> thread is preemptible or a dispatch is forced.  Move the schedule
>>> decision into the change priority operation and use the schedule
>>> operation only in rtems_task_mode() in case preemption is enabled or an
>>> ASR dispatch is necessary.  This is a behaviour change.  Previously, the
>>> RTEMS_NO_PREEMPT also prevented signal delivery in certain cases (not
>>> always).  Now, signal delivery is no longer influenced by
>>> RTEMS_NO_PREEMPT.  Since the currently selected heir thread is used to
>>> determine if a new heir is chosen, non-preemptible heir threads
>>> currently not executing now prevent a new heir.  This may have an
>>> application impact, see change test tm04.  Document this change in sp04.
>>>
>>> Update #2273.
>> ...
>>> diff --git a/cpukit/score/src/schedulercbsunblock.c b/cpukit/score/src/schedulercbsunblock.c
>>> index 688253c..bd27aff 100644
>>> --- a/cpukit/score/src/schedulercbsunblock.c
>>> +++ b/cpukit/score/src/schedulercbsunblock.c
>>> @@ -79,10 +79,10 @@ Scheduler_Void_or_thread _Scheduler_CBS_Unblock(
>>>          _Thread_Heir->current_priority
>>>       )
>>>     ) {
>>> -    _Thread_Heir = the_thread;
>>> -    if ( _Thread_Executing->is_preemptible ||
>>> -         the_thread->current_priority == 0 )
>>> -      _Thread_Dispatch_necessary = true;
>>> +    _Scheduler_Update_heir(
>>> +      the_thread,
>>> +      the_thread->current_priority == 0
>>> +    );
>> The use of an expression without parentheses seems inconsistent
>> style-wise. I don't think this type of code is used often but could
>> you please put parentheses around this?
>>
> This conflicts with our coding conventions to avoid excess parens.

Yes, can we please avoid parentheses in case, since the comma operator 
has the weakest precedence.

>
>> This same pattern is in other places so please do it globally across
>> this patch.
>> I think I spotted a total of four places.
>>
>> Also since this indicates that the thread is at the pseudo-interrupt
>> priority,
>> maybe a macro/static inline with a meaningful name would be even
>> better.
>>
> Yes a macro might be good for this test.

Since we started this discussion. It seems that the MPCI thread is the 
only thread with a priority of 0. Should this be #ifdef 
RTEMS_MULTIPROCESSING? Looks otherwise like dead code. It is possible to 
gain a priority of 0 via the priority ceiling protocol. Is this a bug?

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