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>
next in thread | previous in thread | raw e-mail | index | archive | help
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. Scott
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?45770289.5010002>