On Sat, 02 Apr 2016 13:28:34 +0200, Maarten Brock wrote: > ----- Original Message ----- > From: Jakub Kicinski [mailto:moorray3@xxxxx] > To: Mikael Jansson [mailto:mikael@xxxxxxxxxx] > Cc: linux-serial@xxxxxxxxxxxxxxx > Sent: Wed, 30 Mar 2016 22:25:16 +0200 > Subject: Re: [PATCH] sc16is7xx: Emulate IRQF_ONESHOT. > > > > On Sat, 26 Mar 2016 12:35:14 +0100, Mikael Jansson wrote: > > > Since non-threaded IRQ handlers can't use IRQF_ONESHOT and we return > > directly from the IRQ handler after queueing a worker thread, the IRQ > > handler will be called continuously on level-triggered interrupts. > > > At least on the author's platform, this causes CPU stalls detected by > > rcu_preempt, since the IRQ handler will be called too many times. > > > > > > This patch emulates IRQF_ONESHOT behaviour by disabling the IRQ in the IRQ > > handler and re-enabling it again at the end of the worker thread. > > > --- > > > drivers/tty/serial/sc16is7xx.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/drivers/tty/serial/sc16is7xx.c > > b/drivers/tty/serial/sc16is7xx.c > > > index edb5305..8a6aa71 100644 > > > --- a/drivers/tty/serial/sc16is7xx.c > > > +++ b/drivers/tty/serial/sc16is7xx.c > > > @@ -695,12 +695,15 @@ static void sc16is7xx_ist(struct kthread_work *ws) > > > > > > for (i = 0; i < s->devtype->nr_uart; ++i) > > > sc16is7xx_port_irq(s, i); > > > + > > > + enable_irq(s->p[0].port.irq); > > > > Isn't it necessary to re-check the IRQ status after enabling the > > interrupt? If the IRQ is edge-triggered and irq controller does > > not remember what's pending we can get: > > > > -- irq line goes up > > # IRQ > > irq_disable() > > # thread runs > > check statuses > > -- irq line goes up > > irq_enable() > > > > Since line is up no edge can be detected, no interrupt will ever come? > > I would argue that configuring the interrupt-controller to expect an > edge-triggered IRQ on this device that clearly has a level-triggering IRQ output > is simply wrong. Due to above mentioned problem one currently must improperly > configure for edge-triggered IRQ and this also prevents interrupt sharing. > > The irq line need not even go up (I prefer active) *again*. It could also stay > active. In this case even an interrupt controller that does remember there was > another edge will not retrigger. I agree with you. Apart from the danger of irq line going permanently up and killing the machine in IRQ storm level triggered may be a better idea here. The driver still seems to default to edge-triggered, though (grep IRQF_TRIGGER_FALLING) and we should take that into consideration... -- To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
