Subject: Re: Possible race while masking IRQ on Allwinner A20

From: Jérôme Pouiller <[email-masked]>
Date: Thu, 21 May 2020 15:28:56 +0200

On Thursday 21 May 2020 10:02:48 CEST Marc Zyngier wrote:
> On 2020-05-21 08:26, Maxime Ripard wrote:
> > On Tue, May 19, 2020 at 10:59:26AM +0200, Jérôme Pouiller wrote:
[...]
> >> Nevermind, I tried to use a level triggered IRQ (and my request is on
> >> this part). As you can see in the wfx driver (in  bus_sdio.c and
> >> bh.c), I use a threaded IRQ for that. Unfortunately, I receive some IRQs
> >> twice.
> >> I traced the problem, I get:
> >>
> >>  QSGRenderThread-981   [000] d.h.   247.485524: irq_handler_entry: irq=80 name=wfx
> >>  QSGRenderThread-981   [000] d.h.   247.485547: irq_handler_exit: irq=80 ret=handled
> >>  QSGRenderThread-981   [000] d.h.   247.485600: irq_handler_entry: irq=80 name=wfx
> >>  QSGRenderThread-981   [000] d.h.   247.485606: irq_handler_exit: irq=80 ret=handled
> >>       irq/80-wfx-260   [001] ....   247.485828: io_read32: CONTROL: 0000f046
> >>       irq/80-wfx-260   [001] ....   247.486072: io_read32: CONTROL: 0000f046
> >>     kworker/1:1H-116   [001] ....   247.486214: io_read: QUEUE: 8b 00 84 18 00 00 00 00 01 00 15 82 2b 48 01 1e 88 42 30 00 08 6b d7 c3 53 e0 28 80 88 67 32 af ... (192 bytes)
> >>     kworker/1:1H-116   [001] ....   247.493097: io_read: QUEUE: 00 00 00 00 00 00 00 00 06 06 00 6a 3f 95 00 60 00 00 00 00 08 62 00 00 01 00 5e 00 00 07 28 80 ... (192 bytes)
> >>     [...]
> >>
> >> On this trace, we can see:
> >>   - the hard IRQ handler
> >>   - the IRQ acknowledge from the thread irq/80-wfx-260
> >>   - the access to the data from kworker/1:1H-116
> >>
> >> As far as I understand, the first call to the IRQ handler (at
> >> 247.485524) should mask the IRQ 80. So, the second IRQ (at 247.485600)
> >> should not happen and the thread irq/80 should be triggered only once.
> >>
> >> Do you have any idea of what is going wrong with this IRQ?
> >
> > That's pretty weird indeed. My first guess was that you weren't using
> > IRQF_ONESHOT, but it looks like you are. My next lead would be to see
> > if the mask / unmask hooks in the pinctrl driver are properly called
> > (and actually do what they are supposed to do). I'm not sure we have
> > any in-tree user of a threaded IRQ attached to the pinctrl driver, so
> > it might have been broken for quite some time.
> 
> What is certainly puzzling is that this driver doesn't seem to use
> threaded IRQs at all. Instead, it uses its own workqueue that seems
> to bypass the core IRQ subsystem altogether. So any guarantee we'd
> expect goes at of the window.
>
> It is also pretty unclear to me how whether the HW supports switch
> from edge to level signalling. The request_irq() call definitely asks
> for edge, and I don't know how you'd instruct the HW to change its
> signalling method (in general, it isn't possible).

You are talking about the wfx driver? Be sure you read the right version
of the driver. The ability to use a level-triggered IRQ does not exist in
the stable tree. You have to check the "staging-next" tree from Greg[1].

[1] https://kernel.googlesource.com/pub/scm/linux/kernel/git/gregkh/staging/+/staging-next/drivers/staging/wfx/bus_sdio.c#109

-- 
Jérôme Pouiller

_______________________________________________
linux-arm-kernel mailing list
[email-masked]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel