[PATCH] lib: update DEBUG_SHIRQ docs to match reality

[PATCH] lib: update DEBUG_SHIRQ docs to match reality

[Wolfram Sang]

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/859e8211-2c56-8dd5-d6fb-33e4358e4128@pengutronix.de/T/#mf24d7070d7e0c8f17b6be6ceb51df94b7d7613b3
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---

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

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

[Krzysztof Kozlowski]

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/859e8211-2c56-8dd5-d6fb-33e4358e4128@pengutronix.de/T/#mf24d7070d7e0c8f17b6be6ceb51df94b7d7613b3
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

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

[Andy Shevchenko]

On Fri, Jun 12, 2020 at 3:54 PM Wolfram Sang
<wsa+renesas@sang-engineering.com> 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

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

[Wolfram Sang]

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


> > 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.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Andy Shevchenko]

On Sat, Jun 13, 2020 at 2:39 PM Wolfram Sang
<wsa+renesas@sang-engineering.com> 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

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

[Wolfram Sang]

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

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


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Andy Shevchenko]

On Mon, Jun 22, 2020 at 6:56 PM Wolfram Sang <wsa@the-dreams.de> 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