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

[Nicolai Buchwitz <nb@tipi-net.de>]

Hi Théo

On 17.7.2026 21:48, Théo Lebrun wrote:
> 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 <nb@tipi-net.de>

Thanks
Nicolai