Thread (26 messages) 26 messages, 2 authors, 1d ago

Re: [PATCH net-next v4 13/15] net: macb: read ISR inside bp->lock critical section

From: Nicolai Buchwitz <hidden>
Date: 2026-07-19 10:48:43
Also in: lkml

Hi Théo

On 17.7.2026 21:48, Théo Lebrun wrote:
quoted hunk ↗ jump to hunk
The IRQ handler reads ISR register into the `status` stack variable.
If empty, it early returns. Else, it grabs bp->lock and iterates on
the status bits.

We risk a race on spinlock acquire; status might have changed.
Move the readl(ISR) inside the bp->lock critical section.

One risk remains with spurious interrupts that would, in addition to
taking excessive CPU time, also create lock contention. How bad is it?
Probably not too bad.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb_main.c 
b/drivers/net/ethernet/cadence/macb_main.c
index 4c94c23d925a..c832b6c1b98c 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -2185,13 +2185,14 @@ static irqreturn_t macb_interrupt(int irq, void 
*dev_id)
 	struct net_device *netdev = bp->netdev;
 	u32 status;

-	status = queue_readl(queue, ISR);
-
-	if (unlikely(!status))
-		return IRQ_NONE;
-
 	spin_lock(&bp->lock);

+	status = queue_readl(queue, ISR);
+	if (unlikely(!status)) {
+		spin_unlock(&bp->lock);
+		return IRQ_NONE;
+	}
+
 	while (status) {
 		/* close possible race with dev_close */
 		if (unlikely(!netif_running(netdev))) {
Reviewed-by: Nicolai Buchwitz <redacted>

Thanks
Nicolai
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help