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

[net,PATCH v2] 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: Nicolai Buchwitz <nb@tipi-net.de>
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
---
V2: Register dedicated IRQ handler wrapper which disables BH for the
    parallel variant of the MAC, the variant which uses spinlocks as
    the locking primitive. Use stock IRQ handler with BH unchanged
    for the SPI variant, which uses mutexes as locking primitive.
---
 drivers/net/ethernet/micrel/ks8851.h        |  2 ++
 drivers/net/ethernet/micrel/ks8851_common.c | 22 ++++++++++++++++++++-
 drivers/net/ethernet/micrel/ks8851_par.c    |  1 +
 drivers/net/ethernet/micrel/ks8851_spi.c    |  1 +
 4 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/micrel/ks8851.h b/drivers/net/ethernet/micrel/ks8851.h
index 31f75b4a67fd7..71213b44b0d04 100644
--- a/drivers/net/ethernet/micrel/ks8851.h
+++ b/drivers/net/ethernet/micrel/ks8851.h
@@ -394,6 +394,8 @@ struct ks8851_net {
 	u16			rc_rxqcr;
 	u16			rc_ccr;
 
+	bool			no_bh_in_irq_handler;
+
 	struct mii_if_info	mii;
 	struct ks8851_rxctrl	rxctrl;
 
diff --git a/drivers/net/ethernet/micrel/ks8851_common.c b/drivers/net/ethernet/micrel/ks8851_common.c
index 8048770958d60..0d7c548d18356 100644
--- a/drivers/net/ethernet/micrel/ks8851_common.c
+++ b/drivers/net/ethernet/micrel/ks8851_common.c
@@ -385,6 +385,24 @@ static irqreturn_t ks8851_irq(int irq, void *_ks)
 	return IRQ_HANDLED;
 }
 
+/**
+ * ks8851_irq_nobh - IRQ handler with BH disabled
+ * @irq: IRQ number
+ * @_ks: cookie
+ *
+ * Wrapper which calls ks8851_irq() with BH disabled.
+ */
+static irqreturn_t ks8851_irq_nobh(int irq, void *_ks)
+{
+	irqreturn_t ret;
+
+	local_bh_disable();
+	ret = ks8851_irq(irq, _ks);
+	local_bh_enable();
+
+	return ret;
+}
+
 /**
  * ks8851_flush_tx_work - flush outstanding TX work
  * @ks: The device state
@@ -408,7 +426,9 @@ static int ks8851_net_open(struct net_device *dev)
 	unsigned long flags;
 	int ret;
 
-	ret = request_threaded_irq(dev->irq, NULL, ks8851_irq,
+	ret = request_threaded_irq(dev->irq, NULL,
+				   ks->no_bh_in_irq_handler ?
+				   ks8851_irq_nobh : ks8851_irq,
 				   IRQF_TRIGGER_LOW | IRQF_ONESHOT,
 				   dev->name, ks);
 	if (ret < 0) {
diff --git a/drivers/net/ethernet/micrel/ks8851_par.c b/drivers/net/ethernet/micrel/ks8851_par.c
index 78695be2570bf..0fe3e40291a24 100644
--- a/drivers/net/ethernet/micrel/ks8851_par.c
+++ b/drivers/net/ethernet/micrel/ks8851_par.c
@@ -288,6 +288,7 @@ static int ks8851_probe_par(struct platform_device *pdev)
 	ks->rdfifo = ks8851_rdfifo_par;
 	ks->wrfifo = ks8851_wrfifo_par;
 	ks->start_xmit = ks8851_start_xmit_par;
+	ks->no_bh_in_irq_handler = true;
 
 #define STD_IRQ (IRQ_LCI |	/* Link Change */	\
 		 IRQ_RXI |	/* RX done */		\
diff --git a/drivers/net/ethernet/micrel/ks8851_spi.c b/drivers/net/ethernet/micrel/ks8851_spi.c
index a161ae45743ab..9afdb7eb7953f 100644
--- a/drivers/net/ethernet/micrel/ks8851_spi.c
+++ b/drivers/net/ethernet/micrel/ks8851_spi.c
@@ -424,6 +424,7 @@ static int ks8851_probe_spi(struct spi_device *spi)
 	ks->wrfifo = ks8851_wrfifo_spi;
 	ks->start_xmit = ks8851_start_xmit_spi;
 	ks->flush_tx_work = ks8851_flush_tx_work_spi;
+	ks->no_bh_in_irq_handler = false;
 
 #define STD_IRQ (IRQ_LCI |	/* Link Change */	\
 		 IRQ_TXI |	/* TX done */		\
-- 
2.53.0