From owner-cvs-all@FreeBSD.ORG Sat May 3 10:44:39 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 3F6CB37B401; Sat, 3 May 2003 10:44:39 -0700 (PDT) Received: from harmony.village.org (rover.bsdimp.com [204.144.255.66]) by mx1.FreeBSD.org (Postfix) with ESMTP id DE52743F75; Sat, 3 May 2003 10:44:37 -0700 (PDT) (envelope-from imp@bsdimp.com) Received: from localhost (warner@rover2.village.org [10.0.0.1]) by harmony.village.org (8.12.8/8.12.3) with ESMTP id h43HiPA7029941; Sat, 3 May 2003 11:44:25 -0600 (MDT) (envelope-from imp@bsdimp.com) Date: Sat, 03 May 2003 11:44:16 -0600 (MDT) Message-Id: <20030503.114416.104033857.imp@bsdimp.com> To: jhb@FreeBSD.org From: "M. Warner Losh" In-Reply-To: References: <20030501.101409.57443470.imp@bsdimp.com> X-Mailer: Mew version 2.1 on Emacs 21.2 / Mule 5.0 (SAKAKI) Mime-Version: 1.0 Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit 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: Sat, 03 May 2003 17:44:39 -0000 In message: John Baldwin writes: : : 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() This means you don't have to call the 'is my hardware there' routine in the isr. That's what gone buys you. I'm not sure that's necessarily a big thing... This does imply that we may want two ISRs, one for hot-swappable hardware and one for not, at least for the high performance drivers. Lower performance ones can elect to not care about the slight extra overhead of checking. Warner