Re: [PATCH 1/2] sched/fair: move cpufreq hook to update_cfs_rq_load_avg()

[Rafael J. Wysocki]

On Wed, Apr 13, 2016 at 2:08 AM, Steve Muckle <steve.muckle@xxxxxxxxxx> wrote:
> On Tue, Apr 12, 2016 at 04:29:06PM +0200, Rafael J. Wysocki wrote:
>> This is rather fundamental.
>>
>> For example, if you look at cpufreq_update_util(), it does this:
>>
>> data = rcu_dereference_sched(*this_cpu_ptr(&cpufreq_update_util_data));
>>
>> meaning that it will run the current CPU's utilization update
>> callback.  Of course, that won't work cross-CPU, because in principle
>> different CPUs may use different governors and therefore different
>> util update callbacks.
>
> Will something like the attached (unfinished patches) work? It seems
> to for me, but I haven't tested it much beyond confirming the hook is
> working on remote wakeups.

No, they are not sufficient.

First of all, you need to take all of the governors into account and
they all make assumptions about updates being run on the CPU being
updated.

That should be easy to take into account for ondemand/conservative,
but intel_pstate is a different story.

> I'm relying on the previous comment that it's up to cpufreq drivers to
> run stuff on the target policy's CPUs if the driver needs that.

That's not the case for the fast frequency switching though, which has
to happen on the CPU running the code.

> There's still some more work, fixing up some more smp_processor_id()
> usage in schedutil, but it should be easy (trace, slow path irq_work
> target).
>
>> If you want to do remote updates, I guess that will require an
>> irq_work to run the update on the target CPU, but then you'll probably
>> want to neglect the rate limit on it as well, so it looks like a
>> "need_update" flag in struct update_util_data will be useful for that.
>
> Why is it required to run the update on the target CPU?

The fast switching and intel_pstate are the main reason.

They both have to write to registers of the target CPU and the code to
do that needs to run on that CPU.

Thanks,
Rafael