Re: [PATCH v2 4/6] cpufreq: schedutil: update CFS util only if used

From: Joel Fernandes
Date: Tue Jul 11 2017 - 01:20:30 EST


On Mon, Jul 10, 2017 at 10:49 AM, Vikram Mulukutla
<markivx@xxxxxxxxxxxxxx> 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/