From owner-freebsd-drivers@FreeBSD.ORG Sun Apr 8 03:08:55 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 C9E3C106564A; Sun, 8 Apr 2012 03:08:55 +0000 (UTC) (envelope-from imp@bsdimp.com) Received: from harmony.bsdimp.com (bsdimp.com [199.45.160.85]) by mx1.freebsd.org (Postfix) with ESMTP id 720C58FC0A; Sun, 8 Apr 2012 03:08:55 +0000 (UTC) Received: from 63.imp.bsdimp.com (63.imp.bsdimp.com [10.0.0.63]) (authenticated bits=0) by harmony.bsdimp.com (8.14.4/8.14.3) with ESMTP id q3838eQS076876 (version=TLSv1/SSLv3 cipher=DHE-DSS-AES128-SHA bits=128 verify=NO); Sat, 7 Apr 2012 21:08:41 -0600 (MDT) (envelope-from imp@bsdimp.com) Mime-Version: 1.0 (Apple Message framework v1084) Content-Type: text/plain; charset=us-ascii From: Warner Losh In-Reply-To: <1333810001.1082.36.camel@revolution.hippie.lan> Date: Sat, 7 Apr 2012 21:08:41 -0600 Content-Transfer-Encoding: quoted-printable Message-Id: <7C5089DC-AB9B-458F-A606-B432505BD961@bsdimp.com> References: <20120407125056.GS2358@deviant.kiev.zoral.com.ua> <1333810001.1082.36.camel@revolution.hippie.lan> To: Ian Lepore X-Mailer: Apple Mail (2.1084) X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.0.1 (harmony.bsdimp.com [10.0.0.6]); Sat, 07 Apr 2012 21:08:42 -0600 (MDT) 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: Sun, 08 Apr 2012 03:08:55 -0000 On Apr 7, 2012, at 8:46 AM, 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 = 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. >>=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 = below. >>=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. Also, more generally, don't create the dev nodes before you are ready to = deal with requests. Why do we need to uglify everything here? If you = can't do that, you can check a bit in the softc and return EBUSY or = ENXIO on open if that bit says that your driver isn't ready to accept = requests. Warner=