shkolnick-kun / bugurtos

Breaking backward compatibility since 2010!!!
GNU General Public License v3.0
129 stars 9 forks source link

assertion failed in bgrt_item_insert() from vint #45

Open beefdeadbeef opened 3 months ago

beefdeadbeef commented 3 months ago

So, two vints:

bgrt_vint_init(&dac_vint, 1, func1, arg1);
...
bgrt_vint_init(&usbd_vint, 1, func2, arg2);

regularly pushed into kernel vic from their BGRT_ISR() wrapped isrs

bgrt_vint_push_isr(&usbd_vint, &bgrt_kernel.kblock.vic);
...
bgrt_vint_push_isr(&dac_vint, &bgrt_kernel.kblock.vic);

leads to failed assertion within seconds of run:

(gdb) r

Program received signal SIGTRAP, Trace/breakpoint trap.
0x08009e1a in bgrt_item_insert (head=0x0, item=0x20000d6c <dac_vint>) at bgrt/kernel/item.c:96
96          BGRT_ASSERT(head,       "The #head must not be NULL!");
   0x08009e02 <bgrt_pitem_insert+70>:   b95e            cbnz    r6, 0x8009e1c <bgrt_pitem_insert+96>
   0x08009e04 <bgrt_pitem_insert+72>:   b672            cpsid   i
   0x08009e06 <bgrt_pitem_insert+74>:   4b1a            ldr     r3, [pc, #104]  @ (0x8009e70 <bgrt_pitem_insert+180>)
   0x08009e08 <bgrt_pitem_insert+76>:   4a1e            ldr     r2, [pc, #120]  @ (0x8009e84 <bgrt_pitem_insert+200>)
   0x08009e0a <bgrt_pitem_insert+78>:   f8d3 11ac       ldr.w   r1, [r3, #428]  @ 0x1ac
   0x08009e0e <bgrt_pitem_insert+82>:   4b1e            ldr     r3, [pc, #120]  @ (0x8009e88 <bgrt_pitem_insert+204>)
   0x08009e10 <bgrt_pitem_insert+84>:   9300            str     r3, [sp, #0]
   0x08009e12 <bgrt_pitem_insert+86>:   481a            ldr     r0, [pc, #104]  @ (0x8009e7c <bgrt_pitem_insert+192>)
   0x08009e14 <bgrt_pitem_insert+88>:   2360            movs    r3, #96 @ 0x60
   0x08009e16 <bgrt_pitem_insert+90>:   f7f8 fa0f       bl      0x8002238 <printf_>
=> 0x08009e1a <bgrt_pitem_insert+94>:   be00            bkpt    0x0000
(gdb) bt
#0  0x08009e1a in bgrt_item_insert (head=0x0, item=0x20000d6c <dac_vint>) at bgrt/kernel/item.c:96
#1  bgrt_pitem_insert (pitem=0x20000d6c <dac_vint>, xlist=0x20000d84 <bgrt_kernel>) at bgrt/kernel/pitem.c:107
#2  0x08001ef0 in bgrt_vint_push_isr (vic=0x20000d84 <bgrt_kernel>, vint=0x20000d6c <dac_vint>) at bgrt/kernel/vint.c:114
#3  dma1_channel1_isr_func () at pwmdac.c:212
#4  dma1_channel1_isr () at pwmdac.c:205
#5  <signal handler called>
   0xfffffff9:  0000            movs    r0, r0
#6  0x0800945c in bgrt_pitem_fast_cut (pitem=pitem@entry=0x20000f34 <usbd_vint.lto_priv>) at bgrt/kernel/pitem.c:133
#7  0x08009544 in bgrt_pitem_cut (pitem=pitem@entry=0x20000f34 <usbd_vint.lto_priv>) at bgrt/kernel/pitem.c:153
#8  0x0800c9fa in _vint_pop (vic=<optimized out>, lprio=32) at bgrt/kernel/vint.c:147
#9  bgrt_vic_iterator (vic=<optimized out>) at bgrt/kernel/vint.c:168
#10 bgrt_kblock_do_work (kblock=<optimized out>) at bgrt/kernel/kernel.c:177
#11 bgrt_kblock_main (kblock=<optimized out>) at bgrt/kernel/kernel.c:215
#12 bgrt_start () at bgrt/arch/cm4f/gcc/bugurt_port.c:224
#13 main.isra.0 () at main.c:181
#14 0x08002082 in reset_handler () at libopencm3/lib/at32/f40x/../../cm3/vector.c:93
(gdb) 
(gdb) r

Program received signal SIGTRAP, Trace/breakpoint trap.
0x08009e1a in bgrt_item_insert (head=0x0, item=0x20000f34 <usbd_vint.lto_priv>) at bgrt/kernel/item.c:96
96          BGRT_ASSERT(head,       "The #head must not be NULL!");
   0x08009e02 <bgrt_pitem_insert+70>:   b95e            cbnz    r6, 0x8009e1c <bgrt_pitem_insert+96>
   0x08009e04 <bgrt_pitem_insert+72>:   b672            cpsid   i
   0x08009e06 <bgrt_pitem_insert+74>:   4b1a            ldr     r3, [pc, #104]  @ (0x8009e70 <bgrt_pitem_insert+180>)
   0x08009e08 <bgrt_pitem_insert+76>:   4a1e            ldr     r2, [pc, #120]  @ (0x8009e84 <bgrt_pitem_insert+200>)
   0x08009e0a <bgrt_pitem_insert+78>:   f8d3 11ac       ldr.w   r1, [r3, #428]  @ 0x1ac
   0x08009e0e <bgrt_pitem_insert+82>:   4b1e            ldr     r3, [pc, #120]  @ (0x8009e88 <bgrt_pitem_insert+204>)
   0x08009e10 <bgrt_pitem_insert+84>:   9300            str     r3, [sp, #0]
   0x08009e12 <bgrt_pitem_insert+86>:   481a            ldr     r0, [pc, #104]  @ (0x8009e7c <bgrt_pitem_insert+192>)
   0x08009e14 <bgrt_pitem_insert+88>:   2360            movs    r3, #96 @ 0x60
   0x08009e16 <bgrt_pitem_insert+90>:   f7f8 fa0f       bl      0x8002238 <printf_>
=> 0x08009e1a <bgrt_pitem_insert+94>:   be00            bkpt    0x0000
(gdb) bt
#0  0x08009e1a in bgrt_item_insert (head=0x0, item=0x20000f34 <usbd_vint.lto_priv>) at bgrt/kernel/item.c:96
#1  bgrt_pitem_insert (pitem=0x20000f34 <usbd_vint.lto_priv>, xlist=0x20000d84 <bgrt_kernel>) at bgrt/kernel/pitem.c:107
#2  0x08001f42 in bgrt_vint_push_isr (vic=0x20000d84 <bgrt_kernel>, vint=0x20000f34 <usbd_vint.lto_priv>) at bgrt/kernel/vint.c:114
#3  usb_lp_isr_func () at usbd.c:607
#4  usb_lp_isr () at usbd.c:604
#5  <signal handler called>
   0xfffffff9:  0000            movs    r0, r0
#6  0x0800945c in bgrt_pitem_fast_cut (pitem=pitem@entry=0x20000d6c <dac_vint>) at bgrt/kernel/pitem.c:133
#7  0x08009544 in bgrt_pitem_cut (pitem=pitem@entry=0x20000d6c <dac_vint>) at bgrt/kernel/pitem.c:153
#8  0x0800c9fa in _vint_pop (vic=<optimized out>, lprio=32) at bgrt/kernel/vint.c:147
#9  bgrt_vic_iterator (vic=<optimized out>) at bgrt/kernel/vint.c:168
#10 bgrt_kblock_do_work (kblock=<optimized out>) at bgrt/kernel/kernel.c:177
#11 bgrt_kblock_main (kblock=<optimized out>) at bgrt/kernel/kernel.c:215
#12 bgrt_start () at bgrt/arch/cm4f/gcc/bugurt_port.c:224
#13 main.isra.0 () at main.c:181
#14 0x08002082 in reset_handler () at libopencm3/lib/at32/f40x/../../cm3/vector.c:93

Sometimes it is one vint, sometimes another, as in these two, and happens only if both vints have same priority, iow, it is enough to set 0 & 1, or 1 & 2 as priorities to get rid of asserts. Is it necessary condition to have several vints served simultaneously or there's something else ?

shkolnick-kun commented 3 months ago

Hi! Vints are slow and so they are supposed to be used for low priority interrupts which get disabled in critical sections. If your HW interrupt can fire during critical section then you must either lower interrupt priority or stop using vint.

For faster virtual interrupts please consider using kblock->hpmap and BGRT_CONFIG_LPFIC_HOOK. In case of kblock->hpmapatomics are used.

If you want to use vints anyway then you must know that:

  1. bgrt_vint_push_isr is not atomic.
  2. If you are using bgrt_vint_push_isr for interrupts with different HW priorities then high priority interrupts must be masked/disabled before the bgrt_vint_push_isr call and enabled after it.
  3. Consider using bgrt_vint_push as it calls bgrt_vint_push_isr in critical section with interrupts masked.
beefdeadbeef commented 3 months ago

That clears things a lot, thank you. Indeed, with these HW interrupt priorities set numerically bigger than BGRT_CONFIG_CRITSEC_PRIO and bgrt_vint_push() used i see no more such problems. Please feel free to close.