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