| Message ID | 1353795081.26346.287.camel@shinybook.infradead.org |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
David Woodhouse <dwmw2@infradead.org> : > Recovery doesn't work too well if we leave interrupts disabled... > > Signed-off-by: David Woodhouse <David.Woodhouse@intel.com> Acked-by: Francois Romieu <romieu@fr.zoreil.com> While you are at it, do you see what would prevent the watchdog timer and the rx napi handler to race on two different CPUs ("using a Geode" could be an answer :o) ) ? > --- > For stable too, I suppose. Yes. Watch for http://patchwork.ozlabs.org/bundle/davem/stable/
On Sat, 2012-11-24 at 23:58 +0100, Francois Romieu wrote: > While you are at it, do you see what would prevent the watchdog > timer and the rx napi handler to race on two different CPUs Hm, good point. Is it sufficient just to stick napi_disable() and napi_enable() in there? And should we try to reset the TX state machine without stomping on RX at all?
From: Francois Romieu <romieu@fr.zoreil.com> Date: Sat, 24 Nov 2012 23:58:55 +0100 > David Woodhouse <dwmw2@infradead.org> : >> Recovery doesn't work too well if we leave interrupts disabled... >> >> Signed-off-by: David Woodhouse <David.Woodhouse@intel.com> > > Acked-by: Francois Romieu <romieu@fr.zoreil.com> Applied to net-next -- 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 Woodhouse <dwmw2@infradead.org> : > On Sat, 2012-11-24 at 23:58 +0100, Francois Romieu wrote: > > While you are at it, do you see what would prevent the watchdog > > timer and the rx napi handler to race on two different CPUs > > Hm, good point. Is it sufficient just to stick napi_disable() and > napi_enable() in there? No, napi_disable() may sleep. > And should we try to reset the TX state machine without stomping on > RX at all ? Either that or the watchdog timer will have to queue some user context work. Things may change later when you go lockless (hint, hint).
diff --git a/drivers/net/ethernet/realtek/8139cp.c b/drivers/net/ethernet/realtek/8139cp.c index 1c81825..ad6afc3 100644 --- a/drivers/net/ethernet/realtek/8139cp.c +++ b/drivers/net/ethernet/realtek/8139cp.c @@ -1192,6 +1192,7 @@ static void cp_tx_timeout(struct net_device *dev) cp_clean_rings(cp); rc = cp_init_rings(cp); cp_start_hw(cp); + cp_enable_irq(cp); netif_wake_queue(dev);
Recovery doesn't work too well if we leave interrupts disabled... Signed-off-by: David Woodhouse <David.Woodhouse@intel.com> --- For stable too, I suppose.