[PATCH v1] driver core: Ignore driver_async_probe cmdline param for module drivers

[PATCH v1] driver core: Ignore driver_async_probe cmdline param for module drivers

[Saravana Kannan]

If the module's async_probe option isn't set, the module loading code
will do a async_synchronize_full() before returning to userspace. This
effectively negates any benefits of driver_async_probe listing the
module's driver.

If the module's async_probe is set, then we already do async probe for
that module's driver even if driver_async_probe does not list that
driver. So, again, the driver_async_probe's value doesn't matter for a
module's driver.

So this change ignores the driver_async_probe for module drivers to
avoid useless async probes.

In addition, if driver_async_probe lists a module's driver and the
driver's async probe ends up calling request_module() in the async
thread context, that's going to trigger a spurious WARNING stack dump
without any real benefits of async probing. So this will avoid that
unnecessary warning in that situation.

Signed-off-by: Saravana Kannan <saravanak@google.com>
---
 drivers/base/dd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index e600dd2afc35..f1ac28a4ce62 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -823,7 +823,7 @@ bool driver_allows_async_probing(struct device_driver *drv)
 		return false;
 
 	default:
-		if (cmdline_requested_async_probing(drv->name))
+		if (cmdline_requested_async_probing(drv->name) && drv->owner == NULL)
 			return true;
 
 		if (module_requested_async_probing(drv->owner))
-- 
2.37.0.rc0.161.g10f37bed90-goog

Re: [PATCH v1] driver core: Ignore driver_async_probe cmdline param for module drivers

[Greg Kroah-Hartman]

On Wed, Jun 22, 2022 at 09:21:50PM -0700, Saravana Kannan wrote:
> If the module's async_probe option isn't set, the module loading code
> will do a async_synchronize_full() before returning to userspace. This
> effectively negates any benefits of driver_async_probe listing the
> module's driver.
> 
> If the module's async_probe is set, then we already do async probe for
> that module's driver even if driver_async_probe does not list that
> driver. So, again, the driver_async_probe's value doesn't matter for a
> module's driver.
> 
> So this change ignores the driver_async_probe for module drivers to
> avoid useless async probes.
> 
> In addition, if driver_async_probe lists a module's driver and the
> driver's async probe ends up calling request_module() in the async
> thread context, that's going to trigger a spurious WARNING stack dump
> without any real benefits of async probing. So this will avoid that
> unnecessary warning in that situation.
> 
> Signed-off-by: Saravana Kannan <saravanak@google.com>

Does this "fix" an older commit?  Does this need to be backported
anywhere?  Or is this just a new feature to help make things work
properly going forward?

> ---
>  drivers/base/dd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index e600dd2afc35..f1ac28a4ce62 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -823,7 +823,7 @@ bool driver_allows_async_probing(struct device_driver *drv)
>  		return false;
>  
>  	default:
> -		if (cmdline_requested_async_probing(drv->name))
> +		if (cmdline_requested_async_probing(drv->name) && drv->owner == NULL)

This feels odd, the module owner shouldn't be used for anything other
then the reference for the module being properly handled.  Not doing
different logic for built-in vs. not-built-in feels wrong, especially
asa some module drivers might not set the owner field for various
reasons (hint, networking drivers do not deal with the owner field as
they have different ways of handling the module reference logic.)

So I don't think this is ok, sorry.  Did you test this with networking
drivers?

thanks,

greg k-h