[En-Nut-Discussion] NutEnter|ExitCritical dilemma

[Dušan]

Uwe,

nice overview.

I want just point to a typo, so others will not be confused.

Following "hard" sequence prevents from using the open / close bracket 
solution, also suggested in 2008.
This is why I vote for NutUseCritical(), which does not require any IN / 
OUT parameters for Nut-Enter / Exit-Critical macros.

NutEnterCritical();
...
NutThreadAddPriQueue() {
         ...
         NutExitCritical();
         ...
         NutEnterCritical();
}
... more code needing interrupts off ..
NutExitCritical()

*Dusan*

On 18.1.2018 13:52, Uwe Bonnes wrote:
> Hello,
>
> some 10 year later and the discussion about NutEnter|ExitCritical is still
> going on. See also
> http://lists.egnite.de/pipermail/en-nut-discussion/2008-February/
> "confirmed!!! Re: NutOS 4.4.0 on ARM7: possibly bug in NutEnterCritical /
> NutExitCritical"
>
> I had also some input by private mail.
>
> Let me state some premises:
> ===========================
> 1. Changing the API of an existing function was a NO-NO with Harald and we
> should keep that habit!
>
> 2. In our common code (nut/dev, nut/os) a lot of NutEnterCritical /
> NutExitCritical() usage is used to protect a single access to single 2 or 4
> byte Variable that could also be changed by an interrupt. This is needed for
> any 8-bit architecure, but not for 32-bit arch. I brought up the subject  in
> http://lists.egnite.de/pipermail/en-nut-discussion/2015-November/thread.html
> but the discussion mostly got into esoteric problems.
>
> Things like nut/os/thread.c:NutThreadRemoveQueue()
>      157     NutEnterCritical();
>      158     tqp = *tqpp;
>      159     NutExitCritical();
>
> should get use a new macro that only expand to a critical section on 8-bit
> archs and expand to nothing on 32-bit archs.
>
> 3. nut/arch/<32-bit arch>/os/thread|context.c NutThreadCreate()
> has a code sequence
> ...
> NutEnterCritical();
> ...
> NutThreadAddPriQueue() {
>          ...
>          NutEnterCritical();
>          ...
>          NutExitCritical()
> }
> ... more code needing interrupts off ..
> NutExitCritical()
>
> This is doomed for problems if you create thread during run time with much
> interrupts going on. I have not yet met that problem as I create my threads
> before I enter the "while(1){}" loop in main. But creating threads with a
> lot of interrupts going on is a valid problem and must be handled.
>
> 4. Only in very rare cases should arch-specific code use critical section
> with all interrupts disabled! Disable selected interrupts or use arch-
> specific synchronization mechanisms instead.
>
> 5. os/msg.c is a heavy user of NutEnter|ExitCritical()
> However there is no example code should the intended use. Any code pieces
> showing the use is welcome!
>
> What others do
> =============
> On Avr an ATOMIC_BLOCK() macro is provided.
> http://www.nongnu.org/avr-libc/user-manual/group__util__atomic.html#gaaaea265b31dabcfb3098bec7685c39e4
> This has been ported to cortex:
> https://www.mikrocontroller.net/attachment/294497/cm_atomic.h
>
> Possible solutions:
> ==================
>
> For 1: Tag NutEnter|ExitCritical() as deprecated, but keep it untouched.
>      Changing syntax and scope as Dusan proposed violated premise 1.
>
> For 2: Add NutEnter|ExitCriticalAccess() macro, resolving to nothing on
>         32-bit archs and to a valid critical section on 8-bit.
>         This removes unneeded code for 32-bit archs.
>
> For 3: We should consider the ATOMIC_BLOCK solution.
>      Otherwise what about a solution like:
>      int irq_state;
>      irq_state = NutEnterCriticalNest()
>      <protected code>
>      NutExitCriticalNest(irq_state)
>      Some new macro/function is needed to not violate premise 1.
>
> Solutions 1 and 2 can be implemented probably with not to much hassle. But
> solution 3 will break for architecture where there is no active user and
> probably also for the message queue implemenation in os/msg.c as there is no
> code to test.
>
> Can people with out-of sync active trees live with such a solution so maybe
> we get the trees in sync again? Comments?
>
> Cheers