Commit c956a240 authored by Andrew Gallatin's avatar Andrew Gallatin Committed by Jeff Garzik

Fix myri10ge NAPI oops & warnings

When testing the myri10ge driver with 2.6.24-rc1, I found
that the machine crashed under heavy load:

Unable to handle kernel paging request at 0000000000100108 RIP:
  [<ffffffff803cc8dd>] net_rx_action+0x11b/0x184

The address corresponds to the list_move_tail() in
netif_rx_complete():
                     if (unlikely(work == weight))
                             list_move_tail(&n->poll_list, list);

Eventually, I traced the crashes to calling netif_rx_complete() with
work_done == budget.  From looking at other drivers, it appears that
one should only call netif_rx_complete() when work_done < budget.

To fix it, I changed the test in myri10ge_poll() so that it refers
to to work_done rather than looking at the rx ring status.  If
work_done is < budget, then that implies we have no more packets to
process. Any races will be resolved by the NIC when the write to
irq_claim is made.

In myri10ge_clean_rx_done(), if we ever exceeded our budget, it would
report a work_done one larger than was acutally done.  This is because
the increment was done in the conditional, so work_done would be
incremented regardless of whether or not the test passed or failed.
This would lead to the WARN_ON_ONCE(work > weight); warning in
net_rx_action triggering.  I've moved the increment of work_done
inside the loop.  Note that this would only be a problem when we had
exceeded our budget.

Signed off by: Andrew Gallatin <gallatin@myri.com>

Andrew Gallatin Myricom Inc
Signed-off-by: default avatarJeff Garzik <jeff@garzik.org>
parent 48d58459
...@@ -1151,7 +1151,7 @@ static inline int myri10ge_clean_rx_done(struct myri10ge_priv *mgp, int budget) ...@@ -1151,7 +1151,7 @@ static inline int myri10ge_clean_rx_done(struct myri10ge_priv *mgp, int budget)
u16 length; u16 length;
__wsum checksum; __wsum checksum;
while (rx_done->entry[idx].length != 0 && work_done++ < budget) { while (rx_done->entry[idx].length != 0 && work_done < budget) {
length = ntohs(rx_done->entry[idx].length); length = ntohs(rx_done->entry[idx].length);
rx_done->entry[idx].length = 0; rx_done->entry[idx].length = 0;
checksum = csum_unfold(rx_done->entry[idx].checksum); checksum = csum_unfold(rx_done->entry[idx].checksum);
...@@ -1167,6 +1167,7 @@ static inline int myri10ge_clean_rx_done(struct myri10ge_priv *mgp, int budget) ...@@ -1167,6 +1167,7 @@ static inline int myri10ge_clean_rx_done(struct myri10ge_priv *mgp, int budget)
rx_bytes += rx_ok * (unsigned long)length; rx_bytes += rx_ok * (unsigned long)length;
cnt++; cnt++;
idx = cnt & (myri10ge_max_intr_slots - 1); idx = cnt & (myri10ge_max_intr_slots - 1);
work_done++;
} }
rx_done->idx = idx; rx_done->idx = idx;
rx_done->cnt = cnt; rx_done->cnt = cnt;
...@@ -1233,13 +1234,12 @@ static int myri10ge_poll(struct napi_struct *napi, int budget) ...@@ -1233,13 +1234,12 @@ static int myri10ge_poll(struct napi_struct *napi, int budget)
struct myri10ge_priv *mgp = struct myri10ge_priv *mgp =
container_of(napi, struct myri10ge_priv, napi); container_of(napi, struct myri10ge_priv, napi);
struct net_device *netdev = mgp->dev; struct net_device *netdev = mgp->dev;
struct myri10ge_rx_done *rx_done = &mgp->rx_done;
int work_done; int work_done;
/* process as many rx events as NAPI will allow */ /* process as many rx events as NAPI will allow */
work_done = myri10ge_clean_rx_done(mgp, budget); work_done = myri10ge_clean_rx_done(mgp, budget);
if (rx_done->entry[rx_done->idx].length == 0 || !netif_running(netdev)) { if (work_done < budget || !netif_running(netdev)) {
netif_rx_complete(netdev, napi); netif_rx_complete(netdev, napi);
put_be32(htonl(3), mgp->irq_claim); put_be32(htonl(3), mgp->irq_claim);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment