New: linux-2.6.36-rc5 crash with gianfar ethernet at full line rate traffic
Subsystems (2)
A message that is not in the archive (sent off-list or not yet received): [email protected]/
(switched to email. Please respond via emailed reply-to-all, not via the bugzilla web interface).
Quoted 116 lines · On Mon, 4 Oct 2010 06:25:14 GMT [email protected] wrote:
https://bugzilla.kernel.org/show_bug.cgi?id=19692
Summary: linux-2.6.36-rc5 crash with gianfar ethernet at full
line rate traffic
Product: Drivers
Version: 2.5
Kernel Version: 2.6.36-rc5
Platform: All
OS/Version: Linux
Tree: Mainline
Status: NEW
Severity: blocking
Priority: P1
Component: Network
AssignedTo: [email protected]
ReportedBy: [email protected]
CC: [email protected]
Regression: YesMy problem is kernel crash under full line rate random packet length ip network traffic.
I'am using default unmodified kernel and default SMP kernel configuration, MPC8572DS development board and also using a hardware packet generator.
My test is ip forwarding between eth0 and eth1, and Hardware packet generator produces full duplex, full line rate traffic with random packet length and random payload . After a few millions of packets passed, kernel produces this bellow two different crash messages . I have retry this scenario many times, crash occurs sometimes onskb_put, but mostly occurs onip_rcvfunction. I have aplied same test to latest stable linux 2.6.35.6 kernel. Same errors produced.Here is crash logs:
Thanks.
Emin
First type of crash:
root@mpc8572ds:~# skb_over_panic: text:c0226280 len:1171 put:1171 head:eed6d000 data:eed63040 tail:0xeed6d4d3 end:0xeed63660 dev:<NULL> ------------[ cut here ]------------ kernel BUG at net/core/skbuff.c:127! Oops: Exception in kernel mode, sig: 5 [#1] SMP NR_CPUS=2 MPC8572 DS last sysfs file: /sys/devices/pci0002:03/0002:03:00.0/subsystem_device Modules linked in: NIP: c023bdcc LR: c023bdcc CTR: c01f3ff8 REGS: effe7d70 TRAP: 0700 Not tainted (2.6.36-rc5) MSR: 00029000 <EE,ME,CE> CR: 22028024 XER: 20000000 TASK = ef83e9a0[9] 'ksoftirqd/1' THREAD: ef856000 CPU: 1 GPR00: c023bdcc effe7e20 ef83e9a0 0000007c 00021000 ffffffff c01f7b98 c03ccf1c GPR08: c03c69d4 c03f94b4 00c4e000 00000004 20028048 1001a108 ef211000 efb52d90 GPR16: efb52e38 efb52870 00000000 ef211800 00000008 00000009 efb52800 00000037 GPR24: ef24e180 ef2be040 00000000 ef211948 efb52b80 00000493 ef015940 ef386600 NIP [c023bdcc] skb_put+0x8c/0x94 LR [c023bdcc] skb_put+0x8c/0x94 Call Trace: [effe7e20] [c023bdcc] skb_put+0x8c/0x94 (unreliable) [effe7e30] [c0226280] gfar_clean_rx_ring+0x104/0x4b8 [effe7e90] [c02269dc] gfar_poll+0x3a8/0x60c [effe7f60] [c024928c] net_rx_action+0xf8/0x1a4 [effe7fa0] [c0042524] __do_softirq+0xe0/0x178 [effe7ff0] [c000e59c] call_do_softirq+0x14/0x24 [ef857f50] [c0004840] do_softirq+0x90/0xa0 [ef857f70] [c00430e4] run_ksoftirqd+0xb4/0x164 [ef857fb0] [c00586b4] kthread+0x7c/0x80 [ef857ff0] [c000e9a8] kernel_thread+0x4c/0x68 Instruction dump: 81030098 2f800000 409e000c 3d20c037 3809a19c 3c60c037 7c8802a6 7d695b78 3863b010 90010008 4cc63182 4be016c5 <0fe00000> 48000000 9421fff0 7c0802a6 Kernel panic - not syncing: Fatal exception in interrupt ---------------second type of crash:
Faulting instruction address:
0xc026c1dcOops: Kernel access of bad area, sig: 11 [#1] SMP NR_CPUS=2 MPC8572 DS last sysfs file: /sys/devices/pci0002:03/0002:03:00.0/subsystem_device Modules linked in: NIP: c026c1dc LR: c026bfac CTR: 00000000 REGS: effebd00 TRAP: 0300 Not tainted (2.6.36-rc5) MSR: 00029000 <EE,ME,CE> CR: 42028042 XER: 00000000 DEAR: 0000cad8, ESR: 00000000TASK = ef83cde0[3] 'ksoftirqd/0' THREAD:
ef84a000CPU: 0
GPR00: 00000005effebdb0ef83cde000000000000001b900000000c100806000000000
GPR08:02c3f6050000ca00000005b90000ca00b653a6c77af823f0ef217000efbab590
GPR16:efbab638efbab07000000000ef21780000000008 00000018efbab00000000028
GPR24:c03f971cc0410000c0400000c03f94b4effea000ef316e4000000000eecb685eNIP [c026c1dc]ip_rcv+0x3f8/0x808
LR [c026bfac]ip_rcv+0x1c8/0x808Call Trace: [effebdb0] [c026c204] ip_rcv+0x420/0x808 (unreliable) [effebde0] [c02482dc] __netif_receive_skb+0x2f8/0x324 [effebe10] [c02483a4] netif_receive_skb+0x9c/0xb0 [effebe30] [c0226308] gfar_clean_rx_ring+0x18c/0x4b8 [effebe90] [c02269dc] gfar_poll+0x3a8/0x60c [effebf60] [c024928c] net_rx_action+0xf8/0x1a4 [effebfa0] [c0042524] __do_softirq+0xe0/0x178 [effebff0] [c000e59c] call_do_softirq+0x14/0x24 [ef84bf50] [c0004840] do_softirq+0x90/0xa0 [ef84bf70] [c00430e4] run_ksoftirqd+0xb4/0x164 [ef84bfb0] [c00586b4] kthread+0x7c/0x80 [ef84bff0] [c000e9a8] kernel_thread+0x4c/0x68 Instruction dump: 8148003c 318a0001 7d690194 91680038 9188003c 4bfffd78 7fa3eb78 48002a29 2f830000 40beff50 817d0048 5569003c <a00900d8> 2f800005 419e0034 2f800003 Kernel panic - not syncing: Fatal exception in interrupt
Quoted 10 lines · Andrew Morton
(switched to email. Please respond via emailed reply-to-all, not via the bugzilla web interface).
Quoted 4 lines · On Mon, 4 Oct 2010 06:25:14 GMT [email protected] wrote:
https://bugzilla.kernel.org/show_bug.cgi?id=19692
Summary: linux-2.6.36-rc5 crash with gianfar ethernet at full
line rate traffic
...
Emin, until there is something better I hope you could try this patch. (not tested nor compiled)
Thanks,
Jarek P.
---
drivers/net/gianfar.c +3 -3
@@ -2515,7 +2515,7 @@ static int gfar_clean_tx_ring(struct gfar_priv_tx_q *tx_queue) skb_recycle_check(skb, priv->rx_buffer_size + RXBUF_ALIGNMENT)) {gfar_align_skb (skb); -__skb_queue_head (&priv->rx_recycle, skb); +skb_queue_head (&priv->rx_recycle, skb); } elsedev_kfree_skb_any (skb );
@@ -2598,7 +2598,7 @@ struct sk_buff * gfar_new_skb(struct net_device *dev)struct gfar_private *priv =netdev_priv (dev);struct sk_buff *skb =NULL ; - skb =__skb_dequeue (&priv->rx_recycle); + skb =skb_dequeue (&priv->rx_recycle);if (!skb) skb =gfar_alloc_skb (dev);
@@ -2754,7 +2754,7 @@ int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue, int rx_work_limit)if (unlikely (!newskb)) newskb = skb;else if (skb) -__skb_queue_head (&priv->rx_recycle, skb); +skb_queue_head (&priv->rx_recycle, skb); }else {/* Increment the number of packets */ rx_queue->stats.rx_packets++;
Quoted 11 lines
Quoted 10 lines · Andrew Morton
(switched to email. Please respond via emailed reply-to-all, not via the bugzilla web interface).
Quoted 4 lines · On Mon, 4 Oct 2010 06:25:14 GMT [email protected] wrote:
https://bugzilla.kernel.org/show_bug.cgi?id=19692
Summary: linux-2.6.36-rc5 crash with gianfar ethernet at full line rate traffic
Quoted 7 lines
...
Emin, until there is something better I hope you could try this patch. (not tested nor compiled)
Thanks,
Jarek P.
Hi Jarek,
My test setup is at my office so that I will try your patch on Monday immediately and turn you the results.
Thanks.
Emin
Quoted 3 lines · emin ak
Hi Jarek,
My test setup is at my office so that I will try your patch on Monday immediately and turn you the results.
No need to hurry, especially on Mondays, but thanks for the info.
Jarek P.
Quoted 6 lines · Jarek Poplawski
Quoted 3 lines · emin ak
Hi Jarek,
My test setup is at my office so that I will try your patch on Monday immediately and turn you the results.No need to hurry, especially on Mondays, but thanks for the info.
Hi Emin,
Are there any problems with this testing?
Jarek P.
Quoted 5 lines
Hi Emin,
Are there any problems with this testing?Jarek P.
Hi Jarek.
Sorry for delayed answer. As I promised, I have started the tests on Monday (in spite of dealing with The Monday Syndrome:) I had applied your patch and after two billions packet (and approximatly four hours) passed kernel crashed with skb_over_panic error similar with first type. To ensure the patch is failed or not, I rerun the same test again. That time, surprisingly, it did'nt crashed again for two days with same kernel. But this situation had occured before, I think sometimes because of randomness of the applied ethernet traffic and mostly because I cant apply all full line rate random traffic to my target device because of wrong test setup (switch / hardware packet generator settings etc..), it takes three or more day to crash the kernel and sometimes it never crashes. My device only and only crashes when I can apply full line rate random traffic. Before informing you and the list with (official) test results, I want to be sure with the truth of them. So that please let me apply more test to the target device for a fews day more. After that I wish I'll came with good news!
Thanks for your time and care alot.
Emin
Quoted 10 lines · emin ak
Hi Jarek.
Sorry for delayed answer. As I promised, I have started the tests on Monday (in spite of dealing with The Monday Syndrome:) I had applied your patch and after two billions packet (and approximatly four hours) passed kernel crashed withskb_over_panicerror similar with first type. To ensure the patch is failed or not, I rerun the same test again. That time, surprisingly, it did'nt crashed again for two days with same kernel. But this situation had occured before, I think sometimes because of randomness of the applied ethernet traffic and mostly because I cant apply all full line rate random traffic to my
Quoted 8 lines
target device because of wrong test setup (switch / hardware packet generator settings etc..), it takes three or more day to crash the kernel and sometimes it never crashes. My device only and only crashes when I can apply full line rate random traffic. Before informing you and the list with (official) test results, I want to be sure with the truth of them. So that please let me apply more test to the target device for a fews day more. After that I wish I'll came with good news!
Hi Emin,
Sorry for being impatient. Actually, waiting is no problem for me. I simply didn't know how much this bug is reproducible, and was a bit mislead by your forecast of one day result (and terrified btw seeing words Monday and immediately side by side ;-) So, a few days or weeks, no problem, it's all up to you.
On the other hand, it looks like there might be something else/more, so I'd suggest to add this last skb_over_panic to bugzilla too.
Thanks,
Jarek P.
Hi Jarek;
After 5 days and more then 20 billion packets passed without crash, it seems that this patch is working for me, at least for crash type 2. (For type 1, it only occured once and I can never reproduce this again, but still trying. I think with this patch is also lowers the risk for type 1.
For adding a new bug entry for skb_over_panic, before that I think I must find a reliable way to make this type of crash reproducable, otherwise I don't know how to test it if it solved or not. Lastly, thanks a lot for your valuable help to overcome this problem and also is there anything that I can do for testing / commiting this patch to mainline?
Thanks
Emin
PATCH 1/1 gianfar: Fix crashes on RX path (Was Re: [Bugme-new] [Bug 19692] New: linux-2.6.36-rc5 crash with gianfar ethernet at full line rate traffic)
Quoted 6 lines · emin ak
Hi Jarek;
After 5 days and more then 20 billion packets passed without crash, it seems that this patch is working for me, at least for crash type 2. (For type 1, it only occured once and I can never reproduce this again, but still trying. I think with this patch is also lowers the risk for type 1.
It would be interesting to have a look if it's exactly type 1, because skb_over_panic can happen for different reasons, e.g. like here:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=63b88b9041ceef8217f34de71a2e96f0c3f0fd3b
Quoted 3 lines
For adding a new bug entry for
skb_over_panic, before that I think I must find a reliable way to make this type of crash reproducable, otherwise I don't know how to test it if it solved or not.
Maybe for now let's try to get and see this type 1 again? Since the recycle path is suspicious a bit to me, probably limiting memory or slowing tx (maybe different mtus on eth0 and 1) under heavy multi cpu load might help.
Quoted 3 lines
Lastly, thanks a lot for your valuable help to overcome this problem and also is there anything that I can do for testing / commiting this patch to mainline?
Here it is for David to handle the rest.
Thanks a lot for such an intense testing,
Jarek P.
--------------------------->
The rx_recycle queue is global per device but can be accesed by many napi handlers at the same time, so it needs full skb_queue primitives (with locking). Otherwise, various crashes caused by broken skbs are possible.
This patch resolves, at least partly, bugzilla bug 19692. (Because of some doubts that there could be still something around which is hard to reproduce my proposal is to leave this bug opened for a month.)
Fixes commit: 0fd56bb5be6455d0d42241e65aed057244665e5e
---
drivers/net/gianfar.c +3 -3
@@ -2515,7 +2515,7 @@ static int gfar_clean_tx_ring(struct gfar_priv_tx_q *tx_queue) skb_recycle_check(skb, priv->rx_buffer_size + RXBUF_ALIGNMENT)) {gfar_align_skb (skb); -__skb_queue_head (&priv->rx_recycle, skb); +skb_queue_head (&priv->rx_recycle, skb); } elsedev_kfree_skb_any (skb );
@@ -2598,7 +2598,7 @@ struct sk_buff * gfar_new_skb(struct net_device *dev)struct gfar_private *priv =netdev_priv (dev);struct sk_buff *skb =NULL ; - skb =__skb_dequeue (&priv->rx_recycle); + skb =skb_dequeue (&priv->rx_recycle);if (!skb) skb =gfar_alloc_skb (dev);
@@ -2754,7 +2754,7 @@ int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue, int rx_work_limit)if (unlikely (!newskb)) newskb = skb;else if (skb) -__skb_queue_head (&priv->rx_recycle, skb); +skb_queue_head (&priv->rx_recycle, skb); }else {/* Increment the number of packets */ rx_queue->stats.rx_packets++;
Re: [PATCH] gianfar: Fix crashes on RX path (Was Re: [Bugme-new] [Bug 19692] New: linux-2.6.36-rc5 crash with gianfar ethernet at full line rate traffic)
Hi Jarek;
Quoted 5 lines
Maybe for now let's try to get and see this type 1 again? Since the recycle path is suspicious a bit to me, probably limiting memory or slowing tx (maybe different mtus on eth0 and 1) under heavy multi cpu load might help.
I'll do my best with your recommended test conditions. I have reserved a machine and two ports of hw packet generator, now we'll see if this error will occur again or not. (But I'am not sure if I want to see it again:)
Thanks
Emin
Re: [PATCH] gianfar: Fix crashes on RX path (Was Re: [Bugme-new] [Bug 19692] New: linux-2.6.36-rc5 crash with gianfar ethernet at full line rate traffic)
Quoted 8 lines · emin ak
Hi Jarek;
Quoted 5 lines
Maybe for now let's try to get and see this type 1 again? Since the recycle path is suspicious a bit to me, probably limiting memory or slowing tx (maybe different mtus on eth0 and 1) under heavy multi cpu load might help.
Quoted 4 lines
I'll do my best with your recommended test conditions. I have reserved a machine and two ports of hw packet generator, now we'll see if this error will occur again or not. (But I'am not sure if I want to see it again:)
Really this rx recycle affair is more than suspicious to me too.
Is it really worth the pain ?
Are MTU changes handled correctly ? ( must flush rx_recycle queue...)
Using a single rx_recycle queue in a supposed multiqueue driver makes absolutely _no_ sense to me. This adds an artificial contention point.
Re: [PATCH] gianfar: Fix crashes on RX path (Was Re: [Bugme-new] [Bug 19692] New: linux-2.6.36-rc5 crash with gianfar ethernet at full line rate traffic)
Quoted 14 lines · Eric Dumazet
Quoted 8 lines · emin ak
Hi Jarek;
Quoted 5 lines
Maybe for now let's try to get and see this type 1 again? Since the recycle path is suspicious a bit to me, probably limiting memory or slowing tx (maybe different mtus on eth0 and 1) under heavy multi cpu load might help.
Quoted 4 lines
I'll do my best with your recommended test conditions. I have reserved a machine and two ports of hw packet generator, now we'll see if this error will occur again or not. (But I'am not sure if I want to see it again:)
Quoted 9 lines
Really this rx recycle affair is more than suspicious to me too.
Is it really worth the pain ?
Are MTU changes handled correctly ? ( must flush
rx_recyclequeue...)Using a single
rx_recyclequeue in a supposed multiqueue driver makes absolutely _no_ sense to me. This adds an artificial contention point.
Yes, the design/need of this rx_recycle queue should be reconsidered. But we don't know if the (other?) bug seen by Emin was connected, so it would be better to try to reproduce it without too many changes.
Jarek P.
Re: [PATCH] gianfar: Fix crashes on RX path (Was Re: [Bugme-new] [Bug 19692] New: linux-2.6.36-rc5 crash with gianfar ethernet at full line rate traffic)
In reply to Jarek PoplawskiQuoted 70 lines · Jarek Poplawski
Quoted 6 lines · emin ak
Hi Jarek;
After 5 days and more then 20 billion packets passed without crash, it seems that this patch is working for me, at least for crash type 2. (For type 1, it only occured once and I can never reproduce this again, but still trying. I think with this patch is also lowers the risk for type 1.It would be interesting to have a look if it's exactly type 1, because
skb_over_paniccan happen for different reasons, e.g. like here:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=63b88b9041ceef8217f34de71a2e96f0c3f0fd3bQuoted 3 lines
For adding a new bug entry for
skb_over_panic, before that I think I must find a reliable way to make this type of crash reproducable, otherwise I don't know how to test it if it solved or not.Maybe for now let's try to get and see this type 1 again? Since the recycle path is suspicious a bit to me, probably limiting memory or slowing tx (maybe different mtus on eth0 and 1) under heavy multi cpu load might help.
Quoted 3 lines
Lastly, thanks a lot for your valuable help to overcome this problem and also is there anything that I can do for testing / commiting this patch to mainline?
Here it is for David to handle the rest.
Thanks a lot for such an intense testing,
Jarek P.
--------------------------->The
rx_recyclequeue is global per device but can be accesed by many napi handlers at the same time, so it needs fullskb_queueprimitives (with locking). Otherwise, various crashes caused by broken skbs are possible.This patch resolves, at least partly, bugzilla bug 19692. (Because of some doubts that there could be still something around which is hard to reproduce my proposal is to leave this bug opened for a month.)
Fixes commit:
0fd56bb5be6455d0d42241e65aed057244665e5eReported-by emin ak <[email protected]>Tested-by emin ak <[email protected]>Signed-off-by Jarek Poplawski <[email protected]>CC Andy Fleming <[email protected]>---
drivers/net/gianfar.c +2
-2@@ -2515,7 +2515,7 @@ static int gfar_clean_tx_ring(struct gfar_priv_tx_q *tx_queue) skb_recycle_check(skb, priv->rx_buffer_size + RXBUF_ALIGNMENT)) {gfar_align_skb (skb); -__skb_queue_head (&priv->rx_recycle, skb); +skb_queue_head (&priv->rx_recycle, skb); } elsedev_kfree_skb_any (skb );@@ -2598,7 +2598,7 @@ struct sk_buff * gfar_new_skb(struct net_device *dev)struct gfar_private *priv =netdev_priv (dev);struct sk_buff *skb =NULL ; - skb =__skb_dequeue (&priv->rx_recycle); + skb =skb_dequeue (&priv->rx_recycle);if (!skb) skb =gfar_alloc_skb (dev);
drivers/net/gianfar.c int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue, int rx_work_limit) · line 2760
Quoted 8 lines
drivers/net/gianfar.c +1
-1@@ -2754,7 +2754,7 @@ int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue, int rx_work_limit)if (unlikely (!newskb)) newskb = skb;else if (skb) -__skb_queue_head (&priv->rx_recycle, skb); +skb_queue_head (&priv->rx_recycle, skb); }else {/* Increment the number of packets */ rx_queue->stats.rx_packets++;
Are you sure its needed at all ?
Gianfar claims to be multiqueue, but only one cpu can run gfar_poll() and call gfar_clean_tx_ring() / gfar_clean_rx_ring()
If not, there would be more bugs than only rx_recycle thing
vi +2822 drivers/net/gianfar.c
for_each_set_bit(i, &gfargrp->rx_bit_map, priv->num_rx_queues) {
if (test_bit(i, &serviced_queues))
continue;
rx_queue = priv->rx_queue[i];
tx_queue = priv->tx_queue[rx_queue->qindex];
tx_cleaned += gfar_clean_tx_ring(tx_queue);
rx_cleaned_per_queue = gfar_clean_rx_ring(rx_queue,
budget_per_queue);
rx_cleaned += rx_cleaned_per_queue;
if(rx_cleaned_per_queue < budget_per_queue) {
left_over_budget = left_over_budget +
(budget_per_queue - rx_cleaned_per_queue);
set_bit(i, &serviced_queues);
num_queues--;
}
}Re: [PATCH] gianfar: Fix crashes on RX path (Was Re: [Bugme-new] [Bug 19692] New: linux-2.6.36-rc5 crash with gianfar ethernet at full line rate traffic)
Quoted 70 lines · Eric Dumazet
Quoted 69 lines · Jarek Poplawski
Quoted 6 lines · emin ak
Hi Jarek;
After 5 days and more then 20 billion packets passed without crash, it seems that this patch is working for me, at least for crash type 2. (For type 1, it only occured once and I can never reproduce this again, but still trying. I think with this patch is also lowers the risk for type 1.It would be interesting to have a look if it's exactly type 1, because
skb_over_paniccan happen for different reasons, e.g. like here:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=63b88b9041ceef8217f34de71a2e96f0c3f0fd3bQuoted 3 lines
For adding a new bug entry for
skb_over_panic, before that I think I must find a reliable way to make this type of crash reproducable, otherwise I don't know how to test it if it solved or not.Maybe for now let's try to get and see this type 1 again? Since the recycle path is suspicious a bit to me, probably limiting memory or slowing tx (maybe different mtus on eth0 and 1) under heavy multi cpu load might help.
Quoted 3 lines
Lastly, thanks a lot for your valuable help to overcome this problem and also is there anything that I can do for testing / commiting this patch to mainline?
Here it is for David to handle the rest.
Thanks a lot for such an intense testing,
Jarek P.
--------------------------->The
rx_recyclequeue is global per device but can be accesed by many napi handlers at the same time, so it needs fullskb_queueprimitives (with locking). Otherwise, various crashes caused by broken skbs are possible.This patch resolves, at least partly, bugzilla bug 19692. (Because of some doubts that there could be still something around which is hard to reproduce my proposal is to leave this bug opened for a month.)
Fixes commit:
0fd56bb5be6455d0d42241e65aed057244665e5eReported-by emin ak <[email protected]>Tested-by emin ak <[email protected]>Signed-off-by Jarek Poplawski <[email protected]>CC Andy Fleming <[email protected]>---
drivers/net/gianfar.c +2
-2@@ -2515,7 +2515,7 @@ static int gfar_clean_tx_ring(struct gfar_priv_tx_q *tx_queue) skb_recycle_check(skb, priv->rx_buffer_size + RXBUF_ALIGNMENT)) {gfar_align_skb (skb); -__skb_queue_head (&priv->rx_recycle, skb); +skb_queue_head (&priv->rx_recycle, skb); } elsedev_kfree_skb_any (skb );@@ -2598,7 +2598,7 @@ struct sk_buff * gfar_new_skb(struct net_device *dev)struct gfar_private *priv =netdev_priv (dev);struct sk_buff *skb =NULL ; - skb =__skb_dequeue (&priv->rx_recycle); + skb =skb_dequeue (&priv->rx_recycle);if (!skb) skb =gfar_alloc_skb (dev);
Quoted 11 lines
drivers/net/gianfar.c int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue, int rx_work_limit) · line 2760
Quoted 9 lines
drivers/net/gianfar.c +1
-1@@ -2754,7 +2754,7 @@ int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue, int rx_work_limit)if (unlikely (!newskb)) newskb = skb;else if (skb) -__skb_queue_head (&priv->rx_recycle, skb); +skb_queue_head (&priv->rx_recycle, skb); }else {/* Increment the number of packets */ rx_queue->stats.rx_packets++;Are you sure its needed at all ?
Yes, after Emin's testing I'm quite sure this fix is needed.
Quoted 5 lines
Gianfar claims to be multiqueue, but only one cpu can run
gfar_poll()and callgfar_clean_tx_ring()/gfar_clean_rx_ring()If not, there would be more bugs than only
rx_recyclething
I didn't find what prevents running gfar_poll on many cpus and don't claim there is no more bugs around.
Jarek P.
Quoted 22 lines
vi +2822
drivers/net/gianfar.cfor_each_set_bit(i, &gfargrp->rx_bit_map, priv->num_rx_queues) { if (test_bit(i, &serviced_queues)) continue; rx_queue = priv->rx_queue[i]; tx_queue = priv->tx_queue[rx_queue->qindex]; tx_cleaned += gfar_clean_tx_ring(tx_queue); rx_cleaned_per_queue = gfar_clean_rx_ring(rx_queue, budget_per_queue); rx_cleaned += rx_cleaned_per_queue; if(rx_cleaned_per_queue < budget_per_queue) { left_over_budget = left_over_budget + (budget_per_queue - rx_cleaned_per_queue); set_bit(i, &serviced_queues); num_queues--; } }
Re: [PATCH] gianfar: Fix crashes on RX path (Was Re: [Bugme-new] [Bug 19692] New: linux-2.6.36-rc5 crash with gianfar ethernet at full line rate traffic)
Quoted 1 line · Jarek Poplawski
On Fri, Oct 22, 2010 at 08:11:57AM +0200, Eric Dumazet wrote:
...
Quoted 7 lines
Quoted 4 lines
Gianfar claims to be multiqueue, but only one cpu can run
gfar_poll()and callgfar_clean_tx_ring()/gfar_clean_rx_ring()If not, there would be more bugs than only
rx_recyclethingI didn't find what prevents running
gfar_pollon many cpus and don't claim there is no more bugs around.
On the other hand, I don't see your point in the code below either. These're only per gfargrp queues - not per device, aren't they?
Jarek P.
Quoted 23 lines
Quoted 23 lines
vi +2822
drivers/net/gianfar.cfor_each_set_bit(i, &gfargrp->rx_bit_map, priv->num_rx_queues) { if (test_bit(i, &serviced_queues)) continue; rx_queue = priv->rx_queue[i]; tx_queue = priv->tx_queue[rx_queue->qindex]; tx_cleaned += gfar_clean_tx_ring(tx_queue); rx_cleaned_per_queue = gfar_clean_rx_ring(rx_queue, budget_per_queue); rx_cleaned += rx_cleaned_per_queue; if(rx_cleaned_per_queue < budget_per_queue) { left_over_budget = left_over_budget + (budget_per_queue - rx_cleaned_per_queue); set_bit(i, &serviced_queues); num_queues--; } }
David Miller
maintainerRe: [PATCH] gianfar: Fix crashes on RX path
From: Jarek Poplawski <[email protected]>
Date: Fri, 22 Oct 2010 08:52:48 +0000
Quoted 6 lines · Jarek Poplawski
Quoted 1 line · Eric Dumazet
...
Quoted 3 lines
Quoted 3 lines
Gianfar claims to be multiqueue, but only one cpu can run
gfar_poll()and callgfar_clean_tx_ring()/gfar_clean_rx_ring()
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
If not, there would be more bugs than only
rx_recyclethingI didn't find what prevents running
gfar_pollon many cpus and don't claim there is no more bugs around.On the other hand, I don't see your point in the code below either. These're only per gfargrp queues - not per device, aren't they?
I am still not at the point where I feel confortable applying this bug fix, in fact I am very far from that.
None of the logic is consistent in what we are saying causes the problem.
Anything that would make the RX recycling code racy and corrupt recycling queue of the gianfar driver, would also corrupt all of the other RX side and other driver state.
The NAPI state is unary for gianfar, and inside of that singular ->poll() instance it iterates over the queues.
The only asynchronous path could be netpoll, but again that would break lots of other things.
I want to be shown a code path that results in the recycle SKB queue getting accessed in parallel on two cpus without protection so we can understand what it is that we are fixing.
On another note, I also agree that removing this RX recycling crud wouldn't be a bad idea. In addition to the MTU changing concerns Eric Dumazet brought up, there are many other (less broken) ways to achieve whatever performance gains recycling gives these devices.
Thanks.
Re: [PATCH] gianfar: Fix crashes on RX path
Quoted 23 lines · David Miller
From: Jarek Poplawski <[email protected]>
Date: Fri, 22 Oct 2010 08:52:48 +0000Quoted 6 lines · Jarek Poplawski
Quoted 1 line · Eric Dumazet
...
Quoted 3 lines
Quoted 3 lines
Gianfar claims to be multiqueue, but only one cpu can run
gfar_poll()and callgfar_clean_tx_ring()/gfar_clean_rx_ring()Quoted 7 lines
Quoted 4 lines
Quoted 1 line
If not, there would be more bugs than only
rx_recyclethingI didn't find what prevents running
gfar_pollon many cpus and don't claim there is no more bugs around.On the other hand, I don't see your point in the code below either. These're only per gfargrp queues - not per device, aren't they?
I am still not at the point where I feel confortable applying this bug fix, in fact I am very far from that.
None of the logic is consistent in what we are saying causes the problem.
Quoted 6 lines
Anything that would make the RX recycling code racy and corrupt recycling queue of the gianfar driver, would also corrupt all of the other RX side and other driver state.
The NAPI state is unary for gianfar, and inside of that singular ->poll() instance it iterates over the queues.
IMHO, the NAPI state is unary only for gfargrp, and multiple ->poll() instances share a (unary) rx_recycle queue without proper locking.
Thanks,
Jarek P.
David Miller
maintainerRe: [PATCH] gianfar: Fix crashes on RX path
From: Jarek Poplawski <[email protected]>
Date: Tue, 26 Oct 2010 23:20:42 +0200
Quoted 2 lines
IMHO, the NAPI state is unary only for gfargrp, and multiple ->poll() instances share a (unary)
rx_recyclequeue without proper locking.
Ok, I see it now, thank you.
For now I'll apply your patch, but long term I think we should just eradicate the recycling code from the entire tree.