Re: [PATCH v3 1/3] driver core: Revert default driver_deferred_probe_timeout value to 0

[Marek Szyprowski <m.szyprowski@samsung.com>]

Hi John,

On 22.04.2020 22:32, John Stultz wrote:
> This patch addresses a regression in 5.7-rc1+
>
> In commit c8c43cee29f6 ("driver core: Fix
> driver_deferred_probe_check_state() logic"), we both cleaned up
> the logic and also set the default driver_deferred_probe_timeout
> value to 30 seconds to allow for drivers that are missing
> dependencies to have some time so that the dependency may be
> loaded from userland after initcalls_done is set.
>
> However, Yoshihiro Shimoda reported that on his device that
> expects to have unmet dependencies (due to "optional links" in
> its devicetree), was failing to mount the NFS root.
>
> In digging further, it seemed the problem was that while the
> device properly probes after waiting 30 seconds for any missing
> modules to load, the ip_auto_config() had already failed,
> resulting in NFS to fail. This was due to ip_auto_config()
> calling wait_for_device_probe() which doesn't wait for the
> driver_deferred_probe_timeout to fire.
>
> Fixing that issue is possible, but could also introduce 30
> second delays in bootups for users who don't have any
> missing dependencies, which is not ideal.
>
> So I think the best solution to avoid any regressions is to
> revert back to a default timeout value of zero, and allow
> systems that need to utilize the timeout in order for userland
> to load any modules that supply misisng dependencies in the dts
> to specify the timeout length via the exiting documented boot
> argument.
>
> Thanks to Geert for chasing down that ip_auto_config was why NFS
> was failing in this case!
>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
> Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
> Cc: Sudeep Holla <sudeep.holla@arm.com>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Naresh Kamboju <naresh.kamboju@linaro.org>
> Cc: Basil Eljuse <Basil.Eljuse@arm.com>
> Cc: Ferry Toth <fntoth@gmail.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Anders Roxell <anders.roxell@linaro.org>
> Cc: netdev <netdev@vger.kernel.org>
> Cc: linux-pm@vger.kernel.org
> Reported-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Tested-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Fixes: c8c43cee29f6 ("driver core: Fix driver_deferred_probe_check_state() logic")
> Signed-off-by: John Stultz <john.stultz@linaro.org>

Please also revert dca0b44957e5 "regulator: Use 
driver_deferred_probe_timeout for regulator_init_complete_work" then, 
because now with the default 0 timeout some regulators gets disabled 
during boot, before their supplies gets instantiated.

This patch broke booting of Samsung Exynos5800-based Peach-Pi Chromeboot 
with the default multi_v7_defconfig.

> ---
>   drivers/base/dd.c | 13 ++-----------
>   1 file changed, 2 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index 06ec0e851fa1..908ae4d7805e 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -224,16 +224,7 @@ static int deferred_devs_show(struct seq_file *s, void *data)
>   }
>   DEFINE_SHOW_ATTRIBUTE(deferred_devs);
>   
> -#ifdef CONFIG_MODULES
> -/*
> - * In the case of modules, set the default probe timeout to
> - * 30 seconds to give userland some time to load needed modules
> - */
> -int driver_deferred_probe_timeout = 30;
> -#else
> -/* In the case of !modules, no probe timeout needed */
> -int driver_deferred_probe_timeout = -1;
> -#endif
> +int driver_deferred_probe_timeout;
>   EXPORT_SYMBOL_GPL(driver_deferred_probe_timeout);
>   
>   static int __init deferred_probe_timeout_setup(char *str)
> @@ -266,7 +257,7 @@ int driver_deferred_probe_check_state(struct device *dev)
>   		return -ENODEV;
>   	}
>   
> -	if (!driver_deferred_probe_timeout) {
> +	if (!driver_deferred_probe_timeout && initcalls_done) {
>   		dev_WARN(dev, "deferred probe timeout, ignoring dependency");
>   		return -ETIMEDOUT;
>   	}

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland