Re: Possible race while masking IRQ on Allwinner A20

[Marc Zyngier <maz@kernel.org>]

On 2020-05-21 14:28, Jérôme Pouiller wrote:
> 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].

Right. It still remains that in this (new) code, the threaded handler
seems to kick a workqueue, and returns saying "I'm done". With a level
triggered interrupt, this is likely to result in an interrupt storm if
nothing masks the interrupt.

         M.
-- 
Jazz is not dead. It just smells funny...

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel