[RFC PATCH v2] async: Add cmdline option to specify drivers to be async probed

[RFC PATCH v2] async: Add cmdline option to specify drivers to be async probed

[Feng Tang]

Asynchronous driver probing can help much on kernel fastboot, and
this option can provide a flexible way to optimize and quickly verify
async driver probe.

Also it will help in below cases:
* Some driver actually covers several families of HWs, some of which
  could use async probing while others don't. So we can't simply
  turn on the PROBE_PREFER_ASYNCHRONOUS flag in driver, but use this
  cmdline option, like igb driver async patch discussed at
  https://www.spinics.net/lists/netdev/msg545986.html

* For SOC (System on Chip) with multiple spi or i2c controllers, most
  of the slave spi/i2c devices will be assigned with fixed controller
  number, while async probing may make those controllers get different
  index for each boot, which prevents those controller drivers to be
  async probed. For platforms not using these spi/i2c slave devices,
  they can use this cmdline option to benefit from the async probing.

Suggested-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
Signed-off-by: Feng Tang <feng.tang@intel.com>
---
 drivers/base/dd.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 8ac10af..cfa704a 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -57,6 +57,10 @@ static atomic_t deferred_trigger_count = ATOMIC_INIT(0);
 static struct dentry *deferred_devices;
 static bool initcalls_done;
 
+/* Save the async probe drivers' name from kernel cmdline */
+#define ASYNC_DRV_NAMES_MAX_LEN	256
+static char async_probe_drv_names[ASYNC_DRV_NAMES_MAX_LEN];
+
 /*
  * In some cases, like suspend to RAM or hibernation, It might be reasonable
  * to prohibit probing of devices as it could be unsafe.
@@ -674,8 +678,27 @@ int driver_probe_device(struct device_driver *drv, struct device *dev)
 	return ret;
 }
 
+static inline bool cmdline_requested_async_probing(const char *drv_name)
+{
+	return parse_option_str(async_probe_drv_names, drv_name);
+}
+
+/* The format is like driver_async_probe=drv_name1,drv_name2,drv_name3 */
+static int __init save_async_options(char *buf)
+{
+	if (strlen(buf) >= ASYNC_DRV_NAMES_MAX_LEN)
+		printk(KERN_WARNING "Too long list for async_probe_drv_names!");
+
+	strlcpy(async_probe_drv_names, buf, ASYNC_DRV_NAMES_MAX_LEN);
+	return 0;
+}
+__setup("driver_async_probe=", save_async_options);
+
 bool driver_allows_async_probing(struct device_driver *drv)
 {
+	if (cmdline_requested_async_probing(drv->name))
+		return true;
+
 	switch (drv->probe_type) {
 	case PROBE_PREFER_ASYNCHRONOUS:
 		return true;
-- 
2.7.4

Re: [RFC PATCH v2] async: Add cmdline option to specify drivers to be async probed

[Alexander Duyck]

On Mon, 2019-01-28 at 09:20 +0800, Feng Tang wrote:
> Asynchronous driver probing can help much on kernel fastboot, and
> this option can provide a flexible way to optimize and quickly verify
> async driver probe.
> 
> Also it will help in below cases:
> * Some driver actually covers several families of HWs, some of which
>   could use async probing while others don't. So we can't simply
>   turn on the PROBE_PREFER_ASYNCHRONOUS flag in driver, but use this
>   cmdline option, like igb driver async patch discussed at
>   https://www.spinics.net/lists/netdev/msg545986.html
> 
> * For SOC (System on Chip) with multiple spi or i2c controllers, most
>   of the slave spi/i2c devices will be assigned with fixed controller
>   number, while async probing may make those controllers get different
>   index for each boot, which prevents those controller drivers to be
>   async probed. For platforms not using these spi/i2c slave devices,
>   they can use this cmdline option to benefit from the async probing.
> 
> Suggested-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> Signed-off-by: Feng Tang <feng.tang@intel.com>
> ---
>  drivers/base/dd.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index 8ac10af..cfa704a 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -57,6 +57,10 @@ static atomic_t deferred_trigger_count = ATOMIC_INIT(0);
>  static struct dentry *deferred_devices;
>  static bool initcalls_done;
>  
> +/* Save the async probe drivers' name from kernel cmdline */
> +#define ASYNC_DRV_NAMES_MAX_LEN	256
> +static char async_probe_drv_names[ASYNC_DRV_NAMES_MAX_LEN];
> +
>  /*
>   * In some cases, like suspend to RAM or hibernation, It might be reasonable
>   * to prohibit probing of devices as it could be unsafe.
> @@ -674,8 +678,27 @@ int driver_probe_device(struct device_driver *drv, struct device *dev)
>  	return ret;
>  }
>  
> +static inline bool cmdline_requested_async_probing(const char *drv_name)
> +{
> +	return parse_option_str(async_probe_drv_names, drv_name);
> +}
> +
> +/* The format is like driver_async_probe=drv_name1,drv_name2,drv_name3 */
> +static int __init save_async_options(char *buf)
> +{
> +	if (strlen(buf) >= ASYNC_DRV_NAMES_MAX_LEN)
> +		printk(KERN_WARNING "Too long list for async_probe_drv_names!");
> +
> +	strlcpy(async_probe_drv_names, buf, ASYNC_DRV_NAMES_MAX_LEN);
> +	return 0;
> +}
> +__setup("driver_async_probe=", save_async_options);
> +
>  bool driver_allows_async_probing(struct device_driver *drv)
>  {
> +	if (cmdline_requested_async_probing(drv->name))
> +		return true;
> +

This piece still should be moved. Ideally it should be down in the
"default" case area we use to determine if the module parameter
async_probe was used or not. Where you currently have this will
overrride the driver behavior if it absolutely cannot use an
asynchronous probe.

>  	switch (drv->probe_type) {
>  	case PROBE_PREFER_ASYNCHRONOUS:
>  		return true;

Re: [RFC PATCH v2] async: Add cmdline option to specify drivers to be async probed

[Feng Tang]

Hi Alexander,

Thanks for the review.

On Mon, Jan 28, 2019 at 08:31:11AM -0800, Alexander Duyck wrote:
> On Mon, 2019-01-28 at 09:20 +0800, Feng Tang wrote:
> > Asynchronous driver probing can help much on kernel fastboot, and
> > this option can provide a flexible way to optimize and quickly verify
> > async driver probe.
> > 
> > Also it will help in below cases:
> > * Some driver actually covers several families of HWs, some of which
> >   could use async probing while others don't. So we can't simply
> >   turn on the PROBE_PREFER_ASYNCHRONOUS flag in driver, but use this
> >   cmdline option, like igb driver async patch discussed at
> >   https://www.spinics.net/lists/netdev/msg545986.html
> > 
> > * For SOC (System on Chip) with multiple spi or i2c controllers, most
> >   of the slave spi/i2c devices will be assigned with fixed controller
> >   number, while async probing may make those controllers get different
> >   index for each boot, which prevents those controller drivers to be
> >   async probed. For platforms not using these spi/i2c slave devices,
> >   they can use this cmdline option to benefit from the async probing.
> > 
> > Suggested-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> > Signed-off-by: Feng Tang <feng.tang@intel.com>
> > ---
> >  drivers/base/dd.c | 23 +++++++++++++++++++++++
> >  1 file changed, 23 insertions(+)
> > 
> > diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> > index 8ac10af..cfa704a 100644
> > --- a/drivers/base/dd.c
> > +++ b/drivers/base/dd.c
> > @@ -57,6 +57,10 @@ static atomic_t deferred_trigger_count = ATOMIC_INIT(0);
> >  static struct dentry *deferred_devices;
> >  static bool initcalls_done;
> >  
> > +/* Save the async probe drivers' name from kernel cmdline */
> > +#define ASYNC_DRV_NAMES_MAX_LEN	256
> > +static char async_probe_drv_names[ASYNC_DRV_NAMES_MAX_LEN];
> > +
> >  /*
> >   * In some cases, like suspend to RAM or hibernation, It might be reasonable
> >   * to prohibit probing of devices as it could be unsafe.
> > @@ -674,8 +678,27 @@ int driver_probe_device(struct device_driver *drv, struct device *dev)
> >  	return ret;
> >  }
> >  
> > +static inline bool cmdline_requested_async_probing(const char *drv_name)
> > +{
> > +	return parse_option_str(async_probe_drv_names, drv_name);
> > +}
> > +
> > +/* The format is like driver_async_probe=drv_name1,drv_name2,drv_name3 */
> > +static int __init save_async_options(char *buf)
> > +{
> > +	if (strlen(buf) >= ASYNC_DRV_NAMES_MAX_LEN)
> > +		printk(KERN_WARNING "Too long list for async_probe_drv_names!");
> > +
> > +	strlcpy(async_probe_drv_names, buf, ASYNC_DRV_NAMES_MAX_LEN);
> > +	return 0;
> > +}
> > +__setup("driver_async_probe=", save_async_options);
> > +
> >  bool driver_allows_async_probing(struct device_driver *drv)
> >  {
> > +	if (cmdline_requested_async_probing(drv->name))
> > +		return true;
> > +
> 
> This piece still should be moved. Ideally it should be down in the
> "default" case area we use to determine if the module parameter
> async_probe was used or not. Where you currently have this will
> overrride the driver behavior if it absolutely cannot use an
> asynchronous probe.

I thought this option could also be used as a flexible option to
test and try some drivers what we used to think not appropriate
for async probing (including drivers with PROBE_FORCE_SYNCHRONOUS flag).
But if people think we should enforce this PROBE_FORCE_SYNCHRONOUS
check, I can move the cmdline_requested_async_probing() check into
the "default:" segment.

Thanks,
Feng