[PATCH v2 2/4] driver core: Set deferred probe timeout to 0 if modules are disabled

Javier Martinez Canillas posted 4 patches 3 years, 4 months ago
[PATCH v2 2/4] driver core: Set deferred probe timeout to 0 if modules are disabled
Posted by Javier Martinez Canillas 3 years, 4 months ago
There is no point to schedule the work queue to timeout the deferred probe
if all the initcalls are done and modules are not enabled. The default for
this case is already 0 but can be overridden by the deferred_probe_timeout
parameter. Let's just disable to avoid queuing a work that is not needed.

Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
---

(no changes since v1)

 drivers/base/dd.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 040b4060f903..1e8f1afeac98 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -350,8 +350,10 @@ static int deferred_probe_initcall(void)
 	flush_work(&deferred_probe_work);
 	initcalls_done = true;
 
-	if (!IS_ENABLED(CONFIG_MODULES))
+	if (!IS_ENABLED(CONFIG_MODULES)) {
+		driver_deferred_probe_timeout = 0;
 		fw_devlink_drivers_done();
+	}
 
 	/*
 	 * Trigger deferred probe again, this time we won't defer anything
-- 
2.38.1
Re: [PATCH v2 2/4] driver core: Set deferred probe timeout to 0 if modules are disabled
Posted by Andrew Halaney 3 years, 4 months ago
On Wed, Nov 16, 2022 at 01:00:43PM +0100, Javier Martinez Canillas wrote:
> There is no point to schedule the work queue to timeout the deferred probe
> if all the initcalls are done and modules are not enabled. The default for
> this case is already 0 but can be overridden by the deferred_probe_timeout
> parameter. Let's just disable to avoid queuing a work that is not needed.
> 
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> ---
> 
> (no changes since v1)
> 
>  drivers/base/dd.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index 040b4060f903..1e8f1afeac98 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -350,8 +350,10 @@ static int deferred_probe_initcall(void)
>  	flush_work(&deferred_probe_work);
>  	initcalls_done = true;
>  
> -	if (!IS_ENABLED(CONFIG_MODULES))
> +	if (!IS_ENABLED(CONFIG_MODULES)) {
> +		driver_deferred_probe_timeout = 0;
>  		fw_devlink_drivers_done();
> +	}
>  

Potentially a stupid suggestion, but would it make sense to take out
the ability to actually set that param if !CONFIG_MODULES? Then
driver_deferred_probe_timeout would be the default value already.

>  	/*
>  	 * Trigger deferred probe again, this time we won't defer anything
> -- 
> 2.38.1
>
Re: [PATCH v2 2/4] driver core: Set deferred probe timeout to 0 if modules are disabled
Posted by Javier Martinez Canillas 3 years, 4 months ago
Hello Andrew,

Thanks for your feedback.

On 11/16/22 17:39, Andrew Halaney wrote:
> On Wed, Nov 16, 2022 at 01:00:43PM +0100, Javier Martinez Canillas wrote:
>> There is no point to schedule the work queue to timeout the deferred probe
>> if all the initcalls are done and modules are not enabled. The default for
>> this case is already 0 but can be overridden by the deferred_probe_timeout
>> parameter. Let's just disable to avoid queuing a work that is not needed.
>>
>> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
>> ---
>>
>> (no changes since v1)
>>
>>  drivers/base/dd.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
>> index 040b4060f903..1e8f1afeac98 100644
>> --- a/drivers/base/dd.c
>> +++ b/drivers/base/dd.c
>> @@ -350,8 +350,10 @@ static int deferred_probe_initcall(void)
>>  	flush_work(&deferred_probe_work);
>>  	initcalls_done = true;
>>  
>> -	if (!IS_ENABLED(CONFIG_MODULES))
>> +	if (!IS_ENABLED(CONFIG_MODULES)) {
>> +		driver_deferred_probe_timeout = 0;
>>  		fw_devlink_drivers_done();
>> +	}
>>  
> 
> Potentially a stupid suggestion, but would it make sense to take out
> the ability to actually set that param if !CONFIG_MODULES? Then
> driver_deferred_probe_timeout would be the default value already.
> 

Yes, I think it makes sense. I will do that in the next iteration.

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat