Date:   Wed, 29 Apr 2020 15:46:04 +0200
From:   Marek Szyprowski <m.szyprowski@...sung.com>
To:     John Stultz <john.stultz@...aro.org>,
        lkml <linux-kernel@...r.kernel.org>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Alexey Kuznetsov <kuznet@....inr.ac.ru>,
        Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>,
        Jakub Kicinski <kuba@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "Rafael J . Wysocki" <rjw@...ysocki.net>,
        Rob Herring <robh@...nel.org>,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>,
        Robin Murphy <robin.murphy@....com>,
        Andy Shevchenko <andy.shevchenko@...il.com>,
        Sudeep Holla <sudeep.holla@....com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Naresh Kamboju <naresh.kamboju@...aro.org>,
        Basil Eljuse <Basil.Eljuse@....com>,
        Ferry Toth <fntoth@...il.com>, Arnd Bergmann <arnd@...db.de>,
        Anders Roxell <anders.roxell@...aro.org>,
        netdev <netdev@...r.kernel.org>, linux-pm@...r.kernel.org,
        Mark Brown <broonie@...nel.org>,
        'Linux Samsung SOC' <linux-samsung-soc@...r.kernel.org>
Subject: Re: [PATCH v3 1/3] driver core: Revert default
 driver_deferred_probe_timeout value to 0

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@...emloft.net>
> Cc: Alexey Kuznetsov <kuznet@....inr.ac.ru>
> Cc: Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>
> Cc: Jakub Kicinski <kuba@...nel.org>
> Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> Cc: Rafael J. Wysocki <rjw@...ysocki.net>
> Cc: Rob Herring <robh@...nel.org>
> Cc: Geert Uytterhoeven <geert@...ux-m68k.org>
> Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>
> Cc: Robin Murphy <robin.murphy@....com>
> Cc: Andy Shevchenko <andy.shevchenko@...il.com>
> Cc: Sudeep Holla <sudeep.holla@....com>
> Cc: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
> Cc: Naresh Kamboju <naresh.kamboju@...aro.org>
> Cc: Basil Eljuse <Basil.Eljuse@....com>
> Cc: Ferry Toth <fntoth@...il.com>
> Cc: Arnd Bergmann <arnd@...db.de>
> Cc: Anders Roxell <anders.roxell@...aro.org>
> Cc: netdev <netdev@...r.kernel.org>
> Cc: linux-pm@...r.kernel.org
> Reported-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>
> Tested-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>
> Fixes: c8c43cee29f6 ("driver core: Fix driver_deferred_probe_check_state() logic")
> Signed-off-by: John Stultz <john.stultz@...aro.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

Powered by blists - more mailing lists