From owner-freebsd-new-bus@FreeBSD.ORG Thu Jan 29 07:59:04 2004 Return-Path: Delivered-To: freebsd-new-bus@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id EC77216A4CF for ; Thu, 29 Jan 2004 07:59:04 -0800 (PST) Received: from mail6.speakeasy.net (mail6.speakeasy.net [216.254.0.206]) by mx1.FreeBSD.org (Postfix) with ESMTP id 1C8E543D46 for ; Thu, 29 Jan 2004 07:58:59 -0800 (PST) (envelope-from jhb@FreeBSD.org) Received: (qmail 20206 invoked from network); 29 Jan 2004 15:58:58 -0000 Received: from dsl027-160-063.atl1.dsl.speakeasy.net (HELO server.baldwin.cx) ([216.27.160.63]) (envelope-sender ) encrypted SMTP for ; 29 Jan 2004 15:58:58 -0000 Received: from 10.50.40.205 (gw1.twc.weather.com [216.133.140.1]) by server.baldwin.cx (8.12.10/8.12.10) with ESMTP id i0TFwIM8069201; Thu, 29 Jan 2004 10:58:44 -0500 (EST) (envelope-from jhb@FreeBSD.org) From: John Baldwin To: John Wehle Date: Thu, 29 Jan 2004 10:50:40 -0500 User-Agent: KMail/1.5.4 References: <200401290635.i0T6ZO224579@jwlab.FEITH.COM> In-Reply-To: <200401290635.i0T6ZO224579@jwlab.FEITH.COM> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200401291050.40458.jhb@FreeBSD.org> X-Spam-Checker-Version: SpamAssassin 2.55 (1.174.2.19-2003-05-19-exp) cc: imp@FreeBSD.org cc: new-bus@FreeBSD.org cc: mdodd@FreeBSD.org Subject: Re: nasty device_delete_child interaction X-BeenThere: freebsd-new-bus@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: FreeBSD's new-bus architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 29 Jan 2004 15:59:05 -0000 [ Moved this to new-bus@ list as that is more appropriate. ] On Thursday 29 January 2004 01:35 am, John Wehle wrote: > device_delete_child works by starting with the grandchildren > working back towards the immediate child. Several drivers > (i.e. if_xl.c, if_wx.c, iicbb.c) have code similar to: > > xxx_attach() > { > > ... > sc->child_dev = device_add_child ... > } > > xxx_detach() > { > > bus_generic_detach (); > if (sc->child_dev) > device_delete_child ... > } > > The problem is using device_delete_child on one of these > drivers causes the grandchild to be freed twice. When > device_delete_child is called for xxx, it recurses since > xxx has a child. The grandchild is detached and deleted. > xxx_detach is then called which calls device_delete_child > for the grandchild a second time causing a panic. > > It seems to me that any driver which calls device_delete_child > as part of detaching must also implement something like: > > xxx_child_detached() > { > > if (child == sc->child_dev) > sc->child_dev = NULL; > } > > xxx_detach() > { > > /* > * Remember the child so we can delete it (bus_generic_detach > * indirectly zeroes sc->child_dev). > */ > child = sc->child_dev; > > bus_generic_detach(); > if (child) > device_delete_child ... > } > > or am I missing something? > > -- John > ------------------------------------------------------------------------- > > | Feith Systems | Voice: 1-215-646-8000 | Email: john@feith.com | > | John Wehle | Fax: 1-215-540-5495 | | > > ------------------------------------------------------------------------- -- John Baldwin <>< http://www.FreeBSD.org/~jhb/ "Power Users Use the Power to Serve" = http://www.FreeBSD.org From owner-freebsd-new-bus@FreeBSD.ORG Thu Jan 29 08:11:22 2004 Return-Path: Delivered-To: freebsd-new-bus@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 2828A16A4CF; Thu, 29 Jan 2004 08:11:22 -0800 (PST) Received: from harmony.village.org (rover.bsdimp.com [204.144.255.66]) by mx1.FreeBSD.org (Postfix) with ESMTP id D7AAC43D31; Thu, 29 Jan 2004 08:11:20 -0800 (PST) (envelope-from imp@bsdimp.com) Received: from localhost (warner@rover2.village.org [10.0.0.1]) by harmony.village.org (8.12.10/8.12.9) with ESMTP id i0TGBAET045829; Thu, 29 Jan 2004 09:11:10 -0700 (MST) (envelope-from imp@bsdimp.com) Date: Thu, 29 Jan 2004 09:11:09 -0700 (MST) Message-Id: <20040129.091109.27780542.imp@bsdimp.com> To: jhb@FreeBSD.org From: "M. Warner Losh" In-Reply-To: <200401291050.40458.jhb@FreeBSD.org> References: <200401290635.i0T6ZO224579@jwlab.FEITH.COM> <200401291050.40458.jhb@FreeBSD.org> 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 cc: john@feith.com cc: new-bus@FreeBSD.org cc: mdodd@FreeBSD.org Subject: Re: nasty device_delete_child interaction X-BeenThere: freebsd-new-bus@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: FreeBSD's new-bus architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 29 Jan 2004 16:11:22 -0000 In message: <200401291050.40458.jhb@FreeBSD.org> John Baldwin writes: : On Thursday 29 January 2004 01:35 am, John Wehle wrote: : > device_delete_child works by starting with the grandchildren : > working back towards the immediate child. Several drivers : > (i.e. if_xl.c, if_wx.c, iicbb.c) have code similar to: : > : > xxx_attach() : > { : > : > ... : > sc->child_dev = device_add_child ... : > } : > : > xxx_detach() : > { : > : > bus_generic_detach (); : > if (sc->child_dev) : > device_delete_child ... : > } Don't do that. You are duplicating the storage of children in two places. If you need to cache a copy of a child, that's fine. However, don't delete it explicitly in xxx_detach. I'd say that these drivers are wrong and should be fixed. : > It seems to me that any driver which calls device_delete_child : > as part of detaching must also implement something like: No. They should avoid the problem by using newbus correctly. This sort of solution just adds code to no good purpose. Warner From owner-freebsd-new-bus@FreeBSD.ORG Thu Jan 29 11:59:07 2004 Return-Path: Delivered-To: freebsd-new-bus@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 76D8316A4CE; Thu, 29 Jan 2004 11:59:07 -0800 (PST) Received: from feith1.FEITH.COM (feith1.FEITH.COM [192.251.93.1]) by mx1.FreeBSD.org (Postfix) with ESMTP id 5A3DD43D31; Thu, 29 Jan 2004 11:59:06 -0800 (PST) (envelope-from john@feith.com) Received: from jwlab.FEITH.COM (jwlab.FEITH.COM [192.251.93.16]) by feith1.FEITH.COM (8.12.10+Sun/8.12.9) with ESMTP id i0TJx3AL000159; Thu, 29 Jan 2004 14:59:03 -0500 (EST) Received: (from john@localhost) by jwlab.FEITH.COM (8.11.7+Sun/8.11.7) id i0TJx2m25103; Thu, 29 Jan 2004 14:59:02 -0500 (EST) Date: Thu, 29 Jan 2004 14:59:02 -0500 (EST) From: John Wehle Message-Id: <200401291959.i0TJx2m25103@jwlab.FEITH.COM> To: imp@bsdimp.com Content-Type: text X-Scanned-By: MIMEDefang 2.39 X-Archived: cashew.FEITH.COM cc: new-bus@FreeBSD.org cc: mdodd@FreeBSD.org cc: jhb@FreeBSD.org Subject: Re: nasty device_delete_child interaction X-BeenThere: freebsd-new-bus@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: FreeBSD's new-bus architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 29 Jan 2004 19:59:07 -0000 > Don't do that. You are duplicating the storage of children in two > places. If you need to cache a copy of a child, that's fine. > However, don't delete it explicitly in xxx_detach. > > I'd say that these drivers are wrong and should be fixed. What's the correct approach? Currently (at least in FreeBSD 4.9) if_xl.c uses device_add_child in the attach routine to add on miibus. It then calls device_delete_child in the detach routine to delete miibus (creating a nice symmetry). Should if_xl.c still call device_add_child in the attach and simply not call device_delete_child? Then who's responsible for deleting miibus when if_xl is unloaded? >: > It seems to me that any driver which calls device_delete_child >: > as part of detaching must also implement something like: > > No. They should avoid the problem by using newbus correctly. This > sort of solution just adds code to no good purpose. If the driver has cached a copy of a child, then doesn't xxx_child_detached still need to be implemented so the driver knows when the cached copy is invalid? -- John ------------------------------------------------------------------------- | Feith Systems | Voice: 1-215-646-8000 | Email: john@feith.com | | John Wehle | Fax: 1-215-540-5495 | | -------------------------------------------------------------------------