From owner-freebsd-drivers@FreeBSD.ORG Sat Apr 7 12:51:03 2012 Return-Path: Delivered-To: drivers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id DAB65106566B; Sat, 7 Apr 2012 12:51:03 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from mail.zoral.com.ua (mx0.zoral.com.ua [91.193.166.200]) by mx1.freebsd.org (Postfix) with ESMTP id 4F34D8FC12; Sat, 7 Apr 2012 12:51:03 +0000 (UTC) Received: from skuns.kiev.zoral.com.ua (localhost [127.0.0.1]) by mail.zoral.com.ua (8.14.2/8.14.2) with ESMTP id q37Cou5N002377; Sat, 7 Apr 2012 15:50:56 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: from deviant.kiev.zoral.com.ua (kostik@localhost [127.0.0.1]) by deviant.kiev.zoral.com.ua (8.14.5/8.14.5) with ESMTP id q37CouSS087441; Sat, 7 Apr 2012 15:50:56 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: (from kostik@localhost) by deviant.kiev.zoral.com.ua (8.14.5/8.14.5/Submit) id q37Courc087440; Sat, 7 Apr 2012 15:50:56 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: deviant.kiev.zoral.com.ua: kostik set sender to kostikbel@gmail.com using -f Date: Sat, 7 Apr 2012 15:50:56 +0300 From: Konstantin Belousov To: drivers@freebsd.org Message-ID: <20120407125056.GS2358@deviant.kiev.zoral.com.ua> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="HfUg6XWR+aOWM2JP" Content-Disposition: inline User-Agent: Mutt/1.4.2.3i X-Virus-Scanned: clamav-milter 0.95.2 at skuns.kiev.zoral.com.ua X-Virus-Status: Clean X-Spam-Status: No, score=-4.0 required=5.0 tests=ALL_TRUSTED,AWL,BAYES_00 autolearn=ham version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on skuns.kiev.zoral.com.ua Cc: current@freebsd.org Subject: device_attach(9) and driver initialization X-BeenThere: freebsd-drivers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Writing device drivers for FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 07 Apr 2012 12:51:04 -0000 --HfUg6XWR+aOWM2JP Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hello, there seems to be a problem with device attach sequence offered by newbus. Basically, when device attach method is executing, device is not fully initialized yet. Also the device state in the newbus part of the world is DS_ALIVE. There is definitely no shattering news in the statements, but drivers that e.g. create devfs node to communicate with consumers are prone to a race. If /dev node is created inside device attach method, then usermode can start calling cdevsw methods before device fully initialized itself. Even more, if device tries to use newbus helpers in cdevsw methods, like device_busy(9), then panic occurs "called for unatteched device". I get reports from users about this issues, to it is not something that only could happen. I propose to add DEVICE_AFTER_ATTACH() driver method, to be called from newbus right after device attach finished and newbus considers the device fully initialized. Driver then could create devfs node in the after_attach method instead of attach. Please see the patch below. diff --git a/sys/kern/device_if.m b/sys/kern/device_if.m index eb720eb..9db74e2 100644 --- a/sys/kern/device_if.m +++ b/sys/kern/device_if.m @@ -43,6 +43,10 @@ INTERFACE device; # Default implementations of some methods. # CODE { + static void null_after_attach(device_t dev) + { + } + static int null_shutdown(device_t dev) { return 0; @@ -199,6 +203,21 @@ METHOD int attach { }; =20 /** + * @brief Notify the driver that device is in attached state + * + * Called after driver is successfully attached to the device and + * corresponding device_t is fully operational. Driver now may expose + * the device to the consumers, e.g. create devfs nodes. + * + * @param dev the device to probe + * + * @see DEVICE_ATTACH() + */ +METHOD void after_attach { + device_t dev; +} DEFAULT null_after_attach; + +/** * @brief Detach a driver from a device. * * This can be called if the user is replacing the diff --git a/sys/kern/subr_bus.c b/sys/kern/subr_bus.c index d485b9f..6d849cb 100644 --- a/sys/kern/subr_bus.c +++ b/sys/kern/subr_bus.c @@ -2743,6 +2743,7 @@ device_attach(device_t dev) dev->state =3D DS_ATTACHED; dev->flags &=3D ~DF_DONENOMATCH; devadded(dev); + DEVICE_AFTER_ATTACH(dev); return (0); } =20 --HfUg6XWR+aOWM2JP Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (FreeBSD) iEYEARECAAYFAk+AODAACgkQC3+MBN1Mb4gq0wCePhl0k9l2nFCAcc+Rat/K/iJw EGQAn23FuThAq3iBg1vnt3G50SazHQkP =MudJ -----END PGP SIGNATURE----- --HfUg6XWR+aOWM2JP-- From owner-freebsd-drivers@FreeBSD.ORG Sat Apr 7 14:47:50 2012 Return-Path: Delivered-To: drivers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 5DC1A106566C for ; Sat, 7 Apr 2012 14:47:50 +0000 (UTC) (envelope-from freebsd@damnhippie.dyndns.org) Received: from qmta09.emeryville.ca.mail.comcast.net (qmta09.emeryville.ca.mail.comcast.net [76.96.30.96]) by mx1.freebsd.org (Postfix) with ESMTP id 3E84B8FC12 for ; Sat, 7 Apr 2012 14:47:50 +0000 (UTC) Received: from omta11.emeryville.ca.mail.comcast.net ([76.96.30.36]) by qmta09.emeryville.ca.mail.comcast.net with comcast id v2kl1i0010mlR8UA92mkee; Sat, 07 Apr 2012 14:46:44 +0000 Received: from damnhippie.dyndns.org ([24.8.232.202]) by omta11.emeryville.ca.mail.comcast.net with comcast id v2mj1i00C4NgCEG8X2mklx; Sat, 07 Apr 2012 14:46:44 +0000 Received: from [172.22.42.240] (revolution.hippie.lan [172.22.42.240]) by damnhippie.dyndns.org (8.14.3/8.14.3) with ESMTP id q37Ekf0b055259; Sat, 7 Apr 2012 08:46:41 -0600 (MDT) (envelope-from freebsd@damnhippie.dyndns.org) From: Ian Lepore To: Konstantin Belousov In-Reply-To: <20120407125056.GS2358@deviant.kiev.zoral.com.ua> References: <20120407125056.GS2358@deviant.kiev.zoral.com.ua> Content-Type: text/plain; charset="us-ascii" Date: Sat, 07 Apr 2012 08:46:41 -0600 Message-ID: <1333810001.1082.36.camel@revolution.hippie.lan> Mime-Version: 1.0 X-Mailer: Evolution 2.32.1 FreeBSD GNOME Team Port Content-Transfer-Encoding: 7bit Cc: current@freebsd.org, drivers@freebsd.org Subject: Re: device_attach(9) and driver initialization X-BeenThere: freebsd-drivers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Writing device drivers for FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 07 Apr 2012 14:47:50 -0000 On Sat, 2012-04-07 at 15:50 +0300, Konstantin Belousov wrote: > Hello, > there seems to be a problem with device attach sequence offered by newbus. > Basically, when device attach method is executing, device is not fully > initialized yet. Also the device state in the newbus part of the world > is DS_ALIVE. There is definitely no shattering news in the statements, > but drivers that e.g. create devfs node to communicate with consumers > are prone to a race. > > If /dev node is created inside device attach method, then usermode > can start calling cdevsw methods before device fully initialized itself. > Even more, if device tries to use newbus helpers in cdevsw methods, > like device_busy(9), then panic occurs "called for unatteched device". > I get reports from users about this issues, to it is not something > that only could happen. > > I propose to add DEVICE_AFTER_ATTACH() driver method, to be called > from newbus right after device attach finished and newbus considers > the device fully initialized. Driver then could create devfs node > in the after_attach method instead of attach. Please see the patch below. > > diff --git a/sys/kern/device_if.m b/sys/kern/device_if.m > index eb720eb..9db74e2 100644 > --- a/sys/kern/device_if.m > +++ b/sys/kern/device_if.m > @@ -43,6 +43,10 @@ INTERFACE device; > # Default implementations of some methods. > # > CODE { > + static void null_after_attach(device_t dev) > + { > + } > + > static int null_shutdown(device_t dev) > { > return 0; > @@ -199,6 +203,21 @@ METHOD int attach { > }; > > /** > + * @brief Notify the driver that device is in attached state > + * > + * Called after driver is successfully attached to the device and > + * corresponding device_t is fully operational. Driver now may expose > + * the device to the consumers, e.g. create devfs nodes. > + * > + * @param dev the device to probe > + * > + * @see DEVICE_ATTACH() > + */ > +METHOD void after_attach { > + device_t dev; > +} DEFAULT null_after_attach; > + > +/** > * @brief Detach a driver from a device. > * > * This can be called if the user is replacing the > diff --git a/sys/kern/subr_bus.c b/sys/kern/subr_bus.c > index d485b9f..6d849cb 100644 > --- a/sys/kern/subr_bus.c > +++ b/sys/kern/subr_bus.c > @@ -2743,6 +2743,7 @@ device_attach(device_t dev) > dev->state = DS_ATTACHED; > dev->flags &= ~DF_DONENOMATCH; > devadded(dev); > + DEVICE_AFTER_ATTACH(dev); > return (0); > } > Does device_get_softc() work before attach is completed? (I don't have time to go look in the code right now). If so, then a mutex initialized and acquired early in the driver's attach routine, and also acquired in the driver's cdev implementation routines before using any newbus functions other than device_get_softc(), would solve the problem without a driver api change that would make it harder to backport/MFC driver changes. -- Ian From owner-freebsd-drivers@FreeBSD.ORG Sat Apr 7 14:57:40 2012 Return-Path: Delivered-To: drivers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 6037A1065673; Sat, 7 Apr 2012 14:57:40 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from mail.zoral.com.ua (mx0.zoral.com.ua [91.193.166.200]) by mx1.freebsd.org (Postfix) with ESMTP id C77CE8FC08; Sat, 7 Apr 2012 14:57:39 +0000 (UTC) Received: from skuns.kiev.zoral.com.ua (localhost [127.0.0.1]) by mail.zoral.com.ua (8.14.2/8.14.2) with ESMTP id q37EvTZZ027789; Sat, 7 Apr 2012 17:57:29 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: from deviant.kiev.zoral.com.ua (kostik@localhost [127.0.0.1]) by deviant.kiev.zoral.com.ua (8.14.5/8.14.5) with ESMTP id q37EvSfY087985; Sat, 7 Apr 2012 17:57:28 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: (from kostik@localhost) by deviant.kiev.zoral.com.ua (8.14.5/8.14.5/Submit) id q37EvSMd087984; Sat, 7 Apr 2012 17:57:28 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: deviant.kiev.zoral.com.ua: kostik set sender to kostikbel@gmail.com using -f Date: Sat, 7 Apr 2012 17:57:28 +0300 From: Konstantin Belousov To: Ian Lepore Message-ID: <20120407145728.GV2358@deviant.kiev.zoral.com.ua> References: <20120407125056.GS2358@deviant.kiev.zoral.com.ua> <1333810001.1082.36.camel@revolution.hippie.lan> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="CS4y9zAoaOKmm/jR" Content-Disposition: inline In-Reply-To: <1333810001.1082.36.camel@revolution.hippie.lan> User-Agent: Mutt/1.4.2.3i X-Virus-Scanned: clamav-milter 0.95.2 at skuns.kiev.zoral.com.ua X-Virus-Status: Clean X-Spam-Status: No, score=-4.0 required=5.0 tests=ALL_TRUSTED,AWL,BAYES_00 autolearn=ham version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on skuns.kiev.zoral.com.ua Cc: current@freebsd.org, drivers@freebsd.org Subject: Re: device_attach(9) and driver initialization X-BeenThere: freebsd-drivers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Writing device drivers for FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 07 Apr 2012 14:57:40 -0000 --CS4y9zAoaOKmm/jR Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sat, Apr 07, 2012 at 08:46:41AM -0600, Ian Lepore wrote: > On Sat, 2012-04-07 at 15:50 +0300, Konstantin Belousov wrote: > > Hello, > > there seems to be a problem with device attach sequence offered by newb= us. > > Basically, when device attach method is executing, device is not fully > > initialized yet. Also the device state in the newbus part of the world > > is DS_ALIVE. There is definitely no shattering news in the statements, > > but drivers that e.g. create devfs node to communicate with consumers > > are prone to a race. > >=20 > > If /dev node is created inside device attach method, then usermode > > can start calling cdevsw methods before device fully initialized itself. > > Even more, if device tries to use newbus helpers in cdevsw methods, > > like device_busy(9), then panic occurs "called for unatteched device". > > I get reports from users about this issues, to it is not something > > that only could happen. > >=20 > > I propose to add DEVICE_AFTER_ATTACH() driver method, to be called > > from newbus right after device attach finished and newbus considers > > the device fully initialized. Driver then could create devfs node > > in the after_attach method instead of attach. Please see the patch belo= w. > >=20 > > diff --git a/sys/kern/device_if.m b/sys/kern/device_if.m > > index eb720eb..9db74e2 100644 > > --- a/sys/kern/device_if.m > > +++ b/sys/kern/device_if.m > > @@ -43,6 +43,10 @@ INTERFACE device; > > # Default implementations of some methods. > > # > > CODE { > > + static void null_after_attach(device_t dev) > > + { > > + } > > + > > static int null_shutdown(device_t dev) > > { > > return 0; > > @@ -199,6 +203,21 @@ METHOD int attach { > > }; > > =20 > > /** > > + * @brief Notify the driver that device is in attached state > > + * > > + * Called after driver is successfully attached to the device and > > + * corresponding device_t is fully operational. Driver now may expose > > + * the device to the consumers, e.g. create devfs nodes. > > + * > > + * @param dev the device to probe > > + * > > + * @see DEVICE_ATTACH() > > + */ > > +METHOD void after_attach { > > + device_t dev; > > +} DEFAULT null_after_attach; > > + > > +/** > > * @brief Detach a driver from a device. > > * > > * This can be called if the user is replacing the > > diff --git a/sys/kern/subr_bus.c b/sys/kern/subr_bus.c > > index d485b9f..6d849cb 100644 > > --- a/sys/kern/subr_bus.c > > +++ b/sys/kern/subr_bus.c > > @@ -2743,6 +2743,7 @@ device_attach(device_t dev) > > dev->state =3D DS_ATTACHED; > > dev->flags &=3D ~DF_DONENOMATCH; > > devadded(dev); > > + DEVICE_AFTER_ATTACH(dev); > > return (0); > > } > > =20 >=20 > Does device_get_softc() work before attach is completed? (I don't have > time to go look in the code right now). If so, then a mutex initialized > and acquired early in the driver's attach routine, and also acquired in > the driver's cdev implementation routines before using any newbus > functions other than device_get_softc(), would solve the problem without > a driver api change that would make it harder to backport/MFC driver > changes. No, 'a mutex' does not solve anything. It only adds enourmous burden on the driver developers, because you cannot sleep under mutex. Changing the mutex to the sleepable lock also does not byy you much, since you need to somehow solve the issues with some cdevsw call waking up thread sleeping into another cdevsw call, just for example. Singlethreading a driver due to this race is just silly. And, what do you mean by 'making it harder to MFC' ? How ? --CS4y9zAoaOKmm/jR Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (FreeBSD) iEYEARECAAYFAk+AVdgACgkQC3+MBN1Mb4jTrACdFHKoYkgbu8XZDxumRGi+XhUK TQUAnR0ERsly/TQuBVeUbjyphXO4DtNO =TyEu -----END PGP SIGNATURE----- --CS4y9zAoaOKmm/jR-- From owner-freebsd-drivers@FreeBSD.ORG Sat Apr 7 15:05:07 2012 Return-Path: Delivered-To: drivers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id A4F061065672; Sat, 7 Apr 2012 15:05:07 +0000 (UTC) (envelope-from nwhitehorn@freebsd.org) Received: from argol.doit.wisc.edu (argol.doit.wisc.edu [144.92.197.212]) by mx1.freebsd.org (Postfix) with ESMTP id 6FFDF8FC1B; Sat, 7 Apr 2012 15:05:07 +0000 (UTC) MIME-version: 1.0 Content-transfer-encoding: 7BIT Content-type: text/plain; CHARSET=US-ASCII; format=flowed Received: from avs-daemon.smtpauth3.wiscmail.wisc.edu by smtpauth3.wiscmail.wisc.edu (Sun Java(tm) System Messaging Server 7u2-7.05 32bit (built Jul 30 2009)) id <0M240000278DJB00@smtpauth3.wiscmail.wisc.edu>; Sat, 07 Apr 2012 10:05:01 -0500 (CDT) Received: from comporellon.tachypleus.net ([unknown] [76.210.67.9]) by smtpauth3.wiscmail.wisc.edu (Sun Java(tm) System Messaging Server 7u2-7.05 32bit (built Jul 30 2009)) with ESMTPSA id <0M240005A78BBK00@smtpauth3.wiscmail.wisc.edu>; Sat, 07 Apr 2012 10:05:00 -0500 (CDT) Date: Sat, 07 Apr 2012 10:04:59 -0500 From: Nathan Whitehorn In-reply-to: <20120407125056.GS2358@deviant.kiev.zoral.com.ua> To: Konstantin Belousov Message-id: <4F80579B.4040205@freebsd.org> X-Spam-Report: AuthenticatedSender=yes, SenderIP=76.210.67.9 X-Spam-PmxInfo: Server=avs-15, Version=5.6.1.2065439, Antispam-Engine: 2.7.2.376379, Antispam-Data: 2012.4.7.145414, SenderIP=76.210.67.9 References: <20120407125056.GS2358@deviant.kiev.zoral.com.ua> User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:10.0.2) Gecko/20120311 Thunderbird/10.0.2 Cc: current@freebsd.org, drivers@freebsd.org Subject: Re: device_attach(9) and driver initialization X-BeenThere: freebsd-drivers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Writing device drivers for FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 07 Apr 2012 15:05:07 -0000 On 04/07/12 07:50, Konstantin Belousov wrote: > Hello, > there seems to be a problem with device attach sequence offered by newbus. > Basically, when device attach method is executing, device is not fully > initialized yet. Also the device state in the newbus part of the world > is DS_ALIVE. There is definitely no shattering news in the statements, > but drivers that e.g. create devfs node to communicate with consumers > are prone to a race. > > If /dev node is created inside device attach method, then usermode > can start calling cdevsw methods before device fully initialized itself. > Even more, if device tries to use newbus helpers in cdevsw methods, > like device_busy(9), then panic occurs "called for unatteched device". > I get reports from users about this issues, to it is not something > that only could happen. > > I propose to add DEVICE_AFTER_ATTACH() driver method, to be called > from newbus right after device attach finished and newbus considers > the device fully initialized. Driver then could create devfs node > in the after_attach method instead of attach. Please see the patch below. > Something like this would also be very useful for drivers that need to interact across the device tree, if newbus called it only after all drivers have been attached. Drivers that need to see other potentially attached drivers now need to do some hacks with SYSINIT. Would it be possible to do this? I don't think it changes the functionality you need. -Nathan From owner-freebsd-drivers@FreeBSD.ORG Sat Apr 7 15:15:27 2012 Return-Path: Delivered-To: drivers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id E7811106564A; Sat, 7 Apr 2012 15:15:27 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from mail.zoral.com.ua (mx0.zoral.com.ua [91.193.166.200]) by mx1.freebsd.org (Postfix) with ESMTP id 7383B8FC15; Sat, 7 Apr 2012 15:15:27 +0000 (UTC) Received: from skuns.kiev.zoral.com.ua (localhost [127.0.0.1]) by mail.zoral.com.ua (8.14.2/8.14.2) with ESMTP id q37FFESo031367; Sat, 7 Apr 2012 18:15:14 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: from deviant.kiev.zoral.com.ua (kostik@localhost [127.0.0.1]) by deviant.kiev.zoral.com.ua (8.14.5/8.14.5) with ESMTP id q37FFEtH088081; Sat, 7 Apr 2012 18:15:14 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: (from kostik@localhost) by deviant.kiev.zoral.com.ua (8.14.5/8.14.5/Submit) id q37FFEOC088080; Sat, 7 Apr 2012 18:15:14 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: deviant.kiev.zoral.com.ua: kostik set sender to kostikbel@gmail.com using -f Date: Sat, 7 Apr 2012 18:15:14 +0300 From: Konstantin Belousov To: Nathan Whitehorn Message-ID: <20120407151514.GW2358@deviant.kiev.zoral.com.ua> References: <20120407125056.GS2358@deviant.kiev.zoral.com.ua> <4F80579B.4040205@freebsd.org> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="CN5+qz79j8MMBHjt" Content-Disposition: inline In-Reply-To: <4F80579B.4040205@freebsd.org> User-Agent: Mutt/1.4.2.3i X-Virus-Scanned: clamav-milter 0.95.2 at skuns.kiev.zoral.com.ua X-Virus-Status: Clean X-Spam-Status: No, score=-4.0 required=5.0 tests=ALL_TRUSTED,AWL,BAYES_00 autolearn=ham version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on skuns.kiev.zoral.com.ua Cc: current@freebsd.org, drivers@freebsd.org Subject: Re: device_attach(9) and driver initialization X-BeenThere: freebsd-drivers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Writing device drivers for FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 07 Apr 2012 15:15:28 -0000 --CN5+qz79j8MMBHjt Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sat, Apr 07, 2012 at 10:04:59AM -0500, Nathan Whitehorn wrote: > On 04/07/12 07:50, Konstantin Belousov wrote: > >Hello, > >there seems to be a problem with device attach sequence offered by newbu= s. > >Basically, when device attach method is executing, device is not fully > >initialized yet. Also the device state in the newbus part of the world > >is DS_ALIVE. There is definitely no shattering news in the statements, > >but drivers that e.g. create devfs node to communicate with consumers > >are prone to a race. > > > >If /dev node is created inside device attach method, then usermode > >can start calling cdevsw methods before device fully initialized itself. > >Even more, if device tries to use newbus helpers in cdevsw methods, > >like device_busy(9), then panic occurs "called for unatteched device". > >I get reports from users about this issues, to it is not something > >that only could happen. > > > >I propose to add DEVICE_AFTER_ATTACH() driver method, to be called > >from newbus right after device attach finished and newbus considers > >the device fully initialized. Driver then could create devfs node > >in the after_attach method instead of attach. Please see the patch below. > > >=20 > Something like this would also be very useful for drivers that need to=20 > interact across the device tree, if newbus called it only after all=20 > drivers have been attached. Drivers that need to see other potentially=20 > attached drivers now need to do some hacks with SYSINIT. Would it be=20 > possible to do this? I don't think it changes the functionality you need. I am definitely fine with postponing a call further, but I am not sure how to define that point and how to implement your proposal. I am mostly thinking of the case of kld being loaded, since for compiled-in drivers, there is simply no usermode to make the havoc during attach. For the boot time attachments, I am not sure when to declare the end. E.g. USB does asynchronous device discovery. Could you prototype a change that would do what you propose ? --CN5+qz79j8MMBHjt Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (FreeBSD) iEYEARECAAYFAk+AWgIACgkQC3+MBN1Mb4hlswCghMA1W9QGOErIQUXaukNrBxIb D74AoJ9vcwS2c3f5JPGkfU3zdhrqe7zP =v3zv -----END PGP SIGNATURE----- --CN5+qz79j8MMBHjt-- From owner-freebsd-drivers@FreeBSD.ORG Sat Apr 7 15:48:36 2012 Return-Path: Delivered-To: drivers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id BEF0A1065676; Sat, 7 Apr 2012 15:48:36 +0000 (UTC) (envelope-from nwhitehorn@freebsd.org) Received: from argol.doit.wisc.edu (argol.doit.wisc.edu [144.92.197.212]) by mx1.freebsd.org (Postfix) with ESMTP id 8F2A28FC0A; Sat, 7 Apr 2012 15:48:36 +0000 (UTC) MIME-version: 1.0 Content-transfer-encoding: 7BIT Content-type: text/plain; CHARSET=US-ASCII; format=flowed Received: from avs-daemon.smtpauth3.wiscmail.wisc.edu by smtpauth3.wiscmail.wisc.edu (Sun Java(tm) System Messaging Server 7u2-7.05 32bit (built Jul 30 2009)) id <0M24004009908500@smtpauth3.wiscmail.wisc.edu>; Sat, 07 Apr 2012 10:48:36 -0500 (CDT) Received: from comporellon.tachypleus.net ([unknown] [76.210.67.9]) by smtpauth3.wiscmail.wisc.edu (Sun Java(tm) System Messaging Server 7u2-7.05 32bit (built Jul 30 2009)) with ESMTPSA id <0M24000DK98YBK10@smtpauth3.wiscmail.wisc.edu>; Sat, 07 Apr 2012 10:48:35 -0500 (CDT) Date: Sat, 07 Apr 2012 10:48:34 -0500 From: Nathan Whitehorn In-reply-to: <20120407151514.GW2358@deviant.kiev.zoral.com.ua> To: Konstantin Belousov Message-id: <4F8061D2.4030507@freebsd.org> X-Spam-Report: AuthenticatedSender=yes, SenderIP=76.210.67.9 X-Spam-PmxInfo: Server=avs-15, Version=5.6.1.2065439, Antispam-Engine: 2.7.2.376379, Antispam-Data: 2012.4.7.153916, SenderIP=76.210.67.9 References: <20120407125056.GS2358@deviant.kiev.zoral.com.ua> <4F80579B.4040205@freebsd.org> <20120407151514.GW2358@deviant.kiev.zoral.com.ua> User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:10.0.2) Gecko/20120311 Thunderbird/10.0.2 Cc: current@freebsd.org, drivers@freebsd.org Subject: Re: device_attach(9) and driver initialization X-BeenThere: freebsd-drivers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Writing device drivers for FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 07 Apr 2012 15:48:37 -0000 On 04/07/12 10:15, Konstantin Belousov wrote: > On Sat, Apr 07, 2012 at 10:04:59AM -0500, Nathan Whitehorn wrote: >> On 04/07/12 07:50, Konstantin Belousov wrote: >>> Hello, >>> there seems to be a problem with device attach sequence offered by newbus. >>> Basically, when device attach method is executing, device is not fully >>> initialized yet. Also the device state in the newbus part of the world >>> is DS_ALIVE. There is definitely no shattering news in the statements, >>> but drivers that e.g. create devfs node to communicate with consumers >>> are prone to a race. >>> >>> If /dev node is created inside device attach method, then usermode >>> can start calling cdevsw methods before device fully initialized itself. >>> Even more, if device tries to use newbus helpers in cdevsw methods, >>> like device_busy(9), then panic occurs "called for unatteched device". >>> I get reports from users about this issues, to it is not something >>> that only could happen. >>> >>> I propose to add DEVICE_AFTER_ATTACH() driver method, to be called >> >from newbus right after device attach finished and newbus considers >>> the device fully initialized. Driver then could create devfs node >>> in the after_attach method instead of attach. Please see the patch below. >>> >> Something like this would also be very useful for drivers that need to >> interact across the device tree, if newbus called it only after all >> drivers have been attached. Drivers that need to see other potentially >> attached drivers now need to do some hacks with SYSINIT. Would it be >> possible to do this? I don't think it changes the functionality you need. > I am definitely fine with postponing a call further, but I am not sure > how to define that point and how to implement your proposal. I am mostly > thinking of the case of kld being loaded, since for compiled-in drivers, > there is simply no usermode to make the havoc during attach. > > For the boot time attachments, I am not sure when to declare the end. > E.g. USB does asynchronous device discovery. > > Could you prototype a change that would do what you propose ? The idea would be to have something that walks the tree and calls it from root_bus_configure(), and otherwise does what your patch does. But it is hard to have it work right for modules, and maybe is too invasive anyway. I'll think about it some more and try to mock something up if it doesn't turn out to be unworkable. -Nathan From owner-freebsd-drivers@FreeBSD.ORG Sat Apr 7 21:31:24 2012 Return-Path: Delivered-To: drivers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id E930D106566B; Sat, 7 Apr 2012 21:31:23 +0000 (UTC) (envelope-from julian@freebsd.org) Received: from vps1.elischer.org (vps1.elischer.org [204.109.63.16]) by mx1.freebsd.org (Postfix) with ESMTP id B1EFF8FC0C; Sat, 7 Apr 2012 21:31:23 +0000 (UTC) Received: from julian-mac.elischer.org (c-67-180-24-15.hsd1.ca.comcast.net [67.180.24.15]) (authenticated bits=0) by vps1.elischer.org (8.14.5/8.14.5) with ESMTP id q37LVMBB019934 (version=TLSv1/SSLv3 cipher=DHE-RSA-CAMELLIA256-SHA bits=256 verify=NO); Sat, 7 Apr 2012 14:31:23 -0700 (PDT) (envelope-from julian@freebsd.org) Message-ID: <4F80B254.1010606@freebsd.org> Date: Sat, 07 Apr 2012 14:32:04 -0700 From: Julian Elischer User-Agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X 10.4; en-US; rv:1.9.2.28) Gecko/20120306 Thunderbird/3.1.20 MIME-Version: 1.0 To: Konstantin Belousov References: <20120407125056.GS2358@deviant.kiev.zoral.com.ua> In-Reply-To: <20120407125056.GS2358@deviant.kiev.zoral.com.ua> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: current@freebsd.org, drivers@freebsd.org Subject: Re: device_attach(9) and driver initialization X-BeenThere: freebsd-drivers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Writing device drivers for FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 07 Apr 2012 21:31:24 -0000 On 4/7/12 5:50 AM, Konstantin Belousov wrote: > Hello, > there seems to be a problem with device attach sequence offered by newbus. > Basically, when device attach method is executing, device is not fully > initialized yet. Also the device state in the newbus part of the world > is DS_ALIVE. There is definitely no shattering news in the statements, > but drivers that e.g. create devfs node to communicate with consumers > are prone to a race. > > If /dev node is created inside device attach method, then usermode > can start calling cdevsw methods before device fully initialized itself. > Even more, if device tries to use newbus helpers in cdevsw methods, > like device_busy(9), then panic occurs "called for unatteched device". > I get reports from users about this issues, to it is not something > that only could happen. Yes, we hit that (the tasting fro geom making requests as soona s we added the devfs entries came in before we were quite set up) with the fusion-IO driver but it was "solvable" by moving the device creation to be the very last thing after all the setup was done. We were prepared to handle requests by then. We don't seem to have hit the device_busy(etc) problem, but your solution sounds reasonable. > I propose to add DEVICE_AFTER_ATTACH() driver method, to be called DEVICE_ATTACH_LATE() or DEVICE_ATTACH_DONE() > from newbus right after device attach finished and newbus considers > the device fully initialized. Driver then could create devfs node > in the after_attach method instead of attach. Please see the patch below. > > diff --git a/sys/kern/device_if.m b/sys/kern/device_if.m > index eb720eb..9db74e2 100644 > --- a/sys/kern/device_if.m > +++ b/sys/kern/device_if.m > @@ -43,6 +43,10 @@ INTERFACE device; > # Default implementations of some methods. > # > CODE { > + static void null_after_attach(device_t dev) > + { > + } > + > static int null_shutdown(device_t dev) > { > return 0; > @@ -199,6 +203,21 @@ METHOD int attach { > }; > > /** > + * @brief Notify the driver that device is in attached state > + * > + * Called after driver is successfully attached to the device and > + * corresponding device_t is fully operational. Driver now may expose > + * the device to the consumers, e.g. create devfs nodes. > + * > + * @param dev the device to probe > + * > + * @see DEVICE_ATTACH() > + */ > +METHOD void after_attach { > + device_t dev; > +} DEFAULT null_after_attach; > + > +/** > * @brief Detach a driver from a device. > * > * This can be called if the user is replacing the > diff --git a/sys/kern/subr_bus.c b/sys/kern/subr_bus.c > index d485b9f..6d849cb 100644 > --- a/sys/kern/subr_bus.c > +++ b/sys/kern/subr_bus.c > @@ -2743,6 +2743,7 @@ device_attach(device_t dev) > dev->state = DS_ATTACHED; > dev->flags&= ~DF_DONENOMATCH; > devadded(dev); > + DEVICE_AFTER_ATTACH(dev); > return (0); > } > From owner-freebsd-drivers@FreeBSD.ORG Sat Apr 7 21:34:13 2012 Return-Path: Delivered-To: drivers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 93E80106564A; Sat, 7 Apr 2012 21:34:13 +0000 (UTC) (envelope-from julian@freebsd.org) Received: from vps1.elischer.org (vps1.elischer.org [204.109.63.16]) by mx1.freebsd.org (Postfix) with ESMTP id 4629B8FC0A; Sat, 7 Apr 2012 21:34:13 +0000 (UTC) Received: from julian-mac.elischer.org (c-67-180-24-15.hsd1.ca.comcast.net [67.180.24.15]) (authenticated bits=0) by vps1.elischer.org (8.14.5/8.14.5) with ESMTP id q37LYBpg019961 (version=TLSv1/SSLv3 cipher=DHE-RSA-CAMELLIA256-SHA bits=256 verify=NO); Sat, 7 Apr 2012 14:34:12 -0700 (PDT) (envelope-from julian@freebsd.org) Message-ID: <4F80B2FE.1030007@freebsd.org> Date: Sat, 07 Apr 2012 14:34:54 -0700 From: Julian Elischer User-Agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X 10.4; en-US; rv:1.9.2.28) Gecko/20120306 Thunderbird/3.1.20 MIME-Version: 1.0 To: Konstantin Belousov References: <20120407125056.GS2358@deviant.kiev.zoral.com.ua> <4F80579B.4040205@freebsd.org> <20120407151514.GW2358@deviant.kiev.zoral.com.ua> In-Reply-To: <20120407151514.GW2358@deviant.kiev.zoral.com.ua> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: drivers@freebsd.org, Nathan Whitehorn , current@freebsd.org Subject: Re: device_attach(9) and driver initialization X-BeenThere: freebsd-drivers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Writing device drivers for FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 07 Apr 2012 21:34:13 -0000 On 4/7/12 8:15 AM, Konstantin Belousov wrote: > On Sat, Apr 07, 2012 at 10:04:59AM -0500, Nathan Whitehorn wrote: >> On 04/07/12 07:50, Konstantin Belousov wrote: >>> Hello, >>> there seems to be a problem with device attach sequence offered by newbus. >>> Basically, when device attach method is executing, device is not fully >>> initialized yet. Also the device state in the newbus part of the world >>> is DS_ALIVE. There is definitely no shattering news in the statements, >>> but drivers that e.g. create devfs node to communicate with consumers >>> are prone to a race. >>> >>> If /dev node is created inside device attach method, then usermode >>> can start calling cdevsw methods before device fully initialized itself. >>> Even more, if device tries to use newbus helpers in cdevsw methods, >>> like device_busy(9), then panic occurs "called for unatteched device". >>> I get reports from users about this issues, to it is not something >>> that only could happen. >>> >>> I propose to add DEVICE_AFTER_ATTACH() driver method, to be called >> >from newbus right after device attach finished and newbus considers >>> the device fully initialized. Driver then could create devfs node >>> in the after_attach method instead of attach. Please see the patch below. >>> >> Something like this would also be very useful for drivers that need to >> interact across the device tree, if newbus called it only after all >> drivers have been attached. Drivers that need to see other potentially >> attached drivers now need to do some hacks with SYSINIT. Would it be >> possible to do this? I don't think it changes the functionality you need. > I am definitely fine with postponing a call further, but I am not sure > how to define that point and how to implement your proposal. I am mostly > thinking of the case of kld being loaded, since for compiled-in drivers, > there is simply no usermode to make the havoc during attach. no, Geom starts tasting disk devices as as soon as you make the device. > For the boot time attachments, I am not sure when to declare the end. > E.g. USB does asynchronous device discovery. > > Could you prototype a change that would do what you propose ?