Message ID | 20090206112902.GS4362@tango.lnet.fi |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
From: Ilkka Virta <itvirta@iki.fi> Date: Fri, 6 Feb 2009 13:29:02 +0200 > Looking at gem_do_start() and gem_open(), it seems that the only thing > done while opening the device after the request_irq(), is a call to > napi_enable(). > > I don't know what the ordering requirements are for the > initialization, but I boldly tried to move the napi_enable() call > inside gem_do_start() before the link state is checked and interrupts > subsequently enabled, and it seems to work for me. Doesn't even break > anything too obvious... > > Any ideas on how this really should be fixed? Actually your fix looks good, I'll apply this :-) Thanks! -- 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 wrote, On 02/07/2009 07:01 AM: > From: Ilkka Virta <itvirta@iki.fi> > Date: Fri, 6 Feb 2009 13:29:02 +0200 > >> Looking at gem_do_start() and gem_open(), it seems that the only thing >> done while opening the device after the request_irq(), is a call to >> napi_enable(). >> >> I don't know what the ordering requirements are for the >> initialization, but I boldly tried to move the napi_enable() call >> inside gem_do_start() before the link state is checked and interrupts >> subsequently enabled, and it seems to work for me. Doesn't even break >> anything too obvious... >> >> Any ideas on how this really should be fixed? > > Actually your fix looks good, I'll apply this :-) Alas it could be not enough. It seems this problem is caused by not serving interrupts if napi is disabled. This patch added napi_enable() on one path, but e.g. here: static int gem_close(struct net_device *dev) { struct gem *gp = netdev_priv(dev); mutex_lock(&gp->pm_mutex); napi_disable(&gp->napi); gp->opened = 0; if (!gp->asleep) gem_do_stop(dev, 0); ... similar storm can happen if an interrupt is triggered just after napi_disable(). Jarek P. -- 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
--- linux-2.6.28.2/drivers/net/sungem.c.orig 2009-01-25 02:42:07.000000000 +0200 +++ linux-2.6.28.2/drivers/net/sungem.c 2009-02-05 20:46:23.000000000 +0200 @@ -2222,6 +2222,8 @@ static int gem_do_start(struct net_devic gp->running = 1; + napi_enable(&gp->napi); + if (gp->lstate == link_up) { netif_carrier_on(gp->dev); gem_set_link_modes(gp); @@ -2239,6 +2241,8 @@ static int gem_do_start(struct net_devic spin_lock_irqsave(&gp->lock, flags); spin_lock(&gp->tx_lock); + napi_disable(&gp->napi); + gp->running = 0; gem_reset(gp); gem_clean_rings(gp); @@ -2339,8 +2343,6 @@ static int gem_open(struct net_device *d if (!gp->asleep) rc = gem_do_start(dev); gp->opened = (rc == 0); - if (gp->opened) - napi_enable(&gp->napi); mutex_unlock(&gp->pm_mutex); @@ -2477,8 +2479,6 @@ static int gem_resume(struct pci_dev *pd /* Re-attach net device */ netif_device_attach(dev); - - napi_enable(&gp->napi); } spin_lock_irqsave(&gp->lock, flags);