Re: [PATCH v3 9/10] cpufreq: sched: Re-introduce cpufreq_update_util()

Rafael J. Wysocki
2016-03-04T21:36:16Z

On Fri, Mar 4, 2016 at 10:27 PM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> On Fri, Mar 4, 2016 at 10:21 PM, Steve Muckle <steve.muckle@linaro.org> wrote:
>> On 03/04/2016 05:30 AM, Rafael J. Wysocki wrote:
>>> +void cpufreq_update_util(u64 time, unsigned long util, unsigned long max)
>>> +{
>>> + struct freq_update_hook *hook;
>>> +
>>> +#ifdef CONFIG_LOCKDEP
>>> + WARN_ON(debug_locks && !rcu_read_lock_sched_held());
>>> +#endif
>>> +
>>> + hook = rcu_dereference_sched(*this_cpu_ptr(&cpufreq_freq_update_hook));
>>> + /*
>>> + * If this isn't inside of an RCU-sched read-side critical section, hook
>>> + * may become NULL after the check below.
>>> + */
>>> + if (hook) {
>>> + if (hook->update_util)
>>> + hook->update_util(hook, time, util, max);
>>> + else
>>> + hook->func(hook, time);
>>> + }
>>
>> Is it worth having two hook types?
>
> Well, that's why I said "maybe over the top" in the changelog comments. :-)
>
> If we want to isolate the "old" governors from util/max entirely, then yes.
>
> If we don't care that much, then no.
>
> I'm open to both possibilities.

But in the latter case I don't see a particular reason to put the new
governor under kernel/sched/ too and as I wrote in the changelog
comments to patch [10/10], I personally think that it would be cleaner
to keep it under drivers/cpufreq/.

Thanks,
Rafael