drivers/net/ethernet/broadcom/genet/bcmgenet.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
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;
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
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
© 2016 - 2025 Red Hat, Inc.