Re: [PCNET32] Lock solid with netconsole

Hi,
Tested under very high console activity and it no longer freeze.
Thanks,
Best regards,
Emmanuel.
---

Créez votre adresse électronique prenom.nom@laposte.net 
1 Go d'espace de stockage, anti-spam et anti-virus intégrés.

On Mon, May 28, 2007 at 05:25:51PM +0200, Emmanuel Fust? wrote:

quoted

Any difference if you disable the debug messages in the pcnet32
driver and you apply the patch below ?

diff --git a/drivers/net/pcnet32.c b/drivers/net/pcnet32.c
index 9c171a7..be4513f 100644
--- a/drivers/net/pcnet32.c
+++ b/drivers/net/pcnet32.c

@@ -2556,11 +2556,12 @@ pcnet32_interrupt(int irq, void *dev_id)
 	unsigned long ioaddr;
 	u16 csr0;
 	int boguscnt = max_interrupt_work;
+	unsigned long flags;
 
 	ioaddr = dev->base_addr;
 	lp = netdev_priv(dev);
 
-	spin_lock(&lp->lock);
+	spin_lock_irqsave(&lp->lock, flags);
 
 	csr0 = lp->a.read_csr(ioaddr, CSR0);
 	while ((csr0 & 0x8f00) && --boguscnt >= 0) {

@@ -2632,7 +2633,7 @@ pcnet32_interrupt(int irq, void *dev_id)
 		printk(KERN_DEBUG "%s: exiting interrupt, csr0=%#4.4x.\n",
 		       dev->name, lp->a.read_csr(ioaddr, CSR0));
 
-	spin_unlock(&lp->lock);
+	spin_unlock_irqrestore(&lp->lock, flags);
 
 	return IRQ_HANDLED;
 }

Hi,
Tested under very high console activity and it no longer freeze.

Hmm, I have been seeing lockups too and asked about doing something
almost exactly the same as this recently, but was told that it shouldn't
need irqs disabled at this point.  Well if it makes netconsole more
stable, I think I will try adding it to and see if it makes the problems
go away for good (my problem only happens at random and can be days
between it happening).

--
Len Sorensen

On Mon, May 28, 2007 at 02:31:48PM -0400, Lennart Sorensen wrote:

On Mon, May 28, 2007 at 05:25:51PM +0200, Emmanuel Fust? wrote:

quoted

quoted

Any difference if you disable the debug messages in the pcnet32
driver and you apply the patch below ?

diff --git a/drivers/net/pcnet32.c b/drivers/net/pcnet32.c
index 9c171a7..be4513f 100644
--- a/drivers/net/pcnet32.c
+++ b/drivers/net/pcnet32.c

@@ -2556,11 +2556,12 @@ pcnet32_interrupt(int irq, void *dev_id)
 	unsigned long ioaddr;
 	u16 csr0;
 	int boguscnt = max_interrupt_work;
+	unsigned long flags;
 
 	ioaddr = dev->base_addr;
 	lp = netdev_priv(dev);
 
-	spin_lock(&lp->lock);
+	spin_lock_irqsave(&lp->lock, flags);
 
 	csr0 = lp->a.read_csr(ioaddr, CSR0);
 	while ((csr0 & 0x8f00) && --boguscnt >= 0) {

@@ -2632,7 +2633,7 @@ pcnet32_interrupt(int irq, void *dev_id)
 		printk(KERN_DEBUG "%s: exiting interrupt, csr0=%#4.4x.\n",
 		       dev->name, lp->a.read_csr(ioaddr, CSR0));
 
-	spin_unlock(&lp->lock);
+	spin_unlock_irqrestore(&lp->lock, flags);
 
 	return IRQ_HANDLED;
 }

Hi,
Tested under very high console activity and it no longer freeze.

Hmm, I have been seeing lockups too and asked about doing something
almost exactly the same as this recently, but was told that it shouldn't
need irqs disabled at this point.  Well if it makes netconsole more

for normal interrupt delivery it doesn't matter, because there shouldn't
be any more interrupts coming in at that point. But netconsole uses
pcnet32_interrupt for polling the chip. So if during service of a
a real interrupt a polled pcnet32_interrupt call is done, the machine
will deadlock.

Using spin_lock_irqsave() is probably the only race free solution,
when using NET_POLL.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessary a
good idea.                                                [ RFC1925, 2.3 ]

Lennart Sorensen [off-list ref] :
[...]

Hmm, I have been seeing lockups too and asked about doing something
almost exactly the same as this recently, but was told that it shouldn't
need irqs disabled at this point.

Yes. The patch should not be needed.

OTOH, it is still interesting to know if it makes a difference, though
I do not really figure why afterwards (the local irq thread could get
interrupted and the interrupting thread printk() before returning but
I doubt that it is realistic).

Well if it makes netconsole more stable, I think I will try adding it to
and see if it makes the problems go away for good (my problem only happens
at random and can be days between it happening).

One must ensure that pcnet32_{interrupt/hard_start_xmit} do not try to
printk() too.

-- 
Ueimor

Thomas Bogendoerfer [off-list ref] :
[...]

for normal interrupt delivery it doesn't matter, because there shouldn't
be any more interrupts coming in at that point. But netconsole uses
pcnet32_interrupt for polling the chip. So if during service of a
a real interrupt a polled pcnet32_interrupt call is done, the machine
will deadlock.

Even if the driver-agnostic part of the irq processing can be interrupted
locally, the irq handlers are run sequentially on a given cpu. It does not
leave a lot of room for a deadlock-prone printk().

It _can_ happen but I'd prefer to pinpoint a specific candidate path
where it would have happened.

-- 
Ueimor