[PATCH] sungem: fix napi regression with reset work

[PATCH] sungem: fix napi regression with reset work

[Johannes Berg]

sungem's gem_reset_task() will unconditionally try to disable NAPI even
when it's called while the interface is not operating and hence the NAPI
struct isn't enabled. Make napi_disable() depend on gp->running.

Also removes a superfluous test of gp->running in the same function.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
Not exactly sure how I got there but I did get stuck in there (sysrq-W)
when I plugged in the network cable and booted the computer on the other
end of it... while the interface was down. I would've thought that the
driver wouldn't even notice the fact that I plugged in the cable while
the interface is down, but maybe it does. Or it sets up the device
wrongly at resume time when the interface was down at suspend.

In any case, this should fix the issue.

 drivers/net/sungem.c |   11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

--- everything.orig/drivers/net/sungem.c	2007-11-26 20:48:11.489044815 +0100
+++ everything/drivers/net/sungem.c	2007-11-26 21:01:28.519045736 +0100
@@ -2279,34 +2279,33 @@ static void gem_reset_task(struct work_s
 {
 	struct gem *gp = container_of(work, struct gem, reset_task);
 
 	mutex_lock(&gp->pm_mutex);
 
-	napi_disable(&gp->napi);
+	if (gp->opened)
+		napi_disable(&gp->napi);
 
 	spin_lock_irq(&gp->lock);
 	spin_lock(&gp->tx_lock);
 
-	if (gp->running == 0)
-		goto not_running;
-
 	if (gp->running) {
 		netif_stop_queue(gp->dev);
 
 		/* Reset the chip & rings */
 		gem_reinit_chip(gp);
 		if (gp->lstate == link_up)
 			gem_set_link_modes(gp);
 		netif_wake_queue(gp->dev);
 	}
- not_running:
+
 	gp->reset_task_pending = 0;
 
 	spin_unlock(&gp->tx_lock);
 	spin_unlock_irq(&gp->lock);
 
-	napi_enable(&gp->napi);
+	if (gp->opened)
+		napi_enable(&gp->napi);
 
 	mutex_unlock(&gp->pm_mutex);
 }
 
 

Re: [PATCH] sungem: fix napi regression with reset work

[Benjamin Herrenschmidt]

On Mon, 2007-11-26 at 21:02 +0100, Johannes Berg wrote:
> sungem's gem_reset_task() will unconditionally try to disable NAPI even
> when it's called while the interface is not operating and hence the NAPI
> struct isn't enabled. Make napi_disable() depend on gp->running.
> 
> Also removes a superfluous test of gp->running in the same function.
> 
> Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
> ---
> Not exactly sure how I got there but I did get stuck in there (sysrq-W)
> when I plugged in the network cable and booted the computer on the other
> end of it... while the interface was down. I would've thought that the
> driver wouldn't even notice the fact that I plugged in the cable while
> the interface is down, but maybe it does. Or it sets up the device
> wrongly at resume time when the interface was down at suspend.
> 
> In any case, this should fix the issue.

Thanks.

Ben.

Re: [PATCH] sungem: fix napi regression with reset work

[Herbert Xu]

On Mon, Nov 26, 2007 at 09:02:08PM +0100, Johannes Berg wrote:
> sungem's gem_reset_task() will unconditionally try to disable NAPI even
> when it's called while the interface is not operating and hence the NAPI
> struct isn't enabled. Make napi_disable() depend on gp->running.
> 
> Also removes a superfluous test of gp->running in the same function.
> 
> Signed-off-by: Johannes Berg <johannes@sipsolutions.net>

Patch applied to net-2.6.  Thanks Johannes!
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] sungem: fix napi regression with reset work

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 758 bytes --]


On Thu, 2007-11-29 at 21:57 +1100, Herbert Xu wrote:
> On Mon, Nov 26, 2007 at 09:02:08PM +0100, Johannes Berg wrote:
> > sungem's gem_reset_task() will unconditionally try to disable NAPI even
> > when it's called while the interface is not operating and hence the NAPI
> > struct isn't enabled. Make napi_disable() depend on gp->running.
> > 
> > Also removes a superfluous test of gp->running in the same function.
> > 
> > Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
> 
> Patch applied to net-2.6.  Thanks Johannes!

Thanks. I just noticed that I made a mistake in the patch description,
it should read "Make napi_disable() depend on gp->opened". I don't think
it matters much but wanted to at least mention it.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]