2022-11-14 11:09:07

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] driver core: Disable driver deferred probe timeout by default

On Mon, Nov 14, 2022 at 11:43:33AM +0100, Javier Martinez Canillas wrote:
> The driver_deferred_probe_timeout value has a long story. It was first set
> to -1 when it was introduced by commit 25b4e70dcce9 ("driver core: allow
> stopping deferred probe after init"), meaning that the driver core would
> defer the probe forever unless a subsystem would opt-in by checking if the
> initcalls where done using the driver_deferred_probe_check_state() helper,
> or if a timeout was explicitly set with a "deferred_probe_timeout" param.
>
> Only the power domain, IOMMU and MDIO subsystems currently opt-in to check
> if the initcalls have completed with driver_deferred_probe_check_state().
>
> Commit c8c43cee29f6 ("driver core: Fix driver_deferred_probe_check_state()
> logic") then changed the driver_deferred_probe_check_state() helper logic,
> to take into account whether modules have been enabled or not and also to
> return -EPROBE_DEFER if the probe deferred timeout was still running.
>
> Then in commit e2cec7d68537 ("driver core: Set deferred_probe_timeout to a
> longer default if CONFIG_MODULES is set"), the timeout was increased to 30
> seconds if modules are enabled. Because seems that some of the subsystems
> that were opt-in to not return -EPROBE_DEFER after the initcall where done
> could still have dependencies whose drivers were built as a module.
>
> This commit did a fundamental change to how probe deferral worked though,
> since now the default was not to attempt probing for drivers indefinitely
> but instead it would timeout after 30 seconds unless a different timeout
> was set using the "deferred_probe_timeout" parameter.
>
> The behavior was changed even mere with commit ce68929f07de ("driver core:
> Revert default driver_deferred_probe_timeout value to 0"), since the value
> was set to 0 by default. Meaning that the probe deferral would be disabled
> after the initcalls where done. Unless a timeout was set in the cmdline.
>
> Notice that the commit said that it was reverting the default value to 0,
> but this was never 0. The default was -1 at the beginning and then changed
> to 30 in a later commit.
>
> This default value of 0 was reverted again by commit f516d01b9df2 ("Revert
> "driver core: Set default deferred_probe_timeout back to 0."") and set to
> 10 seconds instead. Which was still less than the 30 seconds that was set
> at some point to allow systems with drivers built as modules and loaded by
> user-land to probe drivers that were previously deferred.
>
> The 10 seconds timeout isn't enough for the mentioned systems, for example
> general purpose distributions attempt to build all the possible drivers as
> a module to keep the Linux kernel image small. But that means that in very
> likely that the probe deferral mechanism will timeout and drivers won't be
> probed correctly.

What specific "mentioned systems" have deferred probe drivers that are
failing on the current value? What drivers are causing the long delay
here? No one should be having to wait 10 seconds for a deferred delay
on a real system as that feels really wrong.

Why not fix the drivers that are causing this delay and maybe move them
to be async so as to not slow down the whole boot process?

thanks,

greg k-h


2022-11-14 11:28:57

by Javier Martinez Canillas

[permalink] [raw]
Subject: Re: [PATCH] driver core: Disable driver deferred probe timeout by default

Hello Greg,

Thanks a lot for your feedback.

On 11/14/22 11:54, Greg Kroah-Hartman wrote:

[...]

>>
>> This default value of 0 was reverted again by commit f516d01b9df2 ("Revert
>> "driver core: Set default deferred_probe_timeout back to 0."") and set to
>> 10 seconds instead. Which was still less than the 30 seconds that was set
>> at some point to allow systems with drivers built as modules and loaded by
>> user-land to probe drivers that were previously deferred.
>>
>> The 10 seconds timeout isn't enough for the mentioned systems, for example
>> general purpose distributions attempt to build all the possible drivers as
>> a module to keep the Linux kernel image small. But that means that in very
>> likely that the probe deferral mechanism will timeout and drivers won't be
>> probed correctly.
>
> What specific "mentioned systems" have deferred probe drivers that are

The "mentioned systems" are the ones mentioned in the paragraph above:

"to allow systems with drivers built as modules and loaded by user-land to
probe drivers that were previously deferred."

I even gave an example about general purpose distributions that build as
much as possible as a module. What more info do you think that is missing?

> failing on the current value? What drivers are causing the long delay
> here? No one should be having to wait 10 seconds for a deferred delay
> on a real system as that feels really wrong.
>

Not really, it depends if the drivers are built-in, built as modules, in
the initramfs or in the rootfs. A 10 seconds might not be enough if these
modules are in the root partition and need to wait for this to be mounted
and udev to load the modules, etc.

Also, it may even be that the module alias is not correct and then users
have to load them by explicitly have /etc/modules-load.d/ configs and so
on.

> Why not fix the drivers that are causing this delay and maybe move them
> to be async so as to not slow down the whole boot process?
>

Yes, these drivers could be fixed to report a proper module alias or the
dependencies can be built-in or added to the initramfs and that does not
change the fact that by default the kernel shouldn't make assumptions
about when is safe to just timeout instead of -EPROBE_DEFER.

Because with modules the kernel has no way to know when all the modules
have been already been loaded by user-space or more drivers are going to
be registered in the future.

Also, that's how probe deferral always worked since the mechanism was
introduced. It's just recently that the behavior was changed to timeout.

A nice feature of the probe deferral mechanism is that it was simple and
reliable. Adding a timeout makes it non-deterministic and more fragile IMO.

--
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat


2022-11-14 13:32:39

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] driver core: Disable driver deferred probe timeout by default

On Mon, Nov 14, 2022 at 12:13:15PM +0100, Javier Martinez Canillas wrote:
> Hello Greg,
>
> Thanks a lot for your feedback.
>
> On 11/14/22 11:54, Greg Kroah-Hartman wrote:
>
> [...]
>
> >>
> >> This default value of 0 was reverted again by commit f516d01b9df2 ("Revert
> >> "driver core: Set default deferred_probe_timeout back to 0."") and set to
> >> 10 seconds instead. Which was still less than the 30 seconds that was set
> >> at some point to allow systems with drivers built as modules and loaded by
> >> user-land to probe drivers that were previously deferred.
> >>
> >> The 10 seconds timeout isn't enough for the mentioned systems, for example
> >> general purpose distributions attempt to build all the possible drivers as
> >> a module to keep the Linux kernel image small. But that means that in very
> >> likely that the probe deferral mechanism will timeout and drivers won't be
> >> probed correctly.
> >
> > What specific "mentioned systems" have deferred probe drivers that are
>
> The "mentioned systems" are the ones mentioned in the paragraph above:
>
> "to allow systems with drivers built as modules and loaded by user-land to
> probe drivers that were previously deferred."
>
> I even gave an example about general purpose distributions that build as
> much as possible as a module. What more info do you think that is missing?

Exact systems that this is failing on would be great to have.

> > failing on the current value? What drivers are causing the long delay
> > here? No one should be having to wait 10 seconds for a deferred delay
> > on a real system as that feels really wrong.
> >
>
> Not really, it depends if the drivers are built-in, built as modules, in
> the initramfs or in the rootfs. A 10 seconds might not be enough if these
> modules are in the root partition and need to wait for this to be mounted
> and udev to load the modules, etc.

How does it take 10 seconds to load the initramfs for a system that
requires deferred probe devices? What typs of hardware is this?

> Also, it may even be that the module alias is not correct and then users
> have to load them by explicitly have /etc/modules-load.d/ configs and so
> on.

Then that's a totally different issue and the module alias needs to be
fixed and is not relevant here.

> > Why not fix the drivers that are causing this delay and maybe move them
> > to be async so as to not slow down the whole boot process?
> >
>
> Yes, these drivers could be fixed to report a proper module alias or the
> dependencies can be built-in or added to the initramfs and that does not
> change the fact that by default the kernel shouldn't make assumptions
> about when is safe to just timeout instead of -EPROBE_DEFER.

Please let me know which drivers these are that are causing problems so
we can fix them.

> Because with modules the kernel has no way to know when all the modules
> have been already been loaded by user-space or more drivers are going to
> be registered in the future.

Of course that is true, so we guess, and so far, 10 seconds is a good
enough guess for normal systems out there that use deferred probe. What
exact system and drivers do not work with this today?

> Also, that's how probe deferral always worked since the mechanism was
> introduced. It's just recently that the behavior was changed to timeout.
>
> A nice feature of the probe deferral mechanism is that it was simple and
> reliable. Adding a timeout makes it non-deterministic and more fragile IMO.

deferred probe was never simple or reliable or determinisitic. It was a
hack we had to implement to handle complex hardware situations and
loadable drivers. Let's not try to paper over driver bugs here by
making the timeout "forever" but rather fix the root problem in the
broken drivers.

So, what drivers do we need to fix up?

thanks,

greg k-h

2022-11-14 13:58:23

by Javier Martinez Canillas

[permalink] [raw]
Subject: Re: [PATCH] driver core: Disable driver deferred probe timeout by default

On 11/14/22 12:38, Greg Kroah-Hartman wrote:
> On Mon, Nov 14, 2022 at 12:13:15PM +0100, Javier Martinez Canillas wrote:
>> Hello Greg,

[...]

>> I even gave an example about general purpose distributions that build as
>> much as possible as a module. What more info do you think that is missing?
>
> Exact systems that this is failing on would be great to have.
>

The exact system is a Snapdragon SC7180 based HP X2 Chromebook with latest
Fedora Rawhide image (kernel version 6.1-rc4). The reason why is timing out
is that the arm_smmu driver is built-in (CONFIG_ARM_SMMU=y) but it depends
on gpucc-sc7180 clk driver that's built as module (CONFIG_SC_GPUCC_7180=m).

>>> failing on the current value? What drivers are causing the long delay
>>> here? No one should be having to wait 10 seconds for a deferred delay
>>> on a real system as that feels really wrong.
>>>
>>
>> Not really, it depends if the drivers are built-in, built as modules, in
>> the initramfs or in the rootfs. A 10 seconds might not be enough if these
>> modules are in the root partition and need to wait for this to be mounted
>> and udev to load the modules, etc.
>
> How does it take 10 seconds to load the initramfs for a system that
> requires deferred probe devices? What typs of hardware is this?
>

That could depend on may things. The dependency of the systemd unit files,
whether NetworkManager-wait-online.service is enabled or not, etc. It can
really take more than 10 seconds on some systems to load all the modules.

[...]

>>
>> A nice feature of the probe deferral mechanism is that it was simple and
>> reliable. Adding a timeout makes it non-deterministic and more fragile IMO.
>
> deferred probe was never simple or reliable or determinisitic. It was a
> hack we had to implement to handle complex hardware situations and
> loadable drivers. Let's not try to paper over driver bugs here by
> making the timeout "forever" but rather fix the root problem in the
> broken drivers.
>

I don't understand how adding a 10 secs timeout would make it more robust than
just letting the driver core to attempt probing the deferred drivers again for
every driver (or device) that gets registered.

> So, what drivers do we need to fix up?
>

So what exactly needs to get fixed on the arm_smmu and gpucc-sc7180 drivers
mentioned? Yes, we could built both of them as =y and make sure that drivers
are registered and probed before the initcalls are done, but if we do that,
we will need to have most of the drivers built-in in the Fedora kernel. That
does not scale for all the platforms that we need to support.

--
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat


2022-11-14 16:50:57

by Steev Klimaszewski

[permalink] [raw]
Subject: Re: [PATCH] driver core: Disable driver deferred probe timeout by default

Hi All,

On 11/14/22 5:56 AM, Javier Martinez Canillas wrote:
> On 11/14/22 12:38, Greg Kroah-Hartman wrote:
>> On Mon, Nov 14, 2022 at 12:13:15PM +0100, Javier Martinez Canillas wrote:
>>> Hello Greg,
> [...]
>
>>> I even gave an example about general purpose distributions that build as
>>> much as possible as a module. What more info do you think that is missing?
>> Exact systems that this is failing on would be great to have.
>>
> The exact system is a Snapdragon SC7180 based HP X2 Chromebook with latest
> Fedora Rawhide image (kernel version 6.1-rc4). The reason why is timing out
> is that the arm_smmu driver is built-in (CONFIG_ARM_SMMU=y) but it depends
> on gpucc-sc7180 clk driver that's built as module (CONFIG_SC_GPUCC_7180=m).

Additionally, this fails on the Snapdragon SC8280XP based Thinkpad X13s
with 6.1-rc4 (and rc5), however, unlike Javier's setup, I *do* have
CONFIG_SC_GPUCC_SC8280XP=y, so it's already built in to the kernel. 
Additionally, all DRM options =y, except for CONFIG_DRM_GEM_HELP=m and
CONFIG_DRM_DISPLAY_CONNECTOR=m.

I am hoping to find some time tonight after work to also test on my
SDM850 based Lenovo Yoga C630, as I believe my previous testing of 6.1
may have been hitting this but I did not have time to dig into what was
going on there.

-- steev