[patch] net, bcmgenet: Fix locking of netpoll facing functions

[patch] net, bcmgenet: Fix locking of netpoll facing functions

[Mike Galbraith]

Lockdep reports ring->lock to not be irq safe during netpoll/netconsole
session, resulting in a potential deadlock scenario.

 Chain exists of:
   &host->lock --> target_list_lock --> &ring->lock
  Possible interrupt unsafe locking scenario:
        CPU0                    CPU1
        ----                    ----
   lock(&ring->lock);
                                local_irq_disable();
                                lock(&host->lock);
                                lock(target_list_lock);
   <Interrupt>
     lock(&host->lock);
  *** DEADLOCK ***

Prevent that via use of irqsave/restore spinlock variant when polling.

Signed-off-by: Mike Galbraith <efault@gmx.de>
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -2022,14 +2022,15 @@ static int bcmgenet_tx_poll(struct napi_
 		container_of(napi, struct bcmgenet_tx_ring, napi);
 	unsigned int work_done = 0;
 	struct netdev_queue *txq;
+	unsigned long flags;
 
-	spin_lock(&ring->lock);
+	spin_lock_irqsave(&ring->lock, flags);
 	work_done = __bcmgenet_tx_reclaim(ring->priv->dev, ring);
 	if (ring->free_bds > (MAX_SKB_FRAGS + 1)) {
 		txq = netdev_get_tx_queue(ring->priv->dev, ring->index);
 		netif_tx_wake_queue(txq);
 	}
-	spin_unlock(&ring->lock);
+	spin_unlock_irqrestore(&ring->lock, flags);
 
 	if (work_done == 0) {
 		napi_complete(napi);
@@ -2128,6 +2129,7 @@ static netdev_tx_t bcmgenet_xmit(struct
 	struct bcmgenet_tx_ring *ring = NULL;
 	struct enet_cb *tx_cb_ptr;
 	struct netdev_queue *txq;
+	unsigned long flags;
 	int nr_frags, index;
 	dma_addr_t mapping;
 	unsigned int size;
@@ -2149,7 +2151,7 @@ static netdev_tx_t bcmgenet_xmit(struct
 
 	nr_frags = skb_shinfo(skb)->nr_frags;
 
-	spin_lock(&ring->lock);
+	spin_lock_irqsave(&ring->lock, flags);
 	if (ring->free_bds <= (nr_frags + 1)) {
 		if (!netif_tx_queue_stopped(txq))
 			netif_tx_stop_queue(txq);
@@ -2239,7 +2241,7 @@ static netdev_tx_t bcmgenet_xmit(struct
 		bcmgenet_tdma_ring_writel(priv, ring->index,
 					  ring->prod_index, TDMA_PROD_INDEX);
 out:
-	spin_unlock(&ring->lock);
+	spin_unlock_irqrestore(&ring->lock, flags);
 
 	return ret;
 

Re: [patch] net, bcmgenet: Fix locking of netpoll facing functions

[Florian Fainelli]

On 8/26/25 01:24, Mike Galbraith wrote:
> Lockdep reports ring->lock to not be irq safe during netpoll/netconsole
> session, resulting in a potential deadlock scenario.
> 
>   Chain exists of:
>     &host->lock --> target_list_lock --> &ring->lock
>    Possible interrupt unsafe locking scenario:
>          CPU0                    CPU1
>          ----                    ----
>     lock(&ring->lock);
>                                  local_irq_disable();
>                                  lock(&host->lock);
>                                  lock(target_list_lock);
>     <Interrupt>
>       lock(&host->lock);
>    *** DEADLOCK ***
> 
> Prevent that via use of irqsave/restore spinlock variant when polling.
> 
> Signed-off-by: Mike Galbraith <efault@gmx.de>

Your patch did not make it to the adequate mailing list which should be 
at least netdev@vger.kernel.org. This is effectively a partial revert of 
b0447ecb533270cf857ebee1133cb8ff67115423 ("net: bcmgenet: relax lock 
constraints to reduce IRQ latency") therefore I would want Doug to chime 
in and review this.

Thanks!

> ---
>   drivers/net/ethernet/broadcom/genet/bcmgenet.c |   10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)
> 
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> @@ -2022,14 +2022,15 @@ static int bcmgenet_tx_poll(struct napi_
>   		container_of(napi, struct bcmgenet_tx_ring, napi);
>   	unsigned int work_done = 0;
>   	struct netdev_queue *txq;
> +	unsigned long flags;
>   
> -	spin_lock(&ring->lock);
> +	spin_lock_irqsave(&ring->lock, flags);
>   	work_done = __bcmgenet_tx_reclaim(ring->priv->dev, ring);
>   	if (ring->free_bds > (MAX_SKB_FRAGS + 1)) {
>   		txq = netdev_get_tx_queue(ring->priv->dev, ring->index);
>   		netif_tx_wake_queue(txq);
>   	}
> -	spin_unlock(&ring->lock);
> +	spin_unlock_irqrestore(&ring->lock, flags);
>   
>   	if (work_done == 0) {
>   		napi_complete(napi);
> @@ -2128,6 +2129,7 @@ static netdev_tx_t bcmgenet_xmit(struct
>   	struct bcmgenet_tx_ring *ring = NULL;
>   	struct enet_cb *tx_cb_ptr;
>   	struct netdev_queue *txq;
> +	unsigned long flags;
>   	int nr_frags, index;
>   	dma_addr_t mapping;
>   	unsigned int size;
> @@ -2149,7 +2151,7 @@ static netdev_tx_t bcmgenet_xmit(struct
>   
>   	nr_frags = skb_shinfo(skb)->nr_frags;
>   
> -	spin_lock(&ring->lock);
> +	spin_lock_irqsave(&ring->lock, flags);
>   	if (ring->free_bds <= (nr_frags + 1)) {
>   		if (!netif_tx_queue_stopped(txq))
>   			netif_tx_stop_queue(txq);
> @@ -2239,7 +2241,7 @@ static netdev_tx_t bcmgenet_xmit(struct
>   		bcmgenet_tdma_ring_writel(priv, ring->index,
>   					  ring->prod_index, TDMA_PROD_INDEX);
>   out:
> -	spin_unlock(&ring->lock);
> +	spin_unlock_irqrestore(&ring->lock, flags);
>   
>   	return ret;
>   
> 


-- 
Florian

Re: [patch] net, bcmgenet: Fix locking of netpoll facing functions

[Mike Galbraith]

On Thu, 2025-08-28 at 11:59 -0700, Florian Fainelli wrote:
> On 8/26/25 01:24, Mike Galbraith wrote:
> > Lockdep reports ring->lock to not be irq safe during netpoll/netconsole
> > session, resulting in a potential deadlock scenario.
> > 
> >   Chain exists of:
> >     &host->lock --> target_list_lock --> &ring->lock
> >    Possible interrupt unsafe locking scenario:
> >          CPU0                    CPU1
> >          ----                    ----
> >     lock(&ring->lock);
> >                                  local_irq_disable();
> >                                  lock(&host->lock);
> >                                  lock(target_list_lock);
> >     <Interrupt>
> >       lock(&host->lock);
> >    *** DEADLOCK ***
> > 
> > Prevent that via use of irqsave/restore spinlock variant when polling.
> > 
> > Signed-off-by: Mike Galbraith <efault@gmx.de>
> 
> Your patch did not make it to the adequate mailing list which should be 
> at least netdev@vger.kernel.org. This is effectively a partial revert of 
> b0447ecb533270cf857ebee1133cb8ff67115423 ("net: bcmgenet: relax lock 
> constraints to reduce IRQ latency") therefore I would want Doug to chime 
> in and review this.

Ah, just drop it, he'll likely do something way better.

	-Mike