[PATCH] rtems: rtems_interrupt_enable/flash/disable()

[Gedare Bloom]

On Fri, Jun 19, 2015 at 2:13 AM, Sebastian Huber
<sebastian.huber at embedded-brains.de> wrote:
>
>
> On 12/06/15 22:26, Pavel Pisa wrote:
>>
>> Hello Sebastian and others,
>>
>> On Friday 12 of June 2015 17:47:42 Sebastian Huber wrote:
>>>
>>> ----- Gedare Bloom <gedare at gwu.edu> schrieb:
>>>>
>>>> I see. It provides the mutual exclusion for (SMP) applications that
>>>> rely on interrupt_disable/enable locks?
>>>>
>>>> I guess we can never get rid of it as long as we allow for users to
>>>> call isr_disable/enable?
>>>
>>> Yes, an alternative is to remove the
>>> rtems_interrupt_disable/enable/flash()
>>> on SMP configurations.  A pure interrupt disable is not really helpful.
>>
>> this is exactly same debate as has been solved in Linux kernel
>> lists more years ago.
>
>
> It seems that every problem was solved by Linux at some time.
>
>
>> The Linux kernel community resolution was
>> to forbid control global interrupts disable from drivers
>> and all kernel code. Only option is to proceed with local
>> (on singel CPU) disabel operations
>>
>>    local_irq_save(flags)
>>    local_irq_disable()
>>
>>    local_irq_restore(flags);
>>
>> The original function names for global irq disable and enable have
>> been kept on single CPU configurations for some transition period.
>> Then they have been removed without replacement.
>>
>> Local IRQ disable is good mostly to build other synchronization
>> primitives. At the level of drivers and higher level code
>> they are not of much use, because there is no guarantee that code
>> manipulating same data is not run on the other CPU.
>> So at the end allmost all sites solely combine disable of interrupt
>> together with spinlock
>>
>> macro to store flags
>>    spin_lock_irqsave(lock, flags)
>> function
>>    spin_unlock_irqrestore(spinlock_t *lock, unsigned long flags)
>>
>> These lock do not deadlock even when called from interrupts,
>> because any lock of this kind could be taken when IRQ
>> is enabled.
>>
>> As for RTEMS, I would suggest to follow the same road.
>> That is to define
>>
>>    rtems_local_interrupt_disable/enable/flash()
>>
>> and for compatibility define rtems_interrupt_disable/enable/flash()
>> to use these on UP targets. Fail the build of applications directly
>> manipulating by IRQ state on the SMP targets. There are not much SMP
>> applications and targets yet runnuning on RTEMS. So the impact is not
>> so wide for start and as the applications are transformed to use
>> irqsave spinlocks when the are moved to SMP environment. If the global
>> IRQ disable is supported then applications do not stop to use it
>> and it would lead to need keep support of this feature for too long.
>> Even if you decide that providing global IRQ disable is required
>> for now it should be defined with deprecated attribute to be extremely
>> noisy.
>
>
> I am fine with this change to use
> rtems_local_interrupt_disable/enable/flash(). We should decide what to do
> before the release.
>
I prefer this rather than the big hammer that might encourage users to
develop broken SMP apps on 4.11. If you have time to get it
implemented before the release, great, but if not, we should not
support the isr_disable/enable for user apps on SMP.

-Gedare

> --
> 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.
>
> _______________________________________________
> devel mailing list
> devel at rtems.org
> http://lists.rtems.org/mailman/listinfo/devel