Linux ARM-MSM sub-architecture
 help / color / mirror / Atom feed
From: Javier Martinez Canillas <javierm@redhat.com>
To: linux-kernel@vger.kernel.org
Cc: "Rafael J . Wysocki" <rafael@kernel.org>,
	Douglas Anderson <dianders@chromium.org>,
	Saravana Kannan <saravanak@google.com>,
	Daniel Vetter <daniel.vetter@ffwll.ch>,
	linux-arm-msm@vger.kernel.org, John Stultz <jstultz@google.com>,
	Peter Robinson <pbrobinson@redhat.com>,
	Steev Klimaszewski <steev@kali.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Enric Balletbo i Serra <eballetbo@redhat.com>,
	Bjorn Andersson <andersson@kernel.org>,
	Brian Masney <bmasney@redhat.com>, Rob Herring <robh@kernel.org>,
	Javier Martinez Canillas <javierm@redhat.com>
Subject: [PATCH v2 4/4] driver core: Disable driver deferred probe timeout by default
Date: Wed, 16 Nov 2022 13:02:36 +0100	[thread overview]
Message-ID: <20221116120236.520017-1-javierm@redhat.com> (raw)
In-Reply-To: <20221116115348.517599-1-javierm@redhat.com>

The driver_deferred_probe_timeout value has a long history. It was first
set to -1 when 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 work 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 to timeout after 30 seconds, unless a different timeout is set
using the "deferred_probe_timeout" command line parameter.

The behavior was changed even more 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
later by user-land to probe drivers that were still in the deferred list.

The 10 seconds timeout isn't enough in some cases, for example the Fedora
kernel builds as much drivers as possible as modules. And this leads to an
Snapdragon SC7180 based HP X2 Chromebook to not have display, due the DRM
driver failing to probe if CONFIG_ARM_SMMU=y and CONFIG_SC_GPUCC_7180=m.

So let's change the default again to -1 as it was at the beginning. That's
how probe deferral always worked. The kernel should try to avoid guessing
when it should be safe to give up on deferred drivers to be probed.

The reason why the default "deferred_probe_timeout" was changed from -1 to
the other values was to allow drivers that have only optional dependencies
to probe even if the suppliers are not available.

But now there is a "fw_devlink.timeout" parameter to timeout the links and
allow drivers to probe even when the dependencies are not present. Let's
set the default for that timeout to 10 seconds, to give the same behaviour
as expected by these driver with optional device links.

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

Changes in v2:
- Mention in the commit messsage the specific machine and drivers that
  are affected by the issue (Greg).
- Double check the commit message for accuracy (John).
- Add a second workqueue to timeout the devlink enforcing and allow
  drivers to probe even without their optional dependencies available.

 drivers/base/dd.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index ea448df94d24..5f18cd497850 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -256,12 +256,8 @@ static int deferred_devs_show(struct seq_file *s, void *data)
 }
 DEFINE_SHOW_ATTRIBUTE(deferred_devs);
 
-#ifdef CONFIG_MODULES
-static int driver_deferred_probe_timeout = 10;
-#else
-static int driver_deferred_probe_timeout;
-#endif
-static int fw_devlink_timeout = -1;
+static int driver_deferred_probe_timeout = -1;
+static int fw_devlink_timeout = 10;
 
 static int __init deferred_probe_timeout_setup(char *str)
 {
-- 
2.38.1


  parent reply	other threads:[~2022-11-16 12:10 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-16 11:53 [PATCH v2 0/4] driver core: Decouple device links enforcing and probe deferral timeouts Javier Martinez Canillas
2022-11-16 11:53 ` [PATCH v2 1/4] driver core: Make driver_deferred_probe_timeout a static variable Javier Martinez Canillas
2022-11-16 12:57   ` Andrew Halaney
2022-11-17 19:14   ` John Stultz
2022-12-12  8:50     ` Javier Martinez Canillas
2022-12-12  8:59       ` Greg Kroah-Hartman
2022-12-12  9:06         ` Javier Martinez Canillas
2022-11-16 12:00 ` [PATCH v2 2/4] driver core: Set deferred probe timeout to 0 if modules are disabled Javier Martinez Canillas
2022-11-16 16:39   ` Andrew Halaney
2022-11-16 16:51     ` Javier Martinez Canillas
2022-11-16 12:01 ` [PATCH v2 3/4] driver core: Add fw_devlink.timeout param to stop waiting for devlinks Javier Martinez Canillas
2022-11-16 16:09   ` Randy Dunlap
2022-11-16 19:05   ` Andrew Halaney
2022-11-17 15:19   ` Andrew Halaney
2022-11-17 18:55     ` Javier Martinez Canillas
2022-11-17 19:07   ` John Stultz
2022-11-17 19:16     ` Javier Martinez Canillas
2022-11-16 12:02 ` Javier Martinez Canillas [this message]
2022-11-16 19:15   ` [PATCH v2 4/4] driver core: Disable driver deferred probe timeout by default Andrew Halaney

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221116120236.520017-1-javierm@redhat.com \
    --to=javierm@redhat.com \
    --cc=andersson@kernel.org \
    --cc=bmasney@redhat.com \
    --cc=daniel.vetter@ffwll.ch \
    --cc=dianders@chromium.org \
    --cc=eballetbo@redhat.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jstultz@google.com \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbrobinson@redhat.com \
    --cc=rafael@kernel.org \
    --cc=robh@kernel.org \
    --cc=saravanak@google.com \
    --cc=steev@kali.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox