Message ID | 20221116120159.519908-1-javierm@redhat.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:6687:0:0:0:0:0 with SMTP id l7csp103871wru; Wed, 16 Nov 2022 04:11:17 -0800 (PST) X-Google-Smtp-Source: AA0mqf68CqkToFyYXgFaLCjGXvN7etEa5rAuSOFlPX8hGWjO82tQ8Qa0m6DLPTxdt4llxcDwTtdY X-Received: by 2002:a17:906:ca4f:b0:7ae:5a4:5356 with SMTP id jx15-20020a170906ca4f00b007ae05a45356mr17836136ejb.748.1668600676877; Wed, 16 Nov 2022 04:11:16 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1668600676; cv=none; d=google.com; s=arc-20160816; b=00LUUev+Kg07Q+7UwS0X3LqyO/W8eF8rXijJXV6EO8X/FWoxwK/cx4PNJdID1N/4hg FfTw1qEkiRp7vvESyli24XMs+OGzOFdLi9YOD5CZDRWKLHJBOxNuaZgQaBYTrMIQCCYo 7EK0ExNcpvUKEDhxH5nlCbjt5ZABZTUU7AVYLCNrYLxlhlhgJWw9R3Dw9fVfJwlRvK6h SSoSNgW/t9/+wjm5C3n1esUqfnfnnyHZ6oUVfRAawYql9QW3GyHTk0LnvyD6X204pyyl 6XtAGTJUBJN5affZmFbwvr/P6PCKgQ186GwUD3VwUuFSEcR2nuYqCqut4EnbhOjai2xs /+UA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=sBCDaltTnEkyasPr95FrZvMKGG7154FaBt72snWNmEY=; b=Qbkvn4bc6kF9ptMlpoT+VfXYfmEr1ZTVJwF0amm/AQGiYN3FfXMe9mSKYkQeu1gHui L/fzp6A0NxPE4nS51u/idLlN9ogwivCrbETQkLvDMJSBX6lORKy7BiBaL0+W+2wnVP1o 1be0o4CgX1arf5DzYGx4ckVnj4VncjQX5ZPZGkhmdod8YyhV/1cwjt4Y4QQpMDMOaqgw hktSAEX3CNPB5Q1ZwfG+ca1ENv3pn+DzmAeUFHqP/LygwmKbvbk9t/NHleLyykKTOqUM yQ8gBZeaYP6ZA6+VY54mhqwl7xoXx0ATaINmQ1K289jlMy0yPIeLtr1bCp0YUWK9cKs2 jOaw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=atfodMCY; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id jg9-20020a170907970900b0078db6f56d51si12854419ejc.808.2022.11.16.04.10.52; Wed, 16 Nov 2022 04:11:16 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=atfodMCY; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232367AbiKPMJn (ORCPT <rfc822;just.gull.subs@gmail.com> + 99 others); Wed, 16 Nov 2022 07:09:43 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53542 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232190AbiKPMIi (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 16 Nov 2022 07:08:38 -0500 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 728E013D3C for <linux-kernel@vger.kernel.org>; Wed, 16 Nov 2022 04:02:09 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1668600128; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=sBCDaltTnEkyasPr95FrZvMKGG7154FaBt72snWNmEY=; b=atfodMCYdecfbDQJ60P8Lqw4eVD3Amu/pklU3y2Xzo6mUL1x5BAthVnWaiHVCJ+hD9WUcz rCfKPbyOqQVw1+GM+5HkHyrcLG1R/clgrK2EJdbmI4JTDupTolclD4ozM6SALDgvxnxaDa PeYDUxY/O7GJV6ZUySr0uXE0Ofkjits= Received: from mail-wr1-f71.google.com (mail-wr1-f71.google.com [209.85.221.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-118-kvOeMQtxNNOVcn-WRH_5qw-1; Wed, 16 Nov 2022 07:02:07 -0500 X-MC-Unique: kvOeMQtxNNOVcn-WRH_5qw-1 Received: by mail-wr1-f71.google.com with SMTP id r4-20020adfbb04000000b00236639438e9so3663704wrg.11 for <linux-kernel@vger.kernel.org>; Wed, 16 Nov 2022 04:02:07 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=sBCDaltTnEkyasPr95FrZvMKGG7154FaBt72snWNmEY=; b=PRTxuDIZVxo0vuRN2k53pI2Il+k4c3zeL0Lcm1N/fNBMmL0qqwpAw02Bm2qDNa/gdJ b3CqUPrTVR8/m+uH4KVGPqQDNQHR1+uhyJWUI764MBNlBTpcKKLBIzw4nz3fT+x7OeEz xFwhdSwLTAWBdcO8lzdpG72bGX0wnr5D1sl12aWEOohbfwrM1LP3c17vDmbwdQ7WKOhh NExdV8LP/1vCxmlxR+c13BP1J+uHO6lEnEKaIyWoanji7fnF/JgGLzcCJJIWaNzn7IqD yhCgiW3SUM00NtJ9AB0oq1LgUcvslUWJESljQQf9Q4hh+FhuJMuhzggtaQkiB75tz41Z 5aBw== X-Gm-Message-State: ANoB5plGJPym+HD0oRJE3MiS0FJqaFa95DqhrMsFyWqNVnUJrqja1l3G llxhGVFgsV6N86cCcTZyeouXTGMeKn5elu22br084wcEo2Y3pXBvV3y61Zv6BHcjKJ7cWi1F7FC TSDi7l9820XsNzJPMX9+I5JkQM0UruT02yfV3OegJ9b50auuG7A+rD3iutZ9eAr7lH4HQjLmrD1 4= X-Received: by 2002:a05:600c:4b17:b0:3cf:8b22:b567 with SMTP id i23-20020a05600c4b1700b003cf8b22b567mr1858796wmp.144.1668600126409; Wed, 16 Nov 2022 04:02:06 -0800 (PST) X-Received: by 2002:a05:600c:4b17:b0:3cf:8b22:b567 with SMTP id i23-20020a05600c4b1700b003cf8b22b567mr1858754wmp.144.1668600126011; Wed, 16 Nov 2022 04:02:06 -0800 (PST) Received: from minerva.home (205.pool92-176-231.dynamic.orange.es. [92.176.231.205]) by smtp.gmail.com with ESMTPSA id i17-20020adfefd1000000b00236b2804d79sm15327240wrp.2.2022.11.16.04.02.05 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 16 Nov 2022 04:02:05 -0800 (PST) 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 3/4] driver core: Add fw_devlink.timeout param to stop waiting for devlinks Date: Wed, 16 Nov 2022 13:01:59 +0100 Message-Id: <20221116120159.519908-1-javierm@redhat.com> X-Mailer: git-send-email 2.38.1 In-Reply-To: <20221116115348.517599-1-javierm@redhat.com> References: <20221116115348.517599-1-javierm@redhat.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1749654623572747508?= X-GMAIL-MSGID: =?utf-8?q?1749654623572747508?= |
Series |
driver core: Decouple device links enforcing and probe deferral timeouts
|
|
Commit Message
Javier Martinez Canillas
Nov. 16, 2022, 12:01 p.m. UTC
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@redhat.com>
---
(no changes since v1)
.../admin-guide/kernel-parameters.txt | 7 ++++
drivers/base/dd.c | 38 ++++++++++++++++++-
2 files changed, 44 insertions(+), 1 deletion(-)
Comments
Hi-- On 11/16/22 04:01, Javier Martinez Canillas wrote: > 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 timeout > + be restarted by each successful driver registration.
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@redhat.com> I like this idea and it looks good as far as I can tell. Acked-by: Andrew Halaney <ahalaney@redhat.com> > --- > > (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 >
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@redhat.com> > --- > > (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= Just thought about this, but I think this should be called fw_devlink_timeout. Generally the $MODULE.$PARAM syntax is reserved for things that can be specificed with module_param(). The advantage is if you accidentally type say fw_devlink_timeut=10 the kernel logs will indicate it has no clue what that means. Including the "." makes the kernel assume that maybe a future module name fw_devlink will be loaded, and at that time will see if that module has the parameter mentioned. A little thing but I think work changing in v3. Thanks, Andrew
Hello Andrew, On 11/17/22 16:19, Andrew Halaney wrote: > 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@redhat.com> >> --- >> >> (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= > > Just thought about this, but I think this should be called > fw_devlink_timeout. Generally the $MODULE.$PARAM syntax is reserved for > things that can be specificed with module_param(). > > The advantage is if you accidentally type say fw_devlink_timeut=10 the > kernel logs will indicate it has no clue what that means. Including the > "." makes the kernel assume that maybe a future module name fw_devlink > will be loaded, and at that time will see if that module has the > parameter mentioned. A little thing but I think work changing in v3. > I was actually on the fence on this one but the reason why I did the .timeout was that the other fw_devlink param [0] is defined as fw_devlink.strict=<bool> so I wanted to keep this one consistent with that. https://www.kernel.org/doc/html/latest/admin-guide/kernel-parameters.html
On Wed, Nov 16, 2022 at 4:02 AM Javier Martinez Canillas <javierm@redhat.com> 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. I'm probably being dim, but it's not immediately clear from this description *why* this is useful. Maybe add some words on the tangible benefit of splitting this up? I'd also push a little bit back on why we need to split this into a separate boot option. Since it's not obvious as to when a user would want to use fw_devlink.timeout vs probe_deferral_timeout. The extra complexity of remembering which timeout is for what might become a burden to users and developers. > > + 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. > + This sounds pretty close to like the deferred_probe_timeout option. I'd suggest some words to make the distinction more clear. thanks -john
Hello John, On 11/17/22 20:07, John Stultz wrote: > On Wed, Nov 16, 2022 at 4:02 AM Javier Martinez Canillas > <javierm@redhat.com> 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. > > I'm probably being dim, but it's not immediately clear from this > description *why* this is useful. Maybe add some words on the tangible > benefit of splitting this up? > Thanks for your feedback. You are right that I need to better explain the why / goal of this patch. But basically is that it would be good to allow timeout waiting for the optional links while still allow the non-optional links to keep make the drivers to keep deferring. I can make a better job at explaining the why in the next iteration. > I'd also push a little bit back on why we need to split this into a > separate boot option. Since it's not obvious as to when a user would > want to use fw_devlink.timeout vs probe_deferral_timeout. > The extra complexity of remembering which timeout is for what might > become a burden to users and developers. > >> >> + 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. >> + > > This sounds pretty close to like the deferred_probe_timeout option. > I'd suggest some words to make the distinction more clear. > Yeah, I can think how to better explain this... but it's similar because there is some overlapping between the two really, but are not exactly the same even though we are tying the two and folding the disable of the two under the same timeout. I'll be OK to drop this parameter btw, and just keep it as an internal var if it's fine to just always use the default 10 seconds or whatever timeout it is decided.
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);