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

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

[Marek Vasut]

If CONFIG_PREEMPT_RT=y is set AND the driver executes ks8851_irq() AND
KSZ_ISR register bit IRQ_RXI is set AND ks8851_rx_pkts() detects that
there are packets in the RX FIFO, then netdev_alloc_skb_ip_align() is
called to allocate SKBs. If netdev_alloc_skb_ip_align() is called with
BH enabled, local_bh_enable() at the end of netdev_alloc_skb_ip_align()
will call __local_bh_enable_ip(), which will call __do_softirq(), which
may trigger net_tx_action() softirq, which may ultimately call the xmit
callback ks8851_start_xmit_par(). The ks8851_start_xmit_par() will try
to lock struct ks8851_net_par .lock spinlock, which is already locked
by ks8851_irq() from which ks8851_start_xmit_par() was called. This
leads to a deadlock, which is reported by the kernel, including a trace
listed below.

Fix the problem by disabling BH around the IRQ handler, thus preventing
the net_tx_action() softirq from triggering during the IRQ handler. The
net_tx_action() softirq is now triggered at the end of the IRQ handler,
once all the other IRQ handler actions have been completed.

  __schedule from schedule_rtlock+0x1c/0x34
  schedule_rtlock from rtlock_slowlock_locked+0x538/0x894
  rtlock_slowlock_locked from rt_spin_lock+0x44/0x5c
  rt_spin_lock from ks8851_start_xmit_par+0x68/0x1a0
  ks8851_start_xmit_par from netdev_start_xmit+0x1c/0x40
  netdev_start_xmit from dev_hard_start_xmit+0xec/0x1b0
  dev_hard_start_xmit from sch_direct_xmit+0xb8/0x25c
  sch_direct_xmit from __qdisc_run+0x20c/0x4fc
  __qdisc_run from qdisc_run+0x1c/0x28
  qdisc_run from net_tx_action+0x1f4/0x244
  net_tx_action from handle_softirqs+0x1c0/0x29c
  handle_softirqs from __local_bh_enable_ip+0xdc/0xf4
  __local_bh_enable_ip from __netdev_alloc_skb+0x140/0x194
  __netdev_alloc_skb from ks8851_irq+0x348/0x4d8
  ks8851_irq from irq_thread_fn+0x24/0x64
  irq_thread_fn from irq_thread+0x110/0x1dc
  irq_thread from kthread+0x104/0x10c
  kthread from ret_from_fork+0x14/0x28

Fixes: e0863634bf9f ("net: ks8851: Queue RX packets in IRQ handler instead of disabling BHs")
Cc: stable@vger.kernel.org
Signed-off-by: Marek Vasut <marex@nabladev.com>
---
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Andrew Lunn <andrew+netdev@lunn.ch>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Ronald Wahl <ronald.wahl@raritan.com>
Cc: Yicong Hui <yiconghui@gmail.com>
Cc: linux-kernel@vger.kernel.org
Cc: netdev@vger.kernel.org
---
 drivers/net/ethernet/micrel/ks8851_common.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/micrel/ks8851_common.c b/drivers/net/ethernet/micrel/ks8851_common.c
index 8048770958d60..dadedea016fac 100644
--- a/drivers/net/ethernet/micrel/ks8851_common.c
+++ b/drivers/net/ethernet/micrel/ks8851_common.c
@@ -316,6 +316,7 @@ static irqreturn_t ks8851_irq(int irq, void *_ks)
 	unsigned int status;
 	struct sk_buff *skb;
 
+	local_bh_disable();
 	ks8851_lock(ks, &flags);
 
 	status = ks8851_rdreg16(ks, KS_ISR);
@@ -381,6 +382,7 @@ static irqreturn_t ks8851_irq(int irq, void *_ks)
 	if (status & IRQ_RXI)
 		while ((skb = __skb_dequeue(&rxq)))
 			netif_rx(skb);
+	local_bh_enable();
 
 	return IRQ_HANDLED;
 }
-- 
2.53.0

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

[Nicolai Buchwitz]

On 7.4.2026 23:23, Marek Vasut wrote:
> If CONFIG_PREEMPT_RT=y is set AND the driver executes ks8851_irq() AND
> KSZ_ISR register bit IRQ_RXI is set AND ks8851_rx_pkts() detects that
> there are packets in the RX FIFO, then netdev_alloc_skb_ip_align() is
> called to allocate SKBs. If netdev_alloc_skb_ip_align() is called with
> BH enabled, local_bh_enable() at the end of netdev_alloc_skb_ip_align()
> will call __local_bh_enable_ip(), which will call __do_softirq(), which
> may trigger net_tx_action() softirq, which may ultimately call the xmit
> callback ks8851_start_xmit_par(). The ks8851_start_xmit_par() will try
> to lock struct ks8851_net_par .lock spinlock, which is already locked
> by ks8851_irq() from which ks8851_start_xmit_par() was called. This
> leads to a deadlock, which is reported by the kernel, including a trace
> listed below.
> 
> Fix the problem by disabling BH around the IRQ handler, thus preventing
> the net_tx_action() softirq from triggering during the IRQ handler. The
> net_tx_action() softirq is now triggered at the end of the IRQ handler,
> once all the other IRQ handler actions have been completed.
> 
>   __schedule from schedule_rtlock+0x1c/0x34
>   schedule_rtlock from rtlock_slowlock_locked+0x538/0x894
>   rtlock_slowlock_locked from rt_spin_lock+0x44/0x5c
>   rt_spin_lock from ks8851_start_xmit_par+0x68/0x1a0
>   ks8851_start_xmit_par from netdev_start_xmit+0x1c/0x40
>   netdev_start_xmit from dev_hard_start_xmit+0xec/0x1b0
>   dev_hard_start_xmit from sch_direct_xmit+0xb8/0x25c
>   sch_direct_xmit from __qdisc_run+0x20c/0x4fc
>   __qdisc_run from qdisc_run+0x1c/0x28
>   qdisc_run from net_tx_action+0x1f4/0x244
>   net_tx_action from handle_softirqs+0x1c0/0x29c
>   handle_softirqs from __local_bh_enable_ip+0xdc/0xf4
>   __local_bh_enable_ip from __netdev_alloc_skb+0x140/0x194
>   __netdev_alloc_skb from ks8851_irq+0x348/0x4d8
>   ks8851_irq from irq_thread_fn+0x24/0x64
>   irq_thread_fn from irq_thread+0x110/0x1dc
>   irq_thread from kthread+0x104/0x10c
>   kthread from ret_from_fork+0x14/0x28
> 
> Fixes: e0863634bf9f ("net: ks8851: Queue RX packets in IRQ handler 
> instead of disabling BHs")
> Cc: stable@vger.kernel.org
> Signed-off-by: Marek Vasut <marex@nabladev.com>
> ---
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Andrew Lunn <andrew+netdev@lunn.ch>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Paolo Abeni <pabeni@redhat.com>
> Cc: Ronald Wahl <ronald.wahl@raritan.com>
> Cc: Yicong Hui <yiconghui@gmail.com>
> Cc: linux-kernel@vger.kernel.org
> Cc: netdev@vger.kernel.org
> ---
>  drivers/net/ethernet/micrel/ks8851_common.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/net/ethernet/micrel/ks8851_common.c 
> b/drivers/net/ethernet/micrel/ks8851_common.c
> index 8048770958d60..dadedea016fac 100644
> --- a/drivers/net/ethernet/micrel/ks8851_common.c
> +++ b/drivers/net/ethernet/micrel/ks8851_common.c
> @@ -316,6 +316,7 @@ static irqreturn_t ks8851_irq(int irq, void *_ks)
>  	unsigned int status;
>  	struct sk_buff *skb;
> 
> +	local_bh_disable();
>  	ks8851_lock(ks, &flags);

I suspect this breaks the SPI variant on non-RT since
ks8851_lock_spi() uses mutex_lock() which can't sleep with
BH disabled. I have KS8851 SPI hardware and will test, will
get back to you.

> 
>  	status = ks8851_rdreg16(ks, KS_ISR);
> @@ -381,6 +382,7 @@ static irqreturn_t ks8851_irq(int irq, void *_ks)
>  	if (status & IRQ_RXI)
>  		while ((skb = __skb_dequeue(&rxq)))
>  			netif_rx(skb);
> +	local_bh_enable();
> 
>  	return IRQ_HANDLED;
>  }

Thanks
Nicolai

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

[Nicolai Buchwitz]

On 7.4.2026 23:23, Marek Vasut wrote:

> [...]

> 
> diff --git a/drivers/net/ethernet/micrel/ks8851_common.c 
> b/drivers/net/ethernet/micrel/ks8851_common.c
> index 8048770958d60..dadedea016fac 100644
> --- a/drivers/net/ethernet/micrel/ks8851_common.c
> +++ b/drivers/net/ethernet/micrel/ks8851_common.c
> @@ -316,6 +316,7 @@ static irqreturn_t ks8851_irq(int irq, void *_ks)
>  	unsigned int status;
>  	struct sk_buff *skb;
> 
> +	local_bh_disable();
>  	ks8851_lock(ks, &flags);

This breaks the SPI variant on non-RT. The SPI path sleeps in
spi_sync() -> wait_for_completion_timeout(), which can't be
done with BH disabled. Confirmed on hardware (KS8851 SPI on
CM4S, PREEMPT non-RT):

   BUG: scheduling while atomic: irq/38-eth2/708/0x00000201
   ...
   spi_transfer_one_message+0x518/0x770
   __spi_pump_transfer_message+0x1dc/0x5f0
   __spi_sync+0x2b4/0x460
   spi_sync+0x38/0x68
   ks8851_rdfifo_spi+0x60/0xc0
   ks8851_irq+0x310/0x3c8

The fix needs to be PAR-specific since the SPI variant doesn't
have the deadlock problem anyway (ks8851_start_xmit_spi doesn't
take the lock).

> 
>  	status = ks8851_rdreg16(ks, KS_ISR);
> @@ -381,6 +382,7 @@ static irqreturn_t ks8851_irq(int irq, void *_ks)
>  	if (status & IRQ_RXI)
>  		while ((skb = __skb_dequeue(&rxq)))
>  			netif_rx(skb);
> +	local_bh_enable();
> 
>  	return IRQ_HANDLED;
>  }

In order to make this work I would propose something like this (which 
works in my SPI setup):

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

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

  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);
+	local_bh_enable();
  }

Tested-by: Nicolai Buchwitz <nb@tipi-net.de>  # KS8851 SPI, non-RT 
(regression + proposed fix)