Received-SPF: pass (google.com: domain of linux-kernel+bounces-96361-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
From: Javier Martinez Canillas <javierm@redhat.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-kernel@vger.kernel.org, javier@dowhile0.org, Andrew Halaney
 <ahalaney@redhat.com>, "Rafael J. Wysocki" <rafael@kernel.org>
Subject: Re: [PATCH v3] driver core: Don't set a deferred probe timeout if
 modules are disabled
In-Reply-To: <2024030722-explicit-juror-9ba7@gregkh>
References: <20240227231001.648698-1-javierm@redhat.com>
 <2024030722-explicit-juror-9ba7@gregkh>
Date: Fri, 08 Mar 2024 00:20:16 +0100
Message-ID: <875xxxsk7j.fsf@minerva.mail-host-address-is-not-set>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain

Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:

> On Wed, Feb 28, 2024 at 12:09:02AM +0100, Javier Martinez Canillas wrote:
>> There is no point to schedule the workqueue 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 skip this and avoid queuing work that is not needed.
>
> As the option is already there to set the timeout to 0, why confuse
> things by trying to tie this to if modules are enabled or not?  And even

Because the timeout is already tied to whether modules are enabled or not [0]:

#ifdef CONFIG_MODULES
static int driver_deferred_probe_timeout = 10;
#else
static int driver_deferred_probe_timeout;
#endif

static int __init deferred_probe_timeout_setup(char *str)
{
	int timeout;

	if (!kstrtoint(str, 10, &timeout))
		driver_deferred_probe_timeout = timeout;
	return 1;
}
__setup("deferred_probe_timeout=", deferred_probe_timeout_setup);

Since that's already the case, it makes no sense to allow this option to
be set when modules is not enabled IMO.

That will just cause to schedule a workqueue for no good reasons, since it
happens at late_initcall() after all the initcall functions for built-in
drivers have already been executed.

[0]: https://elixir.bootlin.com/linux/v6.8-rc7/source/drivers/base/dd.c#L259

> if you do want to do that, where did you now document this new system
> behavior?
>

I thought about it but the current behaviour is also not documented [1],
in fact most of the help text for the deferred_probe_timeout= option is
still from the time when it was introduced _only_ as a debugging option
by 25b4e70dcce9 ("driver core: allow stopping deferred probe after init").

Later, c8c43cee29f6 ("driver core: Fix driver_deferred_probe_check_state()
logic") and e2cec7d68537 ("driver core: Set deferred_probe_timeout to a
longer default if CONFIG_MODULES is set"), tied the behavior and default
timeout to whether the modules were enabled or not. And those two commits
did not document the behavioural changes in the kernel option help either.

I can update the kernel-parameters.txt in the next iteration if you agree
with the change.

[1]: https://elixir.bootlin.com/linux/v6.8-rc7/source/Documentation/admin-guide/kernel-parameters.txt#L1035

> thanks,
>
> greg k-h
>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat