[PATCH net-next v3 13/15] net: macb: re-read ISR inside IRQ handler locked section

[Théo Lebrun]

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.

If we tried grabbing bp->lock while already acquired, we might have
slept and the status might have been updated. Our most likely
competitor in this race (condition) is a swap operation, used in
change_mtu and set_ringparam. It is the only MACB codepath that resets
interrupts and HW inside a bp->lock critical section. Other codepaths
that clear HW IRQ status do so outside the bp->lock critical section.

We can only detect spurious interrupts before grabbing bp->lock if
MACB_CAPS_ISR_CLEAR_ON_WRITE. If we don't, then we only read ISR once.

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

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 7245c345c78f..5a32d5cb759e 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -2184,13 +2184,21 @@ 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;
+	/* detect spurious interrupts without grabbing bp->lock */
+	if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE) {
+		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))) {

-- 
2.55.0