Date: Wed, 06 Dec 2006 09:48:57 -0800 From: Scott Long <scottl@samsco.org> To: Sam Leffler <sam@errno.com> Cc: src-committers@FreeBSD.org, Bruce Evans <bde@zeta.org.au>, cvs-src@FreeBSD.org, Marius Strobl <marius@FreeBSD.org>, cvs-all@FreeBSD.org, Gleb Smirnoff <glebius@FreeBSD.org> Subject: Re: cvs commit: src/sys/pci if_xl.c if_xlreg.h Message-ID: <45770289.5010002@samsco.org> In-Reply-To: <4576F9F7.9090503@errno.com> References: <200612060218.kB62IfVn046324@repoman.freebsd.org> <20061206164242.A32496@delplex.bde.org> <20061206154555.GM32700@FreeBSD.org> <4576F9F7.9090503@errno.com>
index | next in thread | previous in thread | raw e-mail
Sam Leffler wrote: > Gleb Smirnoff wrote: > >>On Wed, Dec 06, 2006 at 06:01:48PM +1100, Bruce Evans wrote: >>B> It's a shame to force all NIC drivers to manage the timeout for this. >>B> Most have a timeout for other purposes so I couldn't see how to save >>B> much code using a callback, but a callback would be cleaner. (To avoid >>B> the race, just move the decrement of the count to drivers.) >> >>It is a shame to have a two extra fields in struct ifnet, just for >>the sake of the drivers that can wedge. It is a shame to go through >>the whole list of interfaces every second. >> >>There are routers with few NICs and dozens of vlan(4) interfaces. There >>are also PPP concentrators with up to thousand interfaces and only >>one NIC that really needs to have its watchdog. >> > > > I agree with both sentiments and as the originator of the ifnet watchdog > mechanism I can only say that it's high time it was replaced by > something better. My main worry with this change is that people will > _blindly_ sweep drivers replacing what was previously a fairly > lightweight mechanism with something much more expensive. > > Sam As Gleb points out, most drivers already have a private callout for stats collecting, so piggybacking off of that doesn't present any extra load on the system. I share your concern about blind changes; the changes made to if_em were meant as a proof of concept and a means to an end for getting the driver ready for the upcoming release. I can't think of any way to keep this functionality in the if layer that doesn't incur extra locking overhead, other than to create an if_slowtimeout() instance per interface. But it's such a small function that I don't think that there is much to be gained by trying to keep it around. I hope that when people replace the if_watchdog mechanism in their driver that they also take the time to make the replacement smarter than just doing blind resets, calling into the interrupt handler, etc. Scotthome | help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?45770289.5010002>