From owner-cvs-all@FreeBSD.ORG Fri Sep 16 20:00:17 2005 Return-Path: X-Original-To: cvs-all@FreeBSD.org 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 78C8616A420; Fri, 16 Sep 2005 20:00:17 +0000 (GMT) (envelope-from imp@bsdimp.com) Received: from harmony.bsdimp.com (vc4-2-0-87.dsl.netrack.net [199.45.160.85]) by mx1.FreeBSD.org (Postfix) with ESMTP id E522943D48; Fri, 16 Sep 2005 20:00:16 +0000 (GMT) (envelope-from imp@bsdimp.com) Received: from localhost (localhost.village.org [127.0.0.1] (may be forged)) by harmony.bsdimp.com (8.13.3/8.13.3) with ESMTP id j8GJwT3s052629; Fri, 16 Sep 2005 13:58:29 -0600 (MDT) (envelope-from imp@bsdimp.com) Date: Fri, 16 Sep 2005 13:58:41 -0600 (MDT) Message-Id: <20050916.135841.130619528.imp@bsdimp.com> To: ru@FreeBSD.org From: "M. Warner Losh" In-Reply-To: <20050916194405.GB24879@ip.net.ua> References: <20050916091928.GG88456@ip.net.ua> <20050916.090140.58827157.imp@bsdimp.com> <20050916194405.GB24879@ip.net.ua> X-Mailer: Mew version 3.3 on Emacs 21.3 / Mule 5.0 (SAKAKI) Mime-Version: 1.0 Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-2.0 (harmony.bsdimp.com [127.0.0.1]); Fri, 16 Sep 2005 13:58:30 -0600 (MDT) Cc: cvs-src@FreeBSD.org, src-committers@FreeBSD.org, cvs-all@FreeBSD.org, jhb@FreeBSD.org Subject: Re: cvs commit: src/sys/dev/re if_re.c X-BeenThere: cvs-all@freebsd.org X-Mailman-Version: 2.1.5 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: Fri, 16 Sep 2005 20:00:17 -0000 In message: <20050916194405.GB24879@ip.net.ua> Ruslan Ermilov writes: : On Fri, Sep 16, 2005 at 09:01:40AM -0600, M. Warner Losh wrote: : > In message: <20050916091928.GG88456@ip.net.ua> : > Ruslan Ermilov writes: : > : On Thu, Sep 15, 2005 at 11:56:39PM +0300, Ruslan Ermilov wrote: : > : > The first is the BPF detach bad interaction with foo_detach(), : > : > as described in re_detach(). This panic is real with (I think) : > : > all drivers. And testing IFF_DRV_RUNNING here doesn't seem to : > : > be able to prevent the panic. Perhaps the fix would be to : > : > move ether_ifdetach() before foo_stop() in foo_detach(), I'm : > : > not yet sure. : > : > : > : I tried with rl(4) PCCARD, by moving ether_ifdetach() before : > : rl_stop() in rl_detach(). It fixes the panic when you eject : > : the card, but doesn't fix it when kldunloading the module. : > : The difference is that rl_detach() is called already after : > : miibus0 and rlphy0 has been detached when kldunloading the : > : module. When ejecting the card, rl_detach() is called first. : > : What happens when you kldunload the module with BPF listener : > : attached, is that bpfdetach() calls rl_ioctl() to reset : > : promisc, that calls rl_init_locked(), and that results in : > : : > : mii = device_get_softc(sc->rl_miibus); : > : : > : being NULL (remember the miibus has already been detached), : > : and that panics later here: : > : : > : mii_mediachg(mii); : > : : > : When we reset IFF_UP, rl_ioctl(SIOCSIFFLAGS) silently exits : > : and no harm is done. So the question is: how do we prevent : > : this from happening without resetting IFF_UP. One possible : > : solution would be to add sc->detaching, similar to : > : sc->suspended, abd check it in rl_ioctl(). : > : > Ugg. In ed, we check to make sure that we still have a child before : > doing things with mii bus. A similar fix could be made. : > : No, ed(4) has the same problem: : : if (sc->miibus != NULL) { : struct mii_data *mii; : mii = device_get_softc(sc->miibus); : mii_mediachg(mii); : } : No it doesn't: void ed_child_detached(device_t dev, device_t child) { struct ed_softc *sc; sc = device_get_softc(dev); if (child == sc->miibus) sc->miibus = NULL; } : The device (sc->miibus) will still be there but already detached, : and its softc will already be freed, so "mii" will be NULL, and : mii_mediachg(NULL) will panic the system. sc->miibus will be NULL after the device is detached, so you don't get an error. How again can this happen? Warner