linux-kernel - Re: [PATCH v5 05/12] PM / devfreq: Add support for policy notifiers

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: <20180731193953.GH68975@google.com>
Date:   Tue, 31 Jul 2018 12:39:53 -0700
From:   Matthias Kaehlcke <mka@...omium.org>
To:     Chanwoo Choi <cw00.choi@...sung.com>
Cc:     MyungJoo Ham <myungjoo.ham@...sung.com>,
        Kyungmin Park <kyungmin.park@...sung.com>,
        Arnd Bergmann <arnd@...db.de>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>, linux-pm@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        Brian Norris <briannorris@...omium.org>,
        Douglas Anderson <dianders@...omium.org>,
        Enric Balletbo i Serra <enric.balletbo@...labora.com>,
        "Rafael J . Wysocki" <rjw@...ysocki.net>,
        Viresh Kumar <viresh.kumar@...aro.org>,
        Lee Jones <lee.jones@...aro.org>,
        Benson Leung <bleung@...omium.org>,
        Olof Johansson <olof@...om.net>
Subject: Re: [PATCH v5 05/12] PM / devfreq: Add support for policy notifiers

On Mon, Jul 16, 2018 at 10:50:50AM -0700, Matthias Kaehlcke wrote:
> On Thu, Jul 12, 2018 at 05:44:33PM +0900, Chanwoo Choi wrote:
> > Hi Matthias,
> > 
> > On 2018년 07월 07일 02:53, Matthias Kaehlcke wrote:
> > > Hi Chanwoo,
> > > 
> > > On Wed, Jul 04, 2018 at 03:41:46PM +0900, Chanwoo Choi wrote:
> > > 
> > >> Firstly,
> > >> I'm not sure why devfreq needs the devfreq_verify_within_limits() function.
> > >>
> > >> devfreq already used the OPP interface as default. It means that
> > >> the outside of 'drivers/devfreq' can disable/enable the frequency
> > >> such as drivers/thermal/devfreq_cooling.c. Also, when some device
> > >> drivers disable/enable the specific frequency, the devfreq core
> > >> consider them.
> > >>
> > >> So, devfreq doesn't need to devfreq_verify_within_limits() because
> > >> already support some interface to change the minimum/maximum frequency
> > >> of devfreq device. 
> > >>
> > >> In case of cpufreq subsystem, cpufreq only provides 'cpufreq_verify_with_limits()'
> > >> to change the minimum/maximum frequency of cpu. some device driver cannot
> > >> change the minimum/maximum frequency through OPP interface.
> > >>
> > >> But, in case of devfreq subsystem, as I explained already, devfreq support
> > >> the OPP interface as default way. devfreq subsystem doesn't need to add
> > >> other way to change the minimum/maximum frequency.
> > > 
> > > Using the OPP interface exclusively works as long as a
> > > enabling/disabling of OPPs is limited to a single driver
> > > (drivers/thermal/devfreq_cooling.c). When multiple drivers are
> > > involved you need a way to resolve conflicts, that's the purpose of
> > > devfreq_verify_within_limits(). Please let me know if there are
> > > existing mechanisms for conflict resolution that I overlooked.
> > > 
> > > Possibly drivers/thermal/devfreq_cooling.c could be migrated to use
> > > devfreq_verify_within_limits() instead of the OPP interface if
> > > desired, however this seems beyond the scope of this series.
> > 
> > Actually, if we uses this approach, it doesn't support the multiple drivers too.
> > If non throttler drivers uses devfreq_verify_within_limits(), the conflict
> > happen.
> 
> As long as drivers limit the max freq there is no conflict, the lowest
> max freq wins. I expect this to be the usual case, apparently it
> worked for cpufreq for 10+ years.
> 
> However it is correct that there would be a conflict if a driver
> requests a min freq that is higher than the max freq requested by
> another. In this case devfreq_verify_within_limits() resolves the
> conflict by raising p->max to the min freq. Not sure if this is
> something that would ever occur in practice though.
> 
> If we are really concerned about this case it would also be an option
> to limit the adjustment to the max frequency.
> 
> > To resolve the conflict for multiple device driver, maybe OPP interface
> > have to support 'usage_count' such as clk_enable/disable().
> 
> This would require supporting negative usage count values, since a OPP
> should not be enabled if e.g. thermal enables it but the throttler
> disabled it or viceversa.
> 
> Theoretically there could also be conflicts, like one driver disabling
> the higher OPPs and another the lower ones, with the outcome of all
> OPPs being disabled, which would be a more drastic conflict resolution
> than that of devfreq_verify_within_limits().
> 
> Viresh, what do you think about an OPP usage count?

Ping, can we try to reach a conclusion on this or at least keep the
discussion going?

Not that it matters, but my preferred solution continues to be
devfreq_verify_within_limits(). It solves conflicts in some way (which
could be adjusted if needed) and has proven to work in practice for
10+ years in a very similar sub-system.