cvs commit: src/sys/pci if_xl.c if_xlreg.h

Sam Leffler sam at errno.com
Wed Dec 6 09:42:36 PST 2006


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


More information about the cvs-src mailing list