diff options
author | David S. Miller <davem@sunset.davemloft.net> | 2007-10-29 21:28:47 -0700 |
---|---|---|
committer | David S. Miller <davem@sunset.davemloft.net> | 2007-10-29 22:37:28 -0700 |
commit | 0a7606c121d58c1831805262c5b764e181429e7d (patch) | |
tree | 4ba68e147c569c83dfedc3b45edf88ce21cde001 | |
parent | b0a713e9e6091b30d0e615d2be88017a57f37c76 (diff) | |
download | lwn-0a7606c121d58c1831805262c5b764e181429e7d.tar.gz lwn-0a7606c121d58c1831805262c5b764e181429e7d.zip |
[NET]: Fix race between poll_napi() and net_rx_action()
netpoll_poll_lock() synchronizes the ->poll() invocation
code paths, but once we have the lock we have to make
sure that NAPI_STATE_SCHED is still set. Otherwise we
get:
cpu 0 cpu 1
net_rx_action() poll_napi()
netpoll_poll_lock() ... spin on ->poll_lock
->poll()
netif_rx_complete
netpoll_poll_unlock() acquire ->poll_lock()
->poll()
netif_rx_complete()
CRASH
Based upon a bug report from Tina Yang.
Signed-off-by: David S. Miller <davem@davemloft.net>
-rw-r--r-- | net/core/dev.c | 10 | ||||
-rw-r--r-- | net/core/netpoll.c | 37 |
2 files changed, 37 insertions, 10 deletions
diff --git a/net/core/dev.c b/net/core/dev.c index 853c8b575f1d..02e7d8377c4a 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -2172,7 +2172,15 @@ static void net_rx_action(struct softirq_action *h) weight = n->weight; - work = n->poll(n, weight); + /* This NAPI_STATE_SCHED test is for avoiding a race + * with netpoll's poll_napi(). Only the entity which + * obtains the lock and sees NAPI_STATE_SCHED set will + * actually make the ->poll() call. Therefore we avoid + * accidently calling ->poll() when NAPI is not scheduled. + */ + work = 0; + if (test_bit(NAPI_STATE_SCHED, &n->state)) + work = n->poll(n, weight); WARN_ON_ONCE(work > weight); diff --git a/net/core/netpoll.c b/net/core/netpoll.c index bf8d18f1b013..c499b5c69bed 100644 --- a/net/core/netpoll.c +++ b/net/core/netpoll.c @@ -116,6 +116,29 @@ static __sum16 checksum_udp(struct sk_buff *skb, struct udphdr *uh, * network adapter, forcing superfluous retries and possibly timeouts. * Thus, we set our budget to greater than 1. */ +static int poll_one_napi(struct netpoll_info *npinfo, + struct napi_struct *napi, int budget) +{ + int work; + + /* net_rx_action's ->poll() invocations and our's are + * synchronized by this test which is only made while + * holding the napi->poll_lock. + */ + if (!test_bit(NAPI_STATE_SCHED, &napi->state)) + return budget; + + npinfo->rx_flags |= NETPOLL_RX_DROP; + atomic_inc(&trapped); + + work = napi->poll(napi, budget); + + atomic_dec(&trapped); + npinfo->rx_flags &= ~NETPOLL_RX_DROP; + + return budget - work; +} + static void poll_napi(struct netpoll *np) { struct netpoll_info *npinfo = np->dev->npinfo; @@ -123,17 +146,13 @@ static void poll_napi(struct netpoll *np) int budget = 16; list_for_each_entry(napi, &np->dev->napi_list, dev_list) { - if (test_bit(NAPI_STATE_SCHED, &napi->state) && - napi->poll_owner != smp_processor_id() && + if (napi->poll_owner != smp_processor_id() && spin_trylock(&napi->poll_lock)) { - npinfo->rx_flags |= NETPOLL_RX_DROP; - atomic_inc(&trapped); - - napi->poll(napi, budget); - - atomic_dec(&trapped); - npinfo->rx_flags &= ~NETPOLL_RX_DROP; + budget = poll_one_napi(npinfo, napi, budget); spin_unlock(&napi->poll_lock); + + if (!budget) + break; } } } |