[Xenomai-core] Re: [patch, RFC] detect unhandled interrupts

[Jan Kiszka <jan.kiszka@domain.hid>]

[-- Attachment #1: Type: text/plain, Size: 2138 bytes --]

Dmitry Adamushko wrote:
> Hello,
> 
> Jan has rised this question initially and I was struggling last week to get
> his request eventually done :)
> 
> The main idea is to prevent system lockups when the cross domain IRQ
> sharing
> isn't properly used (there were a number of reports recently).
> 
> So here is an initial patch as well as some related critics (yep, I
> critisize my own patch).
> 
> I tend to think now :
> 
> 1) "unhandled" is not necessary indeed (do we need chasing "spurious"
> interrupts the way Linux does it? i.e. it disables the line only after a
> number of consequently unhandled interrupts > SOME_LIMIT);
> 
> 2) XN_ISR_NONE --> should _not_ imply that the line gets re-enabled.
> 
> XN_ISR_HANDLED -- implies it, that's ok.
> 
> But XN_ISR_NONE (all ISRs in case of a shared RT line) should really mean
> "something strange happens" and it can be :
> 
> o  either a spurious interrupt;
> 
> o  something is "sitting" on the same line in the linux domain. ISR should
> have returned XN_ISR_PROPAGATE in this case.
> 
> 
> That said, if we are not interested in chasing "spurious" interrupts the
> linux way, then this patch could be highly simplified to just adding the
> following check in all nucleus ISR handlers.
> 
> +       if (unlikely(s == XN_ISR_NONE)) {
> +               xnlogerr("xnintr_check_status: %d of unhandled consequent
> interrupts. "
> +                       "Disabling the IRQ line #%d\n",
> +                       XNINTR_MAX_UNHANDLED, irq);
> +               s |= XN_ISR_NOENABLE;
> +       }
> 
> I tend to think this would be nicer?
> 

I think the additional costs of maintaining an error counter are almost
negligible. The test is in the unlikely path, and the first condition
already keeps us away from touching the counter. The benefit of not
kicking out IRQ lines on spurious IRQs is obvious (think of EMC issues,
though one should better solve them on the board...).

BTW, is there a difference between unlikely(a && b) and unlikely(a) &&
unlikely(b) that we should care about here?

Patch looks good to me.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 249 bytes --]