Commit 932ff279 authored by Herbert Xu's avatar Herbert Xu Committed by David S. Miller
Browse files

[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: default avatarHerbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent bf0857ea

Documentation/networking/netdevices.txt

+4 −4
Original line number Diff line number Diff line
@@ -42,9 +42,9 @@ dev->get_stats: 
	Context: nominally process, but don't sleep inside an rwlock



dev->hard_start_xmit:

	Synchronization: dev->xmit_lock spinlock.

	Synchronization: netif_tx_lock spinlock.

	When the driver sets NETIF_F_LLTX in dev->features this will be

	called without holding xmit_lock. In this case the driver 

	called without holding netif_tx_lock. In this case the driver

	has to lock by itself when needed. It is recommended to use a try lock

	for this and return -1 when the spin lock fails. 

	The locking there should also properly protect against
@@ -62,12 +62,12 @@ dev->hard_start_xmit: 
	  Only valid when NETIF_F_LLTX is set.



dev->tx_timeout:

	Synchronization: dev->xmit_lock spinlock.

	Synchronization: netif_tx_lock spinlock.

	Context: BHs disabled

	Notes: netif_queue_stopped() is guaranteed true



dev->set_multicast_list:

	Synchronization: dev->xmit_lock spinlock.

	Synchronization: netif_tx_lock spinlock.

	Context: BHs disabled



dev->poll:

drivers/infiniband/ulp/ipoib/ipoib_multicast.c

+4 −2
Original line number Diff line number Diff line
@@ -821,7 +821,8 @@ void ipoib_mcast_restart_task(void *dev_ptr) 


	ipoib_mcast_stop_thread(dev, 0);



	spin_lock_irqsave(&dev->xmit_lock, flags);

	local_irq_save(flags);

	netif_tx_lock(dev);

	spin_lock(&priv->lock);



	/*
@@ -896,7 +897,8 @@ void ipoib_mcast_restart_task(void *dev_ptr) 
	}



	spin_unlock(&priv->lock);

	spin_unlock_irqrestore(&dev->xmit_lock, flags);

	netif_tx_unlock(dev);

	local_irq_restore(flags);



	/* We have to cancel outside of the spinlock */

	list_for_each_entry_safe(mcast, tmcast, &remove_list, list) {

drivers/media/dvb/dvb-core/dvb_net.c

+2 −2
Original line number Diff line number Diff line
@@ -1052,7 +1052,7 @@ static void wq_set_multicast_list (void *data) 


	dvb_net_feed_stop(dev);

	priv->rx_mode = RX_MODE_UNI;

	spin_lock_bh(&dev->xmit_lock);

	netif_tx_lock_bh(dev);



	if (dev->flags & IFF_PROMISC) {

		dprintk("%s: promiscuous mode\n", dev->name);
@@ -1077,7 +1077,7 @@ static void wq_set_multicast_list (void *data) 
		}

	}



	spin_unlock_bh(&dev->xmit_lock);

	netif_tx_unlock_bh(dev);

	dvb_net_feed_start(dev);

}

drivers/net/bnx2.c

+2 −2
Original line number Diff line number Diff line
@@ -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

drivers/net/bonding/bond_main.c

+1 −1
Original line number Diff line number Diff line
@@ -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;

CRA Git | Maintained and supported by SUSTech CRA and CCSE