[cross-post] nested interrupts

On 7/2/2012 9:35 AM, #! wrote: [...]

[...]

the main() is only writing to the fifo and the isr_sport() only reading

- except at the beginning of transmission - so why this should result in a race condition?

In the write_to_fifo() I check if the fifo is full and if not I then write to it, incrementing the write pointer. In the read_from_fifo() I check if the fifo is empty and if not read from it, incrementing the read pointer. Maybe I'm missing something here.

On 7/3/2012 1:16 PM, alb wrote: [...]

I think I was really missing something here!!! These are the write_to_fifo() and read_from_fifo() functions:

where fifo has the following structure:

The race condition is on the ef flag, since both read_from_fifo() and write_from_fifo() will set it to either FULL or empty. Uhmmm... I guess I need to revise this part of the code.

I suddenly realize I'm lacking of a methodology to write code which is interruptable, maybe some of you can point me to some good reference.

[ elided code ]

Without analysing that code in detail, at first glance it does look as if it needs to be protected against concurrent access from several threads/tasks/ISRs.

How can a "bool" variable have the values FULL or EMPTY? That contradicts the meaning of "bool". (Yes, I know that "bool" is just a fiction in traditional C, but that does not make it logically correct :-))

And what about the case when the FIFO is neither FULL, nor EMPTY, say it is about half-full?

I like to keep a "count" field in FIFOs. Empty is then count == 0 and full is count == FIFO_LENGTH.

At least for making the FIFO usable by concurrent threads.

The traditional way, with mutually exclusive critical sections:

The new way (mainly useful for multicore processors IMO), with lock-free data structures:

Note that both methods need a known ordering of actions on variables, so they need "volatile", or something like that (plus, in the more complex processors, memory barriers, etc.) If you were using an RT kernel, it would provide some of this stuff, usually at least interrupt-safe FIFO queues.

--
Niklas Holsti
Tidorum Ltd
niklas holsti tidorum fi
       .      @       .

I would say that in that case the race may be won by isr_sport(), instead of (as you have assumed) the main() thread.

This is just to emphasize that (IMO at least) the existence (definition) of the race condition depends only on the possible sequences of actions in the various threads, and not on their relative execution speeds.

If you know the relative execution speeds, you can argue that you know which thread will win the race, and in that case the race condition can be left in the code without risk of errors (until someone changes the execution speeds ands forgets to re-examine the race...)

In that case, the only reason why the code works at all (even without the timer interrupt) is the relatively long latency for interrupt response in the 21020, giving main() time to assign state = TRANSMITTING even while the serial interrupt is asserted.

"That way madness lies" :-) The 21020 program sequencer is a complex thing. When we built our 21020 WCET analyzer I had to try to understand how the zero-overhead, 6-deep-nested HW loop logic interacts with delayed branch instructions, with or without the loop-abort flag, and with special cases for loops that have few instructions and loops that have few iterations. Yuck.

Yes, "volatile" means a bit more than that. Unfortunately (but understandably), the C standard has no strict, formal definition. There are also reports that C compilers frequently miscompile code that uses "volatile" -- at least in the sense that the compiled code does not do what the programmers wanted it to do.

I think most multi-thread stuff (including interrupt handling) must rely on some ordering of actions. But one should try to enforce the order by logical conditions, not by timing that depends on execution speeds.

--
Niklas Holsti
Tidorum Ltd
niklas holsti tidorum fi
       .      @       .

Yes I was thinking of the pointers. write_to_fifo() could be half way through adjusting a pointer value when an interrupt driven read_from_fifo() comes along and also adjusts the pointer values.

Maybe try something like

while(1) { temp_disable_sport_interrupt(); write_to_fifo(data); reenable_sport_interrupt(); if (state != TRANSMITTING) {

Just looked at your code

An interrupt could change the value of rd while evaluating this line if (fifo->wr == fifo->rd) fifo->ef = FULL;

The value of wr may be partly incremented when an interrupt tries to evaluate this line if (fifo->rd == f->wr) fifo->ef = EMPTY;

The code above is flawed. Your main should not write to the sport at all. If no transmission is in progress, the TX interrupt of sport should be disabled. If you write something to the fifo, you just enable the TX interrupt, nothing more. At that time, the sport TX buffers are empty and an interrupt will occur immediately, reading a byte from the fifo and writing it to sport. When the next interrupt occurs, the next byte will be read from the fifo and written to the sport. If the fifo is empty when an interrupt occurs, the interrupt handler should disable the TX interrupt and exit.

Meindert

If

an

g

rom

This is not true for all processors. Some of them have edge triggered interrupts the only interrupt when the transmitter finishes transmission.

I realized that. Here I can only think about disabling the interrupt temporarily, since interrupts will be still latched and servicing resumed on interrupt enabling. Otherwise I would need to invent some 'locking' mechanism with possibility to postpone a write/read to/from the fifo.

A fifo is fully described with a read pointer, a write pointer, the size and a bit to distinguish EMPTY/FULL when pointers are the same. I do not see any logical flow here.

if (wr - rd != 0) fifo is neither empty nor full.

Thanks for the pointers!

Certainly an RTOS will facilitate some of these stuff, but I'm afraid I will bump in several other problems, considering that there's very little activity on this type of DSP (after all is a legacy DSP).

Exactly. And you do not need "fifo empty/full" flags. If write_pointer == read_pointer, fifo is empty. If write_pointer+1 (wrapping at end of buffer) == read_pointer, fifo is full. This way the main program only alters write_pointer (and buffer contents) and the irs code only alters read_pointer and there are no race conditions, ever.

Simple.

-jm

On 7/4/2012 8:39 AM, #! wrote: [...]

[...]

Yep, I also noticed that, even though I do not understand how this problem can produce the effect I see. If the pointers are moved while reading or writing the flag will be wrong. But in either case if the flag is wrong the fifo will be wrongly interpreted as full while is empty or viceversa, meaning data transmitted will be wrong... but they would still be transmitted.

I will nevertheless disable the interrupts and reenable inside write_to_fifo() and read_from_fifo() and see what will happen.

On 7/4/2012 9:12 AM, Meindert Sprang wrote: [...]

[...]

Uhm that is interesting, but since my interrupt is edge sensitive I do not believe that when I enable it the interrupt will be seen immediately.

I may change the interrupt to level sensitive, but I'm not sure if it is beneficial. Nevertheless I don't see how this change will make me understand why the nested interrupts break my code.

When the next interrupt occurs, the next byte will be read from

Your proposal is certainly valid, but it does not help me understanding what I'm doing wrong. Other people have correctly pointed out several flaws in my code but the logic of 'starting' the transmission in main() is not flawed 'per se', it is maybe wrongly implemented.

You are suggesting to remove the flag and keep one memory location of the fifo not used (if I understood correctly). This will prevent the race condition on the flag, it will still not prevent the race on 'state'.

And I should remind you that my problem is why nested interrupts will break my code w.r.t. non nested interrupts. It is still true that I have to prevent fifo to be misused but this will affect only the data being wrong (fifo will be interpreted as full while is empty), not the logic on the transmission.

Yes.

Yes, it does. You update the read and write pointers after you have read or written the buffer. No race.

I'm not sure it's the nested interrupts that's the problem but the race problems in your fifo/isr code.

(I'm also not sure if you need nested interrupts at all. ;-)

-jm

To understand, you will need to look at the machine code and work out what happens when an interrupt occurs at every position in the critical section of code, with every possible combination of rd and wr

As Niklas has suggested maybe the extra interrupts changed the timing, causing you luck to run out.

all. If

and an

writing

interrupt

It is a while since I last looked at the topic, but ISTR that edge-triggered interrupts were to be considered an Abomination Unto Nuggan, at least if you have more than one interrput source.

--------------------------------------- Posted through

What would be the advantage of having an edge triggered interrupt on a UART empty event? Do you have an example of this?

Meindert

all. If

and an

writing

Yes it is because then you can use the procedure I outlined. And this is not pure theoretical, I use this in a commercial product. And I have no race conditions and use nested interrupts.

It sure makes life a lot easier if you don't write to the same hardware in main code AND in an interrupt.

Meindert

RT

8032. Bit of a pain, but once the code is in place it is OK. IIRC the Z80 SIO had a similar issue. The scheme in the PIC for example of disabling the specific interrupt is much easier to use because you just enable the interrupt after adding data to the circular buffer. It doesn't matter if was enabled or not when you re-enable. Worst case is a gratuitous interrupt.