[PATCH] score: Critical section change in _Thread_Dispatch - CAREFUL REVIEW REQUIRED

[Joel Sherrill]

On 7/29/2013 3:33 PM, Gedare Bloom wrote:
> Is there a possibility an ISR happens during the
> set_dispatch_level(1)? It can observe a 0-value dispatch level in that
> case, and dispatch at the end of the isr?
I have always assumed that if this happened, the task
interrupted was not suddenly doing something which
would violate the critical section.   Assume it is a
2 step process.

Step A - dispatch level is 0
    ... ISR and dispatch
Step B - dispatch level --> 1

The task entering the critical section is OK on a uniprocessor
system. The memory and task are in the same state.

Assuming the 1 is set SMP safe, then the same argument
should hold. It just should be in the middle of acquiring an
SMP lock for Step A.

--joel
> On Mon, Jul 29, 2013 at 3:19 PM, Joel Sherrill
> <joel.sherrill at oarcorp.com> wrote:
>> On 7/29/2013 9:17 AM, Gedare Bloom wrote:
>>> It seems like it should work fine, as long as the ISR does not update
>>> _Thread_Executing or reset the disable_level to 0  which I think would
>>> both be bugs anyway...
>> The ISR Handling code changing _Thread_Executing would be
>> a horrible, horrible bug. Context switches are always thread
>> to thread. Not ISR->thread.
>>
>> The assembly ISR Handling code should only be incrementing
>> and decrementing disable_level.  If there are services which
>> do explicitly set it, then they should be forbidden from being
>> called in an ISR and probably reviewed for correctness given
>> the recent changes.
>>
>> One odd thought.. Given the stated assumptions of this patch,
>> I wonder if RTEMS_DEBUG should enable an assertion on these
>> conditions at the top of _Thread_Dispatch.
>>
>> I also wonder if it is worth considering RTEMS_DEBUG or
>> run-time errors being returned for making illegal calls during
>> an ISR. Adding run-time error codes will increase code size so
>> I tend to be against that since these are design errors.
>>
>> Overall I think this is correct. You just opened the door to more
>> internal checks.
>>
>> --joel
>>
>>> On Fri, Jul 26, 2013 at 11:27 AM, Gedare Bloom <gedare at rtems.org> wrote:
>>>> I can look at it and think through what it means hopefully early next
>>>> week.
>>>>
>>>> On Fri, Jul 26, 2013 at 10:48 AM, Sebastian Huber
>>>> <sebastian.huber at embedded-brains.de> wrote:
>>>>> Hello,
>>>>>
>>>>> please review this change carefully.  It may introduce a subtle but
>>>>> devastating problem.
>>>>>
>>>>> --
>>>>> 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.
>>>>> _______________________________________________
>>>>> rtems-devel mailing list
>>>>> rtems-devel at rtems.org
>>>>> http://www.rtems.org/mailman/listinfo/rtems-devel
>>> _______________________________________________
>>> rtems-devel mailing list
>>> rtems-devel at rtems.org
>>> http://www.rtems.org/mailman/listinfo/rtems-devel
>>
>>
>> --
>> Joel Sherrill, Ph.D.             Director of Research & Development
>> joel.sherrill at OARcorp.com        On-Line Applications Research
>> Ask me about RTEMS: a free RTOS  Huntsville AL 35805
>> Support Available                (256) 722-9985
>>


-- 
Joel Sherrill, Ph.D.             Director of Research & Development
joel.sherrill at OARcorp.com        On-Line Applications Research
Ask me about RTEMS: a free RTOS  Huntsville AL 35805
Support Available                (256) 722-9985