Patchwork 3c59x: shared interrupt problem

login
register
mail settings
Submitter Steffen Klassert
Date March 11, 2009, 11:38 a.m.
Message ID <20090311113815.GB26526@newton.mathematik.tu-chemnitz.de>
Download mbox | patch
Permalink /patch/24300/
State RFC
Delegated to: David Miller
Headers show

Comments

Steffen Klassert - March 11, 2009, 11:38 a.m.
On Tue, Mar 10, 2009 at 02:55:42PM -0700, Andrew Morton wrote:
> > 
> > This basically reverts a patch from akpm (bitkeeper cset 1.1046.95.8)
> > This patch was to workaround lots of "nobody cared" warnings generated by
> > boomerang_interrupt(). 
> > I added Andrew to the Cc, perhaps he can remember some details on this.
> > 
> 
> Beats me.  Do you havea full copy of that patch, including changelog?
> 

It was this one:

#### ChangeSet ####
2003-05-19 10:27:49-07:00, akpm@digeo.com 
  [PATCH] 3c59x irqreturn fix
  
  Apparently boomerang_interrupt() is generating lots of "nobody cared"
  warnings - one per packet it seems.  Frankly, I don't have a clue why.
  
  These are ancient cards and the driver is otherwise stable, so just
  change it to return IRQ_HANDLED and move on...

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - March 13, 2009, 10:51 p.m.
From: Steffen Klassert <klassert@mathematik.tu-chemnitz.de>
Date: Wed, 11 Mar 2009 12:38:15 +0100

> On Tue, Mar 10, 2009 at 02:55:42PM -0700, Andrew Morton wrote:
> > > 
> > > This basically reverts a patch from akpm (bitkeeper cset 1.1046.95.8)
> > > This patch was to workaround lots of "nobody cared" warnings generated by
> > > boomerang_interrupt(). 
> > > I added Andrew to the Cc, perhaps he can remember some details on this.
> > > 
> > 
> > Beats me.  Do you havea full copy of that patch, including changelog?
> > 
> 
> It was this one:
> 
> #### ChangeSet ####
> 2003-05-19 10:27:49-07:00, akpm@digeo.com 
>   [PATCH] 3c59x irqreturn fix
>   
>   Apparently boomerang_interrupt() is generating lots of "nobody cared"
>   warnings - one per packet it seems.  Frankly, I don't have a clue why.
>   
>   These are ancient cards and the driver is otherwise stable, so just
>   change it to return IRQ_HANDLED and move on...

So basically it's a band-aid because we didn't investigate why
this happens.

I think we should put the change in, and then look into things
properly if users report this issue again.

The code there right now is just completely wrong when the
3c59x interrupt is shared with another device.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Steffen Klassert - March 14, 2009, 2:08 p.m.
On Fri, Mar 13, 2009 at 03:51:16PM -0700, David Miller wrote:
> > #### ChangeSet ####
> > 2003-05-19 10:27:49-07:00, akpm@digeo.com 
> >   [PATCH] 3c59x irqreturn fix
> >   
> >   Apparently boomerang_interrupt() is generating lots of "nobody cared"
> >   warnings - one per packet it seems.  Frankly, I don't have a clue why.
> >   
> >   These are ancient cards and the driver is otherwise stable, so just
> >   change it to return IRQ_HANDLED and move on...
> 
> So basically it's a band-aid because we didn't investigate why
> this happens.
> 
> I think we should put the change in, and then look into things
> properly if users report this issue again.
> 

Gerhard reported at least one of these "nobody cared" messages at shutdown
after applying this change. He wanted to provide us with further informations
about this issue next week. Best would be to put it in together with a fix.
So I would suggest to wait, perhaps we can fix it with his informations. 


> The code there right now is just completely wrong when the
> 3c59x interrupt is shared with another device.

Indeed.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - March 14, 2009, 6:40 p.m.
From: Steffen Klassert <klassert@mathematik.tu-chemnitz.de>
Date: Sat, 14 Mar 2009 15:08:41 +0100

> Gerhard reported at least one of these "nobody cared" messages at shutdown
> after applying this change. He wanted to provide us with further informations
> about this issue next week. Best would be to put it in together with a fix.
> So I would suggest to wait, perhaps we can fix it with his informations. 

Fair enough.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Gerhard Pircher - March 17, 2009, 9:37 a.m.
-------- Original-Nachricht --------
> Datum: Sat, 14 Mar 2009 15:08:41 +0100
> Von: Steffen Klassert <klassert@mathematik.tu-chemnitz.de>
> An: David Miller <davem@davemloft.net>
> CC: akpm@linux-foundation.org, shemminger@vyatta.com, gerhard_pircher@gmx.net, netdev@vger.kernel.org
> Betreff: Re: 3c59x: shared interrupt problem

> On Fri, Mar 13, 2009 at 03:51:16PM -0700, David Miller wrote:
> > > #### ChangeSet ####
> > > 2003-05-19 10:27:49-07:00, akpm@digeo.com 
> > >   [PATCH] 3c59x irqreturn fix
> > >   
> > >   Apparently boomerang_interrupt() is generating lots of "nobody
> > >   cared" warnings - one per packet it seems.  Frankly, I don't have
> > >   a clue why.
> > >   
> > >   These are ancient cards and the driver is otherwise stable, so
> > >   just change it to return IRQ_HANDLED and move on...
> > 
> > So basically it's a band-aid because we didn't investigate why
> > this happens.
> > 
> > I think we should put the change in, and then look into things
> > properly if users report this issue again.
> > 
> 
> Gerhard reported at least one of these "nobody cared" messages at
> shutdown after applying this change. He wanted to provide us with
> further informations about this issue next week. Best would be to put
> it in together with a fix.
> So I would suggest to wait, perhaps we can fix it with his
> informations.
Okay, here's a small status update in order to show that I'm really
doing something. :)
Increasing the value of vortex_debug doesn't really help. It slows
down the network transfer too much to trigger the bug. Does somebody
know where to insert some printks in the driver to get a useful debug
output?
Bisecting shows that even v2.6.26-rc1 fails. But I have a v2.6.27-rc7
image that works fine!? Looks like I have to put more effort in
bisecting.

Gerhard

Patch

==== drivers/net/3c59x.c ====
2003-05-17 14:09:34-07:00, akpm@digeo.com +2 -7
  3c59x irqreturn fix

--- 1.34/drivers/net/3c59x.c	2003-04-20 22:41:08 -07:00
+++ 1.35/drivers/net/3c59x.c	2003-05-17 14:09:34 -07:00
@@ -2321,7 +2321,6 @@  boomerang_interrupt(int irq, void *dev_i
 	long ioaddr;
 	int status;
 	int work_done = max_interrupt_work;
-	int handled;
 
 	ioaddr = dev->base_addr;
 
@@ -2336,18 +2335,14 @@  boomerang_interrupt(int irq, void *dev_i
 	if (vortex_debug > 6)
 		printk(KERN_DEBUG "boomerang_interrupt. status=0x%4x\n", status);
 
-	if ((status & IntLatch) == 0) {
-		handled = 0;
+	if ((status & IntLatch) == 0)
 		goto handler_exit;		/* No interrupt: shared IRQs can cause this */
-	}
 
 	if (status == 0xffff) {		/* h/w no longer present (hotplug)? */
 		if (vortex_debug > 1)
 			printk(KERN_DEBUG "boomerang_interrupt(1): status = 0xffff\n");
-		handled = 0;
 		goto handler_exit;
 	}
-	handled = 1;
 
 	if (status & IntReq) {
 		status |= vp->deferred;
@@ -2442,7 +2437,7 @@  boomerang_interrupt(int irq, void *dev_i
 			   dev->name, status);
 handler_exit:
 	spin_unlock(&vp->lock);
-	return IRQ_RETVAL(handled);
+	return IRQ_HANDLED;
 }
 
 static int vortex_rx(struct net_device *dev)