Messages in this thread Patch in this message | | | Date | Thu, 26 Mar 2009 09:18:08 +0000 | From | Jarek Poplawski <> | Subject | Re: Revert "gro: Fix legacy path napi_complete crash", |
| |
On Wed, Mar 25, 2009 at 08:20:50PM -0700, David Miller wrote: ... > Adam Richter has successfully tested Jarek's variant, and if Ingo's > tests show that it makes his problem go away too then I'm definitely > going with Jarek's patch.
I hope this patch version on top of Herbert's revert could be useful then.
Thanks, Jarek P. --------------------> GRO: Re-enable GRO on legacy netif_rx path.
Replacing __napi_complete() with napi_complete() in process_backlog() caused various breaks for non-napi NICs. It looks like the following scenario can happen:
process_backlog() netif_rx()
if (!skb) local_irq_enable() if (queue.qlen) //NO napi_schedule() //NOTHING __skb_queue_tail() //qlen > 0 napi_complete() ... ... Every next netif_rx() sees qlen > 0, so napi is never scheduled again.
This patch fixes it by open-coding napi_complete() with additional check for empty queue before __napi_complete().
With help of debugging by Ingo Molnar, Sascha Hauer and Herbert Xu.
Reported-by: Ingo Molnar <mingo@elte.hu> Tested-by: Adam Richter <adam_richter2004@yahoo.com> Signed-off-by: Jarek Poplawski <jarkao2@gmail.com> --- (vs. 2.6.29 with Herbert's "GRO: Disable GRO on legacy netif_rx path")
diff -Nurp a/net/core/dev.c b/net/core/dev.c --- a/net/core/dev.c 2009-03-26 08:32:51.000000000 +0000 +++ b/net/core/dev.c 2009-03-26 08:37:58.000000000 +0000 @@ -2588,15 +2588,22 @@ static int process_backlog(struct napi_s local_irq_disable(); skb = __skb_dequeue(&queue->input_pkt_queue); if (!skb) { - __napi_complete(napi); local_irq_enable(); - break; + napi_gro_flush(napi); + local_irq_disable(); + if (skb_queue_empty(&queue->input_pkt_queue)) + __napi_complete(napi); + local_irq_enable(); + goto out; } local_irq_enable(); - netif_receive_skb(skb); + napi_gro_receive(napi, skb); } while (++work < quota && jiffies == start_time); + napi_gro_flush(napi); + +out: return work; }
| |