I was going to put a fancy explanation in here about what's going on with TOSH_INTERRUPT/SIGNAL, how atomicity checking works, etc.
But then I found out that (AFAICT) the msp430 platform was never updated for the not-so-very-new-anymore atomicity annotations for interrupt handlers (@hwevent, @atomic_hwevent). So in practical terms, there's no atomicity checking on msp430 (or any platform except atm128 and possibly some pxa27x code). To summarize: - reentrant interrupt handlers must be annotated with @hwevent() - non-reentrant interrupt handlers must be annotated with @atomic_hwevent() - if a non-reentrant interrupt handler wants to enable interrupts half-way through, it must do it using a call to __nesc_enable_interrupt() (which should be defined anyway) For reference, here's the atm128 macros that replaced (some time ago...) TOSH_INTERRUPT/TOSH_SIGNAL: /* We need slightly different defs than SIGNAL, INTERRUPT */ #define AVR_ATOMIC_HANDLER(signame) \ void signame() __attribute__ ((signal)) @atomic_hwevent() @C() #define AVR_NONATOMIC_HANDLER(signame) \ void signame() __attribute__ ((interrupt)) @hwevent() @C() David Gay On Wed, Aug 29, 2012 at 10:22 PM, Eric Decker <cire...@gmail.com> wrote: > I've added David Gay (the nesc author and maintainer) to see what he has to > add. > > > On Wed, Aug 29, 2012 at 7:52 PM, Xiaohui Liu <whu...@gmail.com> wrote: >> >> Thanks again for your promptly reply. I still have a few follow-up >> questions, please CIL. >> >> On Wed, Aug 29, 2012 at 8:38 PM, Eric Decker <cire...@gmail.com> wrote: >>> >>> >>> >>> On Wed, Aug 29, 2012 at 4:40 PM, Xiaohui Liu <whu...@gmail.com> wrote: >>>> >>>> Thank you both for sharing your insights. I still have a few comments, >>>> CIL. >>>> >>>> On Tue, Aug 28, 2012 at 3:52 AM, Eric Decker <cire...@gmail.com> wrote: >>>>> >>>>> >>>>> >>>>> On Mon, Aug 27, 2012 at 6:59 PM, Xiaohui Liu <whu...@gmail.com> wrote: >>>>>> >>>>>> Hello all, >>>>>> >>>>>> 1) I have the following code snippet: >>>>>> uint8_t a; >>>>>> >>>>>> async event void Alarm1.fired() { >>>>>> a = 0; >>>>>> } >>>>>> >>>>>> This compiles successfully without any warning. >>>>> >>>>> >>>>>> >>>>>> Isn't there a racing condition here, between Alarm1 and itself? >>>>> >>>>> >>>>> I don't know what you mean by the above. How can Alarm1 have a race >>>>> condition with itself? >>>>> >>>> I was thinking the interrupt that generates Alarm1 can be nested, but >>>> turns out it is TOSH_SIGNAL and cannot be. What I actually want to express >>>> is that if Alarm1.fired() is replaced by another event generated by a >>>> nested >>>> interrupt TOSH_INTERRUPT, should a be protected? >>> >>> >>> If you do indeed cause the interrupts to be nested then yes it should be >>> protected. >>> >>> Nested interrupts on the msp430s have lots of problems and you probably >>> don't want to do this. >>> >>>> >>>> >>>>> Looks to me like in the above program there is only one place where a >>>>> is accessed, so how can there be a race condition. >>>>> >>>>>> >>>>>> >>>>>> 2) If the following is added. >>>>>> async event void Alarm2.fired() { >>>>>> a = 1; >>>>>> } >>>>>> Still, this compiles successfully without any warning. Isn't there an >>>>>> additional racing condition here, between Alarm1 and Alarm2 (and Alarm2 >>>>>> and >>>>>> itself)? >>>>> >>>>> >>>>> async is considered to be one level of execution. So there still >>>>> isn't a race condition. When Alarm1 fires, a gets set to 0. Alarm2 can >>>>> not get in and thus there is not a race condition. >>>>> >>>>> this is a nesc assumption. That async is one level of execution (one >>>>> context). >>>>> >>>> >>>> By assumption, you mean Alarm cannot be preempted? >>> >>> >>> Interrupted is the correct term. Preemption is a little bit different. > > >> >> 1) What is the difference? > > > First, all preemptions are interruptions but not all interruptions are > preemptions. > > Preemption is a technique that interrupts a task temporarily with out its > cooperation with the intention of resuming the task later. ie. time > slicing. Its a context switch that typically is fairly expensive. ie. > tosthreads for example does support preemption. Preemption is typically > built on top of a timer interrupt and implements time-slicing. Full > processor and process (task) state are typically saved to a purpose tasked > block of storage (task/process block) to allow a restart later. > > Interrupts are typically a h/w thing and interrupt the immediate cpu > context. Minimal h/w state is saved on the stack. > > In other words, interrupts are a more primative h/w based construct that can > be used to implement preemption. > > > >> >>> The assumption that nesc makes is that there are two levels of context >>> and that the async context doesn't get interrupted. > > >> >> 2) If async context does not get interrupted, then there should never be >> atomic in async context, at least when no interrupt is TOSH_INTERRUPT. But >> this is clearly not the case. For instance, the following can be found in >> CC2420ReceiveP.nc > > > Don't know what to tell you. People do various things and it isn't > necessarily justified. The code isn't really code reviewed and is developed > in an academic context. Typically that means that the development envelope > isn't as rigorously fleshed out as in other contexts. > >> async event void InterruptFIFOP.fired() { >> if ( m_state == S_STARTED ) { >> #ifndef CC2420_HW_SECURITY >> m_state = S_RX_LENGTH; >> beginReceive(); >> #else >> m_state = S_RX_DEC; >> atomic receivingPacket = TRUE; >> beginDec(); >> #endif >> } else { >> m_missed_packets++; >> } >> } >> So all such atomic are redundant? > > > maybe. depends on if receivingPacket is accessed at sync level. > > Also typically when someone gets hit with the nesc warning, folks tend to go > in and put atomics on any of the references. And TinyOS code typically > isn't code reviewed. So.... > >> >> >>> So if two async pieces access the same variable it assumes that there >>> isn't a race condition. >> >> 3) What about async context cased by TOSH_INTERRUPT in msp430, or >> equivalently, by AVR_NONATOMIC_HANDLER in avr? For example, variable a is >> accessed both from AVR_NONATOMIC_HANDLER(SIG_OUTPUT_COMPARE3B) and >> AVR_NONATOMIC_HANDLER(SIG_OUTPUT_COMPARE3C) in HplAtm1281Timer3P.nc. There >> is a race condition here and it is compliant with nesc. > > > It is my understanding that nesc will not flag those race conditions. > Unless nesc somehow knows about TOSH_INTERRUPT vs. TOSH_SIGNAL. Which > strikes me as pretty strange if it does (too much tinyos coupling). > >> >> >>>> I think the assumption holds just because the interrupt that generates >>>> Alarm is defined as TOSH_SIGNAL rather than TOSH_INTERRUPT. >>> >>> >>> That is why the assumption holds. But it still is a nesc assumption and >>> if violated unless well understood can cause really nasty problems. >>> Typically intermittent failures. >>> >>>> >>>> It is not a nesc assumption. >>> >>> >>> Sure it is. >>> >>>> >>>> What is "one level of execution (one context). ", a code snippet that >>>> cannot be preempted? >>> >>> >>> not sure what you are asking. >>> >>>>>> >>>>>> >>>>>> 3) If the following is added. >>>>>> event void Timer.fired() { >>>>>> a = 2; >>>>>> } >>>>>> Then there is a warning, "non-atomic accesses to shared variable `a'". >>>>>> Why is there no warning for >>>>>> a = 0; >>>>>> in Alarm1? >>>>> >>>>> >>>>> Why do you expect a warning from Alarm1? >>>>> >>>>> Timer.fired is at sync level. Sync level is the other nesc execution >>>>> context. Because there is access to a from the async level nesc beleives >>>>> that there is a potential race condition between the Alarms (async level) >>>>> and Timer.fired (sync level). Hence non-atomic accesses. >>>>> >>>>>> >>>>>> >>>>>> According to the TinyOS book P196, a variable has to be protected by >>>>>> an atomic statement "if it is accessed from an async function". But >>>>>> seems to >>>>>> me, "a" is accessed from an async function in all 3 cases above and none >>>>>> is >>>>>> protected except >>>>>> a = 0; >>>>>> in Timer. >>>>> >>>>> >>>>> The book isn't very clear. >>>>> >>>>> Typically async level is used for functions called from interrupt level >>>>> signals. >>> >>> >>>> >>>> Can we replace the rule in the book by "whenever accessing a shared >>>> variable introduces racing condition, it must be protected by atomic"? >>> >>> >>> you can do that. But.... atomic adds overhead. I've looked >>> carefully at the code generated and introduced mechanisms for mitigating the >>> overhead, but there still is some overhead. Basically, you are disabling >>> interrupts and then reenabling them if there were on to begin with. >>> >>>>> >>>>> >>>>>> >>>>>> >>>>>> Can anybody please share the experience on atomic? Thanks in advance. >>>>>> >>>>>> -- >>>>>> -Xiaohui Liu >>>>>> >>>>>> _______________________________________________ >>>>>> Tinyos-help mailing list >>>>>> Tinyos-help@millennium.berkeley.edu >>>>>> >>>>>> https://www.millennium.berkeley.edu/cgi-bin/mailman/listinfo/tinyos-help >>>>> >>>>> >>>>> >>>>> >>>>> -- >>>>> Eric B. Decker >>>>> Senior (over 50 :-) Researcher >>>>> >>>>> >>>> >>>> >>>> >>>> -- TelosB >>>> -Xiaohui Liu >>> >>> >>> >>> >>> -- >>> Eric B. Decker >>> Senior (over 50 :-) Researcher >>> >>> >> >> >> >> -- >> -Xiaohui Liu > > > > > -- > Eric B. Decker > Senior (over 50 :-) Researcher > > _______________________________________________ Tinyos-help mailing list Tinyos-help@millennium.berkeley.edu https://www.millennium.berkeley.edu/cgi-bin/mailman/listinfo/tinyos-help