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

[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 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 620d0ec..ae0c479 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 cf95428..9596c65 100644
--- a/init/main.c
+++ b/init/main.c
@@ -238,7 +238,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"? */
@@ -255,14 +256,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) {
@@ -279,9 +281,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))
@@ -421,7 +424,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;
 
@@ -440,7 +444,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. */
@@ -545,10 +550,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();
 
@@ -857,7 +862,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 3965511..f5b28b6 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -3178,7 +3178,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);
@@ -3283,7 +3284,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 bd65d136..8534681 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

[Cocci] [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 at 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 at 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 620d0ec..ae0c479 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 cf95428..9596c65 100644
--- a/init/main.c
+++ b/init/main.c
@@ -238,7 +238,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"? */
@@ -255,14 +256,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) {
@@ -279,9 +281,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))
@@ -421,7 +424,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;
 
@@ -440,7 +444,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. */
@@ -545,10 +550,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();
 
@@ -857,7 +862,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 3965511..f5b28b6 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -3178,7 +3178,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);
@@ -3283,7 +3284,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 bd65d136..8534681 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.

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

[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 176d4fe..af034a2 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -928,6 +928,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 f9f4831..b3ec0aa 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -404,7 +404,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 9ace4c0..ae5a165 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -200,16 +200,18 @@ 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.
   */
 enum probe_type {
-	PROBE_SYNCHRONOUS,
+	PROBE_DEFAULT_STRATEGY,
 	PROBE_PREFER_ASYNCHRONOUS,
 };
 
diff --git a/include/linux/module.h b/include/linux/module.h
index ebfb0e1..2b70e71 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 f5b28b6..80639fa 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -3042,7 +3042,7 @@ static 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);
@@ -3181,8 +3181,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>

Some drivers do not work well when their probes are run asynchronously.
Usually it is because driver bug or not optimal driver organization.
Until they are all fixed properly let's allow them opt out of
asynchronous probing.

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 | 15 +++++++++------
 2 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index b3ec0aa..1308e08 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -404,13 +404,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 ae5a165..a17e46f 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -200,19 +200,21 @@ 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 are normally probed
+  *	synchronously, but work just as fine when probed asynchronously
+  *	when user requests it 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.
+  * @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).
   */
 enum probe_type {
 	PROBE_DEFAULT_STRATEGY,
 	PROBE_PREFER_ASYNCHRONOUS,
+	PROBE_FORCE_SYNCHRONOUS,
 };
 
 /**
@@ -222,6 +224,7 @@ enum probe_type {
  * @owner:	The module owner.
  * @mod_name:	Used for built-in modules.
  * @suppress_bind_attrs: Disables bind/unbind via sysfs.
+ * @probe_type: type of probe 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,
-- 
2.2.0.rc0.207.ga3a616c

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

[Dmitry Torokhov]

Because it 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 9421fed..5c9e1b6 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -604,6 +604,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
so enforce just synchronouse probe for now.  Asynchronous probe is not
used by default and requires userepace intervention.  Patches for its
support will be merged later.

The reason async probe fails is that the init call for this driver
relies on probe to have finished for at least one device. This needs to
be addressed before enabling async probe.

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 17638d7..58acced 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -2983,6 +2983,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.

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 forcing async probing for 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 enable
__DEBUG__kernel_force_builtin_async_probe or
__DEBUG__kernel_force_modules_async_probe kernel parameters.

Using 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 af034a2..b09c4ac 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -931,6 +931,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 1308e08..4d00734 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>
@@ -402,6 +403,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) {
@@ -412,10 +417,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 80639fa..8d667ef 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -3196,6 +3196,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,
@@ -3290,6 +3294,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 7/8] module: add core_param_unsafe

[Rusty Russell]

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

Acked-by: Rusty Russell <rusty@rustcorp.com.au>

Thanks,
Rusty.

> ---
>  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

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

[Dmitry Torokhov]

On Fri, Jan 16, 2015 at 03:33:09PM -0800, 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.
> 
> 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
> 

*ping*

-- 
Dmitry

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

[Greg Kroah-Hartman]

On Tue, Feb 03, 2015 at 03:12:19PM -0800, Dmitry Torokhov wrote:
> On Fri, Jan 16, 2015 at 03:33:09PM -0800, 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.
> > 
> > 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
> > 
> 
> *ping*

Sorry, I was waiting for someone else here to speak up.

It's too late for 3.20, I'll look at this when 3.20-rc1 is out, thanks,

greg k-h

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

[Dmitry Torokhov]

On Sat, Feb 07, 2015 at 06:06:15PM +0800, Greg Kroah-Hartman wrote:
> On Tue, Feb 03, 2015 at 03:12:19PM -0800, Dmitry Torokhov wrote:
> > On Fri, Jan 16, 2015 at 03:33:09PM -0800, 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.
> > > 
> > > 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
> > > 
> > 
> > *ping*
> 
> Sorry, I was waiting for someone else here to speak up.
> 
> It's too late for 3.20, I'll look at this when 3.20-rc1 is out, thanks,

Greg, please take another look.

Thanks!

-- 
Dmitry

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

[Dmitry Torokhov]

On Tue, Mar 03, 2015 at 01:18:52PM -0800, Dmitry Torokhov wrote:
> On Sat, Feb 07, 2015 at 06:06:15PM +0800, Greg Kroah-Hartman wrote:
> > On Tue, Feb 03, 2015 at 03:12:19PM -0800, Dmitry Torokhov wrote:
> > > On Fri, Jan 16, 2015 at 03:33:09PM -0800, 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.
> > > > 
> > > > 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
> > > > 
> > > 
> > > *ping*
> > 
> > Sorry, I was waiting for someone else here to speak up.
> > 
> > It's too late for 3.20, I'll look at this when 3.20-rc1 is out, thanks,
> 
> Greg, please take another look.

Greg, since nobody seems to object can it be merged?

Thanks.

-- 
Dmitry

Re: [PATCH 6/8] amd64_edac: enforce synchronous probe

[Tejun Heo]

On Fri, Jan 16, 2015 at 03:33:15PM -0800, Dmitry Torokhov wrote:
> From: Luis R. Rodriguez <mcgrof@suse.com>
> 
> While testing asynchronous PCI probe on this driver I noticed it failed
> so enforce just synchronouse probe for now.  Asynchronous probe is not
> used by default and requires userepace intervention.  Patches for its
> support will be merged later.
> 
> The reason async probe fails is that the init call for this driver
> relies on probe to have finished for at least one device. This needs to
> be addressed before enabling async probe.

I'm still kinda uncomfortable with this both white and black list
behavior.  If we're gonna do this, let's please drop the debug options
and build proper blacklists; otherwise, this will never be complete
and we're gonna left with the in-between situation forever.

Thanks.

-- 
tejun

Re: [PATCH 6/8] amd64_edac: enforce synchronous probe

[Dmitry Torokhov]

On Wed, Mar 18, 2015 at 12:56:18PM -0400, Tejun Heo wrote:
> On Fri, Jan 16, 2015 at 03:33:15PM -0800, Dmitry Torokhov wrote:
> > From: Luis R. Rodriguez <mcgrof@suse.com>
> > 
> > While testing asynchronous PCI probe on this driver I noticed it failed
> > so enforce just synchronouse probe for now.  Asynchronous probe is not
> > used by default and requires userepace intervention.  Patches for its
> > support will be merged later.
> > 
> > The reason async probe fails is that the init call for this driver
> > relies on probe to have finished for at least one device. This needs to
> > be addressed before enabling async probe.
> 
> I'm still kinda uncomfortable with this both white and black list
> behavior.  If we're gonna do this, let's please drop the debug options
> and build proper blacklists; otherwise, this will never be complete
> and we're gonna left with the in-between situation forever.

Without the debug options how can we do that? I will definitely not be
able to go through all the in-tree drivers myself and see if they can be
asynchronously probed or not. The most I can do is to try enabling the
option on our side and fixing the drivers/subsystems that fail with
asynchronous probing. This will be iterative process for some time and
then we'll drop the debug option and flip the flag to do asynchronous
probing by default.

Thanks.

-- 
Dmitry

Re: [PATCH 6/8] amd64_edac: enforce synchronous probe

[Dmitry Torokhov]

On Wed, Mar 18, 2015 at 10:45:44AM -0700, Dmitry Torokhov wrote:
> On Wed, Mar 18, 2015 at 12:56:18PM -0400, Tejun Heo wrote:
> > On Fri, Jan 16, 2015 at 03:33:15PM -0800, Dmitry Torokhov wrote:
> > > From: Luis R. Rodriguez <mcgrof@suse.com>
> > > 
> > > While testing asynchronous PCI probe on this driver I noticed it failed
> > > so enforce just synchronouse probe for now.  Asynchronous probe is not
> > > used by default and requires userepace intervention.  Patches for its
> > > support will be merged later.
> > > 
> > > The reason async probe fails is that the init call for this driver
> > > relies on probe to have finished for at least one device. This needs to
> > > be addressed before enabling async probe.
> > 
> > I'm still kinda uncomfortable with this both white and black list
> > behavior.  If we're gonna do this, let's please drop the debug options
> > and build proper blacklists; otherwise, this will never be complete
> > and we're gonna left with the in-between situation forever.
> 
> Without the debug options how can we do that? I will definitely not be
> able to go through all the in-tree drivers myself and see if they can be
> asynchronously probed or not. The most I can do is to try enabling the
> option on our side and fixing the drivers/subsystems that fail with
> asynchronous probing. This will be iterative process for some time and
> then we'll drop the debug option and flip the flag to do asynchronous
> probing by default.

By the way, at that point I think we should be able to remove the
FORCE_SYNCHRONOUS option (and maybe PREFER_ASYNCHRONOUS as well?).

Thanks.

-- 
Dmitry

Re: [PATCH 6/8] amd64_edac: enforce synchronous probe

[Tejun Heo]

Hello,

On Wed, Mar 18, 2015 at 10:45:44AM -0700, Dmitry Torokhov wrote:
> Without the debug options how can we do that? I will definitely not be
> able to go through all the in-tree drivers myself and see if they can be
> asynchronously probed or not. The most I can do is to try enabling the
> option on our side and fixing the drivers/subsystems that fail with
> asynchronous probing. This will be iterative process for some time and
> then we'll drop the debug option and flip the flag to do asynchronous
> probing by default.

Is this even useful for most drivers?  If not, let's just stick with
whitelisting.  If it is useful, I worry that we're quite unlikely to
build working blacklist with this approach.  idk, having both white
and blacklists tend to end badly.

Thanks.

-- 
tejun

Re: [PATCH 6/8] amd64_edac: enforce synchronous probe

[Dmitry Torokhov]

On Wed, Mar 18, 2015 at 02:16:52PM -0400, Tejun Heo wrote:
> Hello,
> 
> On Wed, Mar 18, 2015 at 10:45:44AM -0700, Dmitry Torokhov wrote:
> > Without the debug options how can we do that? I will definitely not be
> > able to go through all the in-tree drivers myself and see if they can be
> > asynchronously probed or not. The most I can do is to try enabling the
> > option on our side and fixing the drivers/subsystems that fail with
> > asynchronous probing. This will be iterative process for some time and
> > then we'll drop the debug option and flip the flag to do asynchronous
> > probing by default.
> 
> Is this even useful for most drivers?

Define useful. In my tests I was able to shave 2-3 seconds (out of 8-10)
of boot time for the board I was trying it on. Useful for our use case,
not so useful for others.

>  If not, let's just stick with
> whitelisting.  If it is useful, I worry that we're quite unlikely to
> build working blacklist with this approach.  idk, having both white
> and blacklists tend to end badly.

I will try fixing the amd64_edac driver, but I consider
FORCE_SYNCHRONOUS at the moment as an aid for use when trying
fully-asynchronous probing. OTOH I wonder how many more drivers do what
edac does and try to do post-binding setups... and whether it makes
sense to actually try and fix them.

Thanks.

-- 
Dmitry

Re: [PATCH 6/8] amd64_edac: enforce synchronous probe

[Tejun Heo]

Hello, Dmitry.

On Wed, Mar 18, 2015 at 11:23:18AM -0700, Dmitry Torokhov wrote:
> > Is this even useful for most drivers?
> 
> Define useful. In my tests I was able to shave 2-3 seconds (out of 8-10)
> of boot time for the board I was trying it on. Useful for our use case,
> not so useful for others.

That definitely counts as useful in my book.

> >  If not, let's just stick with
> > whitelisting.  If it is useful, I worry that we're quite unlikely to
> > build working blacklist with this approach.  idk, having both white
> > and blacklists tend to end badly.
> 
> I will try fixing the amd64_edac driver, but I consider
> FORCE_SYNCHRONOUS at the moment as an aid for use when trying
> fully-asynchronous probing. OTOH I wonder how many more drivers do what
> edac does and try to do post-binding setups... and whether it makes
> sense to actually try and fix them.

Just to be clear, I'm not saying we need to fix amd64_edac for async
probing.  It's fine if this is something generally useful and some
need to be blacklisted, but in that case let's please drop the
whitelist or at least have a concrete plan to drop the whitelist -
e.g. if we're already too late in this dev cycle, we can merge the
code w/ both white and blacklists now and try to enable it at the
start of the next merge window, but let's make sure we remove it in a
timely manner.

Thanks.

-- 
tejun

Re: [PATCH 6/8] amd64_edac: enforce synchronous probe

[Dmitry Torokhov]

On Wed, Mar 18, 2015 at 02:27:42PM -0400, Tejun Heo wrote:
> Hello, Dmitry.
> 
> On Wed, Mar 18, 2015 at 11:23:18AM -0700, Dmitry Torokhov wrote:
> > > Is this even useful for most drivers?
> > 
> > Define useful. In my tests I was able to shave 2-3 seconds (out of 8-10)
> > of boot time for the board I was trying it on. Useful for our use case,
> > not so useful for others.
> 
> That definitely counts as useful in my book.
> 
> > >  If not, let's just stick with
> > > whitelisting.  If it is useful, I worry that we're quite unlikely to
> > > build working blacklist with this approach.  idk, having both white
> > > and blacklists tend to end badly.
> > 
> > I will try fixing the amd64_edac driver, but I consider
> > FORCE_SYNCHRONOUS at the moment as an aid for use when trying
> > fully-asynchronous probing. OTOH I wonder how many more drivers do what
> > edac does and try to do post-binding setups... and whether it makes
> > sense to actually try and fix them.
> 
> Just to be clear, I'm not saying we need to fix amd64_edac for async
> probing.  It's fine if this is something generally useful and some
> need to be blacklisted, but in that case let's please drop the
> whitelist or at least have a concrete plan to drop the whitelist -
> e.g. if we're already too late in this dev cycle, we can merge the
> code w/ both white and blacklists now and try to enable it at the
> start of the next merge window, but let's make sure we remove it in a
> timely manner.

I do not believe that we will be able to activate asynchronous probing
by default in the next 2, 3, 4 merge windows: distributions will have to
try and use it and see if they are ready for it. However there are
drivers (slow to probe, usually input) that we do know are OK to be
probed asynchronously even today (because the rest of the infrastructure
dealing with input has been converted to deal with hotplug and devices
coming and going in random order at random points of time). Thus
whitelist is useful for now to reduce boot times even if the rest of the
system is probed synchronously because you are not quite ready for your
root device to jump around.

Thanks.

-- 
Dmitry

Re: [PATCH 6/8] amd64_edac: enforce synchronous probe

[Tejun Heo]

On Wed, Mar 18, 2015 at 11:37:31AM -0700, Dmitry Torokhov wrote:
> I do not believe that we will be able to activate asynchronous probing
> by default in the next 2, 3, 4 merge windows: distributions will have to
> try and use it and see if they are ready for it. However there are

Async provides strict completion ordering which storage drivers
already make use of to preserve probe order.  Why isn't this
transitive through asynchronous ->probe calls?  Shouldn't it be?

> drivers (slow to probe, usually input) that we do know are OK to be
> probed asynchronously even today (because the rest of the infrastructure
> dealing with input has been converted to deal with hotplug and devices
> coming and going in random order at random points of time). Thus
> whitelist is useful for now to reduce boot times even if the rest of the
> system is probed synchronously because you are not quite ready for your
> root device to jump around.

Yeah, I can see the short term benefits but at the same time I don't
think this is a healthy long term strategy unless someone really tries
to make it happen that three four merge window is gonna stretch
forever.  If storage drivers are problematic, why not just blacklist
them?

Thanks.

-- 
tejun

Re: [PATCH 6/8] amd64_edac: enforce synchronous probe

[Dmitry Torokhov]

On Wed, Mar 18, 2015 at 02:45:50PM -0400, Tejun Heo wrote:
> On Wed, Mar 18, 2015 at 11:37:31AM -0700, Dmitry Torokhov wrote:
> > I do not believe that we will be able to activate asynchronous probing
> > by default in the next 2, 3, 4 merge windows: distributions will have to
> > try and use it and see if they are ready for it. However there are
> 
> Async provides strict completion ordering which storage drivers
> already make use of to preserve probe order.

Only SCSI. The rest of them do not as far as I can see. And I do not
think they should (and nor SCSI) after we enable everything to async.

>  Why isn't this
> transitive through asynchronous ->probe calls?  Shouldn't it be?

No, I think it should not.

> 
> > drivers (slow to probe, usually input) that we do know are OK to be
> > probed asynchronously even today (because the rest of the infrastructure
> > dealing with input has been converted to deal with hotplug and devices
> > coming and going in random order at random points of time). Thus
> > whitelist is useful for now to reduce boot times even if the rest of the
> > system is probed synchronously because you are not quite ready for your
> > root device to jump around.
> 
> Yeah, I can see the short term benefits but at the same time I don't
> think this is a healthy long term strategy unless someone really tries
> to make it happen that three four merge window is gonna stretch
> forever.  If storage drivers are problematic, why not just blacklist
> them?

Because they are not inherently problematic. I mean from the kernel POV
they work fine, the question is if your userspace can deal with them or
not. For example ChromeOS userspace is fine.

For the record the stuff I had (still have) issues with when enabling
fully async probing on the board I tried were serial and OF-based
regulators.

Thanks.

-- 
Dmitry

Re: [PATCH 6/8] amd64_edac: enforce synchronous probe

[Tejun Heo]

Hello,

On Wed, Mar 18, 2015 at 12:36:22PM -0700, Dmitry Torokhov wrote:
> Because they are not inherently problematic. I mean from the kernel POV
> they work fine, the question is if your userspace can deal with them or
> not. For example ChromeOS userspace is fine.

async already provides mechanisms to solve the above problem.  This
doesn't have to be an either-or thing.  I still don't get why we
aren't converting drivers properly over to async so that they still
follow the ordering rules where necessary.  What's wrong with just
blacklisting the ones which can't follow ordering rules for now and
lifting the blacklist as they get fixed?  That'd provide a gradual
transition path with the matching incentive for converting the drivers
while not disturbing userland.

Thanks.

-- 
tejun

Re: [PATCH 6/8] amd64_edac: enforce synchronous probe

[Dmitry Torokhov]

On Wed, Mar 18, 2015 at 03:51:41PM -0400, Tejun Heo wrote:
> Hello,
> 
> On Wed, Mar 18, 2015 at 12:36:22PM -0700, Dmitry Torokhov wrote:
> > Because they are not inherently problematic. I mean from the kernel POV
> > they work fine, the question is if your userspace can deal with them or
> > not. For example ChromeOS userspace is fine.
> 
> async already provides mechanisms to solve the above problem.  This
> doesn't have to be an either-or thing.  I still don't get why we
> aren't converting drivers properly over to async so that they still
> follow the ordering rules where necessary.  What's wrong with just
> blacklisting the ones which can't follow ordering rules for now and
> lifting the blacklist as they get fixed?  That'd provide a gradual
> transition path with the matching incentive for converting the drivers
> while not disturbing userland.

Tejun, I lost you here. Certainly you are not arguing for going through
the drivers one by one and making their module init code to engage
async_schedule to continue the device creation in link order (well,
sorta, since deferred probing already violates it).

Also, it is not only kernel that may not be prepared for asynchronous
probing, but userspace as well. And I do not think that we should be
working towards preserving the init order in the long run as more and
more bits become hot pluggable and we should be able to handle devices
come and go gracefully anyway.

Thanks.

-- 
Dmitry

Re: [PATCH 6/8] amd64_edac: enforce synchronous probe

[Tejun Heo]

Hello,

On Wed, Mar 18, 2015 at 01:26:05PM -0700, Dmitry Torokhov wrote:
> Tejun, I lost you here. Certainly you are not arguing for going through
> the drivers one by one and making their module init code to engage
> async_schedule to continue the device creation in link order (well,
> sorta, since deferred probing already violates it).

Kind of, yes, but not by driver-by-driver, but by subsystem.  I don't
think we have too many subsystems where probing order matters and the
ordering only matters within each subsystem.

> Also, it is not only kernel that may not be prepared for asynchronous
> probing, but userspace as well. And I do not think that we should be
> working towards preserving the init order in the long run as more and
> more bits become hot pluggable and we should be able to handle devices
> come and go gracefully anyway.

It's not about supporting or not supporting hotplugging.  Most of the
storage devices support hotplugging but still maintain boot order and
at least for storage devices there are pretty good reasons for doing
so especially as we can do so w/o giving up on parallel probing.  The
problem is that if you hinge enabling of general async probing on
virtually all userlands being okay with storage devices (or ttyS
devices and so on), we won't be able to enable this, ever.

This is a solvable problem.  Making e.g. SCSI host probing transitive
in terms of probe ordering shouldn't be too difficult.  We can't add a
generic async probing mechanism at the driver layer ignoring this
aspect.

Thanks.

-- 
tejun

Re: [PATCH 6/8] amd64_edac: enforce synchronous probe

[Dmitry Torokhov]

On Wed, Mar 18, 2015 at 05:02:26PM -0400, Tejun Heo wrote:
> Hello,
> 
> On Wed, Mar 18, 2015 at 01:26:05PM -0700, Dmitry Torokhov wrote:
> > Tejun, I lost you here. Certainly you are not arguing for going through
> > the drivers one by one and making their module init code to engage
> > async_schedule to continue the device creation in link order (well,
> > sorta, since deferred probing already violates it).
> 
> Kind of, yes, but not by driver-by-driver, but by subsystem.  I don't
> think we have too many subsystems where probing order matters and the
> ordering only matters within each subsystem.

I do not think you can always define this by subsystem. SCSI or libata
are exceptions than rule I think.  Take for example I2C. Does the
probing order matter? Not if I2C happens to be an input device. But
maybe if it is a serial port. But maybe not if you can deal with it
being probed out of order. And you probably are since many systems
already ready to handle -EPROBE_DEFER.

And I think libata and SD still rely on the underlying PCI to be probed
synchronously. Try probing PCI asynchronously and see your disks getting
renumbered. And if we try to ensure that all devices are registered in
given order you will end up stalling the boot process because while you
can do some of probing simultaneously you still will have to wait till
slow device is done before allowing drivers "after" the slow one to
register their children objects.

> 
> > Also, it is not only kernel that may not be prepared for asynchronous
> > probing, but userspace as well. And I do not think that we should be
> > working towards preserving the init order in the long run as more and
> > more bits become hot pluggable and we should be able to handle devices
> > come and go gracefully anyway.
> 
> It's not about supporting or not supporting hotplugging.  Most of the
> storage devices support hotplugging but still maintain boot order and
> at least for storage devices there are pretty good reasons for doing
> so especially as we can do so w/o giving up on parallel probing.  The

You are over-stating the boot order guarantees that storage provides.
Yes, you can scan devices and partitions simultaneously on the same
controller, but it will break if controllers are registered in different
order. And if you are delaying registering cone controller because
another that you consider "first" has not done probing, you are stalling
the boot process. It may be OK for "leaf" devices, which disks are
usually are, bit not when you delaying initialization of a device that
is in a middle of the device tree.

> problem is that if you hinge enabling of general async probing on
> virtually all userlands being okay with storage devices (or ttyS
> devices and so on), we won't be able to enable this, ever.

Sure we will. I am pretty sure will do that for ChromeOS reasonably
soon, and I am sure other distributions can follow if needed.

So, to reiterate: right now we are synchronous by default. Certain
drivers can while-list themselves to be async probed when we are sure
userspace can handle this. We have a module option that can be used by
userspace to make drivers registered when module is being loaded to be
probed asynchronously. I expect newer systemd will start using it so
that they can time-out their module loading workers. We also have a
debug kernel option that can force everything asynchronous. I expect
various distributions developers to try using it once they are
comfortable with systemd loading modules asynchronously and then we can
change it to normal option and consider switching the default from sync
to async. I really do not believe that we should continue building
kernel infrastructure to help userspace pretend that the world is static
and unchanging. Userspace is already aware of this past-boot, it is time
for it to recognize the same during boot.

Thanks.

-- 
Dmitry

Re: [PATCH 6/8] amd64_edac: enforce synchronous probe

[Tejun Heo]

Hello, Dmitry.

On Wed, Mar 18, 2015 at 02:41:26PM -0700, Dmitry Torokhov wrote:
> You are over-stating the boot order guarantees that storage provides.
> Yes, you can scan devices and partitions simultaneously on the same
> controller, but it will break if controllers are registered in different
> order. And if you are delaying registering cone controller because
> another that you consider "first" has not done probing, you are stalling
> the boot process. It may be OK for "leaf" devices, which disks are
> usually are, bit not when you delaying initialization of a device that
> is in a middle of the device tree.

Can't we make it "transitive"?  Split ->probe() into two parts so that
attaching the leaf devices run from the completion part of the split
->probe().  Sure, a lot of userlands we have nowadays can handle probe
order changing but we stil have use cases where the order matters.
Why introduce two separate behaviors when we can make the pararell
ordering transitive?

Doing things based on a big switch is going to create two largely
separate modes of operations.  For a lot of cases, the gain in boot
time might not be enough to turn on the switch and we sure can't
default to turning it on.  This is a deviation we can avoid with
reasonable amount of effort.  The trade-off seems pretty clear to me.

Thanks.

-- 
tejun

Re: [PATCH 6/8] amd64_edac: enforce synchronous probe

[Dmitry Torokhov]

On Wed, Mar 18, 2015 at 05:50:28PM -0400, Tejun Heo wrote:
> Hello, Dmitry.
> 
> On Wed, Mar 18, 2015 at 02:41:26PM -0700, Dmitry Torokhov wrote:
> > You are over-stating the boot order guarantees that storage provides.
> > Yes, you can scan devices and partitions simultaneously on the same
> > controller, but it will break if controllers are registered in different
> > order. And if you are delaying registering cone controller because
> > another that you consider "first" has not done probing, you are stalling
> > the boot process. It may be OK for "leaf" devices, which disks are
> > usually are, bit not when you delaying initialization of a device that
> > is in a middle of the device tree.
> 
> Can't we make it "transitive"?  Split ->probe() into two parts so that
> attaching the leaf devices run from the completion part of the split
> ->probe().  Sure, a lot of userlands we have nowadays can handle probe
> order changing but we stil have use cases where the order matters.
> Why introduce two separate behaviors when we can make the pararell
> ordering transitive?

So let's say that we we have 2 devices D1 and D2 which have
children C1 and C2 with leaves L1 and L2:

Device	Probe time
D1	5
D2	1
C1	2
C2	4
L1	1
L2	1

If we run fully async we will need 8 units to probe everything
(max(D1+C1+L1, D2+C2+L2)). With pausing at each level we'd need 10
units (max(D1, D2) + max(C1, C2) + max(L1, L2).

> 
> Doing things based on a big switch is going to create two largely
> separate modes of operations.  For a lot of cases, the gain in boot
> time might not be enough to turn on the switch and we sure can't
> default to turning it on.  This is a deviation we can avoid with
> reasonable amount of effort.  The trade-off seems pretty clear to me.

As I mentioned, the benefit / trade-off depends on your point of view.
For servers nobody would care. For consumer devices it is very
important.

Thanks.

-- 
Dmitry

Re: [PATCH 6/8] amd64_edac: enforce synchronous probe

[Tejun Heo]

Hello, Dmitry.

On Wed, Mar 18, 2015 at 03:15:30PM -0700, Dmitry Torokhov wrote:
> So let's say that we we have 2 devices D1 and D2 which have
> children C1 and C2 with leaves L1 and L2:
> 
> Device	Probe time
> D1	5
> D2	1
> C1	2
> C2	4
> L1	1
> L2	1
> 
> If we run fully async we will need 8 units to probe everything
> (max(D1+C1+L1, D2+C2+L2)). With pausing at each level we'd need 10
> units (max(D1, D2) + max(C1, C2) + max(L1, L2).

Yeah, the details will differ depending on the specifics of layering
but it's likely to add more synchronization points.

> > Doing things based on a big switch is going to create two largely
> > separate modes of operations.  For a lot of cases, the gain in boot
> > time might not be enough to turn on the switch and we sure can't
> > default to turning it on.  This is a deviation we can avoid with
> > reasonable amount of effort.  The trade-off seems pretty clear to me.
> 
> As I mentioned, the benefit / trade-off depends on your point of view.
> For servers nobody would care. For consumer devices it is very
> important.

Shouldn't we still be able to extract most of parallelism this way,
especially given that as the probing walks down the hierarchy, they
get decoupled from each other?

Even if certain use cases can benefit from completely ignoring
ordering, it's a lot more consistent to add an option to ignore device
ordering on top of general async mechanism rather than having fully
async vs. sync probing paths.  We'd still be traveling and testing
most of the same logic.  What bothers me primarily is that this being
some obscure boot flag that only several users know and exploit which
makes the kernel behave very differently.

Thanks.

-- 
tejun

Re: [PATCH 6/8] amd64_edac: enforce synchronous probe

[Dmitry Torokhov]

On Wed, Mar 18, 2015 at 07:24:01PM -0400, Tejun Heo wrote:
> Hello, Dmitry.
> 
> On Wed, Mar 18, 2015 at 03:15:30PM -0700, Dmitry Torokhov wrote:
> > So let's say that we we have 2 devices D1 and D2 which have
> > children C1 and C2 with leaves L1 and L2:
> > 
> > Device	Probe time
> > D1	5
> > D2	1
> > C1	2
> > C2	4
> > L1	1
> > L2	1
> > 
> > If we run fully async we will need 8 units to probe everything
> > (max(D1+C1+L1, D2+C2+L2)). With pausing at each level we'd need 10
> > units (max(D1, D2) + max(C1, C2) + max(L1, L2).
> 
> Yeah, the details will differ depending on the specifics of layering
> but it's likely to add more synchronization points.
> 
> > > Doing things based on a big switch is going to create two largely
> > > separate modes of operations.  For a lot of cases, the gain in boot
> > > time might not be enough to turn on the switch and we sure can't
> > > default to turning it on.  This is a deviation we can avoid with
> > > reasonable amount of effort.  The trade-off seems pretty clear to me.
> > 
> > As I mentioned, the benefit / trade-off depends on your point of view.
> > For servers nobody would care. For consumer devices it is very
> > important.
> 
> Shouldn't we still be able to extract most of parallelism this way,
> especially given that as the probing walks down the hierarchy, they
> get decoupled from each other?

Why would they get decoupled? For example, if we are talking about input
devices, they can be connected to platform bus or one of i2c buses or
HID (via USB). If we want to ensure ordering we'd have to synchronize
all of them somehow and I do not have even sure what the rule should be.
I mean I am probing platform devices simultaneously and I come to an
i2c controller and gpio input device. So I wait till both done probing
before posting new devices to the driver core? What if one returns
-EPROBE_DEFER? Do I stop and wait for the deferral to complete? What if
deferral is satisfied by a 3rd device on platform bus? If I am waiting
for all devices to probe I won't be able to resolve the deferral. And
even without deferral in old world we'd probe i2c and i2c will lead to
discovery of another input device which would be registered before
registering the platform input device. So with async we'd have to pause
platform probing until all children of i2c are done probing, which
pretty much kills all async gains as far as I can see.

> 
> Even if certain use cases can benefit from completely ignoring
> ordering, it's a lot more consistent to add an option to ignore device
> ordering on top of general async mechanism rather than having fully
> async vs. sync probing paths.  We'd still be traveling and testing
> most of the same logic.  What bothers me primarily is that this being

I think the logic is pretty much the same even with async probing,
especially if you take into account -EPROBE_DEFER handling that we
already have. You may not run into it that often on x86 but it is pretty
common on arm devices and it does change the probe order.

>
> some obscure boot flag that only several users know and exploit which
> makes the kernel behave very differently.

I do not think this flag is useful for end users but rather for
distributions. Either their userspace is ready to handle fully async
probe or not quite yet.

Thanks.

-- 
Dmitry

Re: [PATCH 6/8] amd64_edac: enforce synchronous probe

[Tejun Heo]

Hello, Dmitry.

On Wed, Mar 18, 2015 at 05:26:19PM -0700, Dmitry Torokhov wrote:
> Why would they get decoupled? For example, if we are talking about input
> devices, they can be connected to platform bus or one of i2c buses or
> HID (via USB). If we want to ensure ordering we'd have to synchronize
> all of them somehow and I do not have even sure what the rule should be.
> I mean I am probing platform devices simultaneously and I come to an
> i2c controller and gpio input device. So I wait till both done probing
> before posting new devices to the driver core? What if one returns
> -EPROBE_DEFER? Do I stop and wait for the deferral to complete? What if
> deferral is satisfied by a 3rd device on platform bus? If I am waiting
> for all devices to probe I won't be able to resolve the deferral. And
> even without deferral in old world we'd probe i2c and i2c will lead to
> discovery of another input device which would be registered before
> registering the platform input device. So with async we'd have to pause
> platform probing until all children of i2c are done probing, which
> pretty much kills all async gains as far as I can see.
...
> I think the logic is pretty much the same even with async probing,
> especially if you take into account -EPROBE_DEFER handling that we
> already have. You may not run into it that often on x86 but it is pretty
> common on arm devices and it does change the probe order.

I see, so, if ordering has never been reliable for a given platform or
class of devices, there's nothing to worry about.  Or even if ordering
has been reliable but change of ordering wouldn't be noticable from
userland, that'd be fine too.  The thing is that for certain classes
of devices, we've been guaranteeing probe ordering during boot and
there are non-insignificant number of use cases which depend on that
and we should be able to accomodate them.

I don't think this'd be a huge burden.  e.g. even something like
synchronizing once for all async pci probes can be enough.  That
should be enough for most traditional storage devices and that's the
biggest item.

> I do not think this flag is useful for end users but rather for
> distributions. Either their userspace is ready to handle fully async
> probe or not quite yet.

I think we should be able to enable all-async probing by default and
that'd be far beneficial and simpler for everybody.

Thanks.

-- 
tejun

Re: [PATCH 6/8] amd64_edac: enforce synchronous probe

[Dmitry Torokhov]

On Thu, Mar 19, 2015 at 11:41:41AM -0400, Tejun Heo wrote:
> Hello, Dmitry.
> 
> On Wed, Mar 18, 2015 at 05:26:19PM -0700, Dmitry Torokhov wrote:
> > Why would they get decoupled? For example, if we are talking about input
> > devices, they can be connected to platform bus or one of i2c buses or
> > HID (via USB). If we want to ensure ordering we'd have to synchronize
> > all of them somehow and I do not have even sure what the rule should be.
> > I mean I am probing platform devices simultaneously and I come to an
> > i2c controller and gpio input device. So I wait till both done probing
> > before posting new devices to the driver core? What if one returns
> > -EPROBE_DEFER? Do I stop and wait for the deferral to complete? What if
> > deferral is satisfied by a 3rd device on platform bus? If I am waiting
> > for all devices to probe I won't be able to resolve the deferral. And
> > even without deferral in old world we'd probe i2c and i2c will lead to
> > discovery of another input device which would be registered before
> > registering the platform input device. So with async we'd have to pause
> > platform probing until all children of i2c are done probing, which
> > pretty much kills all async gains as far as I can see.
> ...
> > I think the logic is pretty much the same even with async probing,
> > especially if you take into account -EPROBE_DEFER handling that we
> > already have. You may not run into it that often on x86 but it is pretty
> > common on arm devices and it does change the probe order.
> 
> I see, so, if ordering has never been reliable for a given platform or
> class of devices, there's nothing to worry about.  Or even if ordering
> has been reliable but change of ordering wouldn't be noticable from
> userland, that'd be fine too.  The thing is that for certain classes
> of devices, we've been guaranteeing probe ordering during boot and
> there are non-insignificant number of use cases which depend on that
> and we should be able to accomodate them.
> 
> I don't think this'd be a huge burden.  e.g. even something like
> synchronizing once for all async pci probes can be enough.  That
> should be enough for most traditional storage devices and that's the
> biggest item.

OK, I guess I (or maybe somebody else) could look into PCI bus core to
add the necessary sync points for that before we enable wholesale async
probing.

> 
> > I do not think this flag is useful for end users but rather for
> > distributions. Either their userspace is ready to handle fully async
> > probe or not quite yet.
> 
> I think we should be able to enable all-async probing by default and
> that'd be far beneficial and simpler for everybody.

I think that would be the goal, yes, but I think we'd need some "trial"
period before we can do that: I need to look into at least serial and
regulators to make it work (not even considering any userspace). We are
definitely not ready just yet and that is why I have a whitelist: there
are classes of devices that all userspaces learned to deal with long ago
and we can make them not stall boot right now.

Thanks.

-- 
Dmitry

Re: [PATCH 6/8] amd64_edac: enforce synchronous probe

[Tejun Heo]

Hello,

On Thu, Mar 19, 2015 at 09:01:46AM -0700, Dmitry Torokhov wrote:
> I think that would be the goal, yes, but I think we'd need some "trial"
> period before we can do that: I need to look into at least serial and
> regulators to make it work (not even considering any userspace). We are
> definitely not ready just yet and that is why I have a whitelist: there
> are classes of devices that all userspaces learned to deal with long ago
> and we can make them not stall boot right now.

Yeah, as I wrote before, as long as there's a plan and push to finish
the conversion, it's fine.  I'm just worried that this might rot in
the grey area.  Can we please update the patches so that it's clear
that the whitelist is a temporary measure both in the description and
code?

Thanks.

-- 
tejun

Re: [PATCH 6/8] amd64_edac: enforce synchronous probe

[Dmitry Torokhov]

On Thu, Mar 19, 2015 at 12:19:27PM -0400, Tejun Heo wrote:
> Hello,
> 
> On Thu, Mar 19, 2015 at 09:01:46AM -0700, Dmitry Torokhov wrote:
> > I think that would be the goal, yes, but I think we'd need some "trial"
> > period before we can do that: I need to look into at least serial and
> > regulators to make it work (not even considering any userspace). We are
> > definitely not ready just yet and that is why I have a whitelist: there
> > are classes of devices that all userspaces learned to deal with long ago
> > and we can make them not stall boot right now.
> 
> Yeah, as I wrote before, as long as there's a plan and push to finish
> the conversion, it's fine.  I'm just worried that this might rot in
> the grey area.  Can we please update the patches so that it's clear
> that the whitelist is a temporary measure both in the description and
> code?

OK, will do.

Thanks.

-- 
Dmitry