diff mbox

[BUG] oops in net_rx_action on 64-bit powerpc

Message ID 49065942.4040300@nortel.com
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Chris Friesen Oct. 28, 2008, 12:13 a.m. UTC
David Miller wrote:


 > Probably the simplest fix is to get rid of the rx_not_empty label and
 > protect the entire:
 >
 > 	/* Receive descriptor is empty now */
 > 	spin_lock_irqsave(&lp->lock, flags);
 > 	__netif_rx_complete(dev, napi);
 > 	writel(VAL0|RINTEN0, mmio + INTEN0);
 > 	writel(VAL2 | RDMD0, mmio + CMD0);
 > 	spin_unlock_irqrestore(&lp->lock, flags);
 >
 > code block with a test such as:
 >
 > 	if (rx_pkt_limit > 0)
 >
 > (yes, greater than zero, not >= 0)
 >
 > then replace the rx_not_empty goto with a simple break.


Are you sure about that?  Doing that, if we "--rx_pkt_limit < 0" we'll only 
break out of the inner while loop.  We then check then interrupt status 
register and potentially loop through the do/while loop again (maybe 
decrementing rx_pkt_limit again) even though we've used up our budget.

If I leave the label and jump and just add the "rx_pkt_limit > 0" test, it 
seems to work.

Chris


From: Chris Friesen    <cfriesen@nortel.com>
Subject: [PATCH] fix amd8111e rx return code

The amd8111e rx poll routine currently mishandles the case when we process
exactly the number of packets specified in the budget.

This patch is basically as suggested by David Miller.

Signed-off-by: Chris Friesen <cfriesen@nortel.com>

  	return num_rx_pkt;
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

David Miller Oct. 28, 2008, 10:51 p.m. UTC | #1
From: "Chris Friesen" <cfriesen@nortel.com>
Date: Mon, 27 Oct 2008 18:13:54 -0600

> [PATCH] fix amd8111e rx return code
> 
> The amd8111e rx poll routine currently mishandles the case when we process
> exactly the number of packets specified in the budget.
> 
> This patch is basically as suggested by David Miller.
> 
> Signed-off-by: Chris Friesen <cfriesen@nortel.com>

I had to apply this by hand because your email client heavily
corrupted the patch.

Thanks.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Chris Friesen Oct. 28, 2008, 10:59 p.m. UTC | #2
David Miller wrote:
> From: "Chris Friesen" <cfriesen@nortel.com>
> Date: Mon, 27 Oct 2008 18:13:54 -0600
> 
> 
>>[PATCH] fix amd8111e rx return code
>>
>>The amd8111e rx poll routine currently mishandles the case when we process
>>exactly the number of packets specified in the budget.
>>
>>This patch is basically as suggested by David Miller.
>>
>>Signed-off-by: Chris Friesen <cfriesen@nortel.com>
> 
> 
> I had to apply this by hand because your email client heavily
> corrupted the patch.

Crap.  Sorry about that, it looks like I forgot to set my line-wrap to 0 
before creating the email.  @#$#% Thunderbird.

Chris
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/amd8111e.c b/drivers/net/amd8111e.c
index c54967f..ba1be0b 100644
--- a/drivers/net/amd8111e.c
+++ b/drivers/net/amd8111e.c
@@ -833,12 +833,14 @@  static int amd8111e_rx_poll(struct napi_struct *napi, 
int budget)

  	} while(intr0 & RINT0);

-	/* Receive descriptor is empty now */
-	spin_lock_irqsave(&lp->lock, flags);
-	__netif_rx_complete(dev, napi);
-	writel(VAL0|RINTEN0, mmio + INTEN0);
-	writel(VAL2 | RDMD0, mmio + CMD0);
-	spin_unlock_irqrestore(&lp->lock, flags);
+	if (rx_pkt_limit > 0) {
+		/* Receive descriptor is empty now */
+		spin_lock_irqsave(&lp->lock, flags);
+		__netif_rx_complete(dev, napi);
+		writel(VAL0|RINTEN0, mmio + INTEN0);
+		writel(VAL2 | RDMD0, mmio + CMD0);
+		spin_unlock_irqrestore(&lp->lock, flags);
+	}

  rx_not_empty: