From owner-cvs-all@FreeBSD.ORG Tue Apr 29 11:44:27 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 9E09637B404 for ; Tue, 29 Apr 2003 11:44:27 -0700 (PDT) Received: from mail.speakeasy.net (mail15.speakeasy.net [216.254.0.215]) by mx1.FreeBSD.org (Postfix) with ESMTP id 2A76943FD7 for ; Tue, 29 Apr 2003 11:44:25 -0700 (PDT) (envelope-from jhb@FreeBSD.org) Received: (qmail 10312 invoked from network); 29 Apr 2003 18:44:30 -0000 Received: from unknown (HELO server.baldwin.cx) ([216.27.160.63]) (envelope-sender )encrypted SMTP for ; 29 Apr 2003 18:44:30 -0000 Received: from laptop.baldwin.cx (gw1.twc.weather.com [216.133.140.1]) by server.baldwin.cx (8.12.8/8.12.8) with ESMTP id h3TIiLOv019076; Tue, 29 Apr 2003 14:44:22 -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: Date: Tue, 29 Apr 2003 14:44:27 -0400 (EDT) From: John Baldwin To: Nate Lawson cc: cvs-src@FreeBSD.org cc: src-committers@FreeBSD.org cc: 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 List-Id: CVS commit messages for the entire tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 29 Apr 2003 18:44:28 -0000 On 29-Apr-2003 Nate Lawson wrote: > On Mon, 28 Apr 2003, Warner Losh wrote: >> Modified files: >> sys/dev/fxp if_fxp.c if_fxpvar.h >> Log: >> Fix 5 bugs: >> 1) always call fxp_stop in fxp_detach. Since we don't read from >> the card, there's no need to carefully look at things with >> bus_child_present. > > However, we do write to the card registers (i.e. to disable interrupt > generation). Since you were the one who suggested I should add these > calls, can you give more information about when bus_child_present should > be used (and update the man page if anything changed)? > >> 2) Call FXP_UNLOCK() before calling bus_teardown_intr to avoid >> a possible deadlock reported by jhb. > > This adds a race since fxp_intr could occur after the unlock but before > the bus_teardown_intr call. The reason why I tore down the intr while > holding the lock is so fxp_intr would be prevented from accessing the > device until it has been disabled. Then the normal checks in fxp_intr > (IFF_OACTIVE or whatever) would show the card is gone and return without > accessing it. I guess this is ok since ether_ifdetach is still called > with the lock held (since it is what clears IFF_OACTIVE) but I'm > interested in your thoughts. > >> 3) add gone to the softc. Set it to true in detach. >> 4) Return immediately if gone is true in fxp_ioctl >> 5) Return immediately if gone is true in fxp_intr > > Not sure this approach is necessary. It is quite necessary: /* * If the interrupt thread is already running, then just mark this * handler as being dead and let the ithread do the actual removal. */ mtx_lock_spin(&sched_lock); if (!TD_AWAITING_INTR(ithread->it_td)) { handler->ih_flags |= IH_DEAD; ... mtx_unlock_spin(&sched_lock); if ((handler->ih_flags & IH_DEAD) != 0) msleep(handler, &ithread->it_lock, PUSER, "itrmh", 0); and in ithread_loop(): if ((ih->ih_flags & IH_DEAD) != 0) { mtx_lock(&ithd->it_lock); TAILQ_REMOVE(&ithd->it_handlers, ih, ih_next); wakeup(ih); mtx_unlock(&ithd->it_lock); goto restart; } This is how we ensure that the interrupt handler is never executed once bus_teardown_intr() returns. -- John Baldwin <>< http://www.FreeBSD.org/~jhb/ "Power Users Use the Power to Serve!" - http://www.FreeBSD.org/