From owner-cvs-all@FreeBSD.ORG Wed Apr 30 13:52:48 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 F41A037B401; Wed, 30 Apr 2003 13:52:47 -0700 (PDT) Received: from magic.adaptec.com (magic-mail.adaptec.com [208.236.45.100]) by mx1.FreeBSD.org (Postfix) with ESMTP id D1B7943FBF; Wed, 30 Apr 2003 13:52:46 -0700 (PDT) (envelope-from gibbs@scsiguy.com) Received: from redfish.adaptec.com (redfish.adaptec.com [162.62.50.11]) by magic.adaptec.com (8.11.6/8.11.6) with ESMTP id h3UKnkZ16608; Wed, 30 Apr 2003 13:49:46 -0700 Received: from btc.btc.adaptec.com ([10.100.0.52]) by redfish.adaptec.com (8.8.8p2+Sun/8.8.8) with ESMTP id NAA16881; Wed, 30 Apr 2003 13:52:39 -0700 (PDT) Received: from [10.100.253.70] (aslan [10.100.253.70]) by btc.btc.adaptec.com (8.8.8+Sun/8.8.8) with ESMTP id OAA02889; Wed, 30 Apr 2003 14:52:34 -0600 (MDT) Date: Wed, 30 Apr 2003 14:52:33 -0600 From: "Justin T. Gibbs" To: John Baldwin , Andrew Gallatin Message-ID: <1417270000.1051735953@aslan.btc.adaptec.com> In-Reply-To: References: X-Mailer: Mulberry/3.0.3 (Linux/x86) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Content-Disposition: inline cc: cvs-src@freebsd.org cc: src-committers@freebsd.org cc: "M. Warner Losh" 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 Reply-To: "Justin T. Gibbs" List-Id: CVS commit messages for the entire tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 30 Apr 2003 20:52:48 -0000 > First off, it's a gross API violation. ithread's are supposed to be > transparent (except that you can use locks in your interrupt handlers). > Secondly, you still have the same race as if you just removed the gone > condition. We don't hold the ithread lock while executing handlers, > so there is nothing preventing the handler from being executed on another > CPU concurrently with your detach function. In fact, it could easily > be blocked on the FXP lock. You do your magic pointer foo, then unlock > the fxp. The unlock releases the interrupt handler on another CPU > which happily executes _after_ the completion of bus_teardown_intr() > which has all sorts of bad failure modes, like panicing when it tries > to FXP_UNLOCK at the end because back on the first CPU you've done a > mtx_destroy(). I could have sworn I talked to you about this a *long* time ago and requested that bus_teardown_intr() blocked until you were guaranteed to be out of the interrupt handler. The aic7xxx and aic79xx drivers depend on this behavior: ahc_lock(ahc); /* * Disable hardware interrupts so we don't lock the * machine once our handler is removed. */ ahc_enable_intr(ahc, FALSE); ahc_unlock(ahc); /* * Called without our lock so that if we are in our interrupt * handler the handler can complete. */ bus_teardown_intr(); /* continue with detach. */ This means that all detaches must occur from a context that can sleep, but that shouldn't be too hard to make happen. -- Justin