Race condition with slow interrupt handlers and usarts

LeafLabs Garden » Development

Race condition with slow interrupt handlers and usarts

(6 posts) (3 voices)

Started 3 years ago by gauteh
Latest reply from mbolivar

gauteh
Member

Hi,

I've mentioned the problem on #leaflabs, but I thought I would post it here for reference:

I experienced random, but frequent, lock ups with a setup where I used USARTS extensively along with an 250 Hz external interrupt with a rather slow ISR. The lock up was clearly related to, in my case, receiving on the USART. I have not figured out exactly what happens, but I think it is something along the lines of:

1. Data arrives on USART ->regs->DR
2. USART interrupt gets called
3. 250 Hz interrupt gets called before USART interrupt is finished or is started
4. 250 Hz interrupt takes so much time there occurs some error on the USART regs->DR part
5. USART interrupt locks up.

The lock up always happened on the line:
usart.c:usart_irq (usart_dev *dev):
rb_push/safe_insert (dev->rb, (uint8)dev->regs->DR);

As I said I don't know exactly what the error is, but by increasing the priority of the usart interrupt the lock up has been completely removed.

My fix: https://github.com/gauteh/libmaple/commit/1e10299b43e255a4029cacc18bdf248289be7092

libmaple/usart.c View file @ 1e10299 ... ... @@ -102,6 +102,11 @@ void usart_init(usart_dev *dev) { 102 102 rb_init(dev->rb, USART_RX_BUF_SIZE, dev->rx_buf); 103 103 rcc_clk_enable(dev->clk_id); 104 104 nvic_irq_enable(dev->irq_num); 105 + 106 + /* Set to max priority to avoid race condition where register presumably 107 + * might change before it can be picked out by USART interrupt and cause 108 + * it to lock up. */ 109 + nvic_irq_set_priority (dev->irq_num, 0); 105 110 }

the other edit in usart.h is for good measure, I haven't had any problems with it.

Regards, Gaute

Posted 3 years ago #
mbolivar
LeafLabs

sigh. you've run into one of our terrible hacks. sorry for any problems you've been having; glad you found a workaround.

the ring buffer implementation isn't thread-safe at all, but the USART RX ring_buffers are accessed concurrently by the interrupt handlers and other places (e.g. usart_getc(), usart_rx()). so it's totally reasonable that something like this would happen.

i'm not sure what the right thing to do is.

we could try to put together some sort of IRQ bottom half system in wirish, so that the IRQs could just mark the new data and some other routine could lock the RX ring_buffers in usart_dev before pushing them. that seems tricky to get right, though, and it'd add yet another interrupt that would mess with people's real-time concerns in the same way that the USB interrupts already do (we could stuff it into the SysTick handler, but that'd be too surprising to people, i think).

any suggestions are welcome.

(edit: presuming that's the cause of the problem. i guess it could also be due to overrun error? not sure. anyway, thanks again for the report.)

Posted 3 years ago #
gauteh
Member

I tried to put a check for any of the bits described here: http://www.micromouseonline.com/2009/12/31/stm32-usart-basics/#axzz1v8opplfY:

USARTx_SR_ORE: Overrun Error
USARTx_SR_NE: Noise Error
USARTx_SR_FE: Framing Error
USARTx_SR_PE: Parity Error

around the rb_safe_insert line, but to no help. Given I did it correct and that it would actually do any good: for what I know the 250Hz could still interrupt the flow in between.

Also I did not do any reset if any of them were set, so my attempt was fairly basic..

It would be worth a try from someone who has a better overview. But I guess the whole ring_buffer design along with the interrupt is somewhat hard to get right in this case (without the priority workaround).

Perhaps another approach using DMA with a 'locked' and thread safe ringbuffers could be a solution. I don't know if it is possible to do atomic locking between user and interrupt space.

Posted 3 years ago #
ala42
Member
Do you have USART_SAFE_INSERT defined ?
```
static inline int rb_safe_insert(ring_buffer *rb, uint8 element) {
    if (rb_is_full(rb)) {
        return 0;
    }
    rb_insert(rb, element);
    return 1;
}

rb_safe_insert(dev->rb, (uint8)dev->regs->DR);
```
In case the buffer is full, 'element' is not read, so the DR is never read and the interrupt is not cleared. Add a dummy read and check the assembler code so see the dummy read was not optimized out.
Posted 3 years ago #
gauteh
Member

I have had rb_safe_insert hard coded in that function (using the define now), the lock up still happened with no apparent difference in frequency to using the regular rb_push_insert. A shipment with a debugger/adapter should arrive by tomorrow so I might get the chance to investigate a little closer then.

One important thing I forgot to mention here is that I am using the olimexino clone for this project - so although it shouldn't be a difference to the real maple I don't expect any support on it ;) Originally hoped to use the Native, but you ran out.. I would be able to test this error on the one I have though.

Posted 3 years ago #
mbolivar
LeafLabs

One important thing I forgot to mention here is that I am using the olimexino clone for this project - so although it shouldn't be a difference to the real maple I don't expect any support on it ;) Originally hoped to use the Native, but you ran out.. I would be able to test this error on the one I have though.

That's cool. This sounds like a software problem rather than a hardware issue. We say libmaple works on the STM32F103RB, so we shouldn't be bothered if a bug report happens to come from another board ;).

Posted 3 years ago #

RSS feed for this topic

Reply

You must log in to post.