[PATCH] net: fix race in process_backlog

2007-10-03 Thread Peter Zijlstra 
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 [EMAIL PROTECTED]
---
 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 unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] net: fix race in process_backlog

2007-10-03 Thread Stephen Hemminger 
On Wed, 03 Oct 2007 17:44:53 +0200
Peter Zijlstra [EMAIL PROTECTED] wrote:

 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 [EMAIL PROTECTED]

Acked-by: Stephen Hemminger [EMAIL PROTECTED]


-- 
Stephen Hemminger [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] net: fix race in process_backlog

2007-10-03 Thread Stephen Hemminger 
On Wed, 03 Oct 2007 14:58:07 -0700 (PDT)
David Miller [EMAIL PROTECTED] wrote:

 From: Peter Zijlstra [EMAIL PROTECTED]
 Date: Wed, 03 Oct 2007 17:44:53 +0200
 
  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();
 
 What re-enables interrupts in the !skb path?

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

--- a/net/core/dev.c2007-09-27 07:19:10.0 -0700
+++ b/net/core/dev.c2007-10-03 15:03:54.0 -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 [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] net: fix race in process_backlog

2007-10-03 Thread David Miller 
From: Peter Zijlstra [EMAIL PROTECTED]
Date: Wed, 03 Oct 2007 17:44:53 +0200

 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();

What re-enables interrupts in the !skb path?
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] net: fix race in process_backlog

2007-10-03 Thread David Miller 
From: Stephen Hemminger [EMAIL PROTECTED]
Date: Wed, 3 Oct 2007 15:05:19 -0700

 On Wed, 03 Oct 2007 14:58:07 -0700 (PDT)
 David Miller [EMAIL PROTECTED] wrote:
 
  From: Peter Zijlstra [EMAIL PROTECTED]
  Date: Wed, 03 Oct 2007 17:44:53 +0200
  
   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();
  
  What re-enables interrupts in the !skb path?
 
 This looks like a better fix. the irq_enable is needed in both cases.

Yep, applied, thanks Peter and Stephen.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html