Skip to content

Commit

Permalink
cxgb3: Fix lockdep problems with sge.reg_lock
Browse files Browse the repository at this point in the history
Using iWARP with a Chelsio T3 NIC generates the following lockdep warning:

    =================================
    [ INFO: inconsistent lock state ]
    2.6.25-rc6 #50
    ---------------------------------
    inconsistent {softirq-on-W} -> {in-softirq-W} usage.
    swapper/0 [HC0[0]:SC1[1]:HE0:SE0] takes:
     (&adap->sge.reg_lock){-+..}, at: [<ffffffff880e5ee2>] cxgb_offload_ctl+0x3af/0x507 [cxgb3]

The problem is that reg_lock is used with plain spin_lock() in
drivers/net/cxgb3/sge.c but is used with spin_lock_irqsave() in
drivers/net/cxgb3/cxgb3_offload.c.  This is technically a false
positive, since the uses in sge.c are only in the initialization and
cleanup paths and cannot overlap with any use in interrupt context.

The best fix is probably just to use spin_lock_irq() with reg_lock in
sge.c.  Even though it's not strictly required for correctness, it
avoids triggering lockdep and the extra overhead of disabling
interrupts is not important at all in the initialization and cleanup
slow paths.

Signed-off-by: Roland Dreier <rolandd@cisco.com>
Signed-off-by: Jeff Garzik <jeff@garzik.org>
  • Loading branch information
Roland Dreier authored and Jeff Garzik committed Mar 26, 2008
1 parent dc01c44 commit b1186de
Showing 1 changed file with 9 additions and 9 deletions.
18 changes: 9 additions & 9 deletions drivers/net/cxgb3/sge.c
Original file line number Diff line number Diff line change
Expand Up @@ -557,9 +557,9 @@ static void t3_free_qset(struct adapter *adapter, struct sge_qset *q)

for (i = 0; i < SGE_RXQ_PER_SET; ++i)
if (q->fl[i].desc) {
spin_lock(&adapter->sge.reg_lock);
spin_lock_irq(&adapter->sge.reg_lock);
t3_sge_disable_fl(adapter, q->fl[i].cntxt_id);
spin_unlock(&adapter->sge.reg_lock);
spin_unlock_irq(&adapter->sge.reg_lock);
free_rx_bufs(pdev, &q->fl[i]);
kfree(q->fl[i].sdesc);
dma_free_coherent(&pdev->dev,
Expand All @@ -570,9 +570,9 @@ static void t3_free_qset(struct adapter *adapter, struct sge_qset *q)

for (i = 0; i < SGE_TXQ_PER_SET; ++i)
if (q->txq[i].desc) {
spin_lock(&adapter->sge.reg_lock);
spin_lock_irq(&adapter->sge.reg_lock);
t3_sge_enable_ecntxt(adapter, q->txq[i].cntxt_id, 0);
spin_unlock(&adapter->sge.reg_lock);
spin_unlock_irq(&adapter->sge.reg_lock);
if (q->txq[i].sdesc) {
free_tx_desc(adapter, &q->txq[i],
q->txq[i].in_use);
Expand All @@ -586,9 +586,9 @@ static void t3_free_qset(struct adapter *adapter, struct sge_qset *q)
}

if (q->rspq.desc) {
spin_lock(&adapter->sge.reg_lock);
spin_lock_irq(&adapter->sge.reg_lock);
t3_sge_disable_rspcntxt(adapter, q->rspq.cntxt_id);
spin_unlock(&adapter->sge.reg_lock);
spin_unlock_irq(&adapter->sge.reg_lock);
dma_free_coherent(&pdev->dev,
q->rspq.size * sizeof(struct rsp_desc),
q->rspq.desc, q->rspq.phys_addr);
Expand Down Expand Up @@ -2667,7 +2667,7 @@ int t3_sge_alloc_qset(struct adapter *adapter, unsigned int id, int nports,
(16 * 1024) - SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) :
MAX_FRAME_SIZE + 2 + sizeof(struct cpl_rx_pkt);

spin_lock(&adapter->sge.reg_lock);
spin_lock_irq(&adapter->sge.reg_lock);

/* FL threshold comparison uses < */
ret = t3_sge_init_rspcntxt(adapter, q->rspq.cntxt_id, irq_vec_idx,
Expand Down Expand Up @@ -2711,7 +2711,7 @@ int t3_sge_alloc_qset(struct adapter *adapter, unsigned int id, int nports,
goto err_unlock;
}

spin_unlock(&adapter->sge.reg_lock);
spin_unlock_irq(&adapter->sge.reg_lock);

q->adap = adapter;
q->netdev = dev;
Expand All @@ -2728,7 +2728,7 @@ int t3_sge_alloc_qset(struct adapter *adapter, unsigned int id, int nports,
return 0;

err_unlock:
spin_unlock(&adapter->sge.reg_lock);
spin_unlock_irq(&adapter->sge.reg_lock);
err:
t3_free_qset(adapter, q);
return ret;
Expand Down

0 comments on commit b1186de

Please sign in to comment.