[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJWu+oqaWtGNoOcRGevq5KuDUxGAxkjB3SKf=4NnXOgixutxkw@mail.gmail.com>
Date: Mon, 10 Jul 2017 22:19:35 -0700
From: Joel Fernandes <joelaf@...gle.com>
To: Vikram Mulukutla <markivx@...eaurora.org>
Cc: Patrick Bellasi <patrick.bellasi@....com>,
LKML <linux-kernel@...r.kernel.org>,
Linux PM <linux-pm@...r.kernel.org>,
Ingo Molnar <mingo@...hat.com>,
Peter Zijlstra <peterz@...radead.org>,
"Rafael J . Wysocki" <rafael.j.wysocki@...el.com>,
Viresh Kumar <viresh.kumar@...aro.org>,
Vincent Guittot <vincent.guittot@...aro.org>,
Juri Lelli <juri.lelli@....com>,
Andres Oportus <andresoportus@...gle.com>,
Todd Kjos <tkjos@...roid.com>,
Morten Rasmussen <morten.rasmussen@....com>,
Dietmar Eggemann <dietmar.eggemann@....com>,
linux-kernel-owner@...r.kernel.org
Subject: Re: [PATCH v2 4/6] cpufreq: schedutil: update CFS util only if used
On Mon, Jul 10, 2017 at 10:49 AM, Vikram Mulukutla
<markivx@...eaurora.org> wrote:
[..]
>
>>> Given that the utilization update hooks are called with the per-cpu rq
>>> lock
>>> held (for all classes), I don't think PELT utilization can change
>>> throughout
>>> the lifetime of the cpufreq_update_{util,this_cpu} call? Even with
>>> Viresh's
>>> remote cpu callback series we'd still have to hold the rq lock across
>>> cpufreq_update_util.. what can change today is 'max'
>>> (arch_scale_cpu_capacity) when a cpufreq policy is shared, so the patch
>>> is
>>> still needed for that reason I think?
>>>
>>
>> I didn't follow, Could you elaborate more why you think the patch
>> helps with the case where max changes while the per-cpu rq lock held?
>>
>
> So going by Patrick's commit text, the concern was a TOC/TOU
> problem, but since we agree that CFS utilization can't change
> within an rq-locked critical section, the only thing that can
> change is 'max'. So you might be the 8th cpu in line waiting
> for the sg-policy lock, and after you get the lock, the max may
> be outdated.
>
> But come to think of it max changes should be triggering schedutil
> updates and those shouldn't be rate-throttled, so maybe we don't
> need this at all? It's still somewhat future-proof in case there
> is some stat that we read in sugov_get_util that can be updated
> asynchronously. However we can put it in when we need it...
It looks like Juri's patch [1] to split signals already cleaned it up
so we should be all set ;-)
Thanks,
-Joel
[1] https://patchwork.kernel.org/patch/9826201/
Powered by blists - more mailing lists