Re: [PATCH] b44: netpoll locking fix
From: Stephen Hemminger <hidden>
Date: 2007-05-29 22:40:31
On Wed, 30 May 2007 00:20:41 +0200 Francois Romieu [off-list ref] wrote:
Stephen Hemminger [off-list ref] : ⅜...]quoted
Better to just get rid of using the lock as a transmit lock and use netif_tx_lock instead.--- a/drivers/net/b44.c 2007-05-29 09:51:43.000000000 -0700 +++ b/drivers/net/b44.c 2007-05-29 14:26:03.000000000 -0700@@ -607,6 +607,7 @@ static void b44_tx(struct b44 *bp) { u32 cur, cons; + netif_tx_lock(bp->dev); cur = br32(bp, B44_DMATX_STAT) & DMATX_STAT_CDMASK; cur /= sizeof(struct dma_desc);(damn, you are quick) I am not completely convinced. 1. netpoll_send_skb (calls netif_tx_trylock(dev)) -> netpoll_poll(np) -> poll_napi(np) -> np->dev->poll(np->dev, &budget) ( == b44_poll) -> b44_tx -> netif_tx_lock(bp->dev) *deadlock*
Netpoll needs to be fixed. (or scrapped), as is it will break drivers trying to use tx_lock in poll routine. I know sky2 would get borked. Something like this:
--- a/net/core/netpoll.c 2007-05-08 14:19:32.000000000 -0700
+++ b/net/core/netpoll.c 2007-05-29 15:28:22.000000000 -0700@@ -250,22 +250,23 @@ static void netpoll_send_skb(struct netp unsigned long flags; local_irq_save(flags); - if (netif_tx_trylock(dev)) { - /* try until next clock tick */ - for (tries = jiffies_to_usecs(1)/USEC_PER_POLL; - tries > 0; --tries) { + /* try until next clock tick */ + for (tries = jiffies_to_usecs(1)/USEC_PER_POLL; + tries > 0; --tries) { + if (netif_tx_trylock(dev)) { if (!netif_queue_stopped(dev)) status = dev->hard_start_xmit(skb, dev); + netif_tx_unlock(dev); if (status == NETDEV_TX_OK) break; - /* tickle device maybe there is some cleanup */ - netpoll_poll(np); - - udelay(USEC_PER_POLL); } - netif_tx_unlock(dev); + + /* tickle device maybe there is some cleanup */ + netpoll_poll(np); + + udelay(USEC_PER_POLL); } local_irq_restore(flags); }