On 31 March 2015 at 01:20, Dmitry Torokhov <[email protected]> wrote: > Some devices take a long time when initializing, and not all drivers are > suited to initialize their devices when they are open. For example, > input drivers need to interrogate their devices in order to publish > device's capabilities before userspace will open them. When such drivers > are compiled into kernel they may stall entire kernel initialization. > > This change allows drivers request for their probe functions to be > called asynchronously during driver and device registration (manual > binding is still synchronous). Because async_schedule is used to perform > asynchronous calls module loading will still wait for the probing to > complete.
But what about parents? Don't we need to make sure that before probing a device its parent has finished probing already? This backtrace illustrates the problem: [<c0014818>] (__dabt_svc) from [<c03737ac>] (host1x_syncpt_alloc+0x14/0x134) [<c03737ac>] (host1x_syncpt_alloc) from [<c03738f4>] (host1x_syncpt_request+0x28/0x2c) [<c03738f4>] (host1x_syncpt_request) from [<c03b55ec>] (tegra_dc_probe+0x198/0x35c) [<c03b55ec>] (tegra_dc_probe) from [<c03cb5a0>] (platform_drv_probe+0x54/0xbc) [<c03cb5a0>] (platform_drv_probe) from [<c03c96e0>] (driver_probe_device+0x184/0x2c4) [<c03c96e0>] (driver_probe_device) from [<c03c98bc>] (__driver_attach+0x9c/0xa0) [<c03c98bc>] (__driver_attach) from [<c03c78d8>] (bus_for_each_dev+0x78/0xac) [<c03c78d8>] (bus_for_each_dev) from [<c03c9070>] (driver_attach+0x2c/0x30) [<c03c9070>] (driver_attach) from [<c03c7e10>] (driver_attach_async+0x18/0x1c) [<c03c7e10>] (driver_attach_async) from [<c004afd0>] (async_run_entry_fn+0x58/0x128) [<c004afd0>] (async_run_entry_fn) from [<c0042470>] (process_one_work+0x140/0x50c) [<c0042470>] (process_one_work) from [<c0042890>] (worker_thread+0x54/0x52c) [<c0042890>] (worker_thread) from [<c0048554>] (kthread+0xec/0x104) [<c0048554>] (kthread) from [<c000fc28>] (ret_from_fork+0x14/0x2c) host1x_syncpt_request() assumes that the parent of the DC (host1x) has been probed already and its drvdata is available. Thanks, Tomeu > Note that the end goal is to make the probing asynchronous by default, > so annotating drivers with PROBE_PREFER_ASYNCHRONOUS is a temporary > measure that allows us to speed up boot process while we validating and > fixing the rest of the drivers and preparing userspace. > > This change is based on earlier patch by "Luis R. Rodriguez" > <[email protected]> > > Signed-off-by: Dmitry Torokhov <[email protected]> > --- > drivers/base/base.h | 1 + > drivers/base/bus.c | 31 +++++++--- > drivers/base/dd.c | 149 > ++++++++++++++++++++++++++++++++++++++++++------- > include/linux/device.h | 28 ++++++++++ > 4 files changed, 182 insertions(+), 27 deletions(-) > > diff --git a/drivers/base/base.h b/drivers/base/base.h > index 251c5d3..fd3347d 100644 > --- a/drivers/base/base.h > +++ b/drivers/base/base.h > @@ -116,6 +116,7 @@ static inline int driver_match_device(struct > device_driver *drv, > { > return drv->bus->match ? drv->bus->match(dev, drv) : 1; > } > +extern bool driver_allows_async_probing(struct device_driver *drv); > > extern int driver_add_groups(struct device_driver *drv, > const struct attribute_group **groups); > diff --git a/drivers/base/bus.c b/drivers/base/bus.c > index 79bc203..5005924 100644 > --- a/drivers/base/bus.c > +++ b/drivers/base/bus.c > @@ -10,6 +10,7 @@ > * > */ > > +#include <linux/async.h> > #include <linux/device.h> > #include <linux/module.h> > #include <linux/errno.h> > @@ -549,15 +550,12 @@ void bus_probe_device(struct device *dev) > { > struct bus_type *bus = dev->bus; > struct subsys_interface *sif; > - int ret; > > if (!bus) > return; > > - if (bus->p->drivers_autoprobe) { > - ret = device_attach(dev); > - WARN_ON(ret < 0); > - } > + if (bus->p->drivers_autoprobe) > + device_initial_probe(dev); > > mutex_lock(&bus->p->mutex); > list_for_each_entry(sif, &bus->p->interfaces, node) > @@ -659,6 +657,17 @@ static ssize_t uevent_store(struct device_driver *drv, > const char *buf, > } > static DRIVER_ATTR_WO(uevent); > > +static void driver_attach_async(void *_drv, async_cookie_t cookie) > +{ > + struct device_driver *drv = _drv; > + int ret; > + > + ret = driver_attach(drv); > + > + pr_debug("bus: '%s': driver %s async attach completed: %d\n", > + drv->bus->name, drv->name, ret); > +} > + > /** > * bus_add_driver - Add a driver to the bus. > * @drv: driver. > @@ -691,9 +700,15 @@ int bus_add_driver(struct device_driver *drv) > > klist_add_tail(&priv->knode_bus, &bus->p->klist_drivers); > if (drv->bus->p->drivers_autoprobe) { > - error = driver_attach(drv); > - if (error) > - goto out_unregister; > + if (driver_allows_async_probing(drv)) { > + pr_debug("bus: '%s': probing driver %s > asynchronously\n", > + drv->bus->name, drv->name); > + async_schedule(driver_attach_async, drv); > + } else { > + error = driver_attach(drv); > + if (error) > + goto out_unregister; > + } > } > module_add_driver(drv->owner, drv); > > diff --git a/drivers/base/dd.c b/drivers/base/dd.c > index e843fdb..2ad33b2 100644 > --- a/drivers/base/dd.c > +++ b/drivers/base/dd.c > @@ -417,31 +417,95 @@ int driver_probe_device(struct device_driver *drv, > struct device *dev) > return ret; > } > > -static int __device_attach(struct device_driver *drv, void *data) > +bool driver_allows_async_probing(struct device_driver *drv) > { > - struct device *dev = data; > + return drv->probe_type == PROBE_PREFER_ASYNCHRONOUS; > +} > + > +struct device_attach_data { > + struct device *dev; > + > + /* > + * Indicates whether we are are considering asynchronous probing or > + * not. Only initial binding after device or driver registration > + * (including deferral processing) may be done asynchronously, the > + * rest is always synchronous, as we expect it is being done by > + * request from userspace. > + */ > + bool check_async; > + > + /* > + * Indicates if we are binding synchronous or asynchronous drivers. > + * When asynchronous probing is enabled we'll execute 2 passes > + * over drivers: first pass doing synchronous probing and second > + * doing asynchronous probing (if synchronous did not succeed - > + * most likely because there was no driver requiring synchronous > + * probing - and we found asynchronous driver during first pass). > + * The 2 passes are done because we can't shoot asynchronous > + * probe for given device and driver from bus_for_each_drv() since > + * driver pointer is not guaranteed to stay valid once > + * bus_for_each_drv() iterates to the next driver on the bus. > + */ > + bool want_async; > + > + /* > + * We'll set have_async to 'true' if, while scanning for matching > + * driver, we'll encounter one that requests asynchronous probing. > + */ > + bool have_async; > +}; > + > +static int __device_attach_driver(struct device_driver *drv, void *_data) > +{ > + struct device_attach_data *data = _data; > + struct device *dev = data->dev; > + bool async_allowed; > + > + /* > + * Check if device has already been claimed. This may > + * happen with driver loading, device discovery/registration, > + * and deferred probe processing happens all at once with > + * multiple threads. > + */ > + if (dev->driver) > + return -EBUSY; > > if (!driver_match_device(drv, dev)) > return 0; > > + async_allowed = driver_allows_async_probing(drv); > + > + if (async_allowed) > + data->have_async = true; > + > + if (data->check_async && async_allowed != data->want_async) > + return 0; > + > return driver_probe_device(drv, dev); > } > > -/** > - * device_attach - try to attach device to a driver. > - * @dev: device. > - * > - * Walk the list of drivers that the bus has and call > - * driver_probe_device() for each pair. If a compatible > - * pair is found, break out and return. > - * > - * Returns 1 if the device was bound to a driver; > - * 0 if no matching driver was found; > - * -ENODEV if the device is not registered. > - * > - * When called for a USB interface, @dev->parent lock must be held. > - */ > -int device_attach(struct device *dev) > +static void __device_attach_async_helper(void *_dev, async_cookie_t cookie) > +{ > + struct device *dev = _dev; > + struct device_attach_data data = { > + .dev = dev, > + .check_async = true, > + .want_async = true, > + }; > + > + device_lock(dev); > + > + bus_for_each_drv(dev->bus, NULL, &data, __device_attach_driver); > + dev_dbg(dev, "async probe completed\n"); > + > + pm_request_idle(dev); > + > + device_unlock(dev); > + > + put_device(dev); > +} > + > +int __device_attach(struct device *dev, bool allow_async) > { > int ret = 0; > > @@ -459,15 +523,59 @@ int device_attach(struct device *dev) > ret = 0; > } > } else { > - ret = bus_for_each_drv(dev->bus, NULL, dev, __device_attach); > - pm_request_idle(dev); > + struct device_attach_data data = { > + .dev = dev, > + .check_async = allow_async, > + .want_async = false, > + }; > + > + ret = bus_for_each_drv(dev->bus, NULL, &data, > + __device_attach_driver); > + if (!ret && allow_async && data.have_async) { > + /* > + * If we could not find appropriate driver > + * synchronously and we are allowed to do > + * async probes and there are drivers that > + * want to probe asynchronously, we'll > + * try them. > + */ > + dev_dbg(dev, "scheduling asynchronous probe\n"); > + get_device(dev); > + async_schedule(__device_attach_async_helper, dev); > + } else { > + pm_request_idle(dev); > + } > } > out_unlock: > device_unlock(dev); > return ret; > } > + > +/** > + * device_attach - try to attach device to a driver. > + * @dev: device. > + * > + * Walk the list of drivers that the bus has and call > + * driver_probe_device() for each pair. If a compatible > + * pair is found, break out and return. > + * > + * Returns 1 if the device was bound to a driver; > + * 0 if no matching driver was found; > + * -ENODEV if the device is not registered. > + * > + * When called for a USB interface, @dev->parent lock must be held. > + */ > +int device_attach(struct device *dev) > +{ > + return __device_attach(dev, false); > +} > EXPORT_SYMBOL_GPL(device_attach); > > +void device_initial_probe(struct device *dev) > +{ > + __device_attach(dev, true); > +} > + > static int __driver_attach(struct device *dev, void *data) > { > struct device_driver *drv = data; > @@ -522,6 +630,9 @@ static void __device_release_driver(struct device *dev) > > drv = dev->driver; > if (drv) { > + if (driver_allows_async_probing(drv)) > + async_synchronize_full(); > + > pm_runtime_get_sync(dev); > > driver_sysfs_remove(dev); > diff --git a/include/linux/device.h b/include/linux/device.h > index 884aa6e..400cacd 100644 > --- a/include/linux/device.h > +++ b/include/linux/device.h > @@ -196,12 +196,38 @@ extern struct kset *bus_get_kset(struct bus_type *bus); > extern struct klist *bus_get_device_klist(struct bus_type *bus); > > /** > + * enum probe_type - device driver probe type to try > + * Device drivers may opt in for special handling of their > + * respective probe routines. This tells the core what to > + * expect and prefer. > + * > + * @PROBE_SYNCHRONOUS: Default. Drivers expect their probe routines > + * to run synchronously with driver and device registration > + * (with the exception of -EPROBE_DEFER handling - re-probing > + * always ends up being done asynchronously). > + * @PROBE_PREFER_ASYNCHRONOUS: Drivers for "slow" devices which > + * probing order is not essential for booting the system may > + * opt into executing their probes asynchronously. > + * > + * Note that the end goal is to switch the kernel to use asynchronous > + * probing by default, so annotating drivers with > + * %PROBE_PREFER_ASYNCHRONOUS is a temporary measure that allows us > + * to speed up boot process while we are validating the rest of the > + * drivers. > + */ > +enum probe_type { > + PROBE_SYNCHRONOUS, > + PROBE_PREFER_ASYNCHRONOUS, > +}; > + > +/** > * struct device_driver - The basic device driver structure > * @name: Name of the device driver. > * @bus: The bus which the device of this driver belongs to. > * @owner: The module owner. > * @mod_name: Used for built-in modules. > * @suppress_bind_attrs: Disables bind/unbind via sysfs. > + * @probe_type: Type of the probe (synchronous or asynchronous) to > use. > * @of_match_table: The open firmware table. > * @acpi_match_table: The ACPI match table. > * @probe: Called to query the existence of a specific device, > @@ -235,6 +261,7 @@ struct device_driver { > const char *mod_name; /* used for built-in modules > */ > > bool suppress_bind_attrs; /* disables bind/unbind via sysfs */ > + enum probe_type probe_type; > > const struct of_device_id *of_match_table; > const struct acpi_device_id *acpi_match_table; > @@ -972,6 +999,7 @@ extern int __must_check device_bind_driver(struct device > *dev); > extern void device_release_driver(struct device *dev); > extern int __must_check device_attach(struct device *dev); > extern int __must_check driver_attach(struct device_driver *drv); > +extern void device_initial_probe(struct device *dev); > extern int __must_check device_reprobe(struct device *dev); > > /* > -- > 2.2.0.rc0.207.ga3a616c > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to [email protected] > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
