[PATCH v2 0/8] Asynchronous device/driver probing support

[PATCH v2 0/8] Asynchronous device/driver probing support

[Dmitry Torokhov]

This series is a combination of changes proposed by Luis a couple months
ago and implementation used by Chrome OS. The issue we are trying to solve
here is "slow" devices and drivers spending "too much time" in their probe()
methods and it affects:

- overall kernel boot process when drivers are compiled into the kernel
  and slow devices stall entire boot progress;
- systemd desire to time out module loading process.

Unlike Luis' proposal we do make use of asycn_schedule() infrastructure
instead of using a dedicated workqueue, so all  existing synchronization
points in kernel that wait for device registration still work the same.
Also, the asynchronous probing is done not only during driver registration
(i.e. when devices are probed asynchronously only if they are registered
before the driver), but also during device registration and deferred probe
handling. This way slow devices do not stall kernel boot even when drivers
are compiled into the kernel.

The last patch is for adventurous people to try and force
fully-asynchronous boot. It works for me with limited success - I can boot
Rockhip-based box to userspace as long as I force serial to be sychronously
probed and ignore the fact that most devices are using "dummy" regulators
as regulator subsystem really expects regulators to be registered in
orderly fashion on OF-based systems.

Changes from v1:

- Changed verbage in change logs and code to emphasise that
  PROBE_PREFER_ASYNCHRONOUS is a temporary measure and the end goal is
  to enable asynchronous probing by default, as requested by Tejun.

Thanks,
Dmitry

[PATCH 1/8] module: add extra argument for parse_params() callback

[Dmitry Torokhov]

From: "Luis R. Rodriguez" <mcgrof@suse.com>

This adds an extra argument onto parse_params() to be used
as a way to make the unused callback a bit more useful and
generic by allowing the caller to pass on a data structure
of its choice. An example use case is to allow us to easily
make module parameters for every module which we will do
next.

@ parse @
identifier name, args, params, num, level_min, level_max;
identifier unknown, param, val, doing;
type s16;
@@
 extern char *parse_args(const char *name,
 			 char *args,
 			 const struct kernel_param *params,
 			 unsigned num,
 			 s16 level_min,
 			 s16 level_max,
+			 void *arg,
 			 int (*unknown)(char *param, char *val,
					const char *doing
+					, void *arg
					));

@ parse_mod @
identifier name, args, params, num, level_min, level_max;
identifier unknown, param, val, doing;
type s16;
@@
 char *parse_args(const char *name,
 			 char *args,
 			 const struct kernel_param *params,
 			 unsigned num,
 			 s16 level_min,
 			 s16 level_max,
+			 void *arg,
 			 int (*unknown)(char *param, char *val,
					const char *doing
+					, void *arg
					))
{
	...
}

@ parse_args_found @
expression R, E1, E2, E3, E4, E5, E6;
identifier func;
@@

(
	R =
	parse_args(E1, E2, E3, E4, E5, E6,
+		   NULL,
		   func);
|
	R =
	parse_args(E1, E2, E3, E4, E5, E6,
+		   NULL,
		   &func);
|
	R =
	parse_args(E1, E2, E3, E4, E5, E6,
+		   NULL,
		   NULL);
|
	parse_args(E1, E2, E3, E4, E5, E6,
+		   NULL,
		   func);
|
	parse_args(E1, E2, E3, E4, E5, E6,
+		   NULL,
		   &func);
|
	parse_args(E1, E2, E3, E4, E5, E6,
+		   NULL,
		   NULL);
)

@ parse_args_unused depends on parse_args_found @
identifier parse_args_found.func;
@@

int func(char *param, char *val, const char *unused
+		 , void *arg
		 )
{
	...
}

@ mod_unused depends on parse_args_found @
identifier parse_args_found.func;
expression A1, A2, A3;
@@

-	func(A1, A2, A3);
+	func(A1, A2, A3, NULL);

Generated-by: Coccinelle SmPL
Cc: cocci@systeme.lip6.fr
Cc: Tejun Heo <tj@kernel.org>
Cc: Arjan van de Ven <arjan@linux.intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Felipe Contreras <felipe.contreras@gmail.com>
Cc: Ewan Milne <emilne@redhat.com>
Cc: Jean Delvare <jdelvare@suse.de>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: linux-kernel@vger.kernel.org
Reviewed-by: Tejun Heo <tj@kernel.org>
Acked-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
---
 arch/powerpc/mm/hugetlbpage.c |  4 ++--
 include/linux/moduleparam.h   |  3 ++-
 init/main.c                   | 25 +++++++++++++++----------
 kernel/module.c               |  6 ++++--
 kernel/params.c               | 11 +++++++----
 lib/dynamic_debug.c           |  4 ++--
 6 files changed, 32 insertions(+), 21 deletions(-)

diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
index 7e408bf..a444c23 100644
--- a/arch/powerpc/mm/hugetlbpage.c
+++ b/arch/powerpc/mm/hugetlbpage.c
@@ -336,7 +336,7 @@ int alloc_bootmem_huge_page(struct hstate *hstate)
 unsigned long gpage_npages[MMU_PAGE_COUNT];
 
 static int __init do_gpage_early_setup(char *param, char *val,
-				       const char *unused)
+				       const char *unused, void *arg)
 {
 	static phys_addr_t size;
 	unsigned long npages;
@@ -385,7 +385,7 @@ void __init reserve_hugetlb_gpages(void)
 
 	strlcpy(cmdline, boot_command_line, COMMAND_LINE_SIZE);
 	parse_args("hugetlb gpages", cmdline, NULL, 0, 0, 0,
-			&do_gpage_early_setup);
+			NULL, &do_gpage_early_setup);
 
 	/*
 	 * Walk gpage list in reverse, allocating larger page sizes first.
diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
index 1c9effa..1392370 100644
--- a/include/linux/moduleparam.h
+++ b/include/linux/moduleparam.h
@@ -357,8 +357,9 @@ extern char *parse_args(const char *name,
 		      unsigned num,
 		      s16 level_min,
 		      s16 level_max,
+		      void *arg,
 		      int (*unknown)(char *param, char *val,
-			      const char *doing));
+				     const char *doing, void *arg));
 
 /* Called by module remove. */
 #ifdef CONFIG_SYSFS
diff --git a/init/main.c b/init/main.c
index 739a677..b36373a 100644
--- a/init/main.c
+++ b/init/main.c
@@ -235,7 +235,8 @@ static int __init loglevel(char *str)
 early_param("loglevel", loglevel);
 
 /* Change NUL term back to "=", to make "param" the whole string. */
-static int __init repair_env_string(char *param, char *val, const char *unused)
+static int __init repair_env_string(char *param, char *val,
+				    const char *unused, void *arg)
 {
 	if (val) {
 		/* param=val or param="val"? */
@@ -252,14 +253,15 @@ static int __init repair_env_string(char *param, char *val, const char *unused)
 }
 
 /* Anything after -- gets handed straight to init. */
-static int __init set_init_arg(char *param, char *val, const char *unused)
+static int __init set_init_arg(char *param, char *val,
+			       const char *unused, void *arg)
 {
 	unsigned int i;
 
 	if (panic_later)
 		return 0;
 
-	repair_env_string(param, val, unused);
+	repair_env_string(param, val, unused, NULL);
 
 	for (i = 0; argv_init[i]; i++) {
 		if (i == MAX_INIT_ARGS) {
@@ -276,9 +278,10 @@ static int __init set_init_arg(char *param, char *val, const char *unused)
  * Unknown boot options get handed to init, unless they look like
  * unused parameters (modprobe will find them in /proc/cmdline).
  */
-static int __init unknown_bootoption(char *param, char *val, const char *unused)
+static int __init unknown_bootoption(char *param, char *val,
+				     const char *unused, void *arg)
 {
-	repair_env_string(param, val, unused);
+	repair_env_string(param, val, unused, NULL);
 
 	/* Handle obsolete-style parameters */
 	if (obsolete_checksetup(param))
@@ -409,7 +412,8 @@ static noinline void __init_refok rest_init(void)
 }
 
 /* Check for early params. */
-static int __init do_early_param(char *param, char *val, const char *unused)
+static int __init do_early_param(char *param, char *val,
+				 const char *unused, void *arg)
 {
 	const struct obs_kernel_param *p;
 
@@ -428,7 +432,8 @@ static int __init do_early_param(char *param, char *val, const char *unused)
 
 void __init parse_early_options(char *cmdline)
 {
-	parse_args("early options", cmdline, NULL, 0, 0, 0, do_early_param);
+	parse_args("early options", cmdline, NULL, 0, 0, 0, NULL,
+		   do_early_param);
 }
 
 /* Arch code calls this early on, or if not, just before other parsing. */
@@ -534,10 +539,10 @@ asmlinkage __visible void __init start_kernel(void)
 	after_dashes = parse_args("Booting kernel",
 				  static_command_line, __start___param,
 				  __stop___param - __start___param,
-				  -1, -1, &unknown_bootoption);
+				  -1, -1, NULL, &unknown_bootoption);
 	if (!IS_ERR_OR_NULL(after_dashes))
 		parse_args("Setting init args", after_dashes, NULL, 0, -1, -1,
-			   set_init_arg);
+			   NULL, set_init_arg);
 
 	jump_label_init();
 
@@ -846,7 +851,7 @@ static void __init do_initcall_level(int level)
 		   initcall_command_line, __start___param,
 		   __stop___param - __start___param,
 		   level, level,
-		   &repair_env_string);
+		   NULL, &repair_env_string);
 
 	for (fn = initcall_levels[level]; fn < initcall_levels[level+1]; fn++)
 		do_one_initcall(*fn);
diff --git a/kernel/module.c b/kernel/module.c
index 58bee45..05f6931 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -3217,7 +3217,8 @@ out:
 	return err;
 }
 
-static int unknown_module_param_cb(char *param, char *val, const char *modname)
+static int unknown_module_param_cb(char *param, char *val, const char *modname,
+				   void *arg)
 {
 	/* Check for magic 'dyndbg' arg */
 	int ret = ddebug_dyndbg_module_param_cb(param, val, modname);
@@ -3322,7 +3323,8 @@ static int load_module(struct load_info *info, const char __user *uargs,
 
 	/* Module is ready to execute: parsing args may do that. */
 	after_dashes = parse_args(mod->name, mod->args, mod->kp, mod->num_kp,
-				  -32768, 32767, unknown_module_param_cb);
+				  -32768, 32767, NULL,
+				  unknown_module_param_cb);
 	if (IS_ERR(after_dashes)) {
 		err = PTR_ERR(after_dashes);
 		goto bug_cleanup;
diff --git a/kernel/params.c b/kernel/params.c
index 728e05b..91b76d2 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -100,8 +100,9 @@ static int parse_one(char *param,
 		     unsigned num_params,
 		     s16 min_level,
 		     s16 max_level,
+		     void *arg,
 		     int (*handle_unknown)(char *param, char *val,
-				     const char *doing))
+				     const char *doing, void *arg))
 {
 	unsigned int i;
 	int err;
@@ -128,7 +129,7 @@ static int parse_one(char *param,
 
 	if (handle_unknown) {
 		pr_debug("doing %s: %s='%s'\n", doing, param, val);
-		return handle_unknown(param, val, doing);
+		return handle_unknown(param, val, doing, arg);
 	}
 
 	pr_debug("Unknown argument '%s'\n", param);
@@ -194,7 +195,9 @@ char *parse_args(const char *doing,
 		 unsigned num,
 		 s16 min_level,
 		 s16 max_level,
-		 int (*unknown)(char *param, char *val, const char *doing))
+		 void *arg,
+		 int (*unknown)(char *param, char *val,
+				const char *doing, void *arg))
 {
 	char *param, *val;
 
@@ -214,7 +217,7 @@ char *parse_args(const char *doing,
 			return args;
 		irq_was_disabled = irqs_disabled();
 		ret = parse_one(param, val, doing, params, num,
-				min_level, max_level, unknown);
+				min_level, max_level, arg, unknown);
 		if (irq_was_disabled && !irqs_disabled())
 			pr_warn("%s: option '%s' enabled irq's!\n",
 				doing, param);
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index d8f3d31..e491e02 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -887,7 +887,7 @@ static int ddebug_dyndbg_param_cb(char *param, char *val,
 
 /* handle both dyndbg and $module.dyndbg params at boot */
 static int ddebug_dyndbg_boot_param_cb(char *param, char *val,
-				const char *unused)
+				const char *unused, void *arg)
 {
 	vpr_info("%s=\"%s\"\n", param, val);
 	return ddebug_dyndbg_param_cb(param, val, NULL, 0);
@@ -1028,7 +1028,7 @@ static int __init dynamic_debug_init(void)
 	 */
 	cmdline = kstrdup(saved_command_line, GFP_KERNEL);
 	parse_args("dyndbg params", cmdline, NULL,
-		   0, 0, 0, &ddebug_dyndbg_boot_param_cb);
+		   0, 0, 0, NULL, &ddebug_dyndbg_boot_param_cb);
 	kfree(cmdline);
 	return 0;
 
-- 
2.2.0.rc0.207.ga3a616c

[PATCH 2/8] driver-core: add asynchronous probing support for drivers

[Dmitry Torokhov]

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.

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"
<mcgrof@suse.com>

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 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

Re: [PATCH 2/8] driver-core: add asynchronous probing support for drivers

[Tomeu Vizoso]

On 31 March 2015 at 01:20, Dmitry Torokhov <dmitry.torokhov@gmail.com> 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"
> <mcgrof@suse.com>
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>  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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/8] driver-core: add asynchronous probing support for drivers

[Tomeu Vizoso]

On 29 May 2015 at 12:48, Tomeu Vizoso <tomeu.vizoso@gmail.com> wrote:
> On 31 March 2015 at 01:20, Dmitry Torokhov <dmitry.torokhov@gmail.com> 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?

Have realized that this is an existing problem that was just made more
probable by async probe, as before async probing the parent could have
deferred its probe and then its children would be probed.

Wonder if drivers should be modified to defer its probe if their
parent isn't probed yet, or if we can codify that in dd.c.

Regards,

Tomeu

> 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"
>> <mcgrof@suse.com>
>>
>> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> ---
>>  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 majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/8] driver-core: add asynchronous probing support for drivers

[Tomeu Vizoso]

On 29 May 2015 at 15:23, Tomeu Vizoso <tomeu.vizoso@gmail.com> wrote:
> On 29 May 2015 at 12:48, Tomeu Vizoso <tomeu.vizoso@gmail.com> wrote:
>> On 31 March 2015 at 01:20, Dmitry Torokhov <dmitry.torokhov@gmail.com> 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?
>
> Have realized that this is an existing problem that was just made more
> probable by async probe, as before async probing the parent could have
> deferred its probe and then its children would be probed.
>
> Wonder if drivers should be modified to defer its probe if their
> parent isn't probed yet, or if we can codify that in dd.c.

Also wonder what's the plan regarding USB interfaces requiring that
their parent's lock is taken before probing.

Thanks,

Tomeu

> Regards,
>
> Tomeu
>
>> 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"
>>> <mcgrof@suse.com>
>>>
>>> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>>> ---
>>>  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 majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/8] driver-core: add asynchronous probing support for drivers

[Dmitry Torokhov]

On Mon, Jun 01, 2015 at 02:04:01PM +0200, Tomeu Vizoso wrote:
> On 29 May 2015 at 15:23, Tomeu Vizoso <tomeu.vizoso@gmail.com> wrote:
> > On 29 May 2015 at 12:48, Tomeu Vizoso <tomeu.vizoso@gmail.com> wrote:
> >> On 31 March 2015 at 01:20, Dmitry Torokhov <dmitry.torokhov@gmail.com> 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?
> >
> > Have realized that this is an existing problem that was just made more
> > probable by async probe, as before async probing the parent could have
> > deferred its probe and then its children would be probed.
> >
> > Wonder if drivers should be modified to defer its probe if their
> > parent isn't probed yet, or if we can codify that in dd.c.
> 
> Also wonder what's the plan regarding USB interfaces requiring that
> their parent's lock is taken before probing.

Yes, indeed, we need to take paren's lock in async probe too. I'll make
a patch.

Thanks for spotting this.

-- 
Dmitry

Re: [PATCH 2/8] driver-core: add asynchronous probing support for drivers

[Dan Williams]

On Mon, Mar 30, 2015 at 4:20 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> 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.
>
> 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"
> <mcgrof@suse.com>
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>  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(-)

Just noticed this patch.  It caught my eye because I had a hard time
getting an open coded implementation of asynchronous probing to work
in the new libnvdimm subsystem.  Especially the messy races of tearing
things down while probing is still in flight.  I ended up implementing
asynchronous device registration which eliminated a lot of complexity
and of course the bugs.  In general I tend to think that async
registration is less risky than async probe since it keeps wider
portions of the traditional device model synchronous and leverages the
fact that the device model is already well prepared for asynchronous
arrival of devices due to hotplug.  Splitting the "initial probe" from
the "manual probe" case seems like a recipe for confusion.

Re: [PATCH 2/8] driver-core: add asynchronous probing support for drivers

[Luis R. Rodriguez]

On Sat, Jun 27, 2015 at 04:45:25PM -0700, Dan Williams wrote:
> On Mon, Mar 30, 2015 at 4:20 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> 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.
> >
> > 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"
> > <mcgrof@suse.com>
> >
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > ---
> >  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(-)
> 
> Just noticed this patch.  It caught my eye because I had a hard time
> getting an open coded implementation of asynchronous probing to work
> in the new libnvdimm subsystem.  Especially the messy races of tearing
> things down while probing is still in flight.  I ended up implementing
> asynchronous device registration which eliminated a lot of complexity
> and of course the bugs.  In general I tend to think that async
> registration is less risky than async probe since it keeps wider
> portions of the traditional device model synchronous 

but its not see -DEFER_PROBE even before async probe.

> and leverages the
> fact that the device model is already well prepared for asynchronous
> arrival of devices due to hotplug.

I think this sounds reasonable, do you have your code upstream or posted?
If not will you be at Plumbers? Maybe we shoudl talk about this as although
ChromeOS already likely already jumped on async probe we should address a
way forward and path forward for other distributions and I don't think anyone
is looking too much into it. async probe came to Linux for two reasons:

 * chromeos wanting it
 * an incorrect systemd assumption on how the driver core works

So long term we still need to address the systemd approach, are they going
to be defaulting now to async probe for all modules? How about for built-ins?

We should talk about this and maybe at plumbers.

> Splitting the "initial probe" from
> the "manual probe" case seems like a recipe for confusion.

If you can come up with pros / cons on both strategies it'd be
valuable.

  Luis

Re: [PATCH 2/8] driver-core: add asynchronous probing support for drivers

[Dmitry Torokhov]

On Sat, Jun 27, 2015 at 04:45:25PM -0700, Dan Williams wrote:
> On Mon, Mar 30, 2015 at 4:20 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> 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.
> >
> > 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"
> > <mcgrof@suse.com>
> >
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > ---
> >  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(-)
> 
> Just noticed this patch.  It caught my eye because I had a hard time
> getting an open coded implementation of asynchronous probing to work
> in the new libnvdimm subsystem.  Especially the messy races of tearing
> things down while probing is still in flight.

And that is exactly the reason why asynchronous probing was moved into
driver code. It knows the state of the device and knows when it is OK to
remove it or start suspend transitions, etc.

> I ended up implementing
> asynchronous device registration which eliminated a lot of complexity
> and of course the bugs.

serio and gameport subsystems have been using asynchronous registration
for ages (not because of probing but because of issues with serio ports
- such as Synaptics pass-through - stacked on top of each other and
historicaly driver code deadlocking if you try to register child
device on the same bus that parent is). However I was never too happy
with it because with asynchronous registration you can't really handle
errors. What do you do if device registartion fails?

> In general I tend to think that async
> registration is less risky than async probe since it keeps wider
> portions of the traditional device model synchronous and leverages the
> fact that the device model is already well prepared for asynchronous
> arrival of devices due to hotplug.  Splitting the "initial probe" from
> the "manual probe" case seems like a recipe for confusion.

The split is because again serio and USB subsystems resort to somewhat
manual binding due to the need of handling recursion on the bus. But
otherwise we already have asynchronous probing, because we have deferred
probes and also driver modules can be loaded asynchronously with device
registration.

If you have concrete examples where asynchronous probig as it merged
causes issues I'd love to hear and fix them.

Thanks.

-- 
Dmitry

[PATCH 3/8] driver-core: add driver module asynchronous probe support

[Dmitry Torokhov]

From: "Luis R. Rodriguez" <mcgrof@suse.com>

Some init systems may wish to express the desire to have device drivers
run their probe() code asynchronously. This implements support for this
and allows userspace to request async probe as a preference through a
generic shared device driver module parameter, async_probe.

Implementation for async probe is supported through a module parameter
given that since synchronous probe has been prevalent for years some
userspace might exist which relies on the fact that the device driver
will probe synchronously and the assumption that devices it provides
will be immediately available after this.

Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 Documentation/kernel-parameters.txt |  3 +++
 drivers/base/dd.c                   |  8 +++++++-
 include/linux/device.h              |  8 +++++---
 include/linux/module.h              |  2 ++
 kernel/module.c                     | 12 ++++++++++--
 5 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 427a603..035d668 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -929,6 +929,9 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 			Enable debug messages at boot time.  See
 			Documentation/dynamic-debug-howto.txt for details.
 
+	module.async_probe [KNL]
+			Enable asynchronous probe on this module.
+
 	early_ioremap_debug [KNL]
 			Enable debug messages in early_ioremap support. This
 			is useful for tracking down temporary early mappings
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 2ad33b2..7a2fa5d 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -419,7 +419,13 @@ int driver_probe_device(struct device_driver *drv, struct device *dev)
 
 bool driver_allows_async_probing(struct device_driver *drv)
 {
-	return drv->probe_type == PROBE_PREFER_ASYNCHRONOUS;
+	if (drv->probe_type == PROBE_PREFER_ASYNCHRONOUS)
+		return true;
+
+	if (drv->owner && drv->owner->async_probe_requested)
+		return true;
+
+	return false;
 }
 
 struct device_attach_data {
diff --git a/include/linux/device.h b/include/linux/device.h
index 400cacd..22ca487 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -201,10 +201,12 @@ extern struct klist *bus_get_device_klist(struct bus_type *bus);
  *	respective probe routines. This tells the core what to
  *	expect and prefer.
  *
- * @PROBE_SYNCHRONOUS: Default. Drivers expect their probe routines
+ * @PROBE_DEFAULT_STRATEGY: 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).
+ *	always ends up being done asynchronously) unless user
+ *	explicitly requested asynchronous probing via module
+ *	parameter.
  * @PROBE_PREFER_ASYNCHRONOUS: Drivers for "slow" devices which
  *	probing order is not essential for booting the system may
  *	opt into executing their probes asynchronously.
@@ -216,7 +218,7 @@ extern struct klist *bus_get_device_klist(struct bus_type *bus);
  * drivers.
  */
 enum probe_type {
-	PROBE_SYNCHRONOUS,
+	PROBE_DEFAULT_STRATEGY,
 	PROBE_PREFER_ASYNCHRONOUS,
 };
 
diff --git a/include/linux/module.h b/include/linux/module.h
index b03485b..edef82d 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -257,6 +257,8 @@ struct module {
 	bool sig_ok;
 #endif
 
+	bool async_probe_requested;
+
 	/* symbols that will be GPL-only in the near future. */
 	const struct kernel_symbol *gpl_future_syms;
 	const unsigned long *gpl_future_crcs;
diff --git a/kernel/module.c b/kernel/module.c
index 05f6931..7b2fe3e 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -3087,7 +3087,7 @@ static noinline int do_init_module(struct module *mod)
 	 *
 	 * http://thread.gmane.org/gmane.linux.kernel/1420814
 	 */
-	if (current->flags & PF_USED_ASYNC)
+	if (!mod->async_probe_requested && (current->flags & PF_USED_ASYNC))
 		async_synchronize_full();
 
 	mutex_lock(&module_mutex);
@@ -3220,8 +3220,16 @@ out:
 static int unknown_module_param_cb(char *param, char *val, const char *modname,
 				   void *arg)
 {
+	struct module *mod = arg;
+	int ret;
+
+	if (strcmp(param, "async_probe") == 0) {
+		mod->async_probe_requested = true;
+		return 0;
+	}
+
 	/* Check for magic 'dyndbg' arg */
-	int ret = ddebug_dyndbg_module_param_cb(param, val, modname);
+	ret = ddebug_dyndbg_module_param_cb(param, val, modname);
 	if (ret != 0)
 		pr_warn("%s: unknown parameter '%s' ignored\n", modname, param);
 	return 0;
-- 
2.2.0.rc0.207.ga3a616c

[PATCH 4/8] driver-core: enable drivers to opt-out of async probe

[Dmitry Torokhov]

From: "Luis R. Rodriguez" <mcgrof@suse.com>

There are drivers that can not be probed asynchronously. One such group
is platform drivers registered with platform_driver_probe(), which
expects driver's probe routine be discarded after the driver has been
registered and initial binding attempt executed. Also
platform_driver_probe() an error when no devices were bound to the
driver, allowing failing to load such driver module altogether.

Other drivers do not work well with asynchronous probing because of
driver bug or not optimal driver organization.

To allow using such drivers even when user requests asynchronous probing
as default boot strategy, let's allow them to opt out.

Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/base/dd.c      | 14 ++++++++++----
 include/linux/device.h | 13 +++++++------
 2 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 7a2fa5d..3929253 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -419,13 +419,19 @@ int driver_probe_device(struct device_driver *drv, struct device *dev)
 
 bool driver_allows_async_probing(struct device_driver *drv)
 {
-	if (drv->probe_type == PROBE_PREFER_ASYNCHRONOUS)
+	switch (drv->probe_type) {
+	case PROBE_PREFER_ASYNCHRONOUS:
 		return true;
 
-	if (drv->owner && drv->owner->async_probe_requested)
-		return true;
+	case PROBE_FORCE_SYNCHRONOUS:
+		return false;
+
+	default:
+		if (drv->owner && drv->owner->async_probe_requested)
+			return true;
 
-	return false;
+		return false;
+	}
 }
 
 struct device_attach_data {
diff --git a/include/linux/device.h b/include/linux/device.h
index 22ca487..d308fdf 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -201,15 +201,15 @@ extern struct klist *bus_get_device_klist(struct bus_type *bus);
  *	respective probe routines. This tells the core what to
  *	expect and prefer.
  *
- * @PROBE_DEFAULT_STRATEGY: 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) unless user
- *	explicitly requested asynchronous probing via module
- *	parameter.
+ * @PROBE_DEFAULT_STRATEGY: Used by drivers that work equally well
+ *	whether probed synchronously or 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.
+ * @PROBE_FORCE_SYNCHRONOUS: Use this to annotate drivers that need
+ *	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).
  *
  * Note that the end goal is to switch the kernel to use asynchronous
  * probing by default, so annotating drivers with
@@ -220,6 +220,7 @@ extern struct klist *bus_get_device_klist(struct bus_type *bus);
 enum probe_type {
 	PROBE_DEFAULT_STRATEGY,
 	PROBE_PREFER_ASYNCHRONOUS,
+	PROBE_FORCE_SYNCHRONOUS,
 };
 
 /**
-- 
2.2.0.rc0.207.ga3a616c

[PATCH 5/8] driver-core: platform_driver_probe() must probe synchronously

[Dmitry Torokhov]

Because platform_driver_probe() checks, after trying to register driver,
if there are any devices that driver successfully bound to, driver's
probe routine must be run synchronously.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/base/platform.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index ebf034b..063f0ab 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -613,6 +613,19 @@ int __init_or_module __platform_driver_probe(struct platform_driver *drv,
 {
 	int retval, code;
 
+	if (drv->driver.probe_type == PROBE_PREFER_ASYNCHRONOUS) {
+		pr_err("%s: drivers registered with %s can not be probed asynchronously\n",
+			 drv->driver.name, __func__);
+		return -EINVAL;
+	}
+
+	/*
+	 * We have to run our probes synchronously because we check if
+	 * we find any devices to bind to and exit with error if there
+	 * are any.
+	 */
+	drv->driver.probe_type = PROBE_FORCE_SYNCHRONOUS;
+
 	/*
 	 * Prevent driver from requesting probe deferral to avoid further
 	 * futile probe attempts.
-- 
2.2.0.rc0.207.ga3a616c

[PATCH 6/8] amd64_edac: enforce synchronous probe

[Dmitry Torokhov]

From: "Luis R. Rodriguez" <mcgrof@suse.com>

While testing asynchronous PCI probe on this driver I noticed it failed
because the driver checks if any of the PCI devices have been bound to
the driver after registering it, which obviously does not work if
probing is asynchronous.

While there are patches and discussions on how the driver should behave
are ongoing, let's enforce synchronous probe for this driver for now.

Reviewed-by: Tejun Heo <tj@kernel.org>
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/edac/amd64_edac.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index 92772ff..73aea40 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -2964,6 +2964,7 @@ static struct pci_driver amd64_pci_driver = {
 	.probe		= probe_one_instance,
 	.remove		= remove_one_instance,
 	.id_table	= amd64_pci_table,
+	.driver.probe_type = PROBE_FORCE_SYNCHRONOUS,
 };
 
 static void setup_pci_device(void)
-- 
2.2.0.rc0.207.ga3a616c

[PATCH 7/8] module: add core_param_unsafe

[Dmitry Torokhov]

Similarly to module_param_unsafe(), add the helper to be used by core
code wishing to expose unsafe debugging or testing parameters that taint
the kernel when set.

Acked-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 include/linux/moduleparam.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
index 1392370..6480dca 100644
--- a/include/linux/moduleparam.h
+++ b/include/linux/moduleparam.h
@@ -310,6 +310,15 @@ static inline void __kernel_param_unlock(void)
 #define core_param(name, var, type, perm)				\
 	param_check_##type(name, &(var));				\
 	__module_param_call("", name, &param_ops_##type, &var, perm, -1, 0)
+
+/**
+ * core_param_unsafe - same as core_param but taints kernel
+ */
+#define core_param_unsafe(name, var, type, perm)		\
+	param_check_##type(name, &(var));				\
+	__module_param_call("", name, &param_ops_##type, &var, perm,	\
+			    -1, KERNEL_PARAM_FL_UNSAFE)
+
 #endif /* !MODULE */
 
 /**
-- 
2.2.0.rc0.207.ga3a616c

[PATCH 8/8] driver-core: allow enabling async probing for all modules and builtins

[Dmitry Torokhov]

From: "Luis R. Rodriguez" <mcgrof@suse.com>

Folks wishing to test enabling async probe for all built-in drivers
and/or for all modules can use
__DEBUG__kernel_force_builtin_async_probe or
__DEBUG__kernel_force_modules_async_probe kernel parameters.

Activating either one will taint your kernel.

Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
[Dmitry: split off from another patch, split into 2 parameters, moved
over to core_param_unsafe()]
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 Documentation/kernel-parameters.txt | 10 ++++++++++
 drivers/base/dd.c                   | 13 +++++++++----
 kernel/module.c                     |  7 +++++++
 3 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 035d668..464dd56 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -932,6 +932,16 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 	module.async_probe [KNL]
 			Enable asynchronous probe on this module.
 
+	__DEBUG__kernel_force_builtin_async_probe [KNL]
+			Enable asynchronous probe on all built-in drivers.
+			This is is testing parameter and using it will taint
+			your kernel.
+
+	__DEBUG__kernel_force_modules_async_probe [KNL]
+			Enable asynchronous probe on all modules. This is
+			is testing parameter and using it will taint your
+			kernel.
+
 	early_ioremap_debug [KNL]
 			Enable debug messages in early_ioremap support. This
 			is useful for tracking down temporary early mappings
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 3929253..9463457 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -20,6 +20,7 @@
 #include <linux/device.h>
 #include <linux/delay.h>
 #include <linux/module.h>
+#include <linux/moduleparam.h>
 #include <linux/kthread.h>
 #include <linux/wait.h>
 #include <linux/async.h>
@@ -417,6 +418,10 @@ int driver_probe_device(struct device_driver *drv, struct device *dev)
 	return ret;
 }
 
+static bool force_builtin_async_probe;
+core_param_unsafe(__DEBUG__kernel_force_builtin_async_probe,
+		  force_builtin_async_probe, bool, 0644);
+
 bool driver_allows_async_probing(struct device_driver *drv)
 {
 	switch (drv->probe_type) {
@@ -427,10 +432,10 @@ bool driver_allows_async_probing(struct device_driver *drv)
 		return false;
 
 	default:
-		if (drv->owner && drv->owner->async_probe_requested)
-			return true;
-
-		return false;
+		if (drv->owner)
+			return drv->owner->async_probe_requested;
+		else
+			return force_builtin_async_probe;
 	}
 }
 
diff --git a/kernel/module.c b/kernel/module.c
index 7b2fe3e..8dad167 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -3235,6 +3235,10 @@ static int unknown_module_param_cb(char *param, char *val, const char *modname,
 	return 0;
 }
 
+static bool force_modules_async_probe;
+core_param_unsafe(__DEBUG__kernel_force_modules_async_probe,
+		  force_modules_async_probe, bool, 0644);
+
 /* Allocate and load the module: note that size of section 0 is always
    zero, and we rely on this for optional sections. */
 static int load_module(struct load_info *info, const char __user *uargs,
@@ -3329,6 +3333,9 @@ static int load_module(struct load_info *info, const char __user *uargs,
 	if (err)
 		goto ddebug_cleanup;
 
+	if (force_modules_async_probe)
+		mod->async_probe_requested = true;
+
 	/* Module is ready to execute: parsing args may do that. */
 	after_dashes = parse_args(mod->name, mod->args, mod->kp, mod->num_kp,
 				  -32768, 32767, NULL,
-- 
2.2.0.rc0.207.ga3a616c

Re: [PATCH 8/8] driver-core: allow enabling async probing for all modules and builtins

[Greg Kroah-Hartman]

On Mon, Mar 30, 2015 at 04:20:10PM -0700, Dmitry Torokhov wrote:
> From: "Luis R. Rodriguez" <mcgrof@suse.com>
> 
> Folks wishing to test enabling async probe for all built-in drivers
> and/or for all modules can use
> __DEBUG__kernel_force_builtin_async_probe or
> __DEBUG__kernel_force_modules_async_probe kernel parameters.
> 
> Activating either one will taint your kernel.
> 
> Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
> [Dmitry: split off from another patch, split into 2 parameters, moved
> over to core_param_unsafe()]
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

I've dropped this from my tree as I don't want to add these options,
only to have to remove them later on when it's found out that these were
a bad idea.

I don't want to create a user api that we have to keep around for
forever, and this would be such a thing (specifying how the kernel
probing works.)  For debugging, can't you just patch up your kernel and
test this out?  What's the real use of this?  Who do you want to enable
these?  And why?  What will you do with the information?

thanks,

greg k-h

Re: [PATCH 8/8] driver-core: allow enabling async probing for all modules and builtins

[Dmitry Torokhov]

On Wed, May 20, 2015 at 12:27:34AM -0700, Greg Kroah-Hartman wrote:
> On Mon, Mar 30, 2015 at 04:20:10PM -0700, Dmitry Torokhov wrote:
> > From: "Luis R. Rodriguez" <mcgrof@suse.com>
> > 
> > Folks wishing to test enabling async probe for all built-in drivers
> > and/or for all modules can use
> > __DEBUG__kernel_force_builtin_async_probe or
> > __DEBUG__kernel_force_modules_async_probe kernel parameters.
> > 
> > Activating either one will taint your kernel.
> > 
> > Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
> > [Dmitry: split off from another patch, split into 2 parameters, moved
> > over to core_param_unsafe()]
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> 
> I've dropped this from my tree as I don't want to add these options,
> only to have to remove them later on when it's found out that these were
> a bad idea.

OK.

>
> I don't want to create a user api that we have to keep around for
> forever, and this would be such a thing (specifying how the kernel
> probing works.)

Given that they are marked as __DEBUG and taint the kernel I do not
believe they shoudl be considered as an API/ABI. We can emphasise this
in docs and/or kernel messages.

>  For debugging, can't you just patch up your kernel and

I can, but I do not have all hardware in my possession to validate the
behavior.

> test this out?  What's the real use of this?  Who do you want to enable
> these?  And why?  What will you do with the information?

The expectation was that distribution developers might use these
switches when evaluating whether they are ready to switch to
asynchronous probing.

Thanks.

-- 
Dmitry

Re: [PATCH 8/8] driver-core: allow enabling async probing for all modules and builtins

[Greg Kroah-Hartman]

On Wed, May 20, 2015 at 09:44:59AM -0700, Dmitry Torokhov wrote:
> On Wed, May 20, 2015 at 12:27:34AM -0700, Greg Kroah-Hartman wrote:
> > On Mon, Mar 30, 2015 at 04:20:10PM -0700, Dmitry Torokhov wrote:
> > > From: "Luis R. Rodriguez" <mcgrof@suse.com>
> > > 
> > > Folks wishing to test enabling async probe for all built-in drivers
> > > and/or for all modules can use
> > > __DEBUG__kernel_force_builtin_async_probe or
> > > __DEBUG__kernel_force_modules_async_probe kernel parameters.
> > > 
> > > Activating either one will taint your kernel.
> > > 
> > > Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
> > > [Dmitry: split off from another patch, split into 2 parameters, moved
> > > over to core_param_unsafe()]
> > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > 
> > I've dropped this from my tree as I don't want to add these options,
> > only to have to remove them later on when it's found out that these were
> > a bad idea.
> 
> OK.
> 
> >
> > I don't want to create a user api that we have to keep around for
> > forever, and this would be such a thing (specifying how the kernel
> > probing works.)
> 
> Given that they are marked as __DEBUG and taint the kernel I do not
> believe they shoudl be considered as an API/ABI. We can emphasise this
> in docs and/or kernel messages.

But they are options a user can set on the command line, and changing
command lines is a pain.  Yes, it's a bit odd name, but we don't have
any other such naming scheme for command line options, so I don't know
what to suggest here.

> >  For debugging, can't you just patch up your kernel and
> 
> I can, but I do not have all hardware in my possession to validate the
> behavior.
> 
> > test this out?  What's the real use of this?  Who do you want to enable
> > these?  And why?  What will you do with the information?
> 
> The expectation was that distribution developers might use these
> switches when evaluating whether they are ready to switch to
> asynchronous probing.

Distro developers will never do that, they have to support just too many
different hardware types.  And there's no real gain here for them.

greg k-h

Re: [PATCH 8/8] driver-core: allow enabling async probing for all modules and builtins

[Luis R. Rodriguez]

On Wed, May 20, 2015 at 09:34:09PM -0700, Greg Kroah-Hartman wrote:
> On Wed, May 20, 2015 at 09:44:59AM -0700, Dmitry Torokhov wrote:
> > On Wed, May 20, 2015 at 12:27:34AM -0700, Greg Kroah-Hartman wrote:
> > > On Mon, Mar 30, 2015 at 04:20:10PM -0700, Dmitry Torokhov wrote:
> > > > From: "Luis R. Rodriguez" <mcgrof@suse.com>
> > > > 
> > > > Folks wishing to test enabling async probe for all built-in drivers
> > > > and/or for all modules can use
> > > > __DEBUG__kernel_force_builtin_async_probe or
> > > > __DEBUG__kernel_force_modules_async_probe kernel parameters.
> > > > 
> > > > Activating either one will taint your kernel.
> > > > 
> > > > Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
> > > > [Dmitry: split off from another patch, split into 2 parameters, moved
> > > > over to core_param_unsafe()]
> > > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > > 
> > > I've dropped this from my tree as I don't want to add these options,
> > > only to have to remove them later on when it's found out that these were
> > > a bad idea.
> > 
> > OK.
> > 
> > >
> > > I don't want to create a user api that we have to keep around for
> > > forever, and this would be such a thing (specifying how the kernel
> > > probing works.)
> > 
> > Given that they are marked as __DEBUG and taint the kernel I do not
> > believe they shoudl be considered as an API/ABI. We can emphasise this
> > in docs and/or kernel messages.
> 
> But they are options a user can set on the command line, and changing
> command lines is a pain.  Yes, it's a bit odd name, but we don't have
> any other such naming scheme for command line options, so I don't know
> what to suggest here.

What if we document such prefixes are for loose things we can change
at any time? That is, not API and we treat as we do with debugfs.

> > >  For debugging, can't you just patch up your kernel and
> > 
> > I can, but I do not have all hardware in my possession to validate the
> > behavior.
> > 
> > > test this out?  What's the real use of this?  Who do you want to enable
> > > these?  And why?  What will you do with the information?
> > 
> > The expectation was that distribution developers might use these
> > switches when evaluating whether they are ready to switch to
> > asynchronous probing.
> 
> Distro developers will never do that, they have to support just too many
> different hardware types.  And there's no real gain here for them.

Maybe old distros, new distros like ChromeOS will and in this case have.
I think Dmitry did at least. I would bet that some mobile platform distros
might want to do this too.

  Luis

Re: [PATCH v2 0/8] Asynchronous device/driver probing support

[Tejun Heo]

On Mon, Mar 30, 2015 at 04:20:02PM -0700, Dmitry Torokhov wrote:
> This series is a combination of changes proposed by Luis a couple months
> ago and implementation used by Chrome OS. The issue we are trying to solve
> here is "slow" devices and drivers spending "too much time" in their probe()
> methods and it affects:
> 
> - overall kernel boot process when drivers are compiled into the kernel
>   and slow devices stall entire boot progress;
> - systemd desire to time out module loading process.
> 
> Unlike Luis' proposal we do make use of asycn_schedule() infrastructure
> instead of using a dedicated workqueue, so all  existing synchronization
> points in kernel that wait for device registration still work the same.
> Also, the asynchronous probing is done not only during driver registration
> (i.e. when devices are probed asynchronously only if they are registered
> before the driver), but also during device registration and deferred probe
> handling. This way slow devices do not stall kernel boot even when drivers
> are compiled into the kernel.
> 
> The last patch is for adventurous people to try and force
> fully-asynchronous boot. It works for me with limited success - I can boot
> Rockhip-based box to userspace as long as I force serial to be sychronously
> probed and ignore the fact that most devices are using "dummy" regulators
> as regulator subsystem really expects regulators to be registered in
> orderly fashion on OF-based systems.
> 
> Changes from v1:
> 
> - Changed verbage in change logs and code to emphasise that
>   PROBE_PREFER_ASYNCHRONOUS is a temporary measure and the end goal is
>   to enable asynchronous probing by default, as requested by Tejun.

Looks good to me.  Please feel free to add

Acked-by: Tejun Heo <tj@kernel.org>

Thanks.

-- 
tejun

Re: [PATCH v2 0/8] Asynchronous device/driver probing support

[Dmitry Torokhov]

On Tue, Mar 31, 2015 at 04:39:49PM -0400, Tejun Heo wrote:
> On Mon, Mar 30, 2015 at 04:20:02PM -0700, Dmitry Torokhov wrote:
> > This series is a combination of changes proposed by Luis a couple months
> > ago and implementation used by Chrome OS. The issue we are trying to solve
> > here is "slow" devices and drivers spending "too much time" in their probe()
> > methods and it affects:
> > 
> > - overall kernel boot process when drivers are compiled into the kernel
> >   and slow devices stall entire boot progress;
> > - systemd desire to time out module loading process.
> > 
> > Unlike Luis' proposal we do make use of asycn_schedule() infrastructure
> > instead of using a dedicated workqueue, so all  existing synchronization
> > points in kernel that wait for device registration still work the same.
> > Also, the asynchronous probing is done not only during driver registration
> > (i.e. when devices are probed asynchronously only if they are registered
> > before the driver), but also during device registration and deferred probe
> > handling. This way slow devices do not stall kernel boot even when drivers
> > are compiled into the kernel.
> > 
> > The last patch is for adventurous people to try and force
> > fully-asynchronous boot. It works for me with limited success - I can boot
> > Rockhip-based box to userspace as long as I force serial to be sychronously
> > probed and ignore the fact that most devices are using "dummy" regulators
> > as regulator subsystem really expects regulators to be registered in
> > orderly fashion on OF-based systems.
> > 
> > Changes from v1:
> > 
> > - Changed verbage in change logs and code to emphasise that
> >   PROBE_PREFER_ASYNCHRONOUS is a temporary measure and the end goal is
> >   to enable asynchronous probing by default, as requested by Tejun.
> 
> Looks good to me.  Please feel free to add
> 
> Acked-by: Tejun Heo <tj@kernel.org>

Greg, it would be great if it could make it in 4.1.

Thanks!

-- 
Dmitry

Re: [PATCH v2 0/8] Asynchronous device/driver probing support

[Greg Kroah-Hartman]

On Mon, Apr 06, 2015 at 09:22:51AM -0700, Dmitry Torokhov wrote:
> On Tue, Mar 31, 2015 at 04:39:49PM -0400, Tejun Heo wrote:
> > On Mon, Mar 30, 2015 at 04:20:02PM -0700, Dmitry Torokhov wrote:
> > > This series is a combination of changes proposed by Luis a couple months
> > > ago and implementation used by Chrome OS. The issue we are trying to solve
> > > here is "slow" devices and drivers spending "too much time" in their probe()
> > > methods and it affects:
> > > 
> > > - overall kernel boot process when drivers are compiled into the kernel
> > >   and slow devices stall entire boot progress;
> > > - systemd desire to time out module loading process.
> > > 
> > > Unlike Luis' proposal we do make use of asycn_schedule() infrastructure
> > > instead of using a dedicated workqueue, so all  existing synchronization
> > > points in kernel that wait for device registration still work the same.
> > > Also, the asynchronous probing is done not only during driver registration
> > > (i.e. when devices are probed asynchronously only if they are registered
> > > before the driver), but also during device registration and deferred probe
> > > handling. This way slow devices do not stall kernel boot even when drivers
> > > are compiled into the kernel.
> > > 
> > > The last patch is for adventurous people to try and force
> > > fully-asynchronous boot. It works for me with limited success - I can boot
> > > Rockhip-based box to userspace as long as I force serial to be sychronously
> > > probed and ignore the fact that most devices are using "dummy" regulators
> > > as regulator subsystem really expects regulators to be registered in
> > > orderly fashion on OF-based systems.
> > > 
> > > Changes from v1:
> > > 
> > > - Changed verbage in change logs and code to emphasise that
> > >   PROBE_PREFER_ASYNCHRONOUS is a temporary measure and the end goal is
> > >   to enable asynchronous probing by default, as requested by Tejun.
> > 
> > Looks good to me.  Please feel free to add
> > 
> > Acked-by: Tejun Heo <tj@kernel.org>
> 
> Greg, it would be great if it could make it in 4.1.

It's on my list of patches to review next...

Re: [PATCH v2 0/8] Asynchronous device/driver probing support

[Dmitry Torokhov]

On Mon, Apr 06, 2015 at 07:45:30PM +0200, Greg Kroah-Hartman wrote:
> On Mon, Apr 06, 2015 at 09:22:51AM -0700, Dmitry Torokhov wrote:
> > On Tue, Mar 31, 2015 at 04:39:49PM -0400, Tejun Heo wrote:
> > > On Mon, Mar 30, 2015 at 04:20:02PM -0700, Dmitry Torokhov wrote:
> > > > This series is a combination of changes proposed by Luis a couple months
> > > > ago and implementation used by Chrome OS. The issue we are trying to solve
> > > > here is "slow" devices and drivers spending "too much time" in their probe()
> > > > methods and it affects:
> > > > 
> > > > - overall kernel boot process when drivers are compiled into the kernel
> > > >   and slow devices stall entire boot progress;
> > > > - systemd desire to time out module loading process.
> > > > 
> > > > Unlike Luis' proposal we do make use of asycn_schedule() infrastructure
> > > > instead of using a dedicated workqueue, so all  existing synchronization
> > > > points in kernel that wait for device registration still work the same.
> > > > Also, the asynchronous probing is done not only during driver registration
> > > > (i.e. when devices are probed asynchronously only if they are registered
> > > > before the driver), but also during device registration and deferred probe
> > > > handling. This way slow devices do not stall kernel boot even when drivers
> > > > are compiled into the kernel.
> > > > 
> > > > The last patch is for adventurous people to try and force
> > > > fully-asynchronous boot. It works for me with limited success - I can boot
> > > > Rockhip-based box to userspace as long as I force serial to be sychronously
> > > > probed and ignore the fact that most devices are using "dummy" regulators
> > > > as regulator subsystem really expects regulators to be registered in
> > > > orderly fashion on OF-based systems.
> > > > 
> > > > Changes from v1:
> > > > 
> > > > - Changed verbage in change logs and code to emphasise that
> > > >   PROBE_PREFER_ASYNCHRONOUS is a temporary measure and the end goal is
> > > >   to enable asynchronous probing by default, as requested by Tejun.
> > > 
> > > Looks good to me.  Please feel free to add
> > > 
> > > Acked-by: Tejun Heo <tj@kernel.org>
> > 
> > Greg, it would be great if it could make it in 4.1.
> 
> It's on my list of patches to review next...

Greg, could we make 4.2 please? ;)

Thanks.

-- 
Dmitry

Re: [PATCH v2 0/8] Asynchronous device/driver probing support

[Greg Kroah-Hartman]

On Mon, May 18, 2015 at 02:48:19PM -0700, Dmitry Torokhov wrote:
> On Mon, Apr 06, 2015 at 07:45:30PM +0200, Greg Kroah-Hartman wrote:
> > On Mon, Apr 06, 2015 at 09:22:51AM -0700, Dmitry Torokhov wrote:
> > > On Tue, Mar 31, 2015 at 04:39:49PM -0400, Tejun Heo wrote:
> > > > On Mon, Mar 30, 2015 at 04:20:02PM -0700, Dmitry Torokhov wrote:
> > > > > This series is a combination of changes proposed by Luis a couple months
> > > > > ago and implementation used by Chrome OS. The issue we are trying to solve
> > > > > here is "slow" devices and drivers spending "too much time" in their probe()
> > > > > methods and it affects:
> > > > > 
> > > > > - overall kernel boot process when drivers are compiled into the kernel
> > > > >   and slow devices stall entire boot progress;
> > > > > - systemd desire to time out module loading process.
> > > > > 
> > > > > Unlike Luis' proposal we do make use of asycn_schedule() infrastructure
> > > > > instead of using a dedicated workqueue, so all  existing synchronization
> > > > > points in kernel that wait for device registration still work the same.
> > > > > Also, the asynchronous probing is done not only during driver registration
> > > > > (i.e. when devices are probed asynchronously only if they are registered
> > > > > before the driver), but also during device registration and deferred probe
> > > > > handling. This way slow devices do not stall kernel boot even when drivers
> > > > > are compiled into the kernel.
> > > > > 
> > > > > The last patch is for adventurous people to try and force
> > > > > fully-asynchronous boot. It works for me with limited success - I can boot
> > > > > Rockhip-based box to userspace as long as I force serial to be sychronously
> > > > > probed and ignore the fact that most devices are using "dummy" regulators
> > > > > as regulator subsystem really expects regulators to be registered in
> > > > > orderly fashion on OF-based systems.
> > > > > 
> > > > > Changes from v1:
> > > > > 
> > > > > - Changed verbage in change logs and code to emphasise that
> > > > >   PROBE_PREFER_ASYNCHRONOUS is a temporary measure and the end goal is
> > > > >   to enable asynchronous probing by default, as requested by Tejun.
> > > > 
> > > > Looks good to me.  Please feel free to add
> > > > 
> > > > Acked-by: Tejun Heo <tj@kernel.org>
> > > 
> > > Greg, it would be great if it could make it in 4.1.
> > 
> > It's on my list of patches to review next...
> 
> Greg, could we make 4.2 please? ;)

Now queued up.

greg k-h

Re: [PATCH 2/8] driver-core: add asynchronous probing support for drivers

[Dan Williams]

On Fri, Jul 3, 2015 at 11:30 AM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
> On Sat, Jun 27, 2015 at 04:45:25PM -0700, Dan Williams wrote:
>> On Mon, Mar 30, 2015 at 4:20 PM, Dmitry Torokhov
>> <dmitry.torokhov@gmail.com> 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.
>> >
>> > 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"
>> > <mcgrof@suse.com>
>> >
>> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> > ---
>> >  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(-)
>>
>> Just noticed this patch.  It caught my eye because I had a hard time
>> getting an open coded implementation of asynchronous probing to work
>> in the new libnvdimm subsystem.  Especially the messy races of tearing
>> things down while probing is still in flight.  I ended up implementing
>> asynchronous device registration which eliminated a lot of complexity
>> and of course the bugs.  In general I tend to think that async
>> registration is less risky than async probe since it keeps wider
>> portions of the traditional device model synchronous
>
> but its not see -DEFER_PROBE even before async probe.

Except in that case you know probe has been seen by the driver at
least once.  So I see that as less of a surprise, but point taken.

>> and leverages the
>> fact that the device model is already well prepared for asynchronous
>> arrival of devices due to hotplug.
>
> I think this sounds reasonable, do you have your code upstream or posted?

Yes, see nd_device_register() in drivers/nvdimm/bus.c

> If not will you be at Plumbers?

Yes.

> Maybe we shoudl talk about this as although
> ChromeOS already likely already jumped on async probe we should address a
> way forward and path forward for other distributions and I don't think anyone
> is looking too much into it. async probe came to Linux for two reasons:
>
>  * chromeos wanting it
>  * an incorrect systemd assumption on how the driver core works
>
> So long term we still need to address the systemd approach, are they going
> to be defaulting now to async probe for all modules? How about for built-ins?
>
> We should talk about this and maybe at plumbers.
>
>> Splitting the "initial probe" from
>> the "manual probe" case seems like a recipe for confusion.
>
> If you can come up with pros / cons on both strategies it'd be
> valuable.

The problem I ran into was needing to remove devices that still had
yet to be probed and not being able to use registration completion vs
the device_lock() to effectively synchronize the sub-system.

Re: [PATCH 2/8] driver-core: add asynchronous probing support for drivers

[Luis R. Rodriguez]

On Sat, Jul 04, 2015 at 07:09:19AM -0700, Dan Williams wrote:
> On Fri, Jul 3, 2015 at 11:30 AM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
> > On Sat, Jun 27, 2015 at 04:45:25PM -0700, Dan Williams wrote:
> >> On Mon, Mar 30, 2015 at 4:20 PM, Dmitry Torokhov
> >> <dmitry.torokhov@gmail.com> 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.
> >> >
> >> > 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"
> >> > <mcgrof@suse.com>
> >> >
> >> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> >> > ---
> >> >  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(-)
> >>
> >> Just noticed this patch.  It caught my eye because I had a hard time
> >> getting an open coded implementation of asynchronous probing to work
> >> in the new libnvdimm subsystem.  Especially the messy races of tearing
> >> things down while probing is still in flight.  I ended up implementing
> >> asynchronous device registration which eliminated a lot of complexity
> >> and of course the bugs.  In general I tend to think that async
> >> registration is less risky than async probe since it keeps wider
> >> portions of the traditional device model synchronous
> >
> > but its not see -DEFER_PROBE even before async probe.
> 
> Except in that case you know probe has been seen by the driver at
> least once.  So I see that as less of a surprise, but point taken.
> 
> >> and leverages the
> >> fact that the device model is already well prepared for asynchronous
> >> arrival of devices due to hotplug.
> >
> > I think this sounds reasonable, do you have your code upstream or posted?
> 
> Yes, see nd_device_register() in drivers/nvdimm/bus.c

It should be I think rather easy for Dmitry to see if he can convert this input
driver (not yet upstream) to this API and see if the same issues are fixed.
This however does not address systemd's assumption over detachment of module
load and probe. The inherent problem there was the timeout implemented and
carried in systemd over the worker that uses modlib to load modules. Upon
review the code was complex enough already and surely increasing the timeout
helps but that doesn't address all issues with a general timeout in place.
At SUSE we ended up not using a timeout for kmod built-in commands. That
leaves the original timeout purpose in place. The code for async probe was
not put in the kernel though but since its now upstream we should be able
to replace that userspace systemd work around with async probe, but systemd
folks would need to decide what they want to do. For full gory details of
this refer to:

https://bugzilla.novell.com/show_bug.cgi?id=889297

> > If not will you be at Plumbers?
> 
> Yes.

Great, Tom, Dmitry, will you be at Plumbers?

> > Maybe we shoudl talk about this as although
> > ChromeOS already likely already jumped on async probe we should address a
> > way forward and path forward for other distributions and I don't think anyone
> > is looking too much into it. async probe came to Linux for two reasons:
> >
> >  * chromeos wanting it
> >  * an incorrect systemd assumption on how the driver core works
> >
> > So long term we still need to address the systemd approach, are they going
> > to be defaulting now to async probe for all modules? How about for built-ins?
> >
> > We should talk about this and maybe at plumbers.
> >
> >> Splitting the "initial probe" from
> >> the "manual probe" case seems like a recipe for confusion.
> >
> > If you can come up with pros / cons on both strategies it'd be
> > valuable.
> 
> The problem I ran into was needing to remove devices that still had
> yet to be probed and not being able to use registration completion vs
> the device_lock() to effectively synchronize the sub-system.

Interesting, what cases would this happen under?

  Luis

Re: [PATCH 2/8] driver-core: add asynchronous probing support for drivers

[Dmitry Torokhov]

On Tue, Jul 07, 2015 at 01:23:15AM +0200, Luis R. Rodriguez wrote:
> On Sat, Jul 04, 2015 at 07:09:19AM -0700, Dan Williams wrote:
> > On Fri, Jul 3, 2015 at 11:30 AM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
> > > On Sat, Jun 27, 2015 at 04:45:25PM -0700, Dan Williams wrote:
> > >> On Mon, Mar 30, 2015 at 4:20 PM, Dmitry Torokhov
> > >> <dmitry.torokhov@gmail.com> 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.
> > >> >
> > >> > 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"
> > >> > <mcgrof@suse.com>
> > >> >
> > >> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > >> > ---
> > >> >  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(-)
> > >>
> > >> Just noticed this patch.  It caught my eye because I had a hard time
> > >> getting an open coded implementation of asynchronous probing to work
> > >> in the new libnvdimm subsystem.  Especially the messy races of tearing
> > >> things down while probing is still in flight.  I ended up implementing
> > >> asynchronous device registration which eliminated a lot of complexity
> > >> and of course the bugs.  In general I tend to think that async
> > >> registration is less risky than async probe since it keeps wider
> > >> portions of the traditional device model synchronous
> > >
> > > but its not see -DEFER_PROBE even before async probe.
> > 
> > Except in that case you know probe has been seen by the driver at
> > least once.  So I see that as less of a surprise, but point taken.
> > 
> > >> and leverages the
> > >> fact that the device model is already well prepared for asynchronous
> > >> arrival of devices due to hotplug.
> > >
> > > I think this sounds reasonable, do you have your code upstream or posted?
> > 
> > Yes, see nd_device_register() in drivers/nvdimm/bus.c
> 
> It should be I think rather easy for Dmitry to see if he can convert this input
> driver (not yet upstream) to this API and see if the same issues are fixed.

No, I would rather not as it means we lose error handling on device
registration.

Thanks.

-- 
Dmitry

Re: [PATCH 2/8] driver-core: add asynchronous probing support for drivers

[Dan Williams]

On Mon, Jul 6, 2015 at 4:40 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Tue, Jul 07, 2015 at 01:23:15AM +0200, Luis R. Rodriguez wrote:
>> On Sat, Jul 04, 2015 at 07:09:19AM -0700, Dan Williams wrote:
>> > On Fri, Jul 3, 2015 at 11:30 AM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
>> > > On Sat, Jun 27, 2015 at 04:45:25PM -0700, Dan Williams wrote:
>> > >> On Mon, Mar 30, 2015 at 4:20 PM, Dmitry Torokhov
>> > >> <dmitry.torokhov@gmail.com> 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.
>> > >> >
>> > >> > 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"
>> > >> > <mcgrof@suse.com>
>> > >> >
>> > >> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> > >> > ---
>> > >> >  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(-)
>> > >>
>> > >> Just noticed this patch.  It caught my eye because I had a hard time
>> > >> getting an open coded implementation of asynchronous probing to work
>> > >> in the new libnvdimm subsystem.  Especially the messy races of tearing
>> > >> things down while probing is still in flight.  I ended up implementing
>> > >> asynchronous device registration which eliminated a lot of complexity
>> > >> and of course the bugs.  In general I tend to think that async
>> > >> registration is less risky than async probe since it keeps wider
>> > >> portions of the traditional device model synchronous
>> > >
>> > > but its not see -DEFER_PROBE even before async probe.
>> >
>> > Except in that case you know probe has been seen by the driver at
>> > least once.  So I see that as less of a surprise, but point taken.
>> >
>> > >> and leverages the
>> > >> fact that the device model is already well prepared for asynchronous
>> > >> arrival of devices due to hotplug.
>> > >
>> > > I think this sounds reasonable, do you have your code upstream or posted?
>> >
>> > Yes, see nd_device_register() in drivers/nvdimm/bus.c
>>
>> It should be I think rather easy for Dmitry to see if he can convert this input
>> driver (not yet upstream) to this API and see if the same issues are fixed.
>
> No, I would rather not as it means we lose error handling on device
> registration.
>

I think this is a red herring as I don't see how async probing is any
better at handling device registration errors.  The error is logged
and "handled" by the fact that a device fails to appear, what other
action would you take?  In fact libnvdimm does detect registration
failures and reports that in a parent device attribute (at least for a
region device and their namespace child devices).

Re: [PATCH 2/8] driver-core: add asynchronous probing support for drivers

[Dmitry Torokhov]

On Wed, Jul 08, 2015 at 05:36:04PM -0700, Dan Williams wrote:
> On Mon, Jul 6, 2015 at 4:40 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > On Tue, Jul 07, 2015 at 01:23:15AM +0200, Luis R. Rodriguez wrote:
> >> On Sat, Jul 04, 2015 at 07:09:19AM -0700, Dan Williams wrote:
> >> > On Fri, Jul 3, 2015 at 11:30 AM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
> >> > > On Sat, Jun 27, 2015 at 04:45:25PM -0700, Dan Williams wrote:
> >> > >> On Mon, Mar 30, 2015 at 4:20 PM, Dmitry Torokhov
> >> > >> <dmitry.torokhov@gmail.com> 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.
> >> > >> >
> >> > >> > 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"
> >> > >> > <mcgrof@suse.com>
> >> > >> >
> >> > >> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> >> > >> > ---
> >> > >> >  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(-)
> >> > >>
> >> > >> Just noticed this patch.  It caught my eye because I had a hard time
> >> > >> getting an open coded implementation of asynchronous probing to work
> >> > >> in the new libnvdimm subsystem.  Especially the messy races of tearing
> >> > >> things down while probing is still in flight.  I ended up implementing
> >> > >> asynchronous device registration which eliminated a lot of complexity
> >> > >> and of course the bugs.  In general I tend to think that async
> >> > >> registration is less risky than async probe since it keeps wider
> >> > >> portions of the traditional device model synchronous
> >> > >
> >> > > but its not see -DEFER_PROBE even before async probe.
> >> >
> >> > Except in that case you know probe has been seen by the driver at
> >> > least once.  So I see that as less of a surprise, but point taken.
> >> >
> >> > >> and leverages the
> >> > >> fact that the device model is already well prepared for asynchronous
> >> > >> arrival of devices due to hotplug.
> >> > >
> >> > > I think this sounds reasonable, do you have your code upstream or posted?
> >> >
> >> > Yes, see nd_device_register() in drivers/nvdimm/bus.c
> >>
> >> It should be I think rather easy for Dmitry to see if he can convert this input
> >> driver (not yet upstream) to this API and see if the same issues are fixed.
> >
> > No, I would rather not as it means we lose error handling on device
> > registration.
> >
> 
> I think this is a red herring as I don't see how async probing is any
> better at handling device registration errors.  The error is logged
> and "handled" by the fact that a device fails to appear, what other
> action would you take?  In fact libnvdimm does detect registration
> failures and reports that in a parent device attribute (at least for a
> region device and their namespace child devices).

What is libnvdimm behavior if you try to unload a module that tries to
register a device but it failed? Memory leak or crash, right?

-- 
Dmitry

Re: [PATCH 2/8] driver-core: add asynchronous probing support for drivers

[Dan Williams]

On Wed, Jul 8, 2015 at 5:49 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Wed, Jul 08, 2015 at 05:36:04PM -0700, Dan Williams wrote:
>> On Mon, Jul 6, 2015 at 4:40 PM, Dmitry Torokhov
>> <dmitry.torokhov@gmail.com> wrote:
>> > On Tue, Jul 07, 2015 at 01:23:15AM +0200, Luis R. Rodriguez wrote:
>> >> On Sat, Jul 04, 2015 at 07:09:19AM -0700, Dan Williams wrote:
>> >> > On Fri, Jul 3, 2015 at 11:30 AM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
>> >> > > On Sat, Jun 27, 2015 at 04:45:25PM -0700, Dan Williams wrote:
>> >> > >> On Mon, Mar 30, 2015 at 4:20 PM, Dmitry Torokhov
>> >> > >> <dmitry.torokhov@gmail.com> 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.
>> >> > >> >
>> >> > >> > 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"
>> >> > >> > <mcgrof@suse.com>
>> >> > >> >
>> >> > >> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> >> > >> > ---
>> >> > >> >  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(-)
>> >> > >>
>> >> > >> Just noticed this patch.  It caught my eye because I had a hard time
>> >> > >> getting an open coded implementation of asynchronous probing to work
>> >> > >> in the new libnvdimm subsystem.  Especially the messy races of tearing
>> >> > >> things down while probing is still in flight.  I ended up implementing
>> >> > >> asynchronous device registration which eliminated a lot of complexity
>> >> > >> and of course the bugs.  In general I tend to think that async
>> >> > >> registration is less risky than async probe since it keeps wider
>> >> > >> portions of the traditional device model synchronous
>> >> > >
>> >> > > but its not see -DEFER_PROBE even before async probe.
>> >> >
>> >> > Except in that case you know probe has been seen by the driver at
>> >> > least once.  So I see that as less of a surprise, but point taken.
>> >> >
>> >> > >> and leverages the
>> >> > >> fact that the device model is already well prepared for asynchronous
>> >> > >> arrival of devices due to hotplug.
>> >> > >
>> >> > > I think this sounds reasonable, do you have your code upstream or posted?
>> >> >
>> >> > Yes, see nd_device_register() in drivers/nvdimm/bus.c
>> >>
>> >> It should be I think rather easy for Dmitry to see if he can convert this input
>> >> driver (not yet upstream) to this API and see if the same issues are fixed.
>> >
>> > No, I would rather not as it means we lose error handling on device
>> > registration.
>> >
>>
>> I think this is a red herring as I don't see how async probing is any
>> better at handling device registration errors.  The error is logged
>> and "handled" by the fact that a device fails to appear, what other
>> action would you take?  In fact libnvdimm does detect registration
>> failures and reports that in a parent device attribute (at least for a
>> region device and their namespace child devices).
>
> What is libnvdimm behavior if you try to unload a module that tries to
> register a device but it failed? Memory leak or crash, right?

No, in the case of the "region" driver it is part of the core
libnvdimm and it is pinned while any region device is active.  But, it
is a fair point for a generic facility it would need to consider cases
where a driver registers children and then is unloaded.  I'll need to
think about that one more relative to async probing.

Re: [PATCH 2/8] driver-core: add asynchronous probing support for drivers

[Dmitry Torokhov]

On Wed, Jul 08, 2015 at 06:00:41PM -0700, Dan Williams wrote:
> On Wed, Jul 8, 2015 at 5:49 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > On Wed, Jul 08, 2015 at 05:36:04PM -0700, Dan Williams wrote:
> >> On Mon, Jul 6, 2015 at 4:40 PM, Dmitry Torokhov
> >> <dmitry.torokhov@gmail.com> wrote:
> >> > On Tue, Jul 07, 2015 at 01:23:15AM +0200, Luis R. Rodriguez wrote:
> >> >> On Sat, Jul 04, 2015 at 07:09:19AM -0700, Dan Williams wrote:
> >> >> > On Fri, Jul 3, 2015 at 11:30 AM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
> >> >> > > On Sat, Jun 27, 2015 at 04:45:25PM -0700, Dan Williams wrote:
> >> >> > >> On Mon, Mar 30, 2015 at 4:20 PM, Dmitry Torokhov
> >> >> > >> <dmitry.torokhov@gmail.com> 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.
> >> >> > >> >
> >> >> > >> > 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"
> >> >> > >> > <mcgrof@suse.com>
> >> >> > >> >
> >> >> > >> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> >> >> > >> > ---
> >> >> > >> >  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(-)
> >> >> > >>
> >> >> > >> Just noticed this patch.  It caught my eye because I had a hard time
> >> >> > >> getting an open coded implementation of asynchronous probing to work
> >> >> > >> in the new libnvdimm subsystem.  Especially the messy races of tearing
> >> >> > >> things down while probing is still in flight.  I ended up implementing
> >> >> > >> asynchronous device registration which eliminated a lot of complexity
> >> >> > >> and of course the bugs.  In general I tend to think that async
> >> >> > >> registration is less risky than async probe since it keeps wider
> >> >> > >> portions of the traditional device model synchronous
> >> >> > >
> >> >> > > but its not see -DEFER_PROBE even before async probe.
> >> >> >
> >> >> > Except in that case you know probe has been seen by the driver at
> >> >> > least once.  So I see that as less of a surprise, but point taken.
> >> >> >
> >> >> > >> and leverages the
> >> >> > >> fact that the device model is already well prepared for asynchronous
> >> >> > >> arrival of devices due to hotplug.
> >> >> > >
> >> >> > > I think this sounds reasonable, do you have your code upstream or posted?
> >> >> >
> >> >> > Yes, see nd_device_register() in drivers/nvdimm/bus.c
> >> >>
> >> >> It should be I think rather easy for Dmitry to see if he can convert this input
> >> >> driver (not yet upstream) to this API and see if the same issues are fixed.
> >> >
> >> > No, I would rather not as it means we lose error handling on device
> >> > registration.
> >> >
> >>
> >> I think this is a red herring as I don't see how async probing is any
> >> better at handling device registration errors.  The error is logged
> >> and "handled" by the fact that a device fails to appear, what other
> >> action would you take?  In fact libnvdimm does detect registration
> >> failures and reports that in a parent device attribute (at least for a
> >> region device and their namespace child devices).
> >
> > What is libnvdimm behavior if you try to unload a module that tries to
> > register a device but it failed? Memory leak or crash, right?
> 
> No, in the case of the "region" driver it is part of the core
> libnvdimm and it is pinned while any region device is active.

No, not quite. Let's take a look for example at nd_btt_probe(). It calls
__nd_btt_create() which in turn calls __nd_device_register() which
returns void and asynchronously schedules device registration. Now
consider the device registration fails. The async code will drop 2
references to the device, effectively freeing it. In the mean time
nd_btt_probe() stores the device pointer which may or may no longer be
valid and goes on it's merry way using it.

The similar thing in nvdimm_create which returns a pointer that may no
longer be valid. I have not traced enough through the code to make sure
if it can blow up, but this kind of situation is not desirable,
especially if the async registration pattern is applied generally
throughout the kernel.

Thanks.

-- 
Dmitry

Re: [PATCH 2/8] driver-core: add asynchronous probing support for drivers

[Dan Williams]

On Wed, Jul 8, 2015 at 9:44 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Wed, Jul 08, 2015 at 06:00:41PM -0700, Dan Williams wrote:
>> On Wed, Jul 8, 2015 at 5:49 PM, Dmitry Torokhov
>> <dmitry.torokhov@gmail.com> wrote:
>> > On Wed, Jul 08, 2015 at 05:36:04PM -0700, Dan Williams wrote:
>> >> On Mon, Jul 6, 2015 at 4:40 PM, Dmitry Torokhov
>> >> <dmitry.torokhov@gmail.com> wrote:
>> >> > On Tue, Jul 07, 2015 at 01:23:15AM +0200, Luis R. Rodriguez wrote:
>> >> >> On Sat, Jul 04, 2015 at 07:09:19AM -0700, Dan Williams wrote:
>> >> >> > On Fri, Jul 3, 2015 at 11:30 AM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
>> >> >> > > On Sat, Jun 27, 2015 at 04:45:25PM -0700, Dan Williams wrote:
>> >> >> > >> On Mon, Mar 30, 2015 at 4:20 PM, Dmitry Torokhov
>> >> >> > >> <dmitry.torokhov@gmail.com> 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.
>> >> >> > >> >
>> >> >> > >> > 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"
>> >> >> > >> > <mcgrof@suse.com>
>> >> >> > >> >
>> >> >> > >> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> >> >> > >> > ---
>> >> >> > >> >  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(-)
>> >> >> > >>
>> >> >> > >> Just noticed this patch.  It caught my eye because I had a hard time
>> >> >> > >> getting an open coded implementation of asynchronous probing to work
>> >> >> > >> in the new libnvdimm subsystem.  Especially the messy races of tearing
>> >> >> > >> things down while probing is still in flight.  I ended up implementing
>> >> >> > >> asynchronous device registration which eliminated a lot of complexity
>> >> >> > >> and of course the bugs.  In general I tend to think that async
>> >> >> > >> registration is less risky than async probe since it keeps wider
>> >> >> > >> portions of the traditional device model synchronous
>> >> >> > >
>> >> >> > > but its not see -DEFER_PROBE even before async probe.
>> >> >> >
>> >> >> > Except in that case you know probe has been seen by the driver at
>> >> >> > least once.  So I see that as less of a surprise, but point taken.
>> >> >> >
>> >> >> > >> and leverages the
>> >> >> > >> fact that the device model is already well prepared for asynchronous
>> >> >> > >> arrival of devices due to hotplug.
>> >> >> > >
>> >> >> > > I think this sounds reasonable, do you have your code upstream or posted?
>> >> >> >
>> >> >> > Yes, see nd_device_register() in drivers/nvdimm/bus.c
>> >> >>
>> >> >> It should be I think rather easy for Dmitry to see if he can convert this input
>> >> >> driver (not yet upstream) to this API and see if the same issues are fixed.
>> >> >
>> >> > No, I would rather not as it means we lose error handling on device
>> >> > registration.
>> >> >
>> >>
>> >> I think this is a red herring as I don't see how async probing is any
>> >> better at handling device registration errors.  The error is logged
>> >> and "handled" by the fact that a device fails to appear, what other
>> >> action would you take?  In fact libnvdimm does detect registration
>> >> failures and reports that in a parent device attribute (at least for a
>> >> region device and their namespace child devices).
>> >
>> > What is libnvdimm behavior if you try to unload a module that tries to
>> > register a device but it failed? Memory leak or crash, right?
>>
>> No, in the case of the "region" driver it is part of the core
>> libnvdimm and it is pinned while any region device is active.
>
> No, not quite. Let's take a look for example at nd_btt_probe(). It calls
> __nd_btt_create() which in turn calls __nd_device_register() which
> returns void and asynchronously schedules device registration. Now
> consider the device registration fails. The async code will drop 2
> references to the device, effectively freeing it. In the mean time
> nd_btt_probe() stores the device pointer which may or may no longer be
> valid and goes on it's merry way using it.

nd_btt_probe() is the driver probe for the btt device.  If
registration fails then the device is never probed.

> The similar thing in nvdimm_create which returns a pointer that may no
> longer be valid

Exactly, which is why we fail the entire bus probe if any of the
nvdimm devices failed to register.  See nvdimm_bus_check_dimm_count().

> I have not traced enough through the code to make sure
> if it can blow up, but this kind of situation is not desirable,
> especially if the async registration pattern is applied generally
> throughout the kernel.

I do appreciate the review, but I don't think this signals the death
knell for async registration just yet.

Re: [PATCH 2/8] driver-core: add asynchronous probing support for drivers

[Tom Gundersen]

On Tue, Jul 7, 2015 at 1:23 AM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
> On Sat, Jul 04, 2015 at 07:09:19AM -0700, Dan Williams wrote:
>> On Fri, Jul 3, 2015 at 11:30 AM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
>> > On Sat, Jun 27, 2015 at 04:45:25PM -0700, Dan Williams wrote:
>> >> On Mon, Mar 30, 2015 at 4:20 PM, Dmitry Torokhov
>> >> <dmitry.torokhov@gmail.com> 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.
>> >> >
>> >> > 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"
>> >> > <mcgrof@suse.com>
>> >> >
>> >> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> >> > ---
>> >> >  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(-)
>> >>
>> >> Just noticed this patch.  It caught my eye because I had a hard time
>> >> getting an open coded implementation of asynchronous probing to work
>> >> in the new libnvdimm subsystem.  Especially the messy races of tearing
>> >> things down while probing is still in flight.  I ended up implementing
>> >> asynchronous device registration which eliminated a lot of complexity
>> >> and of course the bugs.  In general I tend to think that async
>> >> registration is less risky than async probe since it keeps wider
>> >> portions of the traditional device model synchronous
>> >
>> > but its not see -DEFER_PROBE even before async probe.
>>
>> Except in that case you know probe has been seen by the driver at
>> least once.  So I see that as less of a surprise, but point taken.
>>
>> >> and leverages the
>> >> fact that the device model is already well prepared for asynchronous
>> >> arrival of devices due to hotplug.
>> >
>> > I think this sounds reasonable, do you have your code upstream or posted?
>>
>> Yes, see nd_device_register() in drivers/nvdimm/bus.c
>
> It should be I think rather easy for Dmitry to see if he can convert this input
> driver (not yet upstream) to this API and see if the same issues are fixed.
> This however does not address systemd's assumption over detachment of module
> load and probe. The inherent problem there was the timeout implemented and
> carried in systemd over the worker that uses modlib to load modules. Upon
> review the code was complex enough already and surely increasing the timeout
> helps but that doesn't address all issues with a general timeout in place.
> At SUSE we ended up not using a timeout for kmod built-in commands. That
> leaves the original timeout purpose in place. The code for async probe was
> not put in the kernel though but since its now upstream we should be able
> to replace that userspace systemd work around with async probe, but systemd
> folks would need to decide what they want to do. For full gory details of
> this refer to:
>
> https://bugzilla.novell.com/show_bug.cgi?id=889297

FTR, this does not appear to be public, so I was not able to see it.

>> > If not will you be at Plumbers?
>>
>> Yes.
>
> Great, Tom, Dmitry, will you be at Plumbers?

Sadly, I won't make plumbers this year.

Cheers,

Tom

Re: [PATCH 2/8] driver-core: add asynchronous probing support for drivers

[Dmitry Torokhov]

On Sat, Jul 04, 2015 at 07:09:19AM -0700, Dan Williams wrote:
> On Fri, Jul 3, 2015 at 11:30 AM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
> > On Sat, Jun 27, 2015 at 04:45:25PM -0700, Dan Williams wrote:
> >> On Mon, Mar 30, 2015 at 4:20 PM, Dmitry Torokhov
> >> <dmitry.torokhov@gmail.com> 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.
> >> >
> >> > 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"
> >> > <mcgrof@suse.com>
> >> >
> >> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> >> > ---
> >> >  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(-)
> >>
> >> Just noticed this patch.  It caught my eye because I had a hard time
> >> getting an open coded implementation of asynchronous probing to work
> >> in the new libnvdimm subsystem.  Especially the messy races of tearing
> >> things down while probing is still in flight.  I ended up implementing
> >> asynchronous device registration which eliminated a lot of complexity
> >> and of course the bugs.  In general I tend to think that async
> >> registration is less risky than async probe since it keeps wider
> >> portions of the traditional device model synchronous
> >
> > but its not see -DEFER_PROBE even before async probe.
> 
> Except in that case you know probe has been seen by the driver at
> least once.  So I see that as less of a surprise, but point taken.
> 
> >> and leverages the
> >> fact that the device model is already well prepared for asynchronous
> >> arrival of devices due to hotplug.
> >
> > I think this sounds reasonable, do you have your code upstream or posted?
> 
> Yes, see nd_device_register() in drivers/nvdimm/bus.c

So no error handling whatsoever, as expected...

> 
> > If not will you be at Plumbers?
> 
> Yes.

Me too.

> 
> > Maybe we shoudl talk about this as although
> > ChromeOS already likely already jumped on async probe we should address a
> > way forward and path forward for other distributions and I don't think anyone
> > is looking too much into it. async probe came to Linux for two reasons:
> >
> >  * chromeos wanting it
> >  * an incorrect systemd assumption on how the driver core works
> >
> > So long term we still need to address the systemd approach, are they going
> > to be defaulting now to async probe for all modules? How about for built-ins?
> >
> > We should talk about this and maybe at plumbers.
> >
> >> Splitting the "initial probe" from
> >> the "manual probe" case seems like a recipe for confusion.
> >
> > If you can come up with pros / cons on both strategies it'd be
> > valuable.
> 
> The problem I ran into was needing to remove devices that still had
> yet to be probed and not being able to use registration completion vs
> the device_lock() to effectively synchronize the sub-system.

Why do you need to "synchronize the sub-system"? The asynchronous
probing should be transparent to the driver. Just unregister the device
(or the driver) and driver core will ensure that probe() is not in
flight.

Confused.

-- 
Dmitry

Re: [PATCH 2/8] driver-core: add asynchronous probing support for drivers

[Dan Williams]

On Mon, Jul 6, 2015 at 4:38 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Sat, Jul 04, 2015 at 07:09:19AM -0700, Dan Williams wrote:
>> The problem I ran into was needing to remove devices that still had
>> yet to be probed and not being able to use registration completion vs
>> the device_lock() to effectively synchronize the sub-system.
>
> Why do you need to "synchronize the sub-system"? The asynchronous
> probing should be transparent to the driver. Just unregister the device
> (or the driver) and driver core will ensure that probe() is not in
> flight.

Async registration is indeed transparent to the driver.  The primary
need to "flush registration" is the case of "region" devices that
reference a set of NVDIMM devices.  A region device requires all
related NVDIMMs to be active before the region can be enabled.

I'll look into a more concrete example of the tradeoffs between
asynchronous probing vs registration.

Re: [PATCH 2/8] driver-core: add asynchronous probing support for drivers

[Dmitry Torokhov]

On Wed, Jul 08, 2015 at 05:43:23PM -0700, Dan Williams wrote:
> On Mon, Jul 6, 2015 at 4:38 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > On Sat, Jul 04, 2015 at 07:09:19AM -0700, Dan Williams wrote:
> >> The problem I ran into was needing to remove devices that still had
> >> yet to be probed and not being able to use registration completion vs
> >> the device_lock() to effectively synchronize the sub-system.
> >
> > Why do you need to "synchronize the sub-system"? The asynchronous
> > probing should be transparent to the driver. Just unregister the device
> > (or the driver) and driver core will ensure that probe() is not in
> > flight.
> 
> Async registration is indeed transparent to the driver.  The primary
> need to "flush registration" is the case of "region" devices that
> reference a set of NVDIMM devices.  A region device requires all
> related NVDIMMs to be active before the region can be enabled.

Sounds like you need to call into the subsystem to let it know that the
device is active and activate region devices when they are ready. Could
be either explicit call or you can try using bus notifiers for
bind/unbind events.

BTW, do you handle bind/unbind via sysfs (everyone forgets about this
mechanism)?

Thanks.

-- 
Dmitry

Re: [PATCH 2/8] driver-core: add asynchronous probing support for drivers

[Dan Williams]

On Wed, Jul 8, 2015 at 5:52 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Wed, Jul 08, 2015 at 05:43:23PM -0700, Dan Williams wrote:
>> On Mon, Jul 6, 2015 at 4:38 PM, Dmitry Torokhov
>> <dmitry.torokhov@gmail.com> wrote:
>> > On Sat, Jul 04, 2015 at 07:09:19AM -0700, Dan Williams wrote:
>> >> The problem I ran into was needing to remove devices that still had
>> >> yet to be probed and not being able to use registration completion vs
>> >> the device_lock() to effectively synchronize the sub-system.
>> >
>> > Why do you need to "synchronize the sub-system"? The asynchronous
>> > probing should be transparent to the driver. Just unregister the device
>> > (or the driver) and driver core will ensure that probe() is not in
>> > flight.
>>
>> Async registration is indeed transparent to the driver.  The primary
>> need to "flush registration" is the case of "region" devices that
>> reference a set of NVDIMM devices.  A region device requires all
>> related NVDIMMs to be active before the region can be enabled.
>
> Sounds like you need to call into the subsystem to let it know that the
> device is active and activate region devices when they are ready. Could
> be either explicit call or you can try using bus notifiers for
> bind/unbind events.
>
> BTW, do you handle bind/unbind via sysfs (everyone forgets about this
> mechanism)?

bind/unbind via systs is central to how libnvdimm operates.  It's
covered by our unit tests.

Re: [PATCH 2/8] driver-core: add asynchronous probing support for drivers

[Dmitry Torokhov]

On Wed, Jul 08, 2015 at 05:54:26PM -0700, Dan Williams wrote:
> On Wed, Jul 8, 2015 at 5:52 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > On Wed, Jul 08, 2015 at 05:43:23PM -0700, Dan Williams wrote:
> >> On Mon, Jul 6, 2015 at 4:38 PM, Dmitry Torokhov
> >> <dmitry.torokhov@gmail.com> wrote:
> >> > On Sat, Jul 04, 2015 at 07:09:19AM -0700, Dan Williams wrote:
> >> >> The problem I ran into was needing to remove devices that still had
> >> >> yet to be probed and not being able to use registration completion vs
> >> >> the device_lock() to effectively synchronize the sub-system.
> >> >
> >> > Why do you need to "synchronize the sub-system"? The asynchronous
> >> > probing should be transparent to the driver. Just unregister the device
> >> > (or the driver) and driver core will ensure that probe() is not in
> >> > flight.
> >>
> >> Async registration is indeed transparent to the driver.  The primary
> >> need to "flush registration" is the case of "region" devices that
> >> reference a set of NVDIMM devices.  A region device requires all
> >> related NVDIMMs to be active before the region can be enabled.
> >
> > Sounds like you need to call into the subsystem to let it know that the
> > device is active and activate region devices when they are ready. Could
> > be either explicit call or you can try using bus notifiers for
> > bind/unbind events.
> >
> > BTW, do you handle bind/unbind via sysfs (everyone forgets about this
> > mechanism)?
> 
> bind/unbind via systs is central to how libnvdimm operates.  It's
> covered by our unit tests.

Ah, excellent then.

-- 
Dmitry

[PATCH 0/8] Asynchronous device/driver probing support

[Dmitry Torokhov]

This series is a combination of changes proposed by Luis a couple months
ago and implementation used by Chrome OS. The issue we are trying to solve
here is "slow" devices and drivers spending "too much time" in their probe()
methods and it affects:

- overall kernel boot process when drivers are compiled into the kernel
  and slow devices stall entire boot progress;
- systemd desire to time out module loading process.

Unlike Luis' proposal we do make use of asycn_schedule() infrastructure
instead of using a dedicated workqueue, so all  existing synchronization
points in kernel that wait for device registration still work the same.
Also, the asynchronous probing is done not only during driver registration
(i.e. when devices are probed asynchronously only if they are registered
before the driver), but also during device registration and deferred probe
handling. This way slow devices do not stall kernel boot even when drivers
are compiled into the kernel.

The last patch is for adventurous people to try and force
fully-asynchronous boot. It works for me with limited success - I can boot
Rockhip-based box to userspace as long as I force serial to be sychronously
probed and ignore the fact that most devices are using "dummy" regulators
as regulator subsystem really expects regulators to be registered in
orderly fashion on OF-based systems.

Thanks,
Dmitry


Dmitry Torokhov (3):
  driver-core: add asynchronous probing support for drivers
  driver-core: platform_driver_probe() must probe synchronously
  module: add core_param_unsafe

Luis R. Rodriguez (5):
  module: add extra argument for parse_params() callback
  driver-core: add driver module asynchronous probe support
  driver-core: enable drivers to opt-out of async probe
  amd64_edac: enforce synchronous probe
  driver-core: allow forcing async probing for modules and builtins

 Documentation/kernel-parameters.txt |  13 +++
 arch/powerpc/mm/hugetlbpage.c       |   4 +-
 drivers/base/base.h                 |   1 +
 drivers/base/bus.c                  |  31 +++++--
 drivers/base/dd.c                   | 166 +++++++++++++++++++++++++++++++-----
 drivers/base/platform.c             |  13 +++
 drivers/edac/amd64_edac.c           |   1 +
 include/linux/device.h              |  26 ++++++
 include/linux/module.h              |   2 +
 include/linux/moduleparam.h         |  12 ++-
 init/main.c                         |  25 +++---
 kernel/module.c                     |  25 +++++-
 kernel/params.c                     |  11 ++-
 lib/dynamic_debug.c                 |   4 +-
 14 files changed, 284 insertions(+), 50 deletions(-)

-- 
2.2.0.rc0.207.ga3a616c

[PATCH 2/8] driver-core: add asynchronous probing support for drivers

[Dmitry Torokhov]

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.

This change is based on earlier patch by "Luis R. Rodriguez"
<mcgrof@suse.com>

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/base/base.h    |   1 +
 drivers/base/bus.c     |  31 +++++++---
 drivers/base/dd.c      | 149 ++++++++++++++++++++++++++++++++++++++++++-------
 include/linux/device.h |  21 +++++++
 4 files changed, 175 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 876bae5..a3d71ad 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 cdc779c..f9f4831 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -402,31 +402,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;
 
@@ -444,15 +508,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;
@@ -507,6 +615,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 fb50673..9ace4c0 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -195,6 +195,25 @@ 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.
+  */
+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.
@@ -234,6 +253,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 +992,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