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  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]
Date:   Thu, 13 Feb 2020 18:41:50 -0800
From:   John Stultz <john.stultz@...aro.org>
To:     Rob Herring <robh@...nel.org>
Cc:     lkml <linux-kernel@...r.kernel.org>,
        Alexander Graf <agraf@...e.de>,
        "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>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "open list:THERMAL" <linux-pm@...r.kernel.org>
Subject: Re: [RFC][PATCH] driver core: Extend returning EPROBE_DEFER for two
 minutes after late_initcall

On Thu, Feb 13, 2020 at 5:51 PM Rob Herring <robh@...nel.org> wrote:
> On Thu, Feb 13, 2020 at 6:44 PM John Stultz <john.stultz@...aro.org> wrote:
> > Due to commit e01afc3250255 ("PM / Domains: Stop deferring probe
> > at the end of initcall"), along with commit 25b4e70dcce9
> > ("driver core: allow stopping deferred probe after init") after
> > late_initcall, drivers will stop getting EPROBE_DEFER, and
> > instead see an error causing the driver to fail to load.
> >
> > That change causes trouble when trying to use many clk drivers
> > as modules, as the clk modules may not load until much later
> > after init has started. If a dependent driver loads and gets an
> > error instead of EPROBE_DEFER, it won't try to reload later when
> > the dependency is met, and will thus fail to load.
> >
> > Instead of reverting that patch, this patch tries to extend the
> > time that EPROBE_DEFER is retruned by two minutes, to (hopefully)
> > ensure that everything has had a chance to load.
>
> I think regulators already has some delay like this. We should use the
> same timeouts.

Sounds good. My memory was a bit foggy from the time I initially
brought this up, and I looked briefly before sending this out and
didn't find the regulator change you had mentioned. If you have a
specific pointer (or patch name or something) let me know, otherwise
I'll dig around later tonight/tomorrow.

> We also have the 'deferred_probe_timeout' cmdline option. It's deemed
> a debug option currently, but we could change that and change the
> default.

I looked at that code, but couldn't really make heads or tails of it.
The initcalls_done is checked and returns before the
deferred_probe_timeout is looked at, so I was guessing the
deferred_probe_timeout was addressing a bit more subtle issue than
what I was going for. If its really the same functionality, I'm happy
to try to rework it.

> > Specifically, on db845c, this change allows us to set
> > SDM_GPUCC_845, QCOM_CLK_RPMH and COMMON_CLK_QCOM as modules and
> > get a working system, where as without it the display will fail
> > to load.
> >
> > Cc: Alexander Graf <agraf@...e.de>
> > Cc: Rob Herring <robh@...nel.org>
> > Cc: "Rafael J. Wysocki" <rjw@...ysocki.net>
> > Cc: Kevin Hilman <khilman@...nel.org>
> > Cc: Ulf Hansson <ulf.hansson@...aro.org>
> > Cc: Pavel Machek <pavel@....cz>
> > Cc: Len Brown <len.brown@...el.com>
> > Cc: Todd Kjos <tkjos@...gle.com>
> > Cc: Bjorn Andersson <bjorn.andersson@...aro.org>
> > Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> > Cc: linux-pm@...r.kernel.org
> > Fixes: e01afc3250255 ("PM / Domains: Stop deferring probe at the end of initcall")
> > Fixes: 25b4e70dcce9 ("driver core: allow stopping deferred probe after init")
>
> We can debate the design, but those work as designed. So Fixes?
>

Well, clk module loading would have worked, and then stopped working
with e01afc3250255, so it is a regression of sorts.  And really the
tags are mostly for making sure patches get applied to trees that
backported these commits (and it's not my intention to shame a patch
as broken. :)

thanks
-john

Powered by blists - more mailing lists