| Message ID | 20221216124731.122459-1-marex@denx.de (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: ks8851: Drop IRQ threading | expand |
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 >
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.
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 ?
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.
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?
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;
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(-)