Disabling interrupts to protect data
in [Embedded]
|
Prev: ALP for Real Time Clock using timer unit of HC12
Next: How to create a code in C++ for LM3S811 in IAR Embedded Workbench
From: 42Bastian Schick on 30 Oct 2009 02:37 On Thu, 29 Oct 2009 21:39:03 +0000, ChrisQ <meru(a)devnull.com> wrote: >A related problem is where a single shared variable at driver level that >needs to be accessed from different functions, all of which disable the >interrupt within the critical section. Disabling interrupts is fine, but >it is necessary to know at the time of reenabling any instance, what the >current interrupt state was for the device, prior to the call. The >solution was to write a couple of functions to stack the current device >interrupt state before disabling, then pop on exit, rather than just a >hard disable / reenable. > >I suppose this will turn out to be a standard technique, but new to me >at least... I would not recommend that any function (despite some deeply internal OS routines) should "enable" interrupts if it had not "disabled" them. Of course, if the function may run in either context (i.e. interrupts disabled or not) it should save the current state and restore it. So no need for some kind of global variable to stack the interrupt status. BTW, the same is true for higher level locking using semaphores and mutex. -- 42Bastian Do not email to bastian42(a)yahoo.com, it's a spam-only account :-) Use <same-name>@monlynx.de instead !
From: 42Bastian Schick on 30 Oct 2009 02:45 On Tue, 27 Oct 2009 22:07:40 +0100, David Brown <david.brown(a)hesbynett.removethisbit.no> wrote: >Reservations are hardware intensive to implement, unless you are willing >to pay extra memory latency. But they scale well for multi-processor >systems, making them a good solution for "standard" PPC processors. The >PPC core in the MPC55xx is more like a microcontroller core than a "big" >processor core, and Freescale probably saved a fair amount of silicon >and complexity by dropping reservations so that you must use traditional >interrupt disabling. If FSL only would have placed a big sticker on the manual. :-) Seriously, the manual states reservation works, and some place later in the EBI (external bus interface) description they write it does not work for EBI. => I learned, I have to read every single word of a manual. -- 42Bastian Do not email to bastian42(a)yahoo.com, it's a spam-only account :-) Use <same-name>@monlynx.de instead !
From: David Brown on 30 Oct 2009 04:01 ChrisQ wrote: > Stefan Reuther wrote: > >> By "easy to get wrong" I mean, for example, that you have to tell all >> your interrupt routines about your atomic_swap routine, so that they can >> see that they've interrupted it and can complete or restart it. This is >> nasty asm trickery. It enlarges all your interrupts by a dozen >> instructions (at least some compares and jumps). And all that just >> for a sticker "never disables interrupts" in the glossy brochures? >> Sorry, I'd rather take the additional three-cycle interrupt latency for >> running atomic_link under CLI. > > A related problem is where a single shared variable at driver level that > needs to be accessed from different functions, all of which disable the > interrupt within the critical section. Disabling interrupts is fine, but > it is necessary to know at the time of reenabling any instance, what the > current interrupt state was for the device, prior to the call. The > solution was to write a couple of functions to stack the current device > interrupt state before disabling, then pop on exit, rather than just a > hard disable / reenable. > > I suppose this will turn out to be a standard technique, but new to me > at least... > Do you mean something like this : void foo(void) { doSomethingUnlocked(); memoryBarrier(); oldSREG = SREG; // Preserve old interrupt state cli(); // Ensure interrupts are disabled now doSomethingLocked(); memoryBarrier(); SREG = oldSREG; // Restore old interrupt flag doMoreUnlocked(); } The details vary from processor to processor, but the principle is the same. Some compilers have extensions or extra headers making it easier. For example, avr-gcc standard library has ATOMIC_BLOCK macros for this purpose, and msp430-gcc has an extra "critical" function __attribute__ that wraps a function in an interrupt disable/restore pair.
From: David Brown on 30 Oct 2009 04:41 Stefan Reuther wrote: > D Yuniskis wrote: >> Stefan Reuther wrote: >>> D Yuniskis wrote: <snip> Before I comment on your code below, let me first say I agree entirely that it is almost always better to have a small but clear and predictable interrupt latency using CLI/STI that you can easily see works correctly, than to have a complex scheme that whose correctness is difficult to evaluate. If your system cannot tolerate the small extra interrupt latency caused by such interrupt disable blocks, make a faster system or move the critical parts to better hardware. (Note that this is in the context of microcontroller programming - if you are working with large systems, especially SMP systems, there are many more factors to consider.) > Exactly this "append a pointer" thing is where an atomic operation can > come in very handy. You shouldn't append to the list while it's being > processed, nor should you interrupt another instance in another > interrupt (if you have nested interrupts). > > All this is totally simple when you can use CLI/STI: > void atomic_swap(node** a, node** b, node* c) { > cli(); *a = *b; *b = c; sti(); > } > > // prepend new node to a list (easier than appending...) > node* list; > node* newNode; > atomic_swap(&newNode->next, &list, newNode); > > // get list (for processing) and make it empty > node* oldList; > atomic_swap(&oldList, &list, 0); > (some strategically-placed 'volatile' can be appropriate.) > Here you have missed one important point that many people fail to understand about interrupt disabling (and the related topic of volatile accesses) - memory barriers. Unless your cli() and sti() macros / assembly functions specifically include memory barriers, then your atomic_swap function is not safe. The problem is that C does not have any requirements about ordering of operations except for volatile accesses - all it requires is that the program has the same effect as though the operations were done in order, running on a C abstract machine. But the abstract machine has no concepts of interrupts, multi-threading, or external input and output and peripherals. The compiler can freely make re-arrangements to improve the quality of the generated code, which are correct for the abstract machine but may not be correct for your actual code. In the atomic_swap function above, for example, the compiler will probably see the cli() and sti() as volatile accesses (depending on the compiler and the definition of these macros). But the code in the middle does not use volatile accesses - the compiler can therefore freely move these operations before or after the cli() and sti() operations. To avoid this, you need a memory barrier before and after these operations, to tell the compiler that memory reads before the first barrier are now invalid, and that any pending memory writes must be completed before the second barrier. A typical memory barrier on gcc is a memory clobber: "asm volatile ("" : : : "memory"); On some systems, you may need to issue processor instructions too (such as a "msync" or "mbar" on a PPC) to deal with pipelined write buffers, but that's not typically the case for small microcontrollers. Some compilers will always consider inline assembly as a memory barrier - that makes it easier to write safe code in this case, but harder to use inline assembly optimally in other cases. Either way, you have to know what your compiler is doing.
From: ChrisQ on 30 Oct 2009 07:06
David Brown wrote: >> > > Do you mean something like this : > > void foo(void) { > doSomethingUnlocked(); > memoryBarrier(); > oldSREG = SREG; // Preserve old interrupt state > cli(); // Ensure interrupts are disabled now > doSomethingLocked(); > memoryBarrier(); > SREG = oldSREG; // Restore old interrupt flag > doMoreUnlocked(); > } > > The details vary from processor to processor, but the principle is the > same. > > Some compilers have extensions or extra headers making it easier. For > example, avr-gcc standard library has ATOMIC_BLOCK macros for this > purpose, and msp430-gcc has an extra "critical" function __attribute__ > that wraps a function in an interrupt disable/restore pair. Yes, but using an array, rather than a single variable. The code looked like: static U8 u8PwmInterruptStack [SIZE]; static U8 u8PwmStackPointer = 0; /* Push Pwm Interrupt State and Mask Interrupts */ /* -------------------------------------------- */ void vPwmPushMaskInterruptState (void) { if (u8PwmStackPointer < PWM_STACKDEPTH) { u8PwmInterruptStack [u8PwmStackPointer++] = u8PwmReadIntState (); vPwmMaskInterrupts (); } } /* Pop Pwm Interrupt State */ /* ----------------------- */ void vPwmPopInterruptState (void) { if (u8PwmStackPointer > 0) { u8PwmStackPointer--; if ((u8PwmInterruptStack [u8PwmStackPointer] & PWM_INTMASK) != 0) { vPwmUnmaskInterrupts (); } } } The range check is just there for safety and in practice, the array is sized to suit the application. The first function is called before twiddling the shared data and second after. Turned out to be a simple solution, but the first time i've had to use anything like it... Regards, Chris |