[PATCH net-next v6 13/16] net: macb: move printk() calls out of bp->lock critical section

[Théo Lebrun]

printk() while bp->lock is acquired is dangerous if netconsole is active
on the interface. In that setup, we might land in macb_poll_controller()
-> macb_interrupt() -> spin_lock(&bp->lock) but bp->lock is already
acquired.

This is not an issue currently because macb_interrupt() first checks IRQ
status, potentially early returns, then grabs lock. This early exit is
expected in netpoll scenario.

However if we came to reading the status inside the bp->lock critical
section (as it should be to avoid races), then this would turn into a
deadlock. And we will.

Solution: move printk() calls out of the critical section, to ensure we
can never netpoll under bp->lock's reign. Added benefit is a smaller
and simpler atomic section.

Defer the netdev_err() calls to after the section. Update the timeout
message to highlight it occurred in the past. Inherit the buffer
exhaustion boolean variable name from the old code comment.

Signed-off-by: Théo Lebrun <theo.lebrun@xxxxxxxxxxx>
---
 drivers/net/ethernet/cadence/macb_main.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 987fafb2bf40..65d5ff8a5e23 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1303,6 +1303,7 @@ static void macb_tx_error_task(struct work_struct *work)
 	struct macb_tx_skb *tx_skb;
 	struct macb_dma_desc *desc;
 	bool halt_timeout = false;
+	bool buggy_driver = false;
 	struct sk_buff *skb;
 	unsigned long flags;
 	unsigned int tail;
@@ -1329,7 +1330,6 @@ static void macb_tx_error_task(struct work_struct *work)
 	 * macb/gem must be halted to write TBQP register
 	 */
 	if (macb_halt_tx(bp)) {
-		netdev_err(bp->netdev, "BUG: halt tx timed out\n");
 		macb_writel(bp, NCR, macb_readl(bp, NCR) & (~MACB_BIT(TE)));
 		halt_timeout = true;
 	}
@@ -1358,9 +1358,6 @@ static void macb_tx_error_task(struct work_struct *work)
 			 * since it's the only one written back by the hardware
 			 */
 			if (!(ctrl & MACB_BIT(TX_BUF_EXHAUSTED))) {
-				netdev_vdbg(bp->netdev, "txerr skb %u (data %p) TX complete\n",
-					    macb_tx_ring_wrap(ctx, tail),
-					    skb->data);
 				bp->netdev->stats.tx_packets++;
 				queue->stats.tx_packets++;
 				packets++;
@@ -1374,8 +1371,7 @@ static void macb_tx_error_task(struct work_struct *work)
 			 * those. Statistics are updated by hardware.
 			 */
 			if (ctrl & MACB_BIT(TX_BUF_EXHAUSTED))
-				netdev_err(bp->netdev,
-					   "BUG: TX buffers exhausted mid-frame\n");
+				buggy_driver = true;
 
 			desc->ctrl = ctrl | MACB_BIT(TX_USED);
 		}
@@ -1412,6 +1408,13 @@ static void macb_tx_error_task(struct work_struct *work)
 	macb_writel(bp, NCR, macb_readl(bp, NCR) | MACB_BIT(TSTART));
 
 	spin_unlock_irqrestore(&bp->lock, flags);
+
+	if (halt_timeout)
+		netdev_err(bp->netdev, "BUG: halt tx timed out, we ignored it\n");
+
+	if (buggy_driver)
+		netdev_err(bp->netdev, "BUG: TX buffers exhausted mid-frame\n");
+
 	napi_enable(&queue->napi_tx);
 }
 

-- 
2.55.0