Skip to content

Commit

Permalink
netdev: Fix lockdep warnings in multiqueue configurations.
Browse files Browse the repository at this point in the history 
When support for multiple TX queues were added, the
netif_tx_lock() routines we converted to iterate over
all TX queues and grab each queue's spinlock.

This causes heartburn for lockdep and it's not a healthy
thing to do with lots of TX queues anyways.

So modify this to use a top-level lock and a "frozen"
state for the individual TX queues.

Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
@davem330
davem330 committed Jul 31, 2008
1 parent 967ab99 commit c3f26a2
Show file tree
Hide file tree
Showing 7 changed files with 78 additions and 44 deletions.
12 changes: 7 additions & 5 deletions drivers/net/ifb.c
View file
Expand Up @@ -69,18 +69,20 @@ static void ri_tasklet(unsigned long dev)
struct net_device *_dev = (struct net_device *)dev;
struct ifb_private *dp = netdev_priv(_dev);
struct net_device_stats *stats = &_dev->stats;
struct netdev_queue *txq;
struct sk_buff *skb;

txq = netdev_get_tx_queue(_dev, 0);
dp->st_task_enter++;
if ((skb = skb_peek(&dp->tq)) == NULL) {
dp->st_txq_refl_try++;
if (netif_tx_trylock(_dev)) {
if (__netif_tx_trylock(txq)) {
dp->st_rxq_enter++;
while ((skb = skb_dequeue(&dp->rq)) != NULL) {
skb_queue_tail(&dp->tq, skb);
dp->st_rx2tx_tran++;
}
netif_tx_unlock(_dev);
__netif_tx_unlock(txq);
} else {
/* reschedule */
dp->st_rxq_notenter++;
Expand Down Expand Up @@ -115,18 +117,18 @@ static void ri_tasklet(unsigned long dev)
BUG();
}

if (netif_tx_trylock(_dev)) {
if (__netif_tx_trylock(txq)) {
dp->st_rxq_check++;
if ((skb = skb_peek(&dp->rq)) == NULL) {
dp->tasklet_pending = 0;
if (netif_queue_stopped(_dev))
netif_wake_queue(_dev);
} else {
dp->st_rxq_rsch++;
netif_tx_unlock(_dev);
__netif_tx_unlock(txq);
goto resched;
}
netif_tx_unlock(_dev);
__netif_tx_unlock(txq);
} else {
resched:
dp->tasklet_pending = 1;
Expand Down
86 changes: 55 additions & 31 deletions include/linux/netdevice.h
View file
Expand Up @@ -440,6 +440,7 @@ static inline void napi_synchronize(const struct napi_struct *n)
enum netdev_queue_state_t
{
__QUEUE_STATE_XOFF,
__QUEUE_STATE_FROZEN,
};

struct netdev_queue {
Expand Down Expand Up @@ -636,7 +637,7 @@ struct net_device
unsigned int real_num_tx_queues;

unsigned long tx_queue_len; /* Max frames per queue allowed */

spinlock_t tx_global_lock;
/*
* One part is mostly used on xmit path (device)
*/
Expand Down Expand Up @@ -1099,6 +1100,11 @@ static inline int netif_queue_stopped(const struct net_device *dev)
return netif_tx_queue_stopped(netdev_get_tx_queue(dev, 0));
}

static inline int netif_tx_queue_frozen(const struct netdev_queue *dev_queue)
{
return test_bit(__QUEUE_STATE_FROZEN, &dev_queue->state);
}

/**
* netif_running - test if up
* @dev: network device
Expand Down Expand Up @@ -1475,6 +1481,26 @@ static inline void __netif_tx_lock_bh(struct netdev_queue *txq)
txq->xmit_lock_owner = smp_processor_id();
}

static inline int __netif_tx_trylock(struct netdev_queue *txq)
{
int ok = spin_trylock(&txq->_xmit_lock);
if (likely(ok))
txq->xmit_lock_owner = smp_processor_id();
return ok;
}

static inline void __netif_tx_unlock(struct netdev_queue *txq)
{
txq->xmit_lock_owner = -1;
spin_unlock(&txq->_xmit_lock);
}

static inline void __netif_tx_unlock_bh(struct netdev_queue *txq)
{
txq->xmit_lock_owner = -1;
spin_unlock_bh(&txq->_xmit_lock);
}

/**
* netif_tx_lock - grab network device transmit lock
* @dev: network device
Expand All @@ -1484,12 +1510,23 @@ static inline void __netif_tx_lock_bh(struct netdev_queue *txq)
*/
static inline void netif_tx_lock(struct net_device *dev)
{
int cpu = smp_processor_id();
unsigned int i;
int cpu;

spin_lock(&dev->tx_global_lock);
cpu = smp_processor_id();
for (i = 0; i < dev->num_tx_queues; i++) {
struct netdev_queue *txq = netdev_get_tx_queue(dev, i);

/* We are the only thread of execution doing a
* freeze, but we have to grab the _xmit_lock in
* order to synchronize with threads which are in
* the ->hard_start_xmit() handler and already
* checked the frozen bit.
*/
__netif_tx_lock(txq, cpu);
set_bit(__QUEUE_STATE_FROZEN, &txq->state);
__netif_tx_unlock(txq);
}
}

Expand All @@ -1499,40 +1536,22 @@ static inline void netif_tx_lock_bh(struct net_device *dev)
netif_tx_lock(dev);
}

static inline int __netif_tx_trylock(struct netdev_queue *txq)
{
int ok = spin_trylock(&txq->_xmit_lock);
if (likely(ok))
txq->xmit_lock_owner = smp_processor_id();
return ok;
}

static inline int netif_tx_trylock(struct net_device *dev)
{
return __netif_tx_trylock(netdev_get_tx_queue(dev, 0));
}

static inline void __netif_tx_unlock(struct netdev_queue *txq)
{
txq->xmit_lock_owner = -1;
spin_unlock(&txq->_xmit_lock);
}

static inline void __netif_tx_unlock_bh(struct netdev_queue *txq)
{
txq->xmit_lock_owner = -1;
spin_unlock_bh(&txq->_xmit_lock);
}

static inline void netif_tx_unlock(struct net_device *dev)
{
unsigned int i;

for (i = 0; i < dev->num_tx_queues; i++) {
struct netdev_queue *txq = netdev_get_tx_queue(dev, i);
__netif_tx_unlock(txq);
}

/* No need to grab the _xmit_lock here. If the
* queue is not stopped for another reason, we
* force a schedule.
*/
clear_bit(__QUEUE_STATE_FROZEN, &txq->state);
if (!test_bit(__QUEUE_STATE_XOFF, &txq->state))
__netif_schedule(txq->qdisc);
}
spin_unlock(&dev->tx_global_lock);
}

static inline void netif_tx_unlock_bh(struct net_device *dev)
Expand All @@ -1556,13 +1575,18 @@ static inline void netif_tx_unlock_bh(struct net_device *dev)
static inline void netif_tx_disable(struct net_device *dev)
{
unsigned int i;
int cpu;

netif_tx_lock_bh(dev);
local_bh_disable();
cpu = smp_processor_id();
for (i = 0; i < dev->num_tx_queues; i++) {
struct netdev_queue *txq = netdev_get_tx_queue(dev, i);

__netif_tx_lock(txq, cpu);
netif_tx_stop_queue(txq);
__netif_tx_unlock(txq);
}
netif_tx_unlock_bh(dev);
local_bh_enable();
}

static inline void netif_addr_lock(struct net_device *dev)
Expand Down
1 change: 1 addition & 0 deletions net/core/dev.c
View file
Expand Up @@ -4200,6 +4200,7 @@ static void netdev_init_queues(struct net_device *dev)
{
netdev_init_one_queue(dev, &dev->rx_queue, NULL);
netdev_for_each_tx_queue(dev, netdev_init_one_queue, NULL);
spin_lock_init(&dev->tx_global_lock);
}

/**
Expand Down
1 change: 1 addition & 0 deletions net/core/netpoll.c
View file
Expand Up @@ -70,6 +70,7 @@ static void queue_process(struct work_struct *work)
local_irq_save(flags);
__netif_tx_lock(txq, smp_processor_id());
if (netif_tx_queue_stopped(txq) ||
netif_tx_queue_frozen(txq) ||
dev->hard_start_xmit(skb, dev) != NETDEV_TX_OK) {
skb_queue_head(&npinfo->txq, skb);
__netif_tx_unlock(txq);
Expand Down
7 changes: 5 additions & 2 deletions net/core/pktgen.c
View file
Expand Up @@ -3305,6 +3305,7 @@ static __inline__ void pktgen_xmit(struct pktgen_dev *pkt_dev)

txq = netdev_get_tx_queue(odev, queue_map);
if (netif_tx_queue_stopped(txq) ||
netif_tx_queue_frozen(txq) ||
need_resched()) {
idle_start = getCurUs();

Expand All @@ -3320,7 +3321,8 @@ static __inline__ void pktgen_xmit(struct pktgen_dev *pkt_dev)

pkt_dev->idle_acc += getCurUs() - idle_start;

if (netif_tx_queue_stopped(txq)) {
if (netif_tx_queue_stopped(txq) ||
netif_tx_queue_frozen(txq)) {
pkt_dev->next_tx_us = getCurUs(); /* TODO */
pkt_dev->next_tx_ns = 0;
goto out; /* Try the next interface */
Expand Down Expand Up @@ -3352,7 +3354,8 @@ static __inline__ void pktgen_xmit(struct pktgen_dev *pkt_dev)
txq = netdev_get_tx_queue(odev, queue_map);

__netif_tx_lock_bh(txq);
if (!netif_tx_queue_stopped(txq)) {
if (!netif_tx_queue_stopped(txq) &&
!netif_tx_queue_frozen(txq)) {

atomic_inc(&(pkt_dev->skb->users));
retry_now:
Expand Down
6 changes: 4 additions & 2 deletions net/sched/sch_generic.c
View file
Expand Up @@ -135,7 +135,8 @@ static inline int qdisc_restart(struct Qdisc *q)
txq = netdev_get_tx_queue(dev, skb_get_queue_mapping(skb));

HARD_TX_LOCK(dev, txq, smp_processor_id());
if (!netif_subqueue_stopped(dev, skb))
if (!netif_tx_queue_stopped(txq) &&
!netif_tx_queue_frozen(txq))
ret = dev_hard_start_xmit(skb, dev, txq);
HARD_TX_UNLOCK(dev, txq);

Expand All @@ -162,7 +163,8 @@ static inline int qdisc_restart(struct Qdisc *q)
break;
}

if (ret && netif_tx_queue_stopped(txq))
if (ret && (netif_tx_queue_stopped(txq) ||
netif_tx_queue_frozen(txq)))
ret = 0;

return ret;
Expand Down
9 changes: 5 additions & 4 deletions net/sched/sch_teql.c
View file
Expand Up @@ -305,18 +305,19 @@ static int teql_master_xmit(struct sk_buff *skb, struct net_device *dev)

switch (teql_resolve(skb, skb_res, slave)) {
case 0:
if (netif_tx_trylock(slave)) {
if (!__netif_subqueue_stopped(slave, subq) &&
if (__netif_tx_trylock(slave_txq)) {
if (!netif_tx_queue_stopped(slave_txq) &&
!netif_tx_queue_frozen(slave_txq) &&
slave->hard_start_xmit(skb, slave) == 0) {
netif_tx_unlock(slave);
__netif_tx_unlock(slave_txq);
master->slaves = NEXT_SLAVE(q);
netif_wake_queue(dev);
master->stats.tx_packets++;
master->stats.tx_bytes +=
qdisc_pkt_len(skb);
return 0;
}
netif_tx_unlock(slave);
__netif_tx_unlock(slave_txq);
}
if (netif_queue_stopped(dev))
busy = 1;
Expand Down

0 comments on commit c3f26a2

Please sign in to comment.