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

Previous thread: AW: Does tc-prio really work as advertised? by Joerg Pommnitz on Tuesday, November 27, 2007 - 9:00 am. (3 messages)

Next thread: Re: Does tc-prio really work as advertised? by Joerg Pommnitz on Tuesday, November 27, 2007 - 9:07 am. (2 messages)
To: Herbert Xu <herbert@...>
Cc: netdev <netdev@...>, David S. Miller <davem@...>, Stephen Hemminger <shemminger@...>, Benjamin Herrenschmidt <benh@...>
Date: Monday, November 26, 2007 - 4:02 pm

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);
}

-

To: Johannes Berg <johannes@...>
Cc: netdev <netdev@...>, David S. Miller <davem@...>, Stephen Hemminger <shemminger@...>, Benjamin Herrenschmidt <benh@...>
Date: Thursday, November 29, 2007 - 6:57 am

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
-

To: Herbert Xu <herbert@...>
Cc: netdev <netdev@...>, David S. Miller <davem@...>, Stephen Hemminger <shemminger@...>, Benjamin Herrenschmidt <benh@...>
Date: Monday, December 3, 2007 - 5:29 am

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

To: Johannes Berg <johannes@...>
Cc: Herbert Xu <herbert@...>, netdev <netdev@...>, David S. Miller <davem@...>, Stephen Hemminger <shemminger@...>
Date: Tuesday, November 27, 2007 - 7:28 pm

Thanks.

Ben.

-

Previous thread: AW: Does tc-prio really work as advertised? by Joerg Pommnitz on Tuesday, November 27, 2007 - 9:00 am. (3 messages)

Next thread: Re: Does tc-prio really work as advertised? by Joerg Pommnitz on Tuesday, November 27, 2007 - 9:07 am. (2 messages)