diff options
author | Herbert Xu <herbert@gondor.apana.org.au> | 2006-06-09 12:20:56 -0700 |
---|---|---|
committer | David S. Miller <davem@sunset.davemloft.net> | 2006-06-17 21:30:14 -0700 |
commit | 932ff279a43ab7257942cddff2595acd541cc49b (patch) | |
tree | e60130673a20d71becdac858c2589d8dfbf3ae1f /drivers/net | |
parent | bf0857ea32addb6bc8b46383604b218b8ec09f19 (diff) | |
download | op-kernel-dev-932ff279a43ab7257942cddff2595acd541cc49b.zip op-kernel-dev-932ff279a43ab7257942cddff2595acd541cc49b.tar.gz |
[NET]: Add netif_tx_lock
Various drivers use xmit_lock internally to synchronise with their
transmission routines. They do so without setting xmit_lock_owner.
This is fine as long as netpoll is not in use.
With netpoll it is possible for deadlocks to occur if xmit_lock_owner
isn't set. This is because if a printk occurs while xmit_lock is held
and xmit_lock_owner is not set can cause netpoll to attempt to take
xmit_lock recursively.
While it is possible to resolve this by getting netpoll to use
trylock, it is suboptimal because netpoll's sole objective is to
maximise the chance of getting the printk out on the wire. So
delaying or dropping the message is to be avoided as much as possible.
So the only alternative is to always set xmit_lock_owner. The
following patch does this by introducing the netif_tx_lock family of
functions that take care of setting/unsetting xmit_lock_owner.
I renamed xmit_lock to _xmit_lock to indicate that it should not be
used directly. I didn't provide irq versions of the netif_tx_lock
functions since xmit_lock is meant to be a BH-disabling lock.
This is pretty much a straight text substitution except for a small
bug fix in winbond. It currently uses
netif_stop_queue/spin_unlock_wait to stop transmission. This is
unsafe as an IRQ can potentially wake up the queue. So it is safer to
use netif_tx_disable.
The hamradio bits used spin_lock_irq but it is unnecessary as
xmit_lock must never be taken in an IRQ handler.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'drivers/net')
-rw-r--r-- | drivers/net/bnx2.c | 4 | ||||
-rw-r--r-- | drivers/net/bonding/bond_main.c | 2 | ||||
-rw-r--r-- | drivers/net/forcedeth.c | 18 | ||||
-rw-r--r-- | drivers/net/hamradio/6pack.c | 8 | ||||
-rw-r--r-- | drivers/net/hamradio/mkiss.c | 8 | ||||
-rw-r--r-- | drivers/net/ifb.c | 10 | ||||
-rw-r--r-- | drivers/net/irda/vlsi_ir.c | 2 | ||||
-rw-r--r-- | drivers/net/natsemi.c | 4 | ||||
-rw-r--r-- | drivers/net/tulip/winbond-840.c | 9 | ||||
-rw-r--r-- | drivers/net/wireless/orinoco.c | 4 |
10 files changed, 35 insertions, 34 deletions
diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c index 54161ae..9c5a884 100644 --- a/drivers/net/bnx2.c +++ b/drivers/net/bnx2.c @@ -2009,7 +2009,7 @@ bnx2_poll(struct net_device *dev, int *budget) return 1; } -/* Called with rtnl_lock from vlan functions and also dev->xmit_lock +/* Called with rtnl_lock from vlan functions and also netif_tx_lock * from set_multicast. */ static void @@ -4252,7 +4252,7 @@ bnx2_vlan_rx_kill_vid(struct net_device *dev, uint16_t vid) } #endif -/* Called with dev->xmit_lock. +/* Called with netif_tx_lock. * hard_start_xmit is pseudo-lockless - a lock is only required when * the tx queue is full. This way, we get the benefit of lockless * operations most of the time without the complexities to handle diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 55d2367..46326cd 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -4191,7 +4191,7 @@ static int bond_init(struct net_device *bond_dev, struct bond_params *params) */ bond_dev->features |= NETIF_F_VLAN_CHALLENGED; - /* don't acquire bond device's xmit_lock when + /* don't acquire bond device's netif_tx_lock when * transmitting */ bond_dev->features |= NETIF_F_LLTX; diff --git a/drivers/net/forcedeth.c b/drivers/net/forcedeth.c index feb5b22..5a8651b 100644 --- a/drivers/net/forcedeth.c +++ b/drivers/net/forcedeth.c @@ -533,9 +533,9 @@ typedef union _ring_type { * critical parts: * - rx is (pseudo-) lockless: it relies on the single-threading provided * by the arch code for interrupts. - * - tx setup is lockless: it relies on dev->xmit_lock. Actual submission + * - tx setup is lockless: it relies on netif_tx_lock. Actual submission * needs dev->priv->lock :-( - * - set_multicast_list: preparation lockless, relies on dev->xmit_lock. + * - set_multicast_list: preparation lockless, relies on netif_tx_lock. */ /* in dev: base, irq */ @@ -1213,7 +1213,7 @@ static void drain_ring(struct net_device *dev) /* * nv_start_xmit: dev->hard_start_xmit function - * Called with dev->xmit_lock held. + * Called with netif_tx_lock held. */ static int nv_start_xmit(struct sk_buff *skb, struct net_device *dev) { @@ -1407,7 +1407,7 @@ static void nv_tx_done(struct net_device *dev) /* * nv_tx_timeout: dev->tx_timeout function - * Called with dev->xmit_lock held. + * Called with netif_tx_lock held. */ static void nv_tx_timeout(struct net_device *dev) { @@ -1737,7 +1737,7 @@ static int nv_change_mtu(struct net_device *dev, int new_mtu) * Changing the MTU is a rare event, it shouldn't matter. */ nv_disable_irq(dev); - spin_lock_bh(&dev->xmit_lock); + netif_tx_lock_bh(dev); spin_lock(&np->lock); /* stop engines */ nv_stop_rx(dev); @@ -1768,7 +1768,7 @@ static int nv_change_mtu(struct net_device *dev, int new_mtu) nv_start_rx(dev); nv_start_tx(dev); spin_unlock(&np->lock); - spin_unlock_bh(&dev->xmit_lock); + netif_tx_unlock_bh(dev); nv_enable_irq(dev); } return 0; @@ -1803,7 +1803,7 @@ static int nv_set_mac_address(struct net_device *dev, void *addr) memcpy(dev->dev_addr, macaddr->sa_data, ETH_ALEN); if (netif_running(dev)) { - spin_lock_bh(&dev->xmit_lock); + netif_tx_lock_bh(dev); spin_lock_irq(&np->lock); /* stop rx engine */ @@ -1815,7 +1815,7 @@ static int nv_set_mac_address(struct net_device *dev, void *addr) /* restart rx engine */ nv_start_rx(dev); spin_unlock_irq(&np->lock); - spin_unlock_bh(&dev->xmit_lock); + netif_tx_unlock_bh(dev); } else { nv_copy_mac_to_hw(dev); } @@ -1824,7 +1824,7 @@ static int nv_set_mac_address(struct net_device *dev, void *addr) /* * nv_set_multicast: dev->set_multicast function - * Called with dev->xmit_lock held. + * Called with netif_tx_lock held. */ static void nv_set_multicast(struct net_device *dev) { diff --git a/drivers/net/hamradio/6pack.c b/drivers/net/hamradio/6pack.c index 102c1f0..d12605f 100644 --- a/drivers/net/hamradio/6pack.c +++ b/drivers/net/hamradio/6pack.c @@ -308,9 +308,9 @@ static int sp_set_mac_address(struct net_device *dev, void *addr) { struct sockaddr_ax25 *sa = addr; - spin_lock_irq(&dev->xmit_lock); + netif_tx_lock_bh(dev); memcpy(dev->dev_addr, &sa->sax25_call, AX25_ADDR_LEN); - spin_unlock_irq(&dev->xmit_lock); + netif_tx_unlock_bh(dev); return 0; } @@ -767,9 +767,9 @@ static int sixpack_ioctl(struct tty_struct *tty, struct file *file, break; } - spin_lock_irq(&dev->xmit_lock); + netif_tx_lock_bh(dev); memcpy(dev->dev_addr, &addr, AX25_ADDR_LEN); - spin_unlock_irq(&dev->xmit_lock); + netif_tx_unlock_bh(dev); err = 0; break; diff --git a/drivers/net/hamradio/mkiss.c b/drivers/net/hamradio/mkiss.c index d81a8e1..3ebbbe5 100644 --- a/drivers/net/hamradio/mkiss.c +++ b/drivers/net/hamradio/mkiss.c @@ -357,9 +357,9 @@ static int ax_set_mac_address(struct net_device *dev, void *addr) { struct sockaddr_ax25 *sa = addr; - spin_lock_irq(&dev->xmit_lock); + netif_tx_lock_bh(dev); memcpy(dev->dev_addr, &sa->sax25_call, AX25_ADDR_LEN); - spin_unlock_irq(&dev->xmit_lock); + netif_tx_unlock_bh(dev); return 0; } @@ -886,9 +886,9 @@ static int mkiss_ioctl(struct tty_struct *tty, struct file *file, break; } - spin_lock_irq(&dev->xmit_lock); + netif_tx_lock_bh(dev); memcpy(dev->dev_addr, addr, AX25_ADDR_LEN); - spin_unlock_irq(&dev->xmit_lock); + netif_tx_unlock_bh(dev); err = 0; break; diff --git a/drivers/net/ifb.c b/drivers/net/ifb.c index 31fb2d7..2e222ef 100644 --- a/drivers/net/ifb.c +++ b/drivers/net/ifb.c @@ -76,13 +76,13 @@ static void ri_tasklet(unsigned long dev) dp->st_task_enter++; if ((skb = skb_peek(&dp->tq)) == NULL) { dp->st_txq_refl_try++; - if (spin_trylock(&_dev->xmit_lock)) { + if (netif_tx_trylock(_dev)) { dp->st_rxq_enter++; while ((skb = skb_dequeue(&dp->rq)) != NULL) { skb_queue_tail(&dp->tq, skb); dp->st_rx2tx_tran++; } - spin_unlock(&_dev->xmit_lock); + netif_tx_unlock(_dev); } else { /* reschedule */ dp->st_rxq_notenter++; @@ -110,7 +110,7 @@ static void ri_tasklet(unsigned long dev) } } - if (spin_trylock(&_dev->xmit_lock)) { + if (netif_tx_trylock(_dev)) { dp->st_rxq_check++; if ((skb = skb_peek(&dp->rq)) == NULL) { dp->tasklet_pending = 0; @@ -118,10 +118,10 @@ static void ri_tasklet(unsigned long dev) netif_wake_queue(_dev); } else { dp->st_rxq_rsch++; - spin_unlock(&_dev->xmit_lock); + netif_tx_unlock(_dev); goto resched; } - spin_unlock(&_dev->xmit_lock); + netif_tx_unlock(_dev); } else { resched: dp->tasklet_pending = 1; diff --git a/drivers/net/irda/vlsi_ir.c b/drivers/net/irda/vlsi_ir.c index 97a49e0..d70b9e8 100644 --- a/drivers/net/irda/vlsi_ir.c +++ b/drivers/net/irda/vlsi_ir.c @@ -959,7 +959,7 @@ static int vlsi_hard_start_xmit(struct sk_buff *skb, struct net_device *ndev) || (now.tv_sec==ready.tv_sec && now.tv_usec>=ready.tv_usec)) break; udelay(100); - /* must not sleep here - we are called under xmit_lock! */ + /* must not sleep here - called under netif_tx_lock! */ } } diff --git a/drivers/net/natsemi.c b/drivers/net/natsemi.c index 9062775..2e4eced 100644 --- a/drivers/net/natsemi.c +++ b/drivers/net/natsemi.c @@ -318,12 +318,12 @@ performance critical codepaths: The rx process only runs in the interrupt handler. Access from outside the interrupt handler is only permitted after disable_irq(). -The rx process usually runs under the dev->xmit_lock. If np->intr_tx_reap +The rx process usually runs under the netif_tx_lock. If np->intr_tx_reap is set, then access is permitted under spin_lock_irq(&np->lock). Thus configuration functions that want to access everything must call disable_irq(dev->irq); - spin_lock_bh(dev->xmit_lock); + netif_tx_lock_bh(dev); spin_lock_irq(&np->lock); IV. Notes diff --git a/drivers/net/tulip/winbond-840.c b/drivers/net/tulip/winbond-840.c index 136a70c..56d86c7 100644 --- a/drivers/net/tulip/winbond-840.c +++ b/drivers/net/tulip/winbond-840.c @@ -1605,11 +1605,11 @@ static void __devexit w840_remove1 (struct pci_dev *pdev) * - get_stats: * spin_lock_irq(np->lock), doesn't touch hw if not present * - hard_start_xmit: - * netif_stop_queue + spin_unlock_wait(&dev->xmit_lock); + * synchronize_irq + netif_tx_disable; * - tx_timeout: - * netif_device_detach + spin_unlock_wait(&dev->xmit_lock); + * netif_device_detach + netif_tx_disable; * - set_multicast_list - * netif_device_detach + spin_unlock_wait(&dev->xmit_lock); + * netif_device_detach + netif_tx_disable; * - interrupt handler * doesn't touch hw if not present, synchronize_irq waits for * running instances of the interrupt handler. @@ -1635,11 +1635,10 @@ static int w840_suspend (struct pci_dev *pdev, pm_message_t state) netif_device_detach(dev); update_csr6(dev, 0); iowrite32(0, ioaddr + IntrEnable); - netif_stop_queue(dev); spin_unlock_irq(&np->lock); - spin_unlock_wait(&dev->xmit_lock); synchronize_irq(dev->irq); + netif_tx_disable(dev); np->stats.rx_missed_errors += ioread32(ioaddr + RxMissed) & 0xffff; diff --git a/drivers/net/wireless/orinoco.c b/drivers/net/wireless/orinoco.c index c2d0b09..a5fcfcd 100644 --- a/drivers/net/wireless/orinoco.c +++ b/drivers/net/wireless/orinoco.c @@ -1833,7 +1833,9 @@ static int __orinoco_program_rids(struct net_device *dev) /* Set promiscuity / multicast*/ priv->promiscuous = 0; priv->mc_count = 0; - __orinoco_set_multicast_list(dev); /* FIXME: what about the xmit_lock */ + + /* FIXME: what about netif_tx_lock */ + __orinoco_set_multicast_list(dev); return 0; } |