Re: [PATCH v2 3/4] driver core: Add fw_devlink.timeout param to stop waiting for devlinks

From: Andrew Halaney
Date: Wed Nov 16 2022 - 14:06:06 EST


On Wed, Nov 16, 2022 at 01:01:59PM +0100, Javier Martinez Canillas wrote:
> Currently, the probe deferral timeout does two things:
>
> 1) Call to fw_devlink_drivers_done() to relax the device dependencies and
> allow drivers to be probed if these dependencies are optional.
>
> 2) Disable the probe deferral mechanism so that drivers will fail to probe
> if the required dependencies are not present, instead of adding them to
> the deferred probe pending list.
>
> But there is no need to couple these two, for example the probe deferral
> can be used even when the device links are disable (i.e: fw_devlink=off).
>
> So let's add a separate fw_devlink.timeout command line parameter to allow
> relaxing the device links and prevent drivers to wait for these to probe.
>
> Signed-off-by: Javier Martinez Canillas <javierm@xxxxxxxxxx>

I like this idea and it looks good as far as I can tell.

Acked-by: Andrew Halaney <ahalaney@xxxxxxxxxx>

> ---
>
> (no changes since v1)
>
> .../admin-guide/kernel-parameters.txt | 7 ++++
> drivers/base/dd.c | 38 ++++++++++++++++++-
> 2 files changed, 44 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index a465d5242774..38138a44d5ed 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -1581,6 +1581,13 @@
> dependencies. This only applies for fw_devlink=on|rpm.
> Format: <bool>
>
> + fw_devlink.timeout=
> + [KNL] Debugging option to set a timeout in seconds for
> + drivers to give up waiting on dependencies and to probe
> + these are optional. A timeout of 0 will timeout at the
> + end of initcalls. If the time out hasn't expired, it'll
> + be restarted by each successful driver registration.
> +
> gamecon.map[2|3]=
> [HW,JOY] Multisystem joystick and NES/SNES/PSX pad
> support via parallel port (up to 5 devices per port)
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index 1e8f1afeac98..ea448df94d24 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -261,6 +261,7 @@ static int driver_deferred_probe_timeout = 10;
> #else
> static int driver_deferred_probe_timeout;
> #endif
> +static int fw_devlink_timeout = -1;
>
> static int __init deferred_probe_timeout_setup(char *str)
> {
> @@ -272,6 +273,16 @@ static int __init deferred_probe_timeout_setup(char *str)
> }
> __setup("deferred_probe_timeout=", deferred_probe_timeout_setup);
>
> +static int __init fw_devlink_timeout_setup(char *str)
> +{
> + int timeout;
> +
> + if (!kstrtoint(str, 10, &timeout))
> + fw_devlink_timeout = timeout;
> + return 1;
> +}
> +__setup("fw_devlink.timeout=", fw_devlink_timeout_setup);
> +
> /**
> * driver_deferred_probe_check_state() - Check deferred probe state
> * @dev: device to check
> @@ -318,6 +329,15 @@ static void deferred_probe_timeout_work_func(struct work_struct *work)
> }
> static DECLARE_DELAYED_WORK(deferred_probe_timeout_work, deferred_probe_timeout_work_func);
>
> +static void fw_devlink_timeout_work_func(struct work_struct *work)
> +{
> + fw_devlink_drivers_done();
> +
> + driver_deferred_probe_trigger();
> + flush_work(&deferred_probe_work);
> +}
> +static DECLARE_DELAYED_WORK(fw_devlink_timeout_work, fw_devlink_timeout_work_func);
> +
> void deferred_probe_extend_timeout(void)
> {
> /*
> @@ -330,6 +350,13 @@ void deferred_probe_extend_timeout(void)
> pr_debug("Extended deferred probe timeout by %d secs\n",
> driver_deferred_probe_timeout);
> }
> +
> + if (cancel_delayed_work(&fw_devlink_timeout_work)) {
> + schedule_delayed_work(&fw_devlink_timeout_work,
> + fw_devlink_timeout * HZ);
> + pr_debug("Extended fw_devlink timeout by %d secs\n",
> + fw_devlink_timeout);
> + }
> }
>
> /**
> @@ -352,9 +379,12 @@ static int deferred_probe_initcall(void)
>
> if (!IS_ENABLED(CONFIG_MODULES)) {
> driver_deferred_probe_timeout = 0;
> - fw_devlink_drivers_done();
> + fw_devlink_timeout = 0;
> }
>
> + if (!fw_devlink_timeout)
> + fw_devlink_drivers_done();
> +
> /*
> * Trigger deferred probe again, this time we won't defer anything
> * that is optional
> @@ -366,6 +396,12 @@ static int deferred_probe_initcall(void)
> schedule_delayed_work(&deferred_probe_timeout_work,
> driver_deferred_probe_timeout * HZ);
> }
> +
> + if (fw_devlink_timeout > 0) {
> + schedule_delayed_work(&fw_devlink_timeout_work,
> + fw_devlink_timeout * HZ);
> + }
> +
> return 0;
> }
> late_initcall(deferred_probe_initcall);
> --
> 2.38.1
>