Re: [net,PATCH v2] net: ks8851: Reinstate disabling of BHs around IRQ handler

Sebastian Andrzej Siewior
2026-04-13T12:57:49Z

On 2026-04-12 10:51:25 [-0700], Jakub Kicinski wrote:
> > Does the backtrace make the problem clearer, with the annotation above ?
>
> Sebastian, do you have any recommendation here? tl;dr is that the driver does
…

What about this:

--- a/drivers/net/ethernet/micrel/ks8851_par.c
+++ b/drivers/net/ethernet/micrel/ks8851_par.c
@@ -63,7 +63,7 @@ static void ks8851_lock_par(struct ks8851_net *ks, unsigned long *flags)
{
struct ks8851_net_par *ksp = to_ks8851_par(ks);

- spin_lock_irqsave(&ksp->lock, *flags);
+ spin_lock_bh(&ksp->lock);
}

/**
@@ -77,7 +77,7 @@ static void ks8851_unlock_par(struct ks8851_net *ks, unsigned long *flags)
{
struct ks8851_net_par *ksp = to_ks8851_par(ks);

- spin_unlock_irqrestore(&ksp->lock, *flags);
+ spin_unlock_bh(&ksp->lock);
}

/**

I don't see why it needs to disable interrupts. This seems to be used by
the _par driver and the _common part. The comments refer to DMA but I
see only FIFO access.

And while at it, I would recommend to

diff --git a/drivers/net/ethernet/micrel/ks8851_common.c b/drivers/net/ethernet/micrel/ks8851_common.c
index 8048770958d60..f1c662887646c 100644
--- a/drivers/net/ethernet/micrel/ks8851_common.c
+++ b/drivers/net/ethernet/micrel/ks8851_common.c
@@ -378,9 +378,12 @@ static irqreturn_t ks8851_irq(int irq, void *_ks)
if (status & IRQ_LCI)
mii_check_link(&ks->mii);

- if (status & IRQ_RXI)
+ if (status & IRQ_RXI) {
+ local_bh_disable();
while ((skb = __skb_dequeue(&rxq)))
netif_rx(skb);
+ local_bh_enable();
+ }

return IRQ_HANDLED;
}

Because otherwise it will kick-off backlog NAPI after every packet if
multiple packets are available.

Sebastian