From owner-cvs-src@FreeBSD.ORG Thu May 1 10:50:50 2003 Return-Path: Delivered-To: cvs-src@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 2B93737B401; Thu, 1 May 2003 10:50:50 -0700 (PDT) Received: from mailman.zeta.org.au (mailman.zeta.org.au [203.26.10.16]) by mx1.FreeBSD.org (Postfix) with ESMTP id ED19E43F85; Thu, 1 May 2003 10:50:47 -0700 (PDT) (envelope-from bde@zeta.org.au) Received: from katana.zip.com.au (katana.zip.com.au [61.8.7.246]) by mailman.zeta.org.au (8.9.3p2/8.8.7) with ESMTP id DAA24751; Fri, 2 May 2003 03:50:27 +1000 Date: Fri, 2 May 2003 03:50:26 +1000 (EST) From: Bruce Evans X-X-Sender: bde@gamplex.bde.org To: "M. Warner Losh" In-Reply-To: <20030501.101409.57443470.imp@bsdimp.com> Message-ID: <20030502033051.C20181@gamplex.bde.org> References: <1721460000.1051803729@aslan.btc.adaptec.com> <20030501.101409.57443470.imp@bsdimp.com> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII cc: src-committers@FreeBSD.org cc: gallatin@cs.duke.edu cc: jhb@FreeBSD.org cc: gibbs@scsiguy.com cc: cvs-src@FreeBSD.org cc: cvs-all@FreeBSD.org Subject: Re: cvs commit: src/sys/dev/fxp if_fxp.c if_fxpvar.h X-BeenThere: cvs-src@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: CVS commit messages for the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 01 May 2003 17:50:50 -0000 On Thu, 1 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: You can't easily turn it off either. It may be running, or about to be run, and running it may turn interrupts back on. Perhaps fxp_intr() doesn't need to do this, but some interrupt handler do. They would need potentially expensive flags or locks or a different interrupt handler to tell them to act differently during detach. Perhaps a single lock is enough, but this thread started with a complaint that a single flag test was too expensive (as it may be when you do it a billion times here and there). > 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. It can also physically happen before [4], but might not be very visible to fxp_detach() since it is blocked before or in fxp_intr() on another lock. It just can't proceed past the lock in [4] if it uses that lock to synchronize with fxp_detach() (as it probably should). This case looks almost the same to fxp_detach() as an interrupt after [4]. Bruce