linux-kernel - Re: [PATCH v3 2/2] driver core: Make deferred_probe

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CALAqxLU3k-pqG0C92RCg6_fgeF7f5iGo8uos6ZcJpYGc3ZryfA@mail.gmail.com>
Date:   Wed, 19 Feb 2020 13:15:29 -0800
From:   John Stultz <john.stultz@...aro.org>
To:     Mark Brown <broonie@...nel.org>
Cc:     lkml <linux-kernel@...r.kernel.org>, Rob Herring <robh@...nel.org>,
        "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Kevin Hilman <khilman@...nel.org>,
        Ulf Hansson <ulf.hansson@...aro.org>,
        Pavel Machek <pavel@....cz>, Len Brown <len.brown@...el.com>,
        Todd Kjos <tkjos@...gle.com>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Liam Girdwood <lgirdwood@...il.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Linux PM list <linux-pm@...r.kernel.org>
Subject: Re: [PATCH v3 2/2] driver core: Make deferred_probe_timeout global so
 it can be shared

On Wed, Feb 19, 2020 at 3:59 AM Mark Brown <broonie@...nel.org> wrote:
>
> On Tue, Feb 18, 2020 at 10:07:48PM +0000, John Stultz wrote:
> > This patch, suggested by Rob, allows deferred_probe_timeout to
> > be global so other substems can use it.
>
> > This also sets the default to 30 instead of -1 (no timeout) and
> > modifies the regulator code to make use of it instead of its
> > hard-coded 30 second interval.
>
> This is at least two patches, one adding the new feature and the other
> adding a user of that feature.

Yea, agreed. I combined it here just to get input at this point
without flooding folks with a ton of small patches.

> > @@ -5767,18 +5772,17 @@ static int __init regulator_init_complete(void)
> >               has_full_constraints = true;
> >
> >       /*
> > -      * We punt completion for an arbitrary amount of time since
> > +      * We punt completion for deferred_probe_timeout seconds since
> >        * systems like distros will load many drivers from userspace
> >        * so consumers might not always be ready yet, this is
> >        * particularly an issue with laptops where this might bounce
>
> While I don't see it doing any harm I'm not 100% convinced by this
> change - we're not really doing anything directly to do with deferred
> probe here, we're shutting off regulators that remain unused late in
> boot but even then they'll still be available for use.  It feels a bit
> unclear and the way you've adapted the code to always have a timeout
> even if the deferred probe timeout gets changed feels a bit off.  If
> nothing else this comment needs more of an update than you've done.

This was mostly spurred by Rob's suggestion, as it seemed both
subsystems were doing similar timeouts that may need to be customized
(likely around the same amount of time).  I agree it doesn't quite map
perfectly, but I imagine it would be useful to align those in some
way.

I'll likely take a swing first at getting the driver core bits to make
sense, and then I'll come and revisit this shared timeout issue.

thanks
-john