diff options
author | Andrew Gallatin <gallatin@myri.com> | 2007-10-31 17:40:06 -0400 |
---|---|---|
committer | Jeff Garzik <jeff@garzik.org> | 2007-11-01 16:04:45 -0400 |
commit | c956a24018819bd903fad0cd275a63c089cdba53 (patch) | |
tree | 4a7e534f589e600c11384ee3892c98f8af3d0b58 | |
parent | 48d58459fe991e48bf7e6638a0ded0f8cbd2fa3b (diff) |
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: Jeff Garzik <jeff@garzik.org>
-rw-r--r-- | drivers/net/myri10ge/myri10ge.c | 6 |
1 files changed, 3 insertions, 3 deletions
diff --git a/drivers/net/myri10ge/myri10ge.c b/drivers/net/myri10ge/myri10ge.c index 366e62a2b1e5..0f306ddb5630 100644 --- a/drivers/net/myri10ge/myri10ge.c +++ b/drivers/net/myri10ge/myri10ge.c | |||
@@ -1151,7 +1151,7 @@ static inline int myri10ge_clean_rx_done(struct myri10ge_priv *mgp, int budget) | |||
1151 | u16 length; | 1151 | u16 length; |
1152 | __wsum checksum; | 1152 | __wsum checksum; |
1153 | 1153 | ||
1154 | while (rx_done->entry[idx].length != 0 && work_done++ < budget) { | 1154 | while (rx_done->entry[idx].length != 0 && work_done < budget) { |
1155 | length = ntohs(rx_done->entry[idx].length); | 1155 | length = ntohs(rx_done->entry[idx].length); |
1156 | rx_done->entry[idx].length = 0; | 1156 | rx_done->entry[idx].length = 0; |
1157 | checksum = csum_unfold(rx_done->entry[idx].checksum); | 1157 | 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 | rx_bytes += rx_ok * (unsigned long)length; | 1167 | rx_bytes += rx_ok * (unsigned long)length; |
1168 | cnt++; | 1168 | cnt++; |
1169 | idx = cnt & (myri10ge_max_intr_slots - 1); | 1169 | idx = cnt & (myri10ge_max_intr_slots - 1); |
1170 | work_done++; | ||
1170 | } | 1171 | } |
1171 | rx_done->idx = idx; | 1172 | rx_done->idx = idx; |
1172 | rx_done->cnt = cnt; | 1173 | rx_done->cnt = cnt; |
@@ -1233,13 +1234,12 @@ static int myri10ge_poll(struct napi_struct *napi, int budget) | |||
1233 | struct myri10ge_priv *mgp = | 1234 | struct myri10ge_priv *mgp = |
1234 | container_of(napi, struct myri10ge_priv, napi); | 1235 | container_of(napi, struct myri10ge_priv, napi); |
1235 | struct net_device *netdev = mgp->dev; | 1236 | struct net_device *netdev = mgp->dev; |
1236 | struct myri10ge_rx_done *rx_done = &mgp->rx_done; | ||
1237 | int work_done; | 1237 | int work_done; |
1238 | 1238 | ||
1239 | /* process as many rx events as NAPI will allow */ | 1239 | /* process as many rx events as NAPI will allow */ |
1240 | work_done = myri10ge_clean_rx_done(mgp, budget); | 1240 | work_done = myri10ge_clean_rx_done(mgp, budget); |
1241 | 1241 | ||
1242 | if (rx_done->entry[rx_done->idx].length == 0 || !netif_running(netdev)) { | 1242 | if (work_done < budget || !netif_running(netdev)) { |
1243 | netif_rx_complete(netdev, napi); | 1243 | netif_rx_complete(netdev, napi); |
1244 | put_be32(htonl(3), mgp->irq_claim); | 1244 | put_be32(htonl(3), mgp->irq_claim); |
1245 | } | 1245 | } |