From: "Chris Friesen" <cfriesen@nortel.com>
To: David Miller <davem@davemloft.net>
Cc: linuxppc-dev@ozlabs.org, romieu@fr.zoreil.com,
jesse.brandeburg@intel.com, netdev@vger.kernel.org
Subject: Re: [BUG] oops in net_rx_action on 64-bit powerpc
Date: Mon, 27 Oct 2008 18:13:54 -0600 [thread overview]
Message-ID: <49065942.4040300@nortel.com> (raw)
In-Reply-To: <20081025.001703.261277154.davem@davemloft.net>
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>
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:
return num_rx_pkt;
next prev parent reply other threads:[~2008-10-28 0:14 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-10-23 19:59 [BUG] oops in net_rx_action on 64-bit powerpc Chris Friesen
2008-10-23 21:50 ` Brandeburg, Jesse
2008-10-24 0:16 ` David Miller
2008-10-24 23:39 ` Chris Friesen
2008-10-24 23:41 ` David Miller
2008-10-25 5:42 ` Chris Friesen
2008-10-25 7:17 ` David Miller
2008-10-28 0:13 ` Chris Friesen [this message]
2008-10-28 22:51 ` David Miller
2008-10-28 22:59 ` Chris Friesen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=49065942.4040300@nortel.com \
--to=cfriesen@nortel.com \
--cc=davem@davemloft.net \
--cc=jesse.brandeburg@intel.com \
--cc=linuxppc-dev@ozlabs.org \
--cc=netdev@vger.kernel.org \
--cc=romieu@fr.zoreil.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).