From owner-cvs-all@FreeBSD.ORG Wed Dec 6 17:48:58 2006 Return-Path: X-Original-To: cvs-all@freebsd.org Delivered-To: cvs-all@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id E13B116A415; Wed, 6 Dec 2006 17:48:58 +0000 (UTC) (envelope-from scottl@samsco.org) Received: from kremlin.juniper.net (kremlin.juniper.net [207.17.137.120]) by mx1.FreeBSD.org (Postfix) with ESMTP id 0E6AE43CA5; Wed, 6 Dec 2006 17:48:11 +0000 (GMT) (envelope-from scottl@samsco.org) Received: from unknown (HELO beta.jnpr.net) ([172.24.18.109]) by kremlin.juniper.net with ESMTP; 06 Dec 2006 09:45:48 -0800 X-IronPort-AV: i="4.09,505,1157353200"; d="scan'208"; a="620699752:sNHT35146604" Received: from [172.17.12.69] ([172.17.12.69]) by beta.jnpr.net with Microsoft SMTPSVC(6.0.3790.1830); Wed, 6 Dec 2006 09:48:58 -0800 Message-ID: <45770289.5010002@samsco.org> Date: Wed, 06 Dec 2006 09:48:57 -0800 From: Scott Long User-Agent: Mozilla Thunderbird 1.0.8-1.1.fc4 (X11/20060501) X-Accept-Language: en-us, en MIME-Version: 1.0 To: Sam Leffler References: <200612060218.kB62IfVn046324@repoman.freebsd.org> <20061206164242.A32496@delplex.bde.org> <20061206154555.GM32700@FreeBSD.org> <4576F9F7.9090503@errno.com> In-Reply-To: <4576F9F7.9090503@errno.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-OriginalArrivalTime: 06 Dec 2006 17:48:58.0140 (UTC) FILETIME=[CE41FDC0:01C7195E] Cc: src-committers@FreeBSD.org, Bruce Evans , cvs-src@FreeBSD.org, Marius Strobl , cvs-all@FreeBSD.org, Gleb Smirnoff Subject: Re: cvs commit: src/sys/pci if_xl.c if_xlreg.h X-BeenThere: cvs-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: CVS commit messages for the entire tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 06 Dec 2006 17:48:59 -0000 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