From owner-cvs-all@FreeBSD.ORG Wed Dec 6 17:42:36 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 4938F16A525; Wed, 6 Dec 2006 17:42:36 +0000 (UTC) (envelope-from sam@errno.com) Received: from ebb.errno.com (ebb.errno.com [69.12.149.25]) by mx1.FreeBSD.org (Postfix) with ESMTP id 58FE043CBB; Wed, 6 Dec 2006 17:41:47 +0000 (GMT) (envelope-from sam@errno.com) Received: from [10.0.0.248] (trouble.errno.com [10.0.0.248]) (authenticated bits=0) by ebb.errno.com (8.13.6/8.12.6) with ESMTP id kB6HgWr0075028 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Wed, 6 Dec 2006 09:42:33 -0800 (PST) (envelope-from sam@errno.com) Message-ID: <45770107.9060404@errno.com> Date: Wed, 06 Dec 2006 09:42:31 -0800 From: Sam Leffler User-Agent: Thunderbird 1.5.0.7 (X11/20060920) MIME-Version: 1.0 To: Gleb Smirnoff References: <200612060218.kB62IfVn046324@repoman.freebsd.org> <20061206164242.A32496@delplex.bde.org> <20061206154555.GM32700@FreeBSD.org> <4576F9F7.9090503@errno.com> <20061206173210.GR32700@FreeBSD.org> In-Reply-To: <20061206173210.GR32700@FreeBSD.org> X-Enigmail-Version: 0.94.0.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: cvs-src@FreeBSD.org, Marius Strobl , src-committers@FreeBSD.org, cvs-all@FreeBSD.org, Bruce Evans 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:42:36 -0000 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