Date: Wed, 06 Dec 2006 09:42:31 -0800 From: Sam Leffler <sam@errno.com> To: Gleb Smirnoff <glebius@FreeBSD.org> Cc: cvs-src@FreeBSD.org, Marius Strobl <marius@FreeBSD.org>, src-committers@FreeBSD.org, cvs-all@FreeBSD.org, Bruce Evans <bde@zeta.org.au> Subject: Re: cvs commit: src/sys/pci if_xl.c if_xlreg.h Message-ID: <45770107.9060404@errno.com> In-Reply-To: <20061206173210.GR32700@FreeBSD.org> References: <200612060218.kB62IfVn046324@repoman.freebsd.org> <20061206164242.A32496@delplex.bde.org> <20061206154555.GM32700@FreeBSD.org> <4576F9F7.9090503@errno.com> <20061206173210.GR32700@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Gleb Smirnoff wrote: > On Wed, Dec 06, 2006 at 09:12:23AM -0800, Sam Leffler wrote: > S> > On Wed, Dec 06, 2006 at 06:01:48PM +1100, Bruce Evans wrote: > S> > B> It's a shame to force all NIC drivers to manage the timeout for this. > S> > B> Most have a timeout for other purposes so I couldn't see how to save > S> > B> much code using a callback, but a callback would be cleaner. (To avoid > S> > B> the race, just move the decrement of the count to drivers.) > S> > > S> > It is a shame to have a two extra fields in struct ifnet, just for > S> > the sake of the drivers that can wedge. It is a shame to go through > S> > the whole list of interfaces every second. > S> > > S> > There are routers with few NICs and dozens of vlan(4) interfaces. There > S> > are also PPP concentrators with up to thousand interfaces and only > S> > one NIC that really needs to have its watchdog. > S> > S> I agree with both sentiments and as the originator of the ifnet watchdog > S> mechanism I can only say that it's high time it was replaced by > S> something better. My main worry with this change is that people will > S> _blindly_ sweep drivers replacing what was previously a fairly > S> lightweight mechanism with something much more expensive. > > I have thought a little bit about this. Even in case if every interface > in system schedules its own watchdog, a search through the list of scheduled > callouts isn't worse than going through the list of all interfaces, as we > have now. And it is locked finer, since doesn't holds the IFNET_RLOCK, > just holds the mutex of current callwheel slot. > > Since most drivers already have periodic (usually 1 sec) callouts, > piggybacking on them will not increase number of scheduled callouts. > I've thought about it too. I tend to agree with you but am not certain. I'm also worried about wireless drivers that are layered on top of net80211. The callback from the driver to ieee80211_watchdog was a total hack that was done because the driver's softc lock was not available to grab to protect the com structure (and also because I was lazy :)). I've had changes for ~2 years in p4 to eliminate this callback by handling all the timer processing in the net80211 layer but it's only really been tested with ath. I also eliminated the use of the timer to time out mgt frames; instead requiring drivers to implement a callback mechanism on tx complete. This has significant implications but also reduces the current 5 second timeout to <20ms (typical) which makes things like associating to an ap move much faster. Anyway the point is that fixing up the watchdog timer stuff isn't necessarily a simple "swap if_timer for a callout"; it can be more involved if you examine all the work that potentially is triggered from the routines. Sam
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?45770107.9060404>