Messages in this thread | | | Date | Tue, 11 Dec 2007 02:32:18 -0800 (PST) | Subject | Re: [PATCH] [NET]: Fix Ooops of napi net_rx_action. | From | David Miller <> |
| |
From: "Joonwoo Park" <joonwpark81@gmail.com> Date: Tue, 11 Dec 2007 18:13:34 +0900
Joonwoo-ssi annyoung haseyo,
> [NET]: Fix Ooops of napi net_rx_action. > Before doing list_move_tail napi poll_list, it should be ensured > > Signed-off-by: Joonwoo Park <joonwpark81@gmail.com> > --- > diff --git a/net/core/dev.c b/net/core/dev.c > index 86d6261..74bd5ab 100644 > --- a/net/core/dev.c > +++ b/net/core/dev.c > @@ -2207,7 +2207,8 @@ static void net_rx_action(struct softirq_action *h) > * still "owns" the NAPI instance and therefore can > * move the instance around on the list at-will. > */ > - if (unlikely(work == weight)) > + if (unlikely((test_bit(NAPI_STATE_SCHED, &n->state)) > + && (work == weight))) > list_move_tail(&n->poll_list, list); > > netpoll_poll_unlock(have);
How can the NAPI_STATE_SCHED bit be cleared externally yet we take this list_move_tail() code path?
If NAPI_STATE_SCHED is cleared, work will be zero which will never be equal to 'weight', and this we'll never attempt the list_move_tail().
If something clears NAPI_STATE_SCHED meanwhile, we have a serious race and your patch is an incomplete bandaid. For example, if it can happen, then a case like:
if (test_bit(NAPI_STATE_SCHED, &n->state)) ... something clears NAPI_STATE_SCHED right now ... work = n->poll(n, weight);
can crash too.
| |