Re: [PATCH 0/3] drivers: Frequency constraint infrastructure

[Matthias Kaehlcke <mka@chromium.org>]

On Mon, Jan 21, 2019 at 12:10:55PM +0100, Rafael J. Wysocki wrote:
> On Fri, Jan 18, 2019 at 1:39 PM Juri Lelli <juri.lelli@gmail.com> wrote:
> >
> > On 17/01/19 15:55, Rafael J. Wysocki wrote:
> > > On Thu, Jan 17, 2019 at 2:16 PM Juri Lelli <juri.lelli@gmail.com> wrote:
> > > >
> > > > On 11/01/19 10:47, Rafael J. Wysocki wrote:
> > > > > On Fri, Jan 11, 2019 at 10:18 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > > > > >
> > > > > > Hi,
> > > > > >
> > > > > > This commit introduces the frequency constraint infrastructure, which
> > > > > > provides a generic interface for parts of the kernel to constraint the
> > > > > > working frequency range of a device.
> > > > > >
> > > > > > The primary users of this are the cpufreq and devfreq frameworks. The
> > > > > > cpufreq framework already implements such constraints with help of
> > > > > > notifier chains (for thermal and other constraints) and some local code
> > > > > > (for user-space constraints). The devfreq framework developers have also
> > > > > > shown interest [1] in such a framework, which may use it at a later
> > > > > > point of time.
> > > > > >
> > > > > > The idea here is to provide a generic interface and get rid of the
> > > > > > notifier based mechanism.
> > > > > >
> > > > > > Only one constraint is added for now for the cpufreq framework and the
> > > > > > rest will follow after this stuff is merged.
> > > > > >
> > > > > > Matthias Kaehlcke was involved in the preparation of the first draft of
> > > > > > this work and so I have added him as Co-author to the first patch.
> > > > > > Thanks Matthias.
> > > > > >
> > > > > > FWIW, This doesn't have anything to do with the boot-constraints
> > > > > > framework [2] I was trying to upstream earlier :)
> > > > >
> > > > > This is quite a bit of code to review, so it will take some time.
> > > > >
> > > > > One immediate observation is that it seems to do quite a bit of what
> > > > > is done in the PM QoS framework, so maybe there is an opportunity for
> > > > > some consolidation in there.
> > > >
> > > > Right, had the same impression. :-)
> > > >
> > > > I was also wondering how this new framework is dealing with
> > > > constraints/request imposed/generated by the scheduler and related
> > > > interfaces (thinking about schedutil and Patrick's util_clamp).
> > >
> > > My understanding is that it is orthogonal to them, like adding extra
> > > constraints on top of them etc.
> >
> > Mmm, ok. But, if that is indeed the case, I now wonder why and how
> > existing (or hopefully to be added soon) interfaces are not sufficient.
> > I'm not against this proposal, just trying to understand if this might
> > create unwanted, hard to manage, overlap.
> 
> That is a valid concern IMO.  Especially the utilization clamping and
> the interconnect framework seem to approach the same problem space
> from different directions.
> 
> For cpufreq this work can be regarded as a replacement for notifiers
> which are a bandaid of sorts and it would be good to get rid of them.
> They are mostly used for thermal management and I guess that devfreq
> users also may want to reduce frequency for thermal reasons and I'd
> rather not add notifiers to that framework for this purpose.

FYI: devfreq already reduces frequency for thermal reasons, however
they don't use notifiers, but directly disable OPPs in the cooling driver:

https://elixir.bootlin.com/linux/v4.20.3/source/drivers/thermal/devfreq_cooling.c#L78

The idea to have a frequency constraint framework came up in the
context of the throttler series
(https://lore.kernel.org/patchwork/project/lkml/list/?series=357937)
for non-thermal throttling. My initial approach was to copy the
notifier bandaid ...

> However, as stated previously, this resembles the PM QoS framework
> quite a bit to me and whatever thermal entity, say, sets these
> constraints, it should not work against schedutil and similar.  In
> some situations setting a max frequency limit to control thermals is
> not the most efficient way to go as it effectively turns into
> throttling and makes performance go south.  For example, it may cause
> things to run at the limit frequency all the time which may be too
> slow and it may be more efficient to allow higher frequencies to be
> used, but instead control how much of the time they can be used.  So
> we need to be careful here.