Exactly the same happens, I still get a 'NMI Watchdog detected LOCKUP'
with the r8169 device using the above patch on top of 2.6.10-rc3-bk10.
Thanks
Mark
--
Mark Broadbent [off-list ref]
Web: http://www.wetlettuce.com
From: Matt Mackall <hidden> Date: 2004-12-20 21:15:05
On Mon, Dec 20, 2004 at 09:42:08AM -0000, Mark Broadbent wrote:
Exactly the same happens, I still get a 'NMI Watchdog detected LOCKUP'
with the r8169 device using the above patch on top of 2.6.10-rc3-bk10.
Ok, that suggests a problem localized to netpoll itself. Do you have
spinlock debugging turned on by any chance?
--
Mathematics is the supreme nostalgia of our time.
On Mon, Dec 20, 2004 at 09:42:08AM -0000, Mark Broadbent wrote:
quoted
Exactly the same happens, I still get a 'NMI Watchdog detected LOCKUP'
with the r8169 device using the above patch on top of 2.6.10-rc3-bk10.
Ok, that suggests a problem localized to netpoll itself. Do you have
spinlock debugging turned on by any chance?
Any chance of:
1 dev_queue_xmit
2 dev->xmit_lock taken
3 interruption
4 printk
5 netconsole write
6 dev->xmit_lock again
7 lockup
?
This is probably the silly question of the day.
--
Ueimor
From: Matt Mackall <hidden> Date: 2004-12-21 00:56:14
On Tue, Dec 21, 2004 at 01:22:18AM +0100, Francois Romieu wrote:
Matt Mackall [off-list ref] :
quoted
On Mon, Dec 20, 2004 at 09:42:08AM -0000, Mark Broadbent wrote:
quoted
Exactly the same happens, I still get a 'NMI Watchdog detected LOCKUP'
with the r8169 device using the above patch on top of 2.6.10-rc3-bk10.
Ok, that suggests a problem localized to netpoll itself. Do you have
spinlock debugging turned on by any chance?
Any chance of:
1 dev_queue_xmit
2 dev->xmit_lock taken
3 interruption
4 printk
5 netconsole write
6 dev->xmit_lock again
7 lockup
?
This is probably the silly question of the day.
Maybe, but the answer isn't obvious to me at the moment as I haven't
been thinking about such stuff enough lately. Silly response of the
day:
Mark, can you try this (again completely untested, but at least
compiles) patch? I'm afraid I don't have a proper test rig to
reproduce this at the moment. This will attempt to grab the lock, and
if it fails, will check for recursion. Then it will try to print a
message on the local console, temporarily disabling netconsole to
allow the printk to get through..
Index: l/net/core/netpoll.c
===================================================================
@@ -183,13 +185,24 @@intstatus;repeat:-if(!np||!np->dev||!netif_running(np->dev)){+if(!np||!np->dev||!netif_running(np->dev)||netpoll_kill){__kfree_skb(skb);return;}-spin_lock(&np->dev->xmit_lock);-np->dev->xmit_lock_owner=smp_processor_id();+if(spin_trylock(&np->dev->xmit_lock))+np->dev->xmit_lock_owner=smp_processor_id();+else{+if(np->dev->xmit_lock_owner==smp_processor_id()){+netpoll_kill=1;+__kfree_skb(skb);+printk("Tried to recursively get dev->xmit_lock");+netpoll_kill=0;+return;+}+spin_lock(&np->dev->xmit_lock);++}/**networkdriversdonotexpecttobecalledifthequeueis
--
Mathematics is the supreme nostalgia of our time.
From: Mark Broadbent <hidden> Date: 2004-12-21 10:24:08
Matt Mackall said:
On Tue, Dec 21, 2004 at 01:22:18AM +0100, Francois Romieu wrote:
quoted
Matt Mackall [off-list ref] :
quoted
On Mon, Dec 20, 2004 at 09:42:08AM -0000, Mark Broadbent wrote:
quoted
Exactly the same happens, I still get a 'NMI Watchdog detected
LOCKUP' with the r8169 device using the above patch on top of
2.6.10-rc3-bk10.
Ok, that suggests a problem localized to netpoll itself. Do you have
spinlock debugging turned on by any chance?
Any chance of:
1 dev_queue_xmit
2 dev->xmit_lock taken
3 interruption
4 printk
5 netconsole write
6 dev->xmit_lock again
7 lockup
?
This is probably the silly question of the day.
Maybe, but the answer isn't obvious to me at the moment as I haven't
been thinking about such stuff enough lately. Silly response of the
day:
Mark, can you try this (again completely untested, but at least
compiles) patch? I'm afraid I don't have a proper test rig to
reproduce this at the moment. This will attempt to grab the lock, and
if it fails, will check for recursion. Then it will try to print a
message on the local console, temporarily disabling netconsole to
allow the printk to get through..
OK, patch applied and spinlock debugging enabled. Testing with eth1
(r1869) doesn'tyield any additional messages, just the standard 'NMI Watchdog detected
lockup'.
Thanks
Mark
--
Mark Broadbent [off-list ref]
Web: http://www.wetlettuce.com
OK, patch applied and spinlock debugging enabled. Testing with eth1
(r1869) doesn'tyield any additional messages, just the standard
'NMI Watchdog detected lockup'.
Does the modified version below trigger _exactly_ the same hang ?
From: Mark Broadbent <hidden> Date: 2004-12-21 13:29:58
Francois Romieu said:
Mark Broadbent [off-list ref] :
[...]
quoted
OK, patch applied and spinlock debugging enabled. Testing with eth1
(r1869) doesn'tyield any additional messages, just the standard
'NMI Watchdog detected lockup'.
Does the modified version below trigger _exactly_ the same hang ?
Using the patch supplied I get no hang, just the message 'netconsole
raced' output to the console and the packet capture proceeds as normal.
Thanks
Mark
--
Mark Broadbent [off-list ref]
Web: http://www.wetlettuce.com
Using the patch supplied I get no hang, just the message 'netconsole
raced' output to the console and the packet capture proceeds as normal.
Thanks
The patch is more a bandaid for debugging than a real fix. The netconsole
will drop some messages until its locking is fixed
If you can issue one more test, I'd like to know if some messages appear
on the VGA console around the time at which tcpdump is started (the test
assumes that netconsole is not used/insmoded at all). Please check that
the console log_level is set high enough as it will be really disappointing
if nothing appears :o)
--
Ueimor
From: Matt Mackall <hidden> Date: 2004-12-21 21:28:11
On Tue, Dec 21, 2004 at 09:48:53PM +0100, Francois Romieu wrote:
Mark Broadbent [off-list ref] :
[...]
quoted
Using the patch supplied I get no hang, just the message 'netconsole
raced' output to the console and the packet capture proceeds as normal.
Thanks
The patch is more a bandaid for debugging than a real fix. The netconsole
will drop some messages until its locking is fixed
Unfortunately there's no good way to fix its locking in this
circumstance (or the harder case of driver-private locks). I think
I'll just have to come up with some scheme for queueing messages that
arrive when the queue lock is held.
If you can issue one more test, I'd like to know if some messages appear
on the VGA console around the time at which tcpdump is started (the test
assumes that netconsole is not used/insmoded at all). Please check that
the console log_level is set high enough as it will be really disappointing
if nothing appears :o)
I think it's the promiscuous mode message itself that's the problem
but I've not had time to reproduce it.
--
Mathematics is the supreme nostalgia of our time.
I think it's the promiscuous mode message itself that's the problem
Yes. dev_mc_add takes dev->xmit_lock and the game is over.
Marc, if the patch below happens to work, it should not drop messages
like the previous one (it is an ugly short-term suggestion).
Since the code doesn't dequeue itself, enqueue seems fine to keep
at least the queued messages ordered. But you need to grab
dev->queue_lock, otherwise you risk corrupting qdisc internal data.
You should probably also deal with the noqueue-qdisc, which doesn't
have an enqueue function. So it should look something like this:
while (!spin_trylock(&np->dev->xmit_lock)) {
if (np->dev->xmit_lock_owner == smp_processor_id()) {
struct Qdisc *q;
rcu_read_lock();
q = rcu_dereference(dev->qdisc);
if (q->enqueue) {
spin_lock(&dev->queue_lock);
q->ops->enqueue(skb, q);
spin_unlock(&dev->queue_lock);
netif_schedule(np->dev);
} else
kfree_skb(skb);
rcu_read_unlock();
return;
}
}
Regards
Patrick
at least the queued messages ordered. But you need to grab
dev->queue_lock, otherwise you risk corrupting qdisc internal data.
You should probably also deal with the noqueue-qdisc, which doesn't
have an enqueue function. So it should look something like this:
If I am not mistaken, a failure on spin_trylock + the test on
xmit_lock_owner imply that it is safe to directly handle the
queue. It means that qdisc_run() has been interrupted on the
current cpu and the other paths seem fine as well. Counter-example
is welcome (no joke).
Of course the patch is completely ugly and violates any layering
principle one could think of. It was not submitted for inclusion :o)
while (!spin_trylock(&np->dev->xmit_lock)) {
if (np->dev->xmit_lock_owner == smp_processor_id()) {
struct Qdisc *q;
rcu_read_lock();
q = rcu_dereference(dev->qdisc);
if (q->enqueue) {
spin_lock(&dev->queue_lock);
I'd expect it to deadlock if dev_queue_xmit -> qdisc_run is interrupted
on the current cpu and a printk is issued as dev->queue_lock will have
been taken elsewhere.
--
Ueimor
On Wed, 2004-12-22 at 07:39, Francois Romieu wrote:
If I am not mistaken, a failure on spin_trylock + the test on
xmit_lock_owner imply that it is safe to directly handle the
queue. It means that qdisc_run() has been interrupted on the
current cpu and the other paths seem fine as well. Counter-example
is welcome (no joke).
Since the code doesn't dequeue itself, enqueue seems fine to keep
at least the queued messages ordered. But you need to grab
dev->queue_lock, otherwise you risk corrupting qdisc internal data. You
should probably also deal with the noqueue-qdisc, which doesn't have an
enqueue function. So it should look something like this:
while (!spin_trylock(&np->dev->xmit_lock)) {
if (np->dev->xmit_lock_owner == smp_processor_id()) {
struct Qdisc *q;
rcu_read_lock();
q = rcu_dereference(dev->qdisc);
if (q->enqueue) {
spin_lock(&dev->queue_lock);
q->ops->enqueue(skb, q);
spin_unlock(&dev->queue_lock);
netif_schedule(np->dev);
} else
kfree_skb(skb);
rcu_read_unlock();
return;
}
}
I've tried both patches (modified slightly to get them to compile) but
they both produce hard NMI detected lockups (as before).
Thanks
Mark
Patches after modification to allow compilation:
Francois' patch (against 2.6.10-rc3-bk10):
diff -X dontdiff -urN linux-2.6.9-rc3-bk10.orig/net/core/netpoll.c
linux-2.6.9-rc3-bk10/net/core/netpoll.c--- linux-2.6.9-rc3-bk10.orig/net/core/netpoll.c 2004-12-22
12:09:32.000000000 +0000+++ linux-2.6.9-rc3-bk10/net/core/netpoll.c 2004-12-22 14:13:54.000000000
+0000@@ -22,6 +22,7 @@
#include <net/tcp.h>
#include <net/udp.h>
#include <asm/unaligned.h>
+#include <net/pkt_sched.h>
/*
* We maintain a small pool of fully-sized skbs, to make sure the
From: Patrick McHardy <hidden> Date: 2004-12-22 14:59:06
Francois Romieu wrote:
Patrick McHardy [off-list ref] :
[...]
quoted
at least the queued messages ordered. But you need to grab
dev->queue_lock, otherwise you risk corrupting qdisc internal data.
You should probably also deal with the noqueue-qdisc, which doesn't
have an enqueue function. So it should look something like this:
If I am not mistaken, a failure on spin_trylock + the test on
xmit_lock_owner imply that it is safe to directly handle the
queue. It means that qdisc_run() has been interrupted on the
current cpu and the other paths seem fine as well. Counter-example
is welcome (no joke).
enqueue is only protected by dev->queue_lock, and dev->queue_lock
is dropped as soon as dev->xmit_lock is grabbed, so any other CPU
might call enqueue at the same time.
Example:
CPU1 CPU2
dev_queue_xmit dev_queue_xmit
lock(dev->queue_lock) lock(dev->queue_lock)
q->enqueue
qdisc_run
qdisc_restart
trylock(dev->xmit_lock), ok
unlock(dev->queue_lock)
...
printk("something")
...
netpoll_send_skb
trylock(dev->xmit_lock), fails
q->enqueue q->enqueue
Of course the patch is completely ugly and violates any layering
principle one could think of. It was not submitted for inclusion :o)
Sure, but I think we should have a short-term workaround until
a better solution has been invented. Maybe dropping the packets
would be best for now, it only affects printks issued in paths
starting at qdisc_restart (-> hard_start_xmit -> ...). Queueing
the packets might also cause reordering since not all packets
are queued.
quoted
while (!spin_trylock(&np->dev->xmit_lock)) {
if (np->dev->xmit_lock_owner == smp_processor_id()) {
struct Qdisc *q;
rcu_read_lock();
q = rcu_dereference(dev->qdisc);
if (q->enqueue) {
spin_lock(&dev->queue_lock);
I'd expect it to deadlock if dev_queue_xmit -> qdisc_run is interrupted
on the current cpu and a printk is issued as dev->queue_lock will have
been taken elsewhere.
Hmm this is complicated, let me think some more about it.
Regards
Patrick
From: Matt Mackall <hidden> Date: 2004-12-22 17:19:16
On Wed, Dec 22, 2004 at 03:57:57PM +0100, Patrick McHardy wrote:
quoted
Of course the patch is completely ugly and violates any layering
principle one could think of. It was not submitted for inclusion :o)
Sure, but I think we should have a short-term workaround until
a better solution has been invented. Maybe dropping the packets
would be best for now, it only affects printks issued in paths
starting at qdisc_restart (-> hard_start_xmit -> ...). Queueing
the packets might also cause reordering since not all packets
are queued.
When I mentioned queueing, I was thinking of a netpoll-private queue
that would be hooked to a softirq or some such so that it would be
pushed out as soon as possible. Dropping may be the better approach as
queueing throws away netpoll's immediacy and ordering properties. And
getting netpoll _more_ tangled in the net stack mechanics is
definitely a step in the wrong direction.
More generally, I'm tempted to add some warn_on style functionality so
that printks in such troublesome paths can be lifted out.
--
Mathematics is the supreme nostalgia of our time.
Hi everyone,
Wishing you all A Prosperous Merry ChristMas.
Hope Coming years brings Peace,Happiness,blessings of CHRISTmas to you all
,your family and this World.
With Regards,
Pranav.
From: Jan Engelhardt <hidden> Date: 2004-12-25 11:31:14
Wishing you all A Prosperous Merry ChristMas.
Hope Coming years brings Peace,Happiness,blessings of CHRISTmas to you all
,your family and this World.
With Regards,
Pranav.
I don't see how this is related to linux-kernel.
Jan Engelhardt
--
ENOSPC
On Wed, Dec 22, 2004 at 03:57:57PM +0100, Patrick McHardy wrote:
quoted
quoted
Of course the patch is completely ugly and violates any layering
principle one could think of. It was not submitted for inclusion :o)
Sure, but I think we should have a short-term workaround until
a better solution has been invented. Maybe dropping the packets
would be best for now, it only affects printks issued in paths
starting at qdisc_restart (-> hard_start_xmit -> ...). Queueing
the packets might also cause reordering since not all packets
are queued.
When I mentioned queueing, I was thinking of a netpoll-private queue
that would be hooked to a softirq or some such so that it would be
pushed out as soon as possible. Dropping may be the better approach
I think so - just junk those packets.
cheers,
jamal