Re: [PATCH] net: fix race in process_backlog

Previous thread: [PATCH] hugetlb: Fix pool resizing corner case by Adam Litke on Wednesday, October 3, 2007 - 11:47 am. (6 messages)

Next thread: [GIT PULL] Blackfin fixing for 2.6.23-rc9 by Bryan Wu on Wednesday, October 3, 2007 - 12:50 pm. (1 message)
To: linux-kernel <linux-kernel@...>, netdev <netdev@...>
Cc: Stephen Hemminger <shemminger@...>, Jeff Dike <jdike@...>, David Miller <davem@...>
Date: Wednesday, October 3, 2007 - 11:44 am

Subject: net: fix race in process_backlog

The recent NAPI rework (4fa57c9ea9f36f9ca852f3a88ca5d2f1aebbc960)
introduced a race between netif_rx() and process_backlog() which
resulted in softirq processing to drop dead.

netif_rx() process_backlog()

irq_disable();
skb = __skb_dequeue();
irq_enable();

irq_disable();
__skb_queue_tail();
napi_schedule();
irq_enable();

if (!skb)
napi_complete(); <-- oops!

we cleared the napi bit, even though there is data to process.

Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
net/core/dev.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

Index: linux-2.6/net/core/dev.c
===================================================================
--- linux-2.6.orig/net/core/dev.c
+++ linux-2.6/net/core/dev.c
@@ -2095,11 +2095,11 @@ static int process_backlog(struct napi_s

local_irq_disable();
skb = __skb_dequeue(&queue->input_pkt_queue);
- local_irq_enable();
if (!skb) {
- napi_complete(napi);
+ __napi_complete(napi);
break;
}
+ local_irq_enable();

dev = skb->dev;

-

To: <a.p.zijlstra@...>
Cc: <linux-kernel@...>, <netdev@...>, <shemminger@...>, <jdike@...>
Date: Wednesday, October 3, 2007 - 5:58 pm

From: Peter Zijlstra <a.p.zijlstra@chello.nl>

What re-enables interrupts in the !skb path?
-

To: David Miller <davem@...>
Cc: <a.p.zijlstra@...>, <linux-kernel@...>, <netdev@...>, <jdike@...>
Date: Wednesday, October 3, 2007 - 6:05 pm

On Wed, 03 Oct 2007 14:58:07 -0700 (PDT)

This looks like a better fix. the irq_enable is needed in both cases.

--- a/net/core/dev.c 2007-09-27 07:19:10.000000000 -0700
+++ b/net/core/dev.c 2007-10-03 15:03:54.000000000 -0700
@@ -2077,12 +2077,14 @@ static int process_backlog(struct napi_s

local_irq_disable();
skb = __skb_dequeue(&queue->input_pkt_queue);
- local_irq_enable();
if (!skb) {
- napi_complete(napi);
+ __napi_complete(napi);
+ local_irq_enable();
break;
}

+ local_irq_enable();
+
dev = skb->dev;

netif_receive_skb(skb);

--
Stephen Hemminger <shemminger@linux-foundation.org>
-

To: <shemminger@...>
Cc: <a.p.zijlstra@...>, <linux-kernel@...>, <netdev@...>, <jdike@...>
Date: Wednesday, October 3, 2007 - 7:39 pm

From: Stephen Hemminger <shemminger@linux-foundation.org>

Yep, applied, thanks Peter and Stephen.
-

To: Peter Zijlstra <a.p.zijlstra@...>
Cc: linux-kernel <linux-kernel@...>, netdev <netdev@...>, Jeff Dike <jdike@...>, David Miller <davem@...>
Date: Wednesday, October 3, 2007 - 12:15 pm

On Wed, 03 Oct 2007 17:44:53 +0200

Acked-by: Stephen Hemminger <shemminger@linux-foundation.org>

--
Stephen Hemminger <shemminger@linux-foundation.org>
-

Previous thread: [PATCH] hugetlb: Fix pool resizing corner case by Adam Litke on Wednesday, October 3, 2007 - 11:47 am. (6 messages)

Next thread: [GIT PULL] Blackfin fixing for 2.6.23-rc9 by Bryan Wu on Wednesday, October 3, 2007 - 12:50 pm. (1 message)