| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <5979079.r7hYkyMCOG@aspire.rjw.lan>
Date: Thu, 31 Jan 2019 00:44:55 +0100
From: "Rafael J. Wysocki" <rjw@...ysocki.net>
To: Alexander Duyck <alexander.h.duyck@...ux.intel.com>
Cc: linux-kernel@...r.kernel.org, gregkh@...uxfoundation.org,
mcgrof@...nel.org, linux-nvdimm@...ts.01.org, tj@...nel.org,
akpm@...ux-foundation.org, linux-pm@...r.kernel.org,
jiangshanlai@...il.com, rafael@...nel.org, len.brown@...el.com,
pavel@....cz, zwisler@...nel.org, dan.j.williams@...el.com,
dave.jiang@...el.com, bvanassche@....org
Subject: Re: [driver-core PATCH v10 3/9] driver core: Probe devices asynchronously instead of the driver
On Tuesday, January 22, 2019 7:39:21 PM CET Alexander Duyck wrote:
> Probe devices asynchronously instead of the driver. This results in us
> seeing the same behavior if the device is registered before the driver or
> after. This way we can avoid serializing the initialization should the
> driver not be loaded until after the devices have already been added.
>
> The motivation behind this is that if we have a set of devices that
> take a significant amount of time to load we can greatly reduce the time to
> load by processing them in parallel instead of one at a time. In addition,
> each device can exist on a different node so placing a single thread on one
> CPU to initialize all of the devices for a given driver can result in poor
> performance on a system with multiple nodes.
>
> This approach can reduce the time needed to scan SCSI LUNs significantly.
> The only way to realize that speedup is by enabling more concurrency which
> is what is achieved with this patch.
>
> To achieve this it was necessary to add a new member "async_driver" to the
> device_private structure to store the driver pointer while we wait on the
> deferred probe call.
>
> Reviewed-by: Bart Van Assche <bvanassche@....org>
> Reviewed-by: Dan Williams <dan.j.williams@...el.com>
> Signed-off-by: Alexander Duyck <alexander.h.duyck@...ux.intel.com>
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> ---
> drivers/base/base.h | 2 ++
> drivers/base/bus.c | 23 +++--------------------
> drivers/base/dd.c | 43 +++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 48 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/base/base.h b/drivers/base/base.h
> index b6966511b0ca..b405436ee28e 100644
> --- a/drivers/base/base.h
> +++ b/drivers/base/base.h
> @@ -65,6 +65,7 @@ struct driver_private {
> * binding of drivers which were unable to get all the resources needed by
> * the device; typically because it depends on another driver getting
> * probed first.
> + * @async_driver - pointer to device driver awaiting probe via async_probe
> * @device - pointer back to the struct device that this structure is
> * associated with.
> * @dead - This device is currently either in the process of or has been
> @@ -80,6 +81,7 @@ struct device_private {
> struct klist_node knode_bus;
> struct klist_node knode_class;
> struct list_head deferred_probe;
> + struct device_driver *async_driver;
> struct device *device;
> u8 dead:1;
> };
> diff --git a/drivers/base/bus.c b/drivers/base/bus.c
> index cfa5b63b5582..0a58e969f8b7 100644
> --- a/drivers/base/bus.c
> +++ b/drivers/base/bus.c
> @@ -610,17 +610,6 @@ 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.
> @@ -653,15 +642,9 @@ int bus_add_driver(struct device_driver *drv)
>
> klist_add_tail(&priv->knode_bus, &bus->p->klist_drivers);
> if (drv->bus->p->drivers_autoprobe) {
> - 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;
> - }
> + 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 34556359c7da..627ad05064e0 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -925,6 +925,30 @@ int device_driver_attach(struct device_driver *drv, struct device *dev)
> return ret;
> }
>
> +static void __driver_attach_async_helper(void *_dev, async_cookie_t cookie)
> +{
> + struct device *dev = _dev;
> + struct device_driver *drv;
> + int ret = 0;
> +
> + __device_driver_lock(dev, dev->parent);
> +
> + drv = dev->p->async_driver;
> +
> + /*
> + * If device has been removed or someone has already successfully
> + * bound a driver before us just skip the driver probe call.
> + */
> + if (!dev->p->dead && !dev->driver)
> + ret = driver_probe_device(drv, dev);
> +
> + __device_driver_unlock(dev, dev->parent);
> +
> + dev_dbg(dev, "driver %s async attach completed: %d\n", drv->name, ret);
> +
> + put_device(dev);
> +}
> +
> static int __driver_attach(struct device *dev, void *data)
> {
> struct device_driver *drv = data;
> @@ -952,6 +976,25 @@ static int __driver_attach(struct device *dev, void *data)
> return ret;
> } /* ret > 0 means positive match */
>
> + if (driver_allows_async_probing(drv)) {
> + /*
> + * Instead of probing the device synchronously we will
> + * probe it asynchronously to allow for more parallelism.
> + *
> + * We only take the device lock here in order to guarantee
> + * that the dev->driver and async_driver fields are protected
> + */
> + dev_dbg(dev, "probing driver %s asynchronously\n", drv->name);
> + device_lock(dev);
> + if (!dev->driver) {
> + get_device(dev);
> + dev->p->async_driver = drv;
> + async_schedule(__driver_attach_async_helper, dev);
> + }
> + device_unlock(dev);
> + return 0;
> + }
> +
> device_driver_attach(drv, dev);
>
> return 0;
>
>
Powered by blists - more mailing lists