All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] net: ks8851: Drop IRQ threading
@ 2022-12-16 12:47 Marek Vasut
  2022-12-16 13:23 ` Eric Dumazet
  0 siblings, 1 reply; 6+ messages in thread
From: Marek Vasut @ 2022-12-16 12:47 UTC (permalink / raw)
  To: netdev
  Cc: Marek Vasut, David S. Miller, Dmitry Torokhov, Eric Dumazet,
	Geoff Levand, Jakub Kicinski, Linus Walleij, Paolo Abeni,
	Petr Machata, Wolfram Sang

Request non-threaded IRQ in the KSZ8851 driver, this fixes the following warning:
"
NOHZ tick-stop error: Non-RCU local softirq work is pending, handler #08!!!
"

Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Geoff Levand <geoff@infradead.org>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Petr Machata <petrm@nvidia.com>
Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
To: netdev@vger.kernel.org
---
 drivers/net/ethernet/micrel/ks8851_common.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/micrel/ks8851_common.c b/drivers/net/ethernet/micrel/ks8851_common.c
index cfbc900d4aeb9..1eba4ba0b95cf 100644
--- a/drivers/net/ethernet/micrel/ks8851_common.c
+++ b/drivers/net/ethernet/micrel/ks8851_common.c
@@ -443,9 +443,7 @@ static int ks8851_net_open(struct net_device *dev)
 	unsigned long flags;
 	int ret;
 
-	ret = request_threaded_irq(dev->irq, NULL, ks8851_irq,
-				   IRQF_TRIGGER_LOW | IRQF_ONESHOT,
-				   dev->name, ks);
+	ret = request_irq(dev->irq, ks8851_irq, IRQF_TRIGGER_LOW, dev->name, ks);
 	if (ret < 0) {
 		netdev_err(dev, "failed to get irq\n");
 		return ret;
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] net: ks8851: Drop IRQ threading
  2022-12-16 12:47 [PATCH] net: ks8851: Drop IRQ threading Marek Vasut
@ 2022-12-16 13:23 ` Eric Dumazet
  2022-12-16 21:54   ` Dmitry Torokhov
  0 siblings, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2022-12-16 13:23 UTC (permalink / raw)
  To: Marek Vasut
  Cc: netdev, David S. Miller, Dmitry Torokhov, Geoff Levand,
	Jakub Kicinski, Linus Walleij, Paolo Abeni, Petr Machata,
	Wolfram Sang

On Fri, Dec 16, 2022 at 1:47 PM Marek Vasut <marex@denx.de> wrote:
>
> Request non-threaded IRQ in the KSZ8851 driver, this fixes the following warning:
> "
> NOHZ tick-stop error: Non-RCU local softirq work is pending, handler #08!!!

This changelog is a bit terse.

Why can other drivers use request_threaded_irq(), but not this one ?


> "
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Geoff Levand <geoff@infradead.org>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Paolo Abeni <pabeni@redhat.com>
> Cc: Petr Machata <petrm@nvidia.com>
> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> To: netdev@vger.kernel.org
> ---
>  drivers/net/ethernet/micrel/ks8851_common.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/micrel/ks8851_common.c b/drivers/net/ethernet/micrel/ks8851_common.c
> index cfbc900d4aeb9..1eba4ba0b95cf 100644
> --- a/drivers/net/ethernet/micrel/ks8851_common.c
> +++ b/drivers/net/ethernet/micrel/ks8851_common.c
> @@ -443,9 +443,7 @@ static int ks8851_net_open(struct net_device *dev)
>         unsigned long flags;
>         int ret;
>
> -       ret = request_threaded_irq(dev->irq, NULL, ks8851_irq,
> -                                  IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> -                                  dev->name, ks);
> +       ret = request_irq(dev->irq, ks8851_irq, IRQF_TRIGGER_LOW, dev->name, ks);
>         if (ret < 0) {
>                 netdev_err(dev, "failed to get irq\n");
>                 return ret;
> --
> 2.35.1
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] net: ks8851: Drop IRQ threading
  2022-12-16 13:23 ` Eric Dumazet
@ 2022-12-16 21:54   ` Dmitry Torokhov
  2022-12-16 22:19     ` Marek Vasut
  0 siblings, 1 reply; 6+ messages in thread
From: Dmitry Torokhov @ 2022-12-16 21:54 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Marek Vasut, netdev, David S. Miller, Geoff Levand,
	Jakub Kicinski, Linus Walleij, Paolo Abeni, Petr Machata,
	Wolfram Sang

On Fri, Dec 16, 2022 at 02:23:04PM +0100, Eric Dumazet wrote:
> On Fri, Dec 16, 2022 at 1:47 PM Marek Vasut <marex@denx.de> wrote:
> >
> > Request non-threaded IRQ in the KSZ8851 driver, this fixes the following warning:
> > "
> > NOHZ tick-stop error: Non-RCU local softirq work is pending, handler #08!!!
> 
> This changelog is a bit terse.
> 
> Why can other drivers use request_threaded_irq(), but not this one ?

This one actually *has* to use threading, as (as far as I can see) the
"lock" that is being taken in ks8851_irq for the SPI variant of the
device is actually a mutex, so we have to be able to sleep in the
interrupt handler...

> 
> 
> > "
> >
> > Signed-off-by: Marek Vasut <marex@denx.de>
> > ---
> > Cc: "David S. Miller" <davem@davemloft.net>
> > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > Cc: Eric Dumazet <edumazet@google.com>
> > Cc: Geoff Levand <geoff@infradead.org>
> > Cc: Jakub Kicinski <kuba@kernel.org>
> > Cc: Linus Walleij <linus.walleij@linaro.org>
> > Cc: Paolo Abeni <pabeni@redhat.com>
> > Cc: Petr Machata <petrm@nvidia.com>
> > Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > To: netdev@vger.kernel.org
> > ---
> >  drivers/net/ethernet/micrel/ks8851_common.c | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/micrel/ks8851_common.c b/drivers/net/ethernet/micrel/ks8851_common.c
> > index cfbc900d4aeb9..1eba4ba0b95cf 100644
> > --- a/drivers/net/ethernet/micrel/ks8851_common.c
> > +++ b/drivers/net/ethernet/micrel/ks8851_common.c
> > @@ -443,9 +443,7 @@ static int ks8851_net_open(struct net_device *dev)
> >         unsigned long flags;
> >         int ret;
> >
> > -       ret = request_threaded_irq(dev->irq, NULL, ks8851_irq,
> > -                                  IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> > -                                  dev->name, ks);
> > +       ret = request_irq(dev->irq, ks8851_irq, IRQF_TRIGGER_LOW, dev->name, ks);
> >         if (ret < 0) {
> >                 netdev_err(dev, "failed to get irq\n");
> >                 return ret;
> > --
> > 2.35.1
> >

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] net: ks8851: Drop IRQ threading
  2022-12-16 21:54   ` Dmitry Torokhov
@ 2022-12-16 22:19     ` Marek Vasut
  2022-12-16 22:32       ` Dmitry Torokhov
  0 siblings, 1 reply; 6+ messages in thread
From: Marek Vasut @ 2022-12-16 22:19 UTC (permalink / raw)
  To: Dmitry Torokhov, Eric Dumazet
  Cc: netdev, David S. Miller, Geoff Levand, Jakub Kicinski,
	Linus Walleij, Paolo Abeni, Petr Machata, Wolfram Sang

On 12/16/22 22:54, Dmitry Torokhov wrote:
> On Fri, Dec 16, 2022 at 02:23:04PM +0100, Eric Dumazet wrote:
>> On Fri, Dec 16, 2022 at 1:47 PM Marek Vasut <marex@denx.de> wrote:
>>>
>>> Request non-threaded IRQ in the KSZ8851 driver, this fixes the following warning:
>>> "
>>> NOHZ tick-stop error: Non-RCU local softirq work is pending, handler #08!!!
>>
>> This changelog is a bit terse.
>>
>> Why can other drivers use request_threaded_irq(), but not this one ?
> 
> This one actually *has* to use threading, as (as far as I can see) the
> "lock" that is being taken in ks8851_irq for the SPI variant of the
> device is actually a mutex, so we have to be able to sleep in the
> interrupt handler...

So maybe we should use threaded one for the SPI variant and non-threaded 
one for the Parallel bus variant ?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] net: ks8851: Drop IRQ threading
  2022-12-16 22:19     ` Marek Vasut
@ 2022-12-16 22:32       ` Dmitry Torokhov
  2023-05-31  2:19         ` Marek Vasut
  0 siblings, 1 reply; 6+ messages in thread
From: Dmitry Torokhov @ 2022-12-16 22:32 UTC (permalink / raw)
  To: Marek Vasut
  Cc: Eric Dumazet, netdev, David S. Miller, Geoff Levand,
	Jakub Kicinski, Linus Walleij, Paolo Abeni, Petr Machata,
	Wolfram Sang

On Fri, Dec 16, 2022 at 11:19:27PM +0100, Marek Vasut wrote:
> On 12/16/22 22:54, Dmitry Torokhov wrote:
> > On Fri, Dec 16, 2022 at 02:23:04PM +0100, Eric Dumazet wrote:
> > > On Fri, Dec 16, 2022 at 1:47 PM Marek Vasut <marex@denx.de> wrote:
> > > > 
> > > > Request non-threaded IRQ in the KSZ8851 driver, this fixes the following warning:
> > > > "
> > > > NOHZ tick-stop error: Non-RCU local softirq work is pending, handler #08!!!
> > > 
> > > This changelog is a bit terse.
> > > 
> > > Why can other drivers use request_threaded_irq(), but not this one ?
> > 
> > This one actually *has* to use threading, as (as far as I can see) the
> > "lock" that is being taken in ks8851_irq for the SPI variant of the
> > device is actually a mutex, so we have to be able to sleep in the
> > interrupt handler...
> 
> So maybe we should use threaded one for the SPI variant and non-threaded one
> for the Parallel bus variant ?

I do not think the threading itself is the issue. I did a quick search
and "Non-RCU local softirq work is pending" seems to be a somewhat
common issue in network drivers. I think you should follow for example
thread in https://lore.kernel.org/all/87y28b9nyn.ffs@tglx/t/ and collect
the trace data and bug tglx and Paul. I see you are even on CC in that
thread...

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] net: ks8851: Drop IRQ threading
  2022-12-16 22:32       ` Dmitry Torokhov
@ 2023-05-31  2:19         ` Marek Vasut
  0 siblings, 0 replies; 6+ messages in thread
From: Marek Vasut @ 2023-05-31  2:19 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Eric Dumazet, netdev, David S. Miller, Geoff Levand,
	Jakub Kicinski, Linus Walleij, Paolo Abeni, Petr Machata,
	Wolfram Sang

On 12/16/22 23:32, Dmitry Torokhov wrote:
> On Fri, Dec 16, 2022 at 11:19:27PM +0100, Marek Vasut wrote:
>> On 12/16/22 22:54, Dmitry Torokhov wrote:
>>> On Fri, Dec 16, 2022 at 02:23:04PM +0100, Eric Dumazet wrote:
>>>> On Fri, Dec 16, 2022 at 1:47 PM Marek Vasut <marex@denx.de> wrote:
>>>>>
>>>>> Request non-threaded IRQ in the KSZ8851 driver, this fixes the following warning:
>>>>> "
>>>>> NOHZ tick-stop error: Non-RCU local softirq work is pending, handler #08!!!
>>>>
>>>> This changelog is a bit terse.
>>>>
>>>> Why can other drivers use request_threaded_irq(), but not this one ?
>>>
>>> This one actually *has* to use threading, as (as far as I can see) the
>>> "lock" that is being taken in ks8851_irq for the SPI variant of the
>>> device is actually a mutex, so we have to be able to sleep in the
>>> interrupt handler...
>>
>> So maybe we should use threaded one for the SPI variant and non-threaded one
>> for the Parallel bus variant ?
> 
> I do not think the threading itself is the issue. I did a quick search
> and "Non-RCU local softirq work is pending" seems to be a somewhat
> common issue in network drivers. I think you should follow for example
> thread in https://lore.kernel.org/all/87y28b9nyn.ffs@tglx/t/ and collect
> the trace data and bug tglx and Paul. I see you are even on CC in that
> thread...

I ran into this again, with Linux 6.1.30, the machine with this MAC just 
flat out freezes when the MAC comes up, if I add this patch the machine 
works as it should. I don't have a good explanation however. Any ideas?

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-05-31  2:20 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-16 12:47 [PATCH] net: ks8851: Drop IRQ threading Marek Vasut
2022-12-16 13:23 ` Eric Dumazet
2022-12-16 21:54   ` Dmitry Torokhov
2022-12-16 22:19     ` Marek Vasut
2022-12-16 22:32       ` Dmitry Torokhov
2023-05-31  2:19         ` Marek Vasut

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.