2020-06-12 12:51:18

by Wolfram Sang

[permalink] [raw]
Subject: [PATCH] lib: update DEBUG_SHIRQ docs to match reality

There is no extra interrupt when registering a shared interrupt handler
since 2011. Update the Kconfig text to make it clear and to avoid wrong
assumptions when debugging issues found by it.

Fixes: 6d83f94db95c ("genirq: Disable the SHIRQ_DEBUG call in request_threaded_irq for now")
Link: https://lore.kernel.org/linux-i2c/[email protected]/T/#mf24d7070d7e0c8f17b6be6ceb51df94b7d7613b3
Signed-off-by: Wolfram Sang <[email protected]>
---

I'd think this could go in via one of tglx' trees?

lib/Kconfig.debug | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index ef675beccab1..50522d3a7770 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -845,10 +845,10 @@ config DEBUG_SHIRQ
bool "Debug shared IRQ handlers"
depends on DEBUG_KERNEL
help
- Enable this to generate a spurious interrupt as soon as a shared
- interrupt handler is registered, and just before one is deregistered.
- Drivers ought to be able to handle interrupts coming in at those
- points; some don't and need to be caught.
+ Enable this to generate a spurious interrupt just before a shared
+ interrupt handler is deregistered (generating one when registering
+ is currently disabled). Drivers need to handle this correctly. Some
+ don't and need to be caught.

menu "Debug Oops, Lockups and Hangs"

--
2.20.1


2020-06-12 13:17:59

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH] lib: update DEBUG_SHIRQ docs to match reality

On Fri, Jun 12, 2020 at 02:48:44PM +0200, Wolfram Sang wrote:
> There is no extra interrupt when registering a shared interrupt handler
> since 2011. Update the Kconfig text to make it clear and to avoid wrong
> assumptions when debugging issues found by it.
>
> Fixes: 6d83f94db95c ("genirq: Disable the SHIRQ_DEBUG call in request_threaded_irq for now")
> Link: https://lore.kernel.org/linux-i2c/[email protected]/T/#mf24d7070d7e0c8f17b6be6ceb51df94b7d7613b3
> Signed-off-by: Wolfram Sang <[email protected]>

Reviewed-by: Krzysztof Kozlowski <[email protected]>

Best regards,
Krzysztof

2020-06-13 11:15:47

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH] lib: update DEBUG_SHIRQ docs to match reality

On Fri, Jun 12, 2020 at 3:54 PM Wolfram Sang
<[email protected]> wrote:
>
> There is no extra interrupt when registering a shared interrupt handler
> since 2011. Update the Kconfig text to make it clear and to avoid wrong
> assumptions when debugging issues found by it.
>

I'm not sure.
I have recently fixed a bug in the IIO sensor during ->probe() due to
an issued test interrupt exactly as soon as the handler is registered.

...

> - Enable this to generate a spurious interrupt as soon as a shared
> - interrupt handler is registered, and just before one is deregistered.
> - Drivers ought to be able to handle interrupts coming in at those
> - points; some don't and need to be caught.
> + Enable this to generate a spurious interrupt just before a shared
> + interrupt handler is deregistered (generating one when registering
> + is currently disabled). Drivers need to handle this correctly. Some
> + don't and need to be caught.

--
With Best Regards,
Andy Shevchenko

2020-06-13 11:41:32

by Wolfram Sang

[permalink] [raw]
Subject: Re: [PATCH] lib: update DEBUG_SHIRQ docs to match reality


> > There is no extra interrupt when registering a shared interrupt handler
> > since 2011. Update the Kconfig text to make it clear and to avoid wrong
> > assumptions when debugging issues found by it.
> >
>
> I'm not sure.
> I have recently fixed a bug in the IIO sensor during ->probe() due to
> an issued test interrupt exactly as soon as the handler is registered.

$ git grep DEBUG_SHIRQ_FIXME
kernel/irq/manage.c:#ifdef CONFIG_DEBUG_SHIRQ_FIXME

There is no place to enable this code.

Maybe your case was like Krzysztof's case where the issue turned out to
be the extra interrupt on deregistering after a deferred probe? He
thought it was the initial interrupt but it wasn't.


Attachments:
(No filename) (706.00 B)
signature.asc (849.00 B)
Download all attachments

2020-06-13 12:09:38

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH] lib: update DEBUG_SHIRQ docs to match reality

On Sat, Jun 13, 2020 at 2:39 PM Wolfram Sang
<[email protected]> wrote:
>
>
> > > There is no extra interrupt when registering a shared interrupt handler
> > > since 2011. Update the Kconfig text to make it clear and to avoid wrong
> > > assumptions when debugging issues found by it.
> > >
> >
> > I'm not sure.
> > I have recently fixed a bug in the IIO sensor during ->probe() due to
> > an issued test interrupt exactly as soon as the handler is registered.
>
> $ git grep DEBUG_SHIRQ_FIXME
> kernel/irq/manage.c:#ifdef CONFIG_DEBUG_SHIRQ_FIXME
>
> There is no place to enable this code.
>
> Maybe your case was like Krzysztof's case where the issue turned out to
> be the extra interrupt on deregistering after a deferred probe? He
> thought it was the initial interrupt but it wasn't.

Commit
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/iio/pressure/bmp280-core.c?id=97b31a6f5fb95b1ec6575b78a7240baddba34384

The relevant IRQ core code
https://elixir.bootlin.com/linux/latest/source/kernel/irq/manage.c#L1774

It runs it at deregistering, right.

--
With Best Regards,
Andy Shevchenko

2020-06-22 16:00:45

by Wolfram Sang

[permalink] [raw]
Subject: Re: [PATCH] lib: update DEBUG_SHIRQ docs to match reality

Hi Andy,

> > Maybe your case was like Krzysztof's case where the issue turned out to
> > be the extra interrupt on deregistering after a deferred probe? He
> > thought it was the initial interrupt but it wasn't.
>
> Commit
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/iio/pressure/bmp280-core.c?id=97b31a6f5fb95b1ec6575b78a7240baddba34384
>
> The relevant IRQ core code
> https://elixir.bootlin.com/linux/latest/source/kernel/irq/manage.c#L1774
>
> It runs it at deregistering, right.

So, can I read this as an Acked-by?

Kind regards,

Wolfram


Attachments:
(No filename) (613.00 B)
signature.asc (849.00 B)
Download all attachments

2020-06-22 16:04:26

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH] lib: update DEBUG_SHIRQ docs to match reality

On Mon, Jun 22, 2020 at 6:56 PM Wolfram Sang <[email protected]> wrote:

> > > Maybe your case was like Krzysztof's case where the issue turned out to
> > > be the extra interrupt on deregistering after a deferred probe? He
> > > thought it was the initial interrupt but it wasn't.
> >
> > Commit
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/iio/pressure/bmp280-core.c?id=97b31a6f5fb95b1ec6575b78a7240baddba34384
> >
> > The relevant IRQ core code
> > https://elixir.bootlin.com/linux/latest/source/kernel/irq/manage.c#L1774
> >
> > It runs it at deregistering, right.
>
> So, can I read this as an Acked-by?

Yes. It means I agree that text should be fixed. Alas, I'm not native
speaker, so I can't check the text for (stylistic, spelling, etc)
correctness.

--
With Best Regards,
Andy Shevchenko