From owner-cvs-all@FreeBSD.ORG Thu May 1 11:05:17 2003 Return-Path: Delivered-To: cvs-all@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 6F8A437B405 for ; Thu, 1 May 2003 11:05:17 -0700 (PDT) Received: from mail.speakeasy.net (mail15.speakeasy.net [216.254.0.215]) by mx1.FreeBSD.org (Postfix) with ESMTP id CDB4643F85 for ; Thu, 1 May 2003 11:05:13 -0700 (PDT) (envelope-from jhb@FreeBSD.org) Received: (qmail 1088 invoked from network); 1 May 2003 18:05:18 -0000 Received: from unknown (HELO server.baldwin.cx) ([216.27.160.63]) (envelope-sender )encrypted SMTP for ; 1 May 2003 18:05:18 -0000 Received: from laptop.baldwin.cx ([216.133.140.1]) by server.baldwin.cx (8.12.8/8.12.8) with ESMTP id h41I5AOv026315; Thu, 1 May 2003 14:05:11 -0400 (EDT) (envelope-from jhb@FreeBSD.org) Message-ID: X-Mailer: XFMail 1.5.4 on FreeBSD X-Priority: 3 (Normal) Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 8bit MIME-Version: 1.0 In-Reply-To: <20030501.101409.57443470.imp@bsdimp.com> Date: Thu, 01 May 2003 14:05:15 -0400 (EDT) From: John Baldwin To: "M. Warner Losh" cc: gibbs@scsiguy.com cc: cvs-src@freebsd.org cc: src-committers@freebsd.org cc: gallatin@cs.duke.edu cc: cvs-all@freebsd.org Subject: Re: cvs commit: src/sys/dev/fxp if_fxp.c if_fxpvar.h X-BeenThere: cvs-all@freebsd.org X-Mailman-Version: 2.1.1 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: Thu, 01 May 2003 18:05:17 -0000 On 01-May-2003 M. Warner Losh wrote: > In message: <1721460000.1051803729@aslan.btc.adaptec.com> > "Justin T. Gibbs" writes: >: >> This means that all detaches must occur from a context that can >: >> sleep, but that shouldn't be too hard to make happen. >: > >: > People can't hold the driver lock across bus_teardown_intr() with this >: > model, which does require a possibly smarter interrupt routine or >: > maybe a better detach that only disables interrupts then does a teardown, >: > then finishes shutting down the rest of the hardware along with an >: > interrupt handler that doesn't re-enable interrupts in the shutdown case. >: >: All doable things for all but really broken hardware. fxp is not broken. > > The whole reason for the gone flag may be misunderstood here. You can > easily turn off the fxp device, and there will be no more interrupts > from it. However, its ISR can and will still be called from time to > time until the bus_teardown_intr() is complete? Why you ask? Because > of shared interrupts. If fxp shares an interrupt with another device, > your ISR will execute even if you write 0 into the interrupt enable > register if that other device gets an interrupt between the time you > write to this register and the time bus_teardown_intr is called, even > on a single CPU machine: > > > fxp_detach() > [4] LOCK > [a] write 0 to dis intr > [5] device B on same intr interrupts here > fxp_intr() > LOCK (->sleep) > [b] gone = 0; > UNLOCK > [1] if (gone) return; > [2] bus_teardown_intr(); > [3] bus_teardown_intr returns > > > [1] and [2] can happen in any order, but you know both of them have > happened by [3]. > > The order of [a] and [b] don't really matter because fxp (or anything > that shares its interrupt) could generate an interrupt after the lock > is taken out at [4] and you'd still have a fxp_intr sleeping thread. > The important thing is that an interrupt[5] happens after [4]. This > can happen on both the single CPU case and the SMP case. > > This might argue for blocking interrupts during a device detach. I > think there might be problems with that apprach as well, although I'd > have to think about it a bit to be sure. Ok, here's a question. Why is it bad for fxp_intr() to finish while you are blocked in bus_teardown_intr()? Put another way, perhaps fxp_detach() should do the teardown_intr() a lot sooner and postpone some of it's cleanups until after that retuns. I.e. FXP_LOCK() disable_interrupts_in_hardware() FXP_UNLOCK() bus_teardown_intr() FXP_LOCK() do_rest_of_detach() -- John Baldwin <>< http://www.FreeBSD.org/~jhb/ "Power Users Use the Power to Serve!" - http://www.FreeBSD.org/