Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1347699 > unrolled thread

[PATCH 0/6] cpufreq: schedutil governor

Started by"Rafael J. Wysocki" <rjw@rjwysocki.net>
First post2016-03-02 03:30 +0100
Last post2016-03-04 04:40 +0100
Articles 20 on this page of 40 — 8 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH 0/6] cpufreq: schedutil governor "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-03-02 03:30 +0100
    [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-03-02 03:30 +0100
      Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler  utilization data Vincent Guittot <vincent.guittot@linaro.org> - 2016-03-02 18:20 +0100
        Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler  utilization data "Rafael J. Wysocki" <rafael@kernel.org> - 2016-03-02 19:00 +0100
          Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler  utilization data "Rafael J. Wysocki" <rafael@kernel.org> - 2016-03-02 23:50 +0100
            Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler  utilization data Peter Zijlstra <peterz@infradead.org> - 2016-03-03 13:30 +0100
              Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler  utilization data Juri Lelli <juri.lelli@arm.com> - 2016-03-03 13:40 +0100
              Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler  utilization data "Rafael J. Wysocki" <rafael@kernel.org> - 2016-03-03 17:30 +0100
                Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler  utilization data Peter Zijlstra <peterz@infradead.org> - 2016-03-03 17:40 +0100
                  Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler  utilization data Peter Zijlstra <peterz@infradead.org> - 2016-03-03 17:50 +0100
                    Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler  utilization data "Rafael J. Wysocki" <rafael@kernel.org> - 2016-03-04 02:20 +0100
                  Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler  utilization data Peter Zijlstra <peterz@infradead.org> - 2016-03-03 18:00 +0100
                    Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler  utilization data Juri Lelli <juri.lelli@arm.com> - 2016-03-03 18:20 +0100
                  Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler  utilization data Juri Lelli <juri.lelli@arm.com> - 2016-03-03 18:00 +0100
            Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler  utilization data Vincent Guittot <vincent.guittot@linaro.org> - 2016-03-03 15:10 +0100
              Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler  utilization data Peter Zijlstra <peterz@infradead.org> - 2016-03-03 16:40 +0100
                Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler  utilization data Peter Zijlstra <peterz@infradead.org> - 2016-03-03 17:30 +0100
                  Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler  utilization data Peter Zijlstra <peterz@infradead.org> - 2016-03-03 17:50 +0100
                  Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler  utilization data Dietmar Eggemann <dietmar.eggemann@arm.com> - 2016-03-03 18:30 +0100
                    Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler  utilization data Peter Zijlstra <peterz@infradead.org> - 2016-03-03 19:30 +0100
                      Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler  utilization data Dietmar Eggemann <dietmar.eggemann@arm.com> - 2016-03-03 20:20 +0100
                  Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler  utilization data "Rafael J. Wysocki" <rafael@kernel.org> - 2016-03-03 20:00 +0100
          Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler  utilization data Vincent Guittot <vincent.guittot@linaro.org> - 2016-03-03 14:10 +0100
            Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler  utilization data Steve Muckle <steve.muckle@linaro.org> - 2016-03-03 21:10 +0100
              Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler  utilization data "Rafael J. Wysocki" <rafael@kernel.org> - 2016-03-03 21:30 +0100
                Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler  utilization data Steve Muckle <steve.muckle@linaro.org> - 2016-03-03 22:40 +0100
    [PATCH v2 10/10] cpufreq: schedutil: New governor based on scheduler utilization data "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-03-04 04:40 +0100
    [PATCH v2 6/10] cpufreq: Support for fast frequency switching "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-03-04 04:40 +0100
    [PATCH v2 0/10] cpufreq: schedutil governor "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-03-04 04:40 +0100
      [PATCH v2 1/10] cpufreq: Reduce cpufreq_update_util() overhead a bit "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-03-04 04:40 +0100
      [PATCH v2 5/10] cpufreq: Move governor attribute set headers to cpufreq.h "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-03-04 04:40 +0100
        Re: [PATCH v2 5/10] cpufreq: Move governor attribute set headers to  cpufreq.h Viresh Kumar <viresh.kumar@linaro.org> - 2016-03-04 07:00 +0100
      [PATCH v2 2/10][Resend] cpufreq: acpi-cpufreq: Make read and write operations more efficient "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-03-04 04:40 +0100
      [PATCH v2 3/10] cpufreq: governor: New data type for management part of dbs_data "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-03-04 04:40 +0100
        Re: [PATCH v2 3/10] cpufreq: governor: New data type for management  part of dbs_data Viresh Kumar <viresh.kumar@linaro.org> - 2016-03-04 07:00 +0100
      [PATCH v2 7/10] cpufreq: Rework the scheduler hooks for triggering updates "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-03-04 04:40 +0100
      [PATCH v2 4/10] cpufreq: governor: Move abstract gov_attr_set code to seperate file "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-03-04 04:40 +0100
        Re: [PATCH v2 4/10] cpufreq: governor: Move abstract gov_attr_set  code to seperate file Viresh Kumar <viresh.kumar@linaro.org> - 2016-03-04 07:00 +0100
      [PATCH v2 8/10] cpufreq: Move scheduler-related code to the sched directory "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-03-04 04:40 +0100
      [PATCH v2 9/10] cpufreq: sched: Re-introduce cpufreq_update_util() "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-03-04 04:40 +0100

Page 2 of 2 — ← Prev page 1 [2]


#1349521 — Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data

FromDietmar Eggemann <dietmar.eggemann@arm.com>
Date2016-03-03 20:20 +0100
SubjectRe: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data
Message-ID<r8GX0-3b0-23@gated-at.bofh.it>
In reply to#1349474
On 03/03/16 18:26, Peter Zijlstra wrote:
> On Thu, Mar 03, 2016 at 05:28:55PM +0000, Dietmar Eggemann wrote:
>>> +void arch_scale_freq_tick(void)
>>> +{
>>> +	u64 aperf, mperf;
>>> +	u64 acnt, mcnt;
>>> +
>>> +	if (!static_cpu_has(X86_FEATURE_APERFMPERF))
>>> +		return;
>>> +
>>> +	aperf = rdmsrl(MSR_IA32_APERF);
>>> +	mperf = rdmsrl(MSR_IA32_APERF);
>>> +
>>> +	acnt = aperf - this_cpu_read(arch_prev_aperf);
>>> +	mcnt = mperf - this_cpu_read(arch_prev_mperf);
>>> +
>>> +	this_cpu_write(arch_prev_aperf, aperf);
>>> +	this_cpu_write(arch_prev_mperf, mperf);
>>> +
>>> +	this_cpu_write(arch_cpu_freq, div64_u64(acnt * SCHED_CAPACITY_SCALE, mcnt));
>>
>> Wasn't there the problem that this ratio goes to zero if the cpu is idle
>> in the old power estimation approach on x86?
> 
> Yeah, there was something funky.
> 
> SDM says they only count in C0 (ie. !idle), so it _should_ work.

I see, back than the problem was 0 capacity in idle but this is about
frequency.

[toc] | [prev] | [next] | [standalone]


#1349493 — Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data

From"Rafael J. Wysocki" <rafael@kernel.org>
Date2016-03-03 20:00 +0100
SubjectRe: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data
Message-ID<r8GDE-2P0-29@gated-at.bofh.it>
In reply to#1349340
On Thu, Mar 3, 2016 at 5:28 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Thu, Mar 03, 2016 at 04:38:17PM +0100, Peter Zijlstra wrote:
>> On Thu, Mar 03, 2016 at 03:01:15PM +0100, Vincent Guittot wrote:
>> > > In case a more formal derivation of this formula is needed, it is
>> > > based on the following 3 assumptions:
>> > >
>> > > (1) Performance is a linear function of frequency.
>> > > (2) Required performance is a linear function of the utilization ratio
>> > > x = util/max as provided by the scheduler (0 <= x <= 1).
>> >
>> > Just to mention that the utilization that you are using, varies with
>> > the frequency which add another variable in your equation
>>
>> Right, x86 hasn't implemented arch_scale_freq_capacity(), so the
>> utilization values we use are all over the map. If we lower freq, the
>> util will go up, which would result in us bumping the freq again, etc..
>
> Something like the completely untested below should maybe work.
>
> Rafael?

It looks reasonable (modulo the MPERF reading typo you've noticed),
but can we get back to that later?

I'll first try to address the Ingo's feedback (which I hope I
understood correctly) and some other comments people had and resend
the series.

> ---
>  arch/x86/include/asm/topology.h | 19 +++++++++++++++++++
>  arch/x86/kernel/smpboot.c       | 24 ++++++++++++++++++++++++
>  kernel/sched/core.c             |  1 +
>  kernel/sched/sched.h            |  7 +++++++
>  4 files changed, 51 insertions(+)
>
> diff --git a/arch/x86/include/asm/topology.h b/arch/x86/include/asm/topology.h
> index 7f991bd5031b..af7b7259db94 100644
> --- a/arch/x86/include/asm/topology.h
> +++ b/arch/x86/include/asm/topology.h
> @@ -146,4 +146,23 @@ struct pci_bus;
>  int x86_pci_root_bus_node(int bus);
>  void x86_pci_root_bus_resources(int bus, struct list_head *resources);
>
> +#ifdef CONFIG_SMP
> +
> +#define arch_scale_freq_tick arch_scale_freq_tick
> +#define arch_scale_freq_capacity arch_scale_freq_capacity
> +
> +DECLARE_PER_CPU(unsigned long, arch_cpu_freq);
> +
> +static inline arch_scale_freq_capacity(struct sched_domain *sd, int cpu)
> +{
> +       if (static_cpu_has(X86_FEATURE_APERFMPERF))
> +               return per_cpu(arch_cpu_freq, cpu);
> +       else
> +               return SCHED_CAPACITY_SCALE;
> +}
> +
> +extern void arch_scale_freq_tick(void);
> +
> +#endif
> +
>  #endif /* _ASM_X86_TOPOLOGY_H */
> diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
> index 3bf1e0b5f827..7d459577ee44 100644
> --- a/arch/x86/kernel/smpboot.c
> +++ b/arch/x86/kernel/smpboot.c
> @@ -1647,3 +1647,27 @@ void native_play_dead(void)
>  }
>
>  #endif
> +
> +static DEFINE_PER_CPU(u64, arch_prev_aperf);
> +static DEFINE_PER_CPU(u64, arch_prev_mperf);
> +DEFINE_PER_CPU(unsigned long, arch_cpu_freq);
> +
> +void arch_scale_freq_tick(void)
> +{
> +       u64 aperf, mperf;
> +       u64 acnt, mcnt;
> +
> +       if (!static_cpu_has(X86_FEATURE_APERFMPERF))
> +               return;
> +
> +       aperf = rdmsrl(MSR_IA32_APERF);
> +       mperf = rdmsrl(MSR_IA32_APERF);
> +
> +       acnt = aperf - this_cpu_read(arch_prev_aperf);
> +       mcnt = mperf - this_cpu_read(arch_prev_mperf);
> +
> +       this_cpu_write(arch_prev_aperf, aperf);
> +       this_cpu_write(arch_prev_mperf, mperf);
> +
> +       this_cpu_write(arch_cpu_freq, div64_u64(acnt * SCHED_CAPACITY_SCALE, mcnt));
> +}
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 96e323b26ea9..35dbf909afb2 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -2901,6 +2901,7 @@ void scheduler_tick(void)
>         struct rq *rq = cpu_rq(cpu);
>         struct task_struct *curr = rq->curr;
>
> +       arch_scale_freq_tick();
>         sched_clock_tick();
>
>         raw_spin_lock(&rq->lock);
> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index baa32075f98e..c3825c920e3f 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -1408,6 +1408,13 @@ unsigned long arch_scale_freq_capacity(struct sched_domain *sd, int cpu)
>  }
>  #endif
>
> +#ifndef arch_scale_freq_tick
> +static __always_inline
> +void arch_scale_freq_tick(void)
> +{
> +}
> +#endif
> +
>  #ifndef arch_scale_cpu_capacity
>  static __always_inline
>  unsigned long arch_scale_cpu_capacity(struct sched_domain *sd, int cpu)

[toc] | [prev] | [next] | [standalone]


#1349145 — Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data

FromVincent Guittot <vincent.guittot@linaro.org>
Date2016-03-03 14:10 +0100
SubjectRe: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data
Message-ID<r8BaX-7xs-39@gated-at.bofh.it>
In reply to#1348366
On 2 March 2016 at 18:58, Rafael J. Wysocki <rafael@kernel.org> wrote:
> On Wed, Mar 2, 2016 at 6:10 PM, Vincent Guittot
> <vincent.guittot@linaro.org> wrote:
>> Hi Rafael,
>>
>>
>> On 2 March 2016 at 03:27, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>>
>>> Add a new cpufreq scaling governor, called "schedutil", that uses
>>> scheduler-provided CPU utilization information as input for making
>>> its decisions.
>>>
>>> Doing that is possible after commit fe7034338ba0 (cpufreq: Add
>>> mechanism for registering utilization update callbacks) that
>>> introduced cpufreq_update_util() called by the scheduler on
>>> utilization changes (from CFS) and RT/DL task status updates.
>>> In particular, CPU frequency scaling decisions may be based on
>>> the the utilization data passed to cpufreq_update_util() by CFS.
>>>
>>> The new governor is relatively simple.
>>>
>>> The frequency selection formula used by it is essentially the same
>>> as the one used by the "ondemand" governor, although it doesn't use
>>> the additional up_threshold parameter, but instead of computing the
>>> load as the "non-idle CPU time" to "total CPU time" ratio, it takes
>>> the utilization data provided by CFS as input.  More specifically,
>>> it represents "load" as the util/max ratio, where util and max
>>> are the utilization and CPU capacity coming from CFS.
>>>
>>
>> [snip]
>>
>>> +
>>> +static void sugov_update_commit(struct sugov_policy *sg_policy, u64 time,
>>> +                               unsigned long util, unsigned long max,
>>> +                               unsigned int next_freq)
>>> +{
>>> +       struct cpufreq_policy *policy = sg_policy->policy;
>>> +       unsigned int rel;
>>> +
>>> +       if (next_freq > policy->max)
>>> +               next_freq = policy->max;
>>> +       else if (next_freq < policy->min)
>>> +               next_freq = policy->min;
>>> +
>>> +       sg_policy->last_freq_update_time = time;
>>> +       if (sg_policy->next_freq == next_freq)
>>> +               return;
>>> +
>>> +       sg_policy->next_freq = next_freq;
>>> +       /*
>>> +        * If utilization is less than max / 4, use RELATION_C to allow the
>>> +        * minimum frequency to be selected more often in case the distance from
>>> +        * it to the next available frequency in the table is significant.
>>> +        */
>>> +       rel = util < (max >> 2) ? CPUFREQ_RELATION_C : CPUFREQ_RELATION_L;
>>> +       if (policy->fast_switch_possible) {
>>> +               cpufreq_driver_fast_switch(policy, next_freq, rel);
>>> +       } else {
>>> +               sg_policy->relation = rel;
>>> +               sg_policy->work_in_progress = true;
>>> +               irq_work_queue(&sg_policy->irq_work);
>>> +       }
>>> +}
>>> +
>>> +static void sugov_update_single(struct update_util_data *data, u64 time,
>>> +                               unsigned long util, unsigned long max)
>>> +{
>>> +       struct sugov_cpu *sg_cpu = container_of(data, struct sugov_cpu, update_util);
>>> +       struct sugov_policy *sg_policy = sg_cpu->sg_policy;
>>> +       unsigned int min_f, max_f, next_f;
>>> +
>>> +       if (!sugov_should_update_freq(sg_policy, time))
>>> +               return;
>>> +
>>> +       min_f = sg_policy->policy->cpuinfo.min_freq;
>>> +       max_f = sg_policy->policy->cpuinfo.max_freq;
>>> +       next_f = util > max ? max_f : min_f + util * (max_f - min_f) / max;
>>
>> I think it has been pointed out in another email's thread but you
>> should change the way the next_f is computed. util reflects the
>> utilization of a CPU from 0 to its max compute capacity whereas
>> ondemand was using the load at the current frequency during the last
>> time window. I have understood that you want to keep same formula than
>> ondemand as a starting point but you use a different input to
>> calculate the next frequency so i don't see the rational of keeping
>> this formula.
>
> It is a formula that causes the entire available frequency range to be
> utilized proportionally to the utilization as reported by the
> scheduler (modulo the policy->min/max limits).  Its (significant IMO)
> advantage is that it doesn't require any additional factors that would
> need to be determined somehow.
>
>> Saying that, even the simple formula next_f = util > max
>> ? max_f : util * (max_f) / max will not work properly if the frequency
>> invariance is enable because the utilization becomes capped by the
>> current compute capacity so next_f will never be higher than current
>> freq (unless a task move on the rq).  That was one reason of using a
>> threshold in sched-freq proposal (and there are on going dev to try to
>> solve this limitation).
>
> Well, a different formula will have to be used along with frequency
> invariance, then.
>
>> IIIUC, frequency invariance is not enable on your platform so you have
>> not seen the problem but you have probably see that selection of your
>> next_f was not really stable. Without frequency invariance, the
>> utilization will be overestimated when running at lower frequency so
>> the governor will probably select a frequency that is higher than
>> necessary but then the utilization will decrease at this higher
>> frequency so the governor will probably decrease the frequency and so
>> on until you found the right frequency that will generate the right
>> utilisation value
>
> I don't have any problems with that to be honest and if you aim at
> selecting the perfect frequency at the first attempt, then good luck
> with that anyway.

I mainly want to prevent any useless and periodic frequency switch
because of an utilization that changes with the current frequency (if
frequency invariance is not used) and that can make the formula
selects another frequency than the current one. That what i can see
when testing it .

Sorry for the late reply, i was trying to do some test on my board but
was facing some crash issue (not link with your patchset). So i have
done some tests and i can see such instable behavior. I have generated
a load of 33% at max frequency (3ms runs every 9ms) and i can see the
frequency that toggles without any good reason. Saying that, i can see
similar thing with ondemand.

Vincent

>
> Now, I'm not saying that the formula used in this patch cannot be
> improved or similar.  It very well may be possible to improve it.  I'm
> only saying that it is good enough to start with, because of the
> reasons mentioned above.
>
> Still, if you can suggest to me what other formula specifically should
> be used here, I'll consider using it.  Which will probably mean
> comparing the two and seeing which one leads to better results.
>
> Thanks,
> Rafael

[toc] | [prev] | [next] | [standalone]


#1349550 — Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data

FromSteve Muckle <steve.muckle@linaro.org>
Date2016-03-03 21:10 +0100
SubjectRe: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data
Message-ID<r8HJp-3Jy-41@gated-at.bofh.it>
In reply to#1349145
On 03/03/2016 05:07 AM, Vincent Guittot wrote:
> I mainly want to prevent any useless and periodic frequency switch
> because of an utilization that changes with the current frequency (if
> frequency invariance is not used) and that can make the formula
> selects another frequency than the current one. That what i can see
> when testing it .
> 
> Sorry for the late reply, i was trying to do some test on my board but
> was facing some crash issue (not link with your patchset). So i have
> done some tests and i can see such instable behavior. I have generated
> a load of 33% at max frequency (3ms runs every 9ms) and i can see the
> frequency that toggles without any good reason. Saying that, i can see
> similar thing with ondemand.

FWIW I ran some performance numbers on my chromebook 2. Initially I
forgot to bring in the frequency invariance support but that yielded an
opportunity to see the impact of it.

The tests below consist of a periodic workload. The OH (overhead)
numbers show how close the workload got to running as slow as fmin (100%
= as slow as powersave gov, 0% = as fast as perf gov). The OR (overrun)
number is the count of instances where the busy work exceeded the period.

First a comparison of schedutil with and without frequency invariance.
Run and period are in milliseconds.

			scu (no inv)	scu (w/inv)	
run	period	busy %	OR	OH	OR	OH
1	100	1.00%	0	79.72%	0	95.86%
10	1000	1.00%	0	24.52%	0	71.61%
1	10	10.00%	0	21.25%	0	41.78%
10	100	10.00%	0	26.06%	0	47.96%
100	1000	10.00%	0	6.36%	0	26.03%
6	33	18.18%	0	15.67%	0	31.61%
66	333	19.82%	0	8.94%	0	29.46%
4	10	40.00%	0	6.26%	0	12.93%
40	100	40.00%	0	6.93%	2	14.08%
400	1000	40.00%	0	1.65%	0	11.58%
5	9	55.56%	0	3.70%	0	7.70%
50	90	55.56%	1	4.19%	6	8.06%
500	900	55.56%	0	1.35%	5	6.94%
9	12	75.00%	0	1.60%	56	3.59%
90	120	75.00%	0	1.88%	21	3.94%
900	1200	75.00%	0	0.73%	4	4.41%

Frequency invariance causes schedutil overhead to increase noticeably. I
haven't dug into traces or anything. Perhaps this is due to the
algorithm overshooting then overcorrecting etc., I do not yet know.

Here is a comparison, with frequency invariance, of ondemand and
interactive with schedfreq and schedutil. The first two columns (run and
period) are omitted so the table will fit.

	ondemand	interactive	schedfreq	schedutil	
busy %	OR	OH	OR	OH	OR	OH	OR	OH
1.00%	0	68.96%	0	100.04%	0	78.49%	0	95.86%
1.00%	0	25.04%	0	22.59%	0	72.56%	0	71.61%
10.00%	0	21.75%	0	63.08%	0	52.40%	0	41.78%
10.00%	0	12.17%	0	14.41%	0	17.33%	0	47.96%
10.00%	0	2.57%	0	2.17%	0	0.29%	0	26.03%
18.18%	0	12.39%	0	9.39%	0	17.34%	0	31.61%
19.82%	0	3.74%	0	3.42%	0	12.26%	0	29.46%
40.00%	2	6.26%	1	12.23%	0	6.15%	0	12.93%
40.00%	0	0.47%	0	0.05%	0	2.68%	2	14.08%
40.00%	0	0.60%	0	0.50%	0	1.22%	0	11.58%
55.56%	2	4.25%	5	5.97%	0	2.51%	0	7.70%
55.56%	0	1.89%	0	0.04%	0	1.71%	6	8.06%
55.56%	0	0.50%	0	0.47%	0	1.82%	5	6.94%
75.00%	2	1.65%	1	0.46%	0	0.26%	56	3.59%
75.00%	0	1.68%	0	0.05%	0	0.49%	21	3.94%
75.00%	0	0.28%	0	0.23%	0	0.62%	4	4.41%

Aside from the 2nd and 3rd tests schedutil is showing decreased
performance across the board. The fifth test is particularly bad.

The catch is that I do not have power numbers to go with this data, as
I'm not currently equipped to gather them. So more analysis is
definitely needed to capture the full story.

thanks,
Steve

[toc] | [prev] | [next] | [standalone]


#1349554 — Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data

From"Rafael J. Wysocki" <rafael@kernel.org>
Date2016-03-03 21:30 +0100
SubjectRe: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data
Message-ID<r8I2K-3U3-3@gated-at.bofh.it>
In reply to#1349550
On Thu, Mar 3, 2016 at 9:06 PM, Steve Muckle <steve.muckle@linaro.org> wrote:
> On 03/03/2016 05:07 AM, Vincent Guittot wrote:
>> I mainly want to prevent any useless and periodic frequency switch
>> because of an utilization that changes with the current frequency (if
>> frequency invariance is not used) and that can make the formula
>> selects another frequency than the current one. That what i can see
>> when testing it .
>>
>> Sorry for the late reply, i was trying to do some test on my board but
>> was facing some crash issue (not link with your patchset). So i have
>> done some tests and i can see such instable behavior. I have generated
>> a load of 33% at max frequency (3ms runs every 9ms) and i can see the
>> frequency that toggles without any good reason. Saying that, i can see
>> similar thing with ondemand.
>
> FWIW I ran some performance numbers on my chromebook 2. Initially I
> forgot to bring in the frequency invariance support but that yielded an
> opportunity to see the impact of it.
>
> The tests below consist of a periodic workload. The OH (overhead)
> numbers show how close the workload got to running as slow as fmin (100%
> = as slow as powersave gov, 0% = as fast as perf gov). The OR (overrun)
> number is the count of instances where the busy work exceeded the period.
>
> First a comparison of schedutil with and without frequency invariance.
> Run and period are in milliseconds.
>
>                         scu (no inv)    scu (w/inv)
> run     period  busy %  OR      OH      OR      OH
> 1       100     1.00%   0       79.72%  0       95.86%
> 10      1000    1.00%   0       24.52%  0       71.61%
> 1       10      10.00%  0       21.25%  0       41.78%
> 10      100     10.00%  0       26.06%  0       47.96%
> 100     1000    10.00%  0       6.36%   0       26.03%
> 6       33      18.18%  0       15.67%  0       31.61%
> 66      333     19.82%  0       8.94%   0       29.46%
> 4       10      40.00%  0       6.26%   0       12.93%
> 40      100     40.00%  0       6.93%   2       14.08%
> 400     1000    40.00%  0       1.65%   0       11.58%
> 5       9       55.56%  0       3.70%   0       7.70%
> 50      90      55.56%  1       4.19%   6       8.06%
> 500     900     55.56%  0       1.35%   5       6.94%
> 9       12      75.00%  0       1.60%   56      3.59%
> 90      120     75.00%  0       1.88%   21      3.94%
> 900     1200    75.00%  0       0.73%   4       4.41%
>
> Frequency invariance causes schedutil overhead to increase noticeably. I
> haven't dug into traces or anything. Perhaps this is due to the
> algorithm overshooting then overcorrecting etc., I do not yet know.

So as I said, the formula I used didn't take invariance into account,
so that's quite as expected.

> Here is a comparison, with frequency invariance, of ondemand and
> interactive with schedfreq and schedutil. The first two columns (run and
> period) are omitted so the table will fit.
>
>         ondemand        interactive     schedfreq       schedutil
> busy %  OR      OH      OR      OH      OR      OH      OR      OH
> 1.00%   0       68.96%  0       100.04% 0       78.49%  0       95.86%
> 1.00%   0       25.04%  0       22.59%  0       72.56%  0       71.61%
> 10.00%  0       21.75%  0       63.08%  0       52.40%  0       41.78%
> 10.00%  0       12.17%  0       14.41%  0       17.33%  0       47.96%
> 10.00%  0       2.57%   0       2.17%   0       0.29%   0       26.03%
> 18.18%  0       12.39%  0       9.39%   0       17.34%  0       31.61%
> 19.82%  0       3.74%   0       3.42%   0       12.26%  0       29.46%
> 40.00%  2       6.26%   1       12.23%  0       6.15%   0       12.93%
> 40.00%  0       0.47%   0       0.05%   0       2.68%   2       14.08%
> 40.00%  0       0.60%   0       0.50%   0       1.22%   0       11.58%
> 55.56%  2       4.25%   5       5.97%   0       2.51%   0       7.70%
> 55.56%  0       1.89%   0       0.04%   0       1.71%   6       8.06%
> 55.56%  0       0.50%   0       0.47%   0       1.82%   5       6.94%
> 75.00%  2       1.65%   1       0.46%   0       0.26%   56      3.59%
> 75.00%  0       1.68%   0       0.05%   0       0.49%   21      3.94%
> 75.00%  0       0.28%   0       0.23%   0       0.62%   4       4.41%
>
> Aside from the 2nd and 3rd tests schedutil is showing decreased
> performance across the board. The fifth test is particularly bad.

I guess you mean performance in terms of the overhead?

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]


#1349599 — Re: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data

FromSteve Muckle <steve.muckle@linaro.org>
Date2016-03-03 22:40 +0100
SubjectRe: [PATCH 6/6] cpufreq: schedutil: New governor based on scheduler utilization data
Message-ID<r8J8u-4LK-11@gated-at.bofh.it>
In reply to#1349554
On 03/03/2016 12:20 PM, Rafael J. Wysocki wrote:
>> Here is a comparison, with frequency invariance, of ondemand and
>> interactive with schedfreq and schedutil. The first two columns (run and
>> period) are omitted so the table will fit.
>>
>>         ondemand        interactive     schedfreq       schedutil
>> busy %  OR      OH      OR      OH      OR      OH      OR      OH
>> 1.00%   0       68.96%  0       100.04% 0       78.49%  0       95.86%
>> 1.00%   0       25.04%  0       22.59%  0       72.56%  0       71.61%
>> 10.00%  0       21.75%  0       63.08%  0       52.40%  0       41.78%
>> 10.00%  0       12.17%  0       14.41%  0       17.33%  0       47.96%
>> 10.00%  0       2.57%   0       2.17%   0       0.29%   0       26.03%
>> 18.18%  0       12.39%  0       9.39%   0       17.34%  0       31.61%
>> 19.82%  0       3.74%   0       3.42%   0       12.26%  0       29.46%
>> 40.00%  2       6.26%   1       12.23%  0       6.15%   0       12.93%
>> 40.00%  0       0.47%   0       0.05%   0       2.68%   2       14.08%
>> 40.00%  0       0.60%   0       0.50%   0       1.22%   0       11.58%
>> 55.56%  2       4.25%   5       5.97%   0       2.51%   0       7.70%
>> 55.56%  0       1.89%   0       0.04%   0       1.71%   6       8.06%
>> 55.56%  0       0.50%   0       0.47%   0       1.82%   5       6.94%
>> 75.00%  2       1.65%   1       0.46%   0       0.26%   56      3.59%
>> 75.00%  0       1.68%   0       0.05%   0       0.49%   21      3.94%
>> 75.00%  0       0.28%   0       0.23%   0       0.62%   4       4.41%
>>
>> Aside from the 2nd and 3rd tests schedutil is showing decreased
>> performance across the board. The fifth test is particularly bad.
> 
> I guess you mean performance in terms of the overhead?

Correct. This overhead metric describes how fast the workload completes,
with 0% equaling the perf governor and 100% equaling the powersave
governor. So it's a reflection of general performance using the
governor. It's called "overhead" I imagine (the metric predates my
involvement) as it is something introduced/caused by the policy of the
governor.

thanks,
Steve

[toc] | [prev] | [next] | [standalone]


#1349811 — [PATCH v2 10/10] cpufreq: schedutil: New governor based on scheduler utilization data

From"Rafael J. Wysocki" <rjw@rjwysocki.net>
Date2016-03-04 04:40 +0100
Subject[PATCH v2 10/10] cpufreq: schedutil: New governor based on scheduler utilization data
Message-ID<r8OKR-B1-1@gated-at.bofh.it>
In reply to#1347699
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Add a new cpufreq scaling governor, called "schedutil", that uses
scheduler-provided CPU utilization information as input for making
its decisions.

Doing that is possible after commit fe7034338ba0 (cpufreq: Add
mechanism for registering utilization update callbacks) that
introduced cpufreq_update_util() called by the scheduler on
utilization changes (from CFS) and RT/DL task status updates.
In particular, CPU frequency scaling decisions may be based on
the the utilization data passed to cpufreq_update_util() by CFS.

The new governor is relatively simple.

The frequency selection formula used by it is

	next_freq = util * max_freq / max

where util and max are the utilization and CPU capacity coming from CFS.

All of the computations are carried out in the utilization update
handlers provided by the new governor.  One of those handlers is
used for cpufreq policies shared between multiple CPUs and the other
one is for policies with one CPU only (and therefore it doesn't need
to use any extra synchronization means).

The governor supports fast frequency switching if that is supported
by the cpufreq driver in use and possible for the given policy.
In the fast switching case, all operations of the governor take
place in its utilization update handlers.  If fast switching cannot
be used, the frequency switch operations are carried out with the
help of a work item which only calls __cpufreq_driver_target()
(under a mutex) to trigger a frequency update (to a value already
computed beforehand in one of the utilization update handlers).

Currently, the governor treats all of the RT and DL tasks as
"unknown utilization" and sets the frequency to the allowed
maximum when updated from the RT or DL sched classes.  That
heavy-handed approach should be replaced with something more
subtle and specifically targeted at RT and DL tasks.

The governor shares some sysfs attributes management code with
the "ondemand" and "conservative" governors and uses some common
definitions from cpufreq.h, but apart from that it is stand-alone.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

Changes from the previous version:
- New frequency selection formula and modifications related to that.
- The file is now located in kernel/sched/.

Initially, I had hoped that it would be possible to split the code
into a library part that might go into kernel/sched/ and the governor
interface plus sysfs-related code, but that split would have been
artificial and I wanted the governor to be one module as a whole.  So
that didn't work out.

Also the way it is configured and built is somewhat bizarre, as the
Kconfig options are in the cpufreq Kconfig, but the code they are
related to is located in kernel/sched/ (which is not exactly
straightforward).

Overall, I'd be happier if the governor could stay in drivers/cpufreq/.

---
 drivers/cpufreq/Kconfig            |   26 +
 drivers/cpufreq/cpufreq_governor.h |    1 
 include/linux/cpufreq.h            |    3 
 kernel/sched/Makefile              |    1 
 kernel/sched/cpufreq_schedutil.c   |  487 +++++++++++++++++++++++++++++++++++++
 5 files changed, 517 insertions(+), 1 deletion(-)

Index: linux-pm/drivers/cpufreq/Kconfig
===================================================================
--- linux-pm.orig/drivers/cpufreq/Kconfig
+++ linux-pm/drivers/cpufreq/Kconfig
@@ -107,6 +107,16 @@ config CPU_FREQ_DEFAULT_GOV_CONSERVATIVE
 	  Be aware that not all cpufreq drivers support the conservative
 	  governor. If unsure have a look at the help section of the
 	  driver. Fallback governor will be the performance governor.
+
+config CPU_FREQ_DEFAULT_GOV_SCHEDUTIL
+	bool "schedutil"
+	select CPU_FREQ_GOV_SCHEDUTIL
+	select CPU_FREQ_GOV_PERFORMANCE
+	help
+	  Use the 'schedutil' CPUFreq governor by default. If unsure,
+	  have a look at the help section of that governor. The fallback
+	  governor will be 'performance'.
+
 endchoice
 
 config CPU_FREQ_GOV_PERFORMANCE
@@ -188,6 +198,22 @@ config CPU_FREQ_GOV_CONSERVATIVE
 
 	  If in doubt, say N.
 
+config CPU_FREQ_GOV_SCHEDUTIL
+	tristate "'schedutil' cpufreq policy governor"
+	depends on CPU_FREQ
+	select CPU_FREQ_GOV_ATTR_SET
+	select IRQ_WORK
+	help
+	  The frequency selection formula used by this governor is analogous
+	  to the one used by 'ondemand', but instead of computing CPU load
+	  as the "non-idle CPU time" to "total CPU time" ratio, it uses CPU
+	  utilization data provided by the scheduler as input.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called cpufreq_schedutil.
+
+	  If in doubt, say N.
+
 comment "CPU frequency scaling drivers"
 
 config CPUFREQ_DT
Index: linux-pm/kernel/sched/cpufreq_schedutil.c
===================================================================
--- /dev/null
+++ linux-pm/kernel/sched/cpufreq_schedutil.c
@@ -0,0 +1,487 @@
+/*
+ * CPUFreq governor based on scheduler-provided CPU utilization data.
+ *
+ * Copyright (C) 2016, Intel Corporation
+ * Author: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+
+#include "sched.h"
+
+struct sugov_tunables {
+	struct gov_attr_set attr_set;
+	unsigned int rate_limit_us;
+};
+
+struct sugov_policy {
+	struct cpufreq_policy *policy;
+
+	struct sugov_tunables *tunables;
+	struct list_head tunables_hook;
+
+	raw_spinlock_t update_lock;  /* For shared policies */
+	u64 last_freq_update_time;
+	s64 freq_update_delay_ns;
+	unsigned int next_freq;
+
+	/* The next fields are only needed if fast switch cannot be used. */
+	struct irq_work irq_work;
+	struct work_struct work;
+	struct mutex work_lock;
+	bool work_in_progress;
+
+	bool need_freq_update;
+};
+
+struct sugov_cpu {
+	struct freq_update_hook update_hook;
+	struct sugov_policy *sg_policy;
+
+	/* The fields below are only needed when sharing a policy. */
+	unsigned long util;
+	unsigned long max;
+	u64 last_update;
+};
+
+static DEFINE_PER_CPU(struct sugov_cpu, sugov_cpu);
+
+/************************ Governor internals ***********************/
+
+static bool sugov_should_update_freq(struct sugov_policy *sg_policy, u64 time)
+{
+	u64 delta_ns;
+
+	if (sg_policy->work_in_progress)
+		return false;
+
+	if (unlikely(sg_policy->need_freq_update)) {
+		sg_policy->need_freq_update = false;
+		return true;
+	}
+
+	delta_ns = time - sg_policy->last_freq_update_time;
+	return (s64)delta_ns >= sg_policy->freq_update_delay_ns;
+}
+
+static void sugov_update_commit(struct sugov_policy *sg_policy, u64 time,
+				unsigned int next_freq)
+{
+	struct cpufreq_policy *policy = sg_policy->policy;
+
+	if (next_freq > policy->max)
+		next_freq = policy->max;
+	else if (next_freq < policy->min)
+		next_freq = policy->min;
+
+	sg_policy->last_freq_update_time = time;
+	if (sg_policy->next_freq == next_freq)
+		return;
+
+	sg_policy->next_freq = next_freq;
+	if (policy->fast_switch_possible) {
+		cpufreq_driver_fast_switch(policy, next_freq, CPUFREQ_RELATION_L);
+	} else {
+		sg_policy->work_in_progress = true;
+		irq_work_queue(&sg_policy->irq_work);
+	}
+}
+
+static void sugov_update_single(struct freq_update_hook *hook, u64 time,
+				unsigned long util, unsigned long max)
+{
+	struct sugov_cpu *sg_cpu = container_of(hook, struct sugov_cpu, update_hook);
+	struct sugov_policy *sg_policy = sg_cpu->sg_policy;
+	unsigned int max_f, next_f;
+
+	if (!sugov_should_update_freq(sg_policy, time))
+		return;
+
+	max_f = sg_policy->policy->cpuinfo.max_freq;
+	next_f = util > max ? max_f : util * max_f / max;
+	sugov_update_commit(sg_policy, time, next_f);
+}
+
+static unsigned int sugov_next_freq(struct sugov_policy *sg_policy,
+				    unsigned long util, unsigned long max)
+{
+	struct cpufreq_policy *policy = sg_policy->policy;
+	unsigned int max_f = policy->cpuinfo.max_freq;
+	u64 last_freq_update_time = sg_policy->last_freq_update_time;
+	unsigned int j;
+
+	if (util > max)
+		return max_f;
+
+	for_each_cpu(j, policy->cpus) {
+		struct sugov_cpu *j_sg_cpu;
+		unsigned long j_util, j_max;
+		u64 delta_ns;
+
+		if (j == smp_processor_id())
+			continue;
+
+		j_sg_cpu = &per_cpu(sugov_cpu, j);
+		/*
+		 * If the CPU utilization was last updated before the previous
+		 * frequency update and the time elapsed between the last update
+		 * of the CPU utilization and the last frequency update is long
+		 * enough, don't take the CPU into account as it probably is
+		 * idle now.
+		 */
+		delta_ns = last_freq_update_time - j_sg_cpu->last_update;
+		if ((s64)delta_ns > NSEC_PER_SEC / HZ)
+			continue;
+
+		j_util = j_sg_cpu->util;
+		j_max = j_sg_cpu->max;
+		if (j_util > j_max)
+			return max_f;
+
+		if (j_util * max > j_max * util) {
+			util = j_util;
+			max = j_max;
+		}
+	}
+
+	return  util * max_f / max;
+}
+
+static void sugov_update_shared(struct freq_update_hook *hook, u64 time,
+				unsigned long util, unsigned long max)
+{
+	struct sugov_cpu *sg_cpu = container_of(hook, struct sugov_cpu, update_hook);
+	struct sugov_policy *sg_policy = sg_cpu->sg_policy;
+	unsigned int next_f;
+
+	raw_spin_lock(&sg_policy->update_lock);
+
+	sg_cpu->util = util;
+	sg_cpu->max = max;
+	sg_cpu->last_update = time;
+
+	if (sugov_should_update_freq(sg_policy, time)) {
+		next_f = sugov_next_freq(sg_policy, util, max);
+		sugov_update_commit(sg_policy, time, next_f);
+	}
+
+	raw_spin_unlock(&sg_policy->update_lock);
+}
+
+static void sugov_work(struct work_struct *work)
+{
+	struct sugov_policy *sg_policy = container_of(work, struct sugov_policy, work);
+
+	mutex_lock(&sg_policy->work_lock);
+	__cpufreq_driver_target(sg_policy->policy, sg_policy->next_freq,
+				CPUFREQ_RELATION_L);
+	mutex_unlock(&sg_policy->work_lock);
+
+	sg_policy->work_in_progress = false;
+}
+
+static void sugov_irq_work(struct irq_work *irq_work)
+{
+	struct sugov_policy *sg_policy;
+
+	sg_policy = container_of(irq_work, struct sugov_policy, irq_work);
+	schedule_work(&sg_policy->work);
+}
+
+/************************** sysfs interface ************************/
+
+static struct sugov_tunables *global_tunables;
+static DEFINE_MUTEX(global_tunables_lock);
+
+static inline struct sugov_tunables *to_sugov_tunables(struct gov_attr_set *attr_set)
+{
+	return container_of(attr_set, struct sugov_tunables, attr_set);
+}
+
+static ssize_t rate_limit_us_show(struct gov_attr_set *attr_set, char *buf)
+{
+	struct sugov_tunables *tunables = to_sugov_tunables(attr_set);
+
+	return sprintf(buf, "%u\n", tunables->rate_limit_us);
+}
+
+static ssize_t rate_limit_us_store(struct gov_attr_set *attr_set, const char *buf,
+				   size_t count)
+{
+	struct sugov_tunables *tunables = to_sugov_tunables(attr_set);
+	struct sugov_policy *sg_policy;
+	unsigned int rate_limit_us;
+	int ret;
+
+	ret = sscanf(buf, "%u", &rate_limit_us);
+	if (ret != 1)
+		return -EINVAL;
+
+	tunables->rate_limit_us = rate_limit_us;
+
+	list_for_each_entry(sg_policy, &attr_set->policy_list, tunables_hook)
+		sg_policy->freq_update_delay_ns = rate_limit_us * NSEC_PER_USEC;
+
+	return count;
+}
+
+static struct governor_attr rate_limit_us = __ATTR_RW(rate_limit_us);
+
+static struct attribute *sugov_attributes[] = {
+	&rate_limit_us.attr,
+	NULL
+};
+
+static struct kobj_type sugov_tunables_ktype = {
+	.default_attrs = sugov_attributes,
+	.sysfs_ops = &governor_sysfs_ops,
+};
+
+/********************** cpufreq governor interface *********************/
+
+static struct cpufreq_governor schedutil_gov;
+
+static struct sugov_policy *sugov_policy_alloc(struct cpufreq_policy *policy)
+{
+	struct sugov_policy *sg_policy;
+
+	sg_policy = kzalloc(sizeof(*sg_policy), GFP_KERNEL);
+	if (!sg_policy)
+		return NULL;
+
+	sg_policy->policy = policy;
+	init_irq_work(&sg_policy->irq_work, sugov_irq_work);
+	INIT_WORK(&sg_policy->work, sugov_work);
+	mutex_init(&sg_policy->work_lock);
+	raw_spin_lock_init(&sg_policy->update_lock);
+	return sg_policy;
+}
+
+static void sugov_policy_free(struct sugov_policy *sg_policy)
+{
+	mutex_destroy(&sg_policy->work_lock);
+	kfree(sg_policy);
+}
+
+static struct sugov_tunables *sugov_tunables_alloc(struct sugov_policy *sg_policy)
+{
+	struct sugov_tunables *tunables;
+
+	tunables = kzalloc(sizeof(*tunables), GFP_KERNEL);
+	if (tunables)
+		gov_attr_set_init(&tunables->attr_set, &sg_policy->tunables_hook);
+
+	return tunables;
+}
+
+static void sugov_tunables_free(struct sugov_tunables *tunables)
+{
+	if (!have_governor_per_policy())
+		global_tunables = NULL;
+
+	kfree(tunables);
+}
+
+static int sugov_init(struct cpufreq_policy *policy)
+{
+	struct sugov_policy *sg_policy;
+	struct sugov_tunables *tunables;
+	unsigned int lat;
+	int ret = 0;
+
+	/* State should be equivalent to EXIT */
+	if (policy->governor_data)
+		return -EBUSY;
+
+	sg_policy = sugov_policy_alloc(policy);
+	if (!sg_policy)
+		return -ENOMEM;
+
+	mutex_lock(&global_tunables_lock);
+
+	if (global_tunables) {
+		if (WARN_ON(have_governor_per_policy())) {
+			ret = -EINVAL;
+			goto free_sg_policy;
+		}
+		policy->governor_data = sg_policy;
+		sg_policy->tunables = global_tunables;
+
+		gov_attr_set_get(&global_tunables->attr_set, &sg_policy->tunables_hook);
+		goto out;
+	}
+
+	tunables = sugov_tunables_alloc(sg_policy);
+	if (!tunables) {
+		ret = -ENOMEM;
+		goto free_sg_policy;
+	}
+
+	tunables->rate_limit_us = LATENCY_MULTIPLIER;
+	lat = policy->cpuinfo.transition_latency / NSEC_PER_USEC;
+	if (lat)
+		tunables->rate_limit_us *= lat;
+
+	if (!have_governor_per_policy())
+		global_tunables = tunables;
+
+	policy->governor_data = sg_policy;
+	sg_policy->tunables = tunables;
+
+	ret = kobject_init_and_add(&tunables->attr_set.kobj, &sugov_tunables_ktype,
+				   get_governor_parent_kobj(policy), "%s",
+				   schedutil_gov.name);
+	if (!ret)
+		goto out;
+
+	/* Failure, so roll back. */
+	policy->governor_data = NULL;
+	sugov_tunables_free(tunables);
+
+ free_sg_policy:
+	pr_err("cpufreq: schedutil governor initialization failed (error %d)\n", ret);
+	sugov_policy_free(sg_policy);
+
+ out:
+	mutex_unlock(&global_tunables_lock);
+	return ret;
+}
+
+static int sugov_exit(struct cpufreq_policy *policy)
+{
+	struct sugov_policy *sg_policy = policy->governor_data;
+	struct sugov_tunables *tunables = sg_policy->tunables;
+	unsigned int count;
+
+	mutex_lock(&global_tunables_lock);
+
+	count = gov_attr_set_put(&tunables->attr_set, &sg_policy->tunables_hook);
+	policy->governor_data = NULL;
+	if (!count)
+		sugov_tunables_free(tunables);
+
+	mutex_unlock(&global_tunables_lock);
+
+	sugov_policy_free(sg_policy);
+	return 0;
+}
+
+static int sugov_start(struct cpufreq_policy *policy)
+{
+	struct sugov_policy *sg_policy = policy->governor_data;
+	unsigned int cpu;
+
+	sg_policy->freq_update_delay_ns = sg_policy->tunables->rate_limit_us * NSEC_PER_USEC;
+	sg_policy->last_freq_update_time = 0;
+	sg_policy->next_freq = UINT_MAX;
+	sg_policy->work_in_progress = false;
+	sg_policy->need_freq_update = false;
+
+	for_each_cpu(cpu, policy->cpus) {
+		struct sugov_cpu *sg_cpu = &per_cpu(sugov_cpu, cpu);
+
+		sg_cpu->sg_policy = sg_policy;
+		if (policy_is_shared(policy)) {
+			sg_cpu->util = ULONG_MAX;
+			sg_cpu->max = 0;
+			sg_cpu->last_update = 0;
+			cpufreq_set_update_util_hook(cpu, &sg_cpu->update_hook,
+						     sugov_update_shared);
+		} else {
+			cpufreq_set_update_util_hook(cpu, &sg_cpu->update_hook,
+						     sugov_update_single);
+		}
+	}
+	return 0;
+}
+
+static int sugov_stop(struct cpufreq_policy *policy)
+{
+	struct sugov_policy *sg_policy = policy->governor_data;
+	unsigned int cpu;
+
+	for_each_cpu(cpu, policy->cpus)
+		cpufreq_clear_update_util_hook(cpu);
+
+	synchronize_sched();
+
+	irq_work_sync(&sg_policy->irq_work);
+	cancel_work_sync(&sg_policy->work);
+	return 0;
+}
+
+static int sugov_limits(struct cpufreq_policy *policy)
+{
+	struct sugov_policy *sg_policy = policy->governor_data;
+
+	if (!policy->fast_switch_possible) {
+		mutex_lock(&sg_policy->work_lock);
+
+		if (policy->max < policy->cur)
+			__cpufreq_driver_target(policy, policy->max,
+						CPUFREQ_RELATION_H);
+		else if (policy->min > policy->cur)
+			__cpufreq_driver_target(policy, policy->min,
+						CPUFREQ_RELATION_L);
+
+		mutex_unlock(&sg_policy->work_lock);
+	}
+
+	sg_policy->need_freq_update = true;
+	return 0;
+}
+
+int sugov_governor(struct cpufreq_policy *policy, unsigned int event)
+{
+	if (event == CPUFREQ_GOV_POLICY_INIT) {
+		return sugov_init(policy);
+	} else if (policy->governor_data) {
+		switch (event) {
+		case CPUFREQ_GOV_POLICY_EXIT:
+			return sugov_exit(policy);
+		case CPUFREQ_GOV_START:
+			return sugov_start(policy);
+		case CPUFREQ_GOV_STOP:
+			return sugov_stop(policy);
+		case CPUFREQ_GOV_LIMITS:
+			return sugov_limits(policy);
+		}
+	}
+	return -EINVAL;
+}
+
+static struct cpufreq_governor schedutil_gov = {
+	.name = "schedutil",
+	.governor = sugov_governor,
+	.owner = THIS_MODULE,
+};
+
+static int __init sugov_module_init(void)
+{
+	return cpufreq_register_governor(&schedutil_gov);
+}
+
+static void __exit sugov_module_exit(void)
+{
+	cpufreq_unregister_governor(&schedutil_gov);
+}
+
+MODULE_AUTHOR("Rafael J. Wysocki <rafael.j.wysocki@intel.com>");
+MODULE_DESCRIPTION("Utilization-based CPU frequency selection");
+MODULE_LICENSE("GPL");
+
+#ifdef CONFIG_CPU_FREQ_DEFAULT_GOV_SCHEDUTIL
+struct cpufreq_governor *cpufreq_default_governor(void)
+{
+	return &schedutil_gov;
+}
+
+fs_initcall(sugov_module_init);
+#else
+module_init(sugov_module_init);
+#endif
+module_exit(sugov_module_exit);
Index: linux-pm/kernel/sched/Makefile
===================================================================
--- linux-pm.orig/kernel/sched/Makefile
+++ linux-pm/kernel/sched/Makefile
@@ -20,3 +20,4 @@ obj-$(CONFIG_SCHEDSTATS) += stats.o
 obj-$(CONFIG_SCHED_DEBUG) += debug.o
 obj-$(CONFIG_CGROUP_CPUACCT) += cpuacct.o
 obj-$(CONFIG_CPU_FREQ) += cpufreq.o
+obj-$(CONFIG_CPU_FREQ_GOV_SCHEDUTIL) += cpufreq_schedutil.o
Index: linux-pm/drivers/cpufreq/cpufreq_governor.h
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_governor.h
+++ linux-pm/drivers/cpufreq/cpufreq_governor.h
@@ -34,7 +34,6 @@
  * this governor will not work. All times here are in us (micro seconds).
  */
 #define MIN_SAMPLING_RATE_RATIO			(2)
-#define LATENCY_MULTIPLIER			(1000)
 #define MIN_LATENCY_MULTIPLIER			(20)
 #define TRANSITION_LATENCY_LIMIT		(10 * 1000 * 1000)
 
Index: linux-pm/include/linux/cpufreq.h
===================================================================
--- linux-pm.orig/include/linux/cpufreq.h
+++ linux-pm/include/linux/cpufreq.h
@@ -468,6 +468,9 @@ void cpufreq_unregister_governor(struct
 struct cpufreq_governor *cpufreq_default_governor(void);
 struct cpufreq_governor *cpufreq_fallback_governor(void);
 
+/* Coefficient for computing default sampling rate/rate limit in governors */
+#define LATENCY_MULTIPLIER	(1000)
+
 /* Governor attribute set */
 struct gov_attr_set {
 	struct kobject kobj;

[toc] | [prev] | [next] | [standalone]


#1349812 — [PATCH v2 6/10] cpufreq: Support for fast frequency switching

From"Rafael J. Wysocki" <rjw@rjwysocki.net>
Date2016-03-04 04:40 +0100
Subject[PATCH v2 6/10] cpufreq: Support for fast frequency switching
Message-ID<r8OKR-B1-5@gated-at.bofh.it>
In reply to#1347699
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Modify the ACPI cpufreq driver to provide a method for switching
CPU frequencies from interrupt context and update the cpufreq core
to support that method if available.

Introduce a new cpufreq driver callback, ->fast_switch, to be
invoked for frequency switching from interrupt context via a
new helper function, cpufreq_driver_fast_switch().  Add a new
policy flag, fast_switch_possible, to be set if fast frequency
switching can be used for the given policy.

Implement the ->fast_switch callback in the ACPI cpufreq driver
and make it set fast_switch_possible during policy initialization
as appropriate.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

Changes from the previous version:
- Drop a bogus check from cpufreq_driver_fast_switch().

---
 drivers/cpufreq/acpi-cpufreq.c |   53 +++++++++++++++++++++++++++++++++++++++++
 drivers/cpufreq/cpufreq.c      |   30 +++++++++++++++++++++++
 include/linux/cpufreq.h        |    6 ++++
 3 files changed, 89 insertions(+)

Index: linux-pm/drivers/cpufreq/acpi-cpufreq.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/acpi-cpufreq.c
+++ linux-pm/drivers/cpufreq/acpi-cpufreq.c
@@ -458,6 +458,55 @@ static int acpi_cpufreq_target(struct cp
 	return result;
 }
 
+unsigned int acpi_cpufreq_fast_switch(struct cpufreq_policy *policy,
+				      unsigned int target_freq,
+				      unsigned int relation)
+{
+	struct acpi_cpufreq_data *data = policy->driver_data;
+	struct acpi_processor_performance *perf;
+	struct cpufreq_frequency_table *entry, *found;
+	unsigned int next_perf_state, next_freq, freq;
+
+	/*
+	 * Find the closest frequency above target_freq or equal to it.
+	 *
+	 * The table is sorted in the reverse order with respect to the
+	 * frequency and all of the entries are valid (see the initialization).
+	 */
+	entry = data->freq_table;
+	do {
+		entry++;
+		freq = entry->frequency;
+	} while (freq >= target_freq && freq != CPUFREQ_TABLE_END);
+	found = entry - 1;
+	/*
+	 * Use the one found or the previous one, depending on the relation.
+	 * CPUFREQ_RELATION_H is not taken into account here, but it is not
+	 * expected to be passed to this function anyway.
+	 */
+	next_freq = found->frequency;
+	if (freq == CPUFREQ_TABLE_END || relation != CPUFREQ_RELATION_C ||
+	    target_freq - freq >= next_freq - target_freq) {
+		next_perf_state = found->driver_data;
+	} else {
+		next_freq = freq;
+		next_perf_state = entry->driver_data;
+	}
+
+	perf = to_perf_data(data);
+	if (perf->state == next_perf_state) {
+		if (unlikely(data->resume))
+			data->resume = 0;
+		else
+			return next_freq;
+	}
+
+	data->cpu_freq_write(&perf->control_register,
+			     perf->states[next_perf_state].control);
+	perf->state = next_perf_state;
+	return next_freq;
+}
+
 static unsigned long
 acpi_cpufreq_guess_freq(struct acpi_cpufreq_data *data, unsigned int cpu)
 {
@@ -740,6 +789,9 @@ static int acpi_cpufreq_cpu_init(struct
 		goto err_unreg;
 	}
 
+	policy->fast_switch_possible = !acpi_pstate_strict &&
+		!(policy_is_shared(policy) && policy->shared_type != CPUFREQ_SHARED_TYPE_ANY);
+
 	data->freq_table = kzalloc(sizeof(*data->freq_table) *
 		    (perf->state_count+1), GFP_KERNEL);
 	if (!data->freq_table) {
@@ -874,6 +926,7 @@ static struct freq_attr *acpi_cpufreq_at
 static struct cpufreq_driver acpi_cpufreq_driver = {
 	.verify		= cpufreq_generic_frequency_table_verify,
 	.target_index	= acpi_cpufreq_target,
+	.fast_switch	= acpi_cpufreq_fast_switch,
 	.bios_limit	= acpi_processor_get_bios_limit,
 	.init		= acpi_cpufreq_cpu_init,
 	.exit		= acpi_cpufreq_cpu_exit,
Index: linux-pm/drivers/cpufreq/cpufreq.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq.c
+++ linux-pm/drivers/cpufreq/cpufreq.c
@@ -1719,6 +1719,36 @@ EXPORT_SYMBOL(cpufreq_unregister_notifie
  *                              GOVERNORS                            *
  *********************************************************************/
 
+/**
+ * cpufreq_driver_fast_switch - Carry out a fast CPU frequency switch.
+ * @policy: cpufreq policy to switch the frequency for.
+ * @target_freq: New frequency to set (may be approximate).
+ * @relation: Relation to use for frequency selection.
+ *
+ * Carry out a fast frequency switch from interrupt context.
+ *
+ * This function must not be called if policy->fast_switch_possible is unset.
+ *
+ * Governors calling this function must guarantee that it will never be invoked
+ * twice in parallel for the same policy and that it will never be called in
+ * parallel with either ->target() or ->target_index() for the same policy.
+ *
+ * If CPUFREQ_ENTRY_INVALID is returned by the driver's ->fast_switch()
+ * callback, the hardware configuration must be preserved.
+ */
+void cpufreq_driver_fast_switch(struct cpufreq_policy *policy,
+				unsigned int target_freq, unsigned int relation)
+{
+	unsigned int freq;
+
+	freq = cpufreq_driver->fast_switch(policy, target_freq, relation);
+	if (freq != CPUFREQ_ENTRY_INVALID) {
+		policy->cur = freq;
+		trace_cpu_frequency(freq, smp_processor_id());
+	}
+}
+EXPORT_SYMBOL_GPL(cpufreq_driver_fast_switch);
+
 /* Must set freqs->new to intermediate frequency */
 static int __target_intermediate(struct cpufreq_policy *policy,
 				 struct cpufreq_freqs *freqs, int index)
Index: linux-pm/include/linux/cpufreq.h
===================================================================
--- linux-pm.orig/include/linux/cpufreq.h
+++ linux-pm/include/linux/cpufreq.h
@@ -81,6 +81,7 @@ struct cpufreq_policy {
 	struct cpufreq_governor	*governor; /* see below */
 	void			*governor_data;
 	char			last_governor[CPUFREQ_NAME_LEN]; /* last governor used */
+	bool			fast_switch_possible;
 
 	struct work_struct	update; /* if update_policy() needs to be
 					 * called, but you're in IRQ context */
@@ -236,6 +237,9 @@ struct cpufreq_driver {
 				  unsigned int relation);	/* Deprecated */
 	int		(*target_index)(struct cpufreq_policy *policy,
 					unsigned int index);
+	unsigned int	(*fast_switch)(struct cpufreq_policy *policy,
+				       unsigned int target_freq,
+				       unsigned int relation);
 	/*
 	 * Only for drivers with target_index() and CPUFREQ_ASYNC_NOTIFICATION
 	 * unset.
@@ -450,6 +454,8 @@ struct cpufreq_governor {
 };
 
 /* Pass a target to the cpufreq driver */
+void cpufreq_driver_fast_switch(struct cpufreq_policy *policy,
+				unsigned int target_freq, unsigned int relation);
 int cpufreq_driver_target(struct cpufreq_policy *policy,
 				 unsigned int target_freq,
 				 unsigned int relation);

[toc] | [prev] | [next] | [standalone]


#1349813 — [PATCH v2 0/10] cpufreq: schedutil governor

From"Rafael J. Wysocki" <rjw@rjwysocki.net>
Date2016-03-04 04:40 +0100
Subject[PATCH v2 0/10] cpufreq: schedutil governor
Message-ID<r8OKR-B1-3@gated-at.bofh.it>
In reply to#1347699
On Wednesday, March 02, 2016 02:56:28 AM Rafael J. Wysocki wrote:
> Hi,
> 
> My previous intro message still applies somewhat, so here's a link:
> 
> http://marc.info/?l=linux-pm&m=145609673008122&w=2
> 
> The executive summary of the motivation is that I wanted to do two things:
> use the utilization data from the scheduler (it's passed to the governor
> as aguments of update callbacks anyway) and make it possible to set
> CPU frequency without involving process context (fast frequency switching).
> 
> Both have been prototyped in the previous RFCs:
> 
> https://patchwork.kernel.org/patch/8426691/
> https://patchwork.kernel.org/patch/8426741/
> 

[cut]

> 
> Comments welcome.

There were quite a few comments to address, so here's a new version.

First off, my interpretation of what Ingo said earlier today (or yesterday
depending on your time zone) is that he wants all of the code dealing with
the util and max values to be located in kernel/sched/.  I can understand
the motivation here, although schedutil shares some amount of code with
the other governors, so the dependency on cpufreq will still be there, even
if the code goes to kernel/sched/.  Nevertheless, I decided to make that
change just to see how it would look like if not for anything else.

To that end, I revived a patch I had before the first schedutil one to
remove util/max from the cpufreq hooks [7/10], moved the scheduler-related
code from drivers/cpufreq/cpufreq.c to kernel/sched/cpufreq.c (new file)
on top of that [8/10] and reintroduced cpufreq_update_util() in a slightly
different form [9/10].  I did it this way in case it turns out to be
necessary to apply [7/10] and [8/10] for the time being and defer the rest
to the next cycle.

Apart from that, I changed the frequency selection formula in the new
governor to next_freq = util * max_freq / max and it seems to work.  That
allowed the code to be simplified somewhat as I don't need the extra
relation field in struct sugov_policy now (RELATION_L is used everywhere).

Finally, I tried to address the bikeshed comment from Viresh about the
"wrong" names of data types etc related to governor sysfs attributes
handling.  Hopefully, the new ones are better.

There are small tweaks all over on top of that.

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]


#1349814 — [PATCH v2 1/10] cpufreq: Reduce cpufreq_update_util() overhead a bit

From"Rafael J. Wysocki" <rjw@rjwysocki.net>
Date2016-03-04 04:40 +0100
Subject[PATCH v2 1/10] cpufreq: Reduce cpufreq_update_util() overhead a bit
Message-ID<r8OKS-B1-7@gated-at.bofh.it>
In reply to#1349813
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Use the observation that cpufreq_update_util() is only called
by the scheduler with rq->lock held, so the callers of
cpufreq_set_update_util_data() can use synchronize_sched()
instead of synchronize_rcu() to wait for cpufreq_update_util()
to complete.  Moreover, if they are updated to do that,
rcu_read_(un)lock() calls in cpufreq_update_util() might be
replaced with rcu_read_(un)lock_sched(), respectively, but
those aren't really necessary, because the scheduler calls
that function from RCU-sched read-side critical sections
already.

In addition to that, if cpufreq_set_update_util_data() checks
the func field in the struct update_util_data before setting
the per-CPU pointer to it, the data->func check may be dropped
from cpufreq_update_util() as well.

Make the above changes to reduce the overhead from
cpufreq_update_util() in the scheduler paths invoking it
and to make the cleanup after removing its callbacks less
heavy-weight somewhat.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
---

Changes from the previous version:
- Use rcu_dereference_sched() in cpufreq_update_util().

---
 drivers/cpufreq/cpufreq.c          |   25 +++++++++++++++++--------
 drivers/cpufreq/cpufreq_governor.c |    2 +-
 drivers/cpufreq/intel_pstate.c     |    4 ++--
 3 files changed, 20 insertions(+), 11 deletions(-)

Index: linux-pm/drivers/cpufreq/cpufreq.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq.c
+++ linux-pm/drivers/cpufreq/cpufreq.c
@@ -77,12 +77,15 @@ static DEFINE_PER_CPU(struct update_util
  * to call from cpufreq_update_util().  That function will be called from an RCU
  * read-side critical section, so it must not sleep.
  *
- * Callers must use RCU callbacks to free any memory that might be accessed
- * via the old update_util_data pointer or invoke synchronize_rcu() right after
- * this function to avoid use-after-free.
+ * Callers must use RCU-sched callbacks to free any memory that might be
+ * accessed via the old update_util_data pointer or invoke synchronize_sched()
+ * right after this function to avoid use-after-free.
  */
 void cpufreq_set_update_util_data(int cpu, struct update_util_data *data)
 {
+	if (WARN_ON(data && !data->func))
+		return;
+
 	rcu_assign_pointer(per_cpu(cpufreq_update_util_data, cpu), data);
 }
 EXPORT_SYMBOL_GPL(cpufreq_set_update_util_data);
@@ -95,18 +98,24 @@ EXPORT_SYMBOL_GPL(cpufreq_set_update_uti
  *
  * This function is called by the scheduler on every invocation of
  * update_load_avg() on the CPU whose utilization is being updated.
+ *
+ * It can only be called from RCU-sched read-side critical sections.
  */
 void cpufreq_update_util(u64 time, unsigned long util, unsigned long max)
 {
 	struct update_util_data *data;
 
-	rcu_read_lock();
+#ifdef CONFIG_LOCKDEP
+	WARN_ON(debug_locks && !rcu_read_lock_sched_held());
+#endif
 
-	data = rcu_dereference(*this_cpu_ptr(&cpufreq_update_util_data));
-	if (data && data->func)
+	data = rcu_dereference_sched(*this_cpu_ptr(&cpufreq_update_util_data));
+	/*
+	 * If this isn't inside of an RCU-sched read-side critical section, data
+	 * may become NULL after the check below.
+	 */
+	if (data)
 		data->func(data, time, util, max);
-
-	rcu_read_unlock();
 }
 
 /* Flag to suspend/resume CPUFreq governors */
Index: linux-pm/drivers/cpufreq/cpufreq_governor.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_governor.c
+++ linux-pm/drivers/cpufreq/cpufreq_governor.c
@@ -280,7 +280,7 @@ static inline void gov_clear_update_util
 	for_each_cpu(i, policy->cpus)
 		cpufreq_set_update_util_data(i, NULL);
 
-	synchronize_rcu();
+	synchronize_sched();
 }
 
 static void gov_cancel_work(struct cpufreq_policy *policy)
Index: linux-pm/drivers/cpufreq/intel_pstate.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/intel_pstate.c
+++ linux-pm/drivers/cpufreq/intel_pstate.c
@@ -1174,7 +1174,7 @@ static void intel_pstate_stop_cpu(struct
 	pr_debug("intel_pstate: CPU %d exiting\n", cpu_num);
 
 	cpufreq_set_update_util_data(cpu_num, NULL);
-	synchronize_rcu();
+	synchronize_sched();
 
 	if (hwp_active)
 		return;
@@ -1442,7 +1442,7 @@ out:
 	for_each_online_cpu(cpu) {
 		if (all_cpu_data[cpu]) {
 			cpufreq_set_update_util_data(cpu, NULL);
-			synchronize_rcu();
+			synchronize_sched();
 			kfree(all_cpu_data[cpu]);
 		}
 	}

[toc] | [prev] | [next] | [standalone]


#1349815 — [PATCH v2 5/10] cpufreq: Move governor attribute set headers to cpufreq.h

From"Rafael J. Wysocki" <rjw@rjwysocki.net>
Date2016-03-04 04:40 +0100
Subject[PATCH v2 5/10] cpufreq: Move governor attribute set headers to cpufreq.h
Message-ID<r8OKS-B1-11@gated-at.bofh.it>
In reply to#1349813
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Move definitions and function headers related to struct gov_attr_set
to include/linux/cpufreq.h so they can be used by (future) goverernors
located outside of drivers/cpufreq/.

No functional changes.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

New patch.  Needed to move cpufreq_schedutil.c to kernel/sched/.

---
 drivers/cpufreq/cpufreq_governor.h |   21 ---------------------
 include/linux/cpufreq.h            |   23 +++++++++++++++++++++++
 2 files changed, 23 insertions(+), 21 deletions(-)

Index: linux-pm/drivers/cpufreq/cpufreq_governor.h
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_governor.h
+++ linux-pm/drivers/cpufreq/cpufreq_governor.h
@@ -41,19 +41,6 @@
 /* Ondemand Sampling types */
 enum {OD_NORMAL_SAMPLE, OD_SUB_SAMPLE};
 
-struct gov_attr_set {
-	struct kobject kobj;
-	struct list_head policy_list;
-	struct mutex update_lock;
-	int usage_count;
-};
-
-extern const struct sysfs_ops governor_sysfs_ops;
-
-void gov_attr_set_init(struct gov_attr_set *attr_set, struct list_head *list_node);
-void gov_attr_set_get(struct gov_attr_set *attr_set, struct list_head *list_node);
-unsigned int gov_attr_set_put(struct gov_attr_set *attr_set, struct list_head *list_node);
-
 /*
  * Abbreviations:
  * dbs: used as a shortform for demand based switching It helps to keep variable
@@ -80,14 +67,6 @@ static inline struct dbs_data *to_dbs_da
 	return container_of(attr_set, struct dbs_data, attr_set);
 }
 
-/* Governor's specific attributes */
-struct governor_attr {
-	struct attribute attr;
-	ssize_t (*show)(struct gov_attr_set *attr_set, char *buf);
-	ssize_t (*store)(struct gov_attr_set *attr_set, const char *buf,
-			 size_t count);
-};
-
 #define gov_show_one(_gov, file_name)					\
 static ssize_t show_##file_name						\
 (struct gov_attr_set *attr_set, char *buf)				\
Index: linux-pm/include/linux/cpufreq.h
===================================================================
--- linux-pm.orig/include/linux/cpufreq.h
+++ linux-pm/include/linux/cpufreq.h
@@ -462,6 +462,29 @@ void cpufreq_unregister_governor(struct
 struct cpufreq_governor *cpufreq_default_governor(void);
 struct cpufreq_governor *cpufreq_fallback_governor(void);
 
+/* Governor attribute set */
+struct gov_attr_set {
+	struct kobject kobj;
+	struct list_head policy_list;
+	struct mutex update_lock;
+	int usage_count;
+};
+
+/* sysfs ops for cpufreq governors */
+extern const struct sysfs_ops governor_sysfs_ops;
+
+void gov_attr_set_init(struct gov_attr_set *attr_set, struct list_head *list_node);
+void gov_attr_set_get(struct gov_attr_set *attr_set, struct list_head *list_node);
+unsigned int gov_attr_set_put(struct gov_attr_set *attr_set, struct list_head *list_node);
+
+/* Governor sysfs attribute */
+struct governor_attr {
+	struct attribute attr;
+	ssize_t (*show)(struct gov_attr_set *attr_set, char *buf);
+	ssize_t (*store)(struct gov_attr_set *attr_set, const char *buf,
+			 size_t count);
+};
+
 /*********************************************************************
  *                     FREQUENCY TABLE HELPERS                       *
  *********************************************************************/

[toc] | [prev] | [next] | [standalone]


#1349861 — Re: [PATCH v2 5/10] cpufreq: Move governor attribute set headers to cpufreq.h

FromViresh Kumar <viresh.kumar@linaro.org>
Date2016-03-04 07:00 +0100
SubjectRe: [PATCH v2 5/10] cpufreq: Move governor attribute set headers to cpufreq.h
Message-ID<r8QWm-20V-15@gated-at.bofh.it>
In reply to#1349815
On 04-03-16, 04:05, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Move definitions and function headers related to struct gov_attr_set
> to include/linux/cpufreq.h so they can be used by (future) goverernors
> located outside of drivers/cpufreq/.
> 
> No functional changes.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> 
> New patch.  Needed to move cpufreq_schedutil.c to kernel/sched/.

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

[toc] | [prev] | [next] | [standalone]


#1349816 — [PATCH v2 2/10][Resend] cpufreq: acpi-cpufreq: Make read and write operations more efficient

From"Rafael J. Wysocki" <rjw@rjwysocki.net>
Date2016-03-04 04:40 +0100
Subject[PATCH v2 2/10][Resend] cpufreq: acpi-cpufreq: Make read and write operations more efficient
Message-ID<r8OKS-B1-15@gated-at.bofh.it>
In reply to#1349813
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Setting a new CPU frequency and reading the current request value
in the ACPI cpufreq driver involves each at least two switch
instructions (there's more if the policy is shared).  One of
them is present in drv_read/write() that prepares a command
structure and the other happens in subsequent do_drv_read/write()
when that structure is interpreted.  However, all of those switches
may be avoided by using function pointers.

To that end, add two function pointers to struct acpi_cpufreq_data
to represent read and write operations on the frequency register
and set them up during policy intitialization to point to the pair
of routines suitable for the given processor (Intel/AMD MSR access
or I/O port access).  Then, use those pointers in do_drv_read/write()
and modify drv_read/write() to prepare the command structure for
them without any checks.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

No changes.

---
 drivers/cpufreq/acpi-cpufreq.c |  208 ++++++++++++++++++-----------------------
 1 file changed, 95 insertions(+), 113 deletions(-)

Index: linux-pm/drivers/cpufreq/acpi-cpufreq.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/acpi-cpufreq.c
+++ linux-pm/drivers/cpufreq/acpi-cpufreq.c
@@ -70,6 +70,8 @@ struct acpi_cpufreq_data {
 	unsigned int cpu_feature;
 	unsigned int acpi_perf_cpu;
 	cpumask_var_t freqdomain_cpus;
+	void (*cpu_freq_write)(struct acpi_pct_register *reg, u32 val);
+	u32 (*cpu_freq_read)(struct acpi_pct_register *reg);
 };
 
 /* acpi_perf_data is a pointer to percpu data. */
@@ -243,125 +245,119 @@ static unsigned extract_freq(u32 val, st
 	}
 }
 
-struct msr_addr {
-	u32 reg;
-};
+u32 cpu_freq_read_intel(struct acpi_pct_register *not_used)
+{
+	u32 val, dummy;
 
-struct io_addr {
-	u16 port;
-	u8 bit_width;
-};
+	rdmsr(MSR_IA32_PERF_CTL, val, dummy);
+	return val;
+}
+
+void cpu_freq_write_intel(struct acpi_pct_register *not_used, u32 val)
+{
+	u32 lo, hi;
+
+	rdmsr(MSR_IA32_PERF_CTL, lo, hi);
+	lo = (lo & ~INTEL_MSR_RANGE) | (val & INTEL_MSR_RANGE);
+	wrmsr(MSR_IA32_PERF_CTL, lo, hi);
+}
+
+u32 cpu_freq_read_amd(struct acpi_pct_register *not_used)
+{
+	u32 val, dummy;
+
+	rdmsr(MSR_AMD_PERF_CTL, val, dummy);
+	return val;
+}
+
+void cpu_freq_write_amd(struct acpi_pct_register *not_used, u32 val)
+{
+	wrmsr(MSR_AMD_PERF_CTL, val, 0);
+}
+
+u32 cpu_freq_read_io(struct acpi_pct_register *reg)
+{
+	u32 val;
+
+	acpi_os_read_port(reg->address, &val, reg->bit_width);
+	return val;
+}
+
+void cpu_freq_write_io(struct acpi_pct_register *reg, u32 val)
+{
+	acpi_os_write_port(reg->address, val, reg->bit_width);
+}
 
 struct drv_cmd {
-	unsigned int type;
-	const struct cpumask *mask;
-	union {
-		struct msr_addr msr;
-		struct io_addr io;
-	} addr;
+	struct acpi_pct_register *reg;
 	u32 val;
+	union {
+		void (*write)(struct acpi_pct_register *reg, u32 val);
+		u32 (*read)(struct acpi_pct_register *reg);
+	} func;
 };
 
 /* Called via smp_call_function_single(), on the target CPU */
 static void do_drv_read(void *_cmd)
 {
 	struct drv_cmd *cmd = _cmd;
-	u32 h;
 
-	switch (cmd->type) {
-	case SYSTEM_INTEL_MSR_CAPABLE:
-	case SYSTEM_AMD_MSR_CAPABLE:
-		rdmsr(cmd->addr.msr.reg, cmd->val, h);
-		break;
-	case SYSTEM_IO_CAPABLE:
-		acpi_os_read_port((acpi_io_address)cmd->addr.io.port,
-				&cmd->val,
-				(u32)cmd->addr.io.bit_width);
-		break;
-	default:
-		break;
-	}
+	cmd->val = cmd->func.read(cmd->reg);
 }
 
-/* Called via smp_call_function_many(), on the target CPUs */
-static void do_drv_write(void *_cmd)
+static u32 drv_read(struct acpi_cpufreq_data *data, const struct cpumask *mask)
 {
-	struct drv_cmd *cmd = _cmd;
-	u32 lo, hi;
+	struct acpi_processor_performance *perf = to_perf_data(data);
+	struct drv_cmd cmd = {
+		.reg = &perf->control_register,
+		.func.read = data->cpu_freq_read,
+	};
+	int err;
 
-	switch (cmd->type) {
-	case SYSTEM_INTEL_MSR_CAPABLE:
-		rdmsr(cmd->addr.msr.reg, lo, hi);
-		lo = (lo & ~INTEL_MSR_RANGE) | (cmd->val & INTEL_MSR_RANGE);
-		wrmsr(cmd->addr.msr.reg, lo, hi);
-		break;
-	case SYSTEM_AMD_MSR_CAPABLE:
-		wrmsr(cmd->addr.msr.reg, cmd->val, 0);
-		break;
-	case SYSTEM_IO_CAPABLE:
-		acpi_os_write_port((acpi_io_address)cmd->addr.io.port,
-				cmd->val,
-				(u32)cmd->addr.io.bit_width);
-		break;
-	default:
-		break;
-	}
+	err = smp_call_function_any(mask, do_drv_read, &cmd, 1);
+	WARN_ON_ONCE(err);	/* smp_call_function_any() was buggy? */
+	return cmd.val;
 }
 
-static void drv_read(struct drv_cmd *cmd)
+/* Called via smp_call_function_many(), on the target CPUs */
+static void do_drv_write(void *_cmd)
 {
-	int err;
-	cmd->val = 0;
+	struct drv_cmd *cmd = _cmd;
 
-	err = smp_call_function_any(cmd->mask, do_drv_read, cmd, 1);
-	WARN_ON_ONCE(err);	/* smp_call_function_any() was buggy? */
+	cmd->func.write(cmd->reg, cmd->val);
 }
 
-static void drv_write(struct drv_cmd *cmd)
+static void drv_write(struct acpi_cpufreq_data *data,
+		      const struct cpumask *mask, u32 val)
 {
+	struct acpi_processor_performance *perf = to_perf_data(data);
+	struct drv_cmd cmd = {
+		.reg = &perf->control_register,
+		.val = val,
+		.func.write = data->cpu_freq_write,
+	};
 	int this_cpu;
 
 	this_cpu = get_cpu();
-	if (cpumask_test_cpu(this_cpu, cmd->mask))
-		do_drv_write(cmd);
-	smp_call_function_many(cmd->mask, do_drv_write, cmd, 1);
+	if (cpumask_test_cpu(this_cpu, mask))
+		do_drv_write(&cmd);
+
+	smp_call_function_many(mask, do_drv_write, &cmd, 1);
 	put_cpu();
 }
 
-static u32
-get_cur_val(const struct cpumask *mask, struct acpi_cpufreq_data *data)
+static u32 get_cur_val(const struct cpumask *mask, struct acpi_cpufreq_data *data)
 {
-	struct acpi_processor_performance *perf;
-	struct drv_cmd cmd;
+	u32 val;
 
 	if (unlikely(cpumask_empty(mask)))
 		return 0;
 
-	switch (data->cpu_feature) {
-	case SYSTEM_INTEL_MSR_CAPABLE:
-		cmd.type = SYSTEM_INTEL_MSR_CAPABLE;
-		cmd.addr.msr.reg = MSR_IA32_PERF_CTL;
-		break;
-	case SYSTEM_AMD_MSR_CAPABLE:
-		cmd.type = SYSTEM_AMD_MSR_CAPABLE;
-		cmd.addr.msr.reg = MSR_AMD_PERF_CTL;
-		break;
-	case SYSTEM_IO_CAPABLE:
-		cmd.type = SYSTEM_IO_CAPABLE;
-		perf = to_perf_data(data);
-		cmd.addr.io.port = perf->control_register.address;
-		cmd.addr.io.bit_width = perf->control_register.bit_width;
-		break;
-	default:
-		return 0;
-	}
-
-	cmd.mask = mask;
-	drv_read(&cmd);
+	val = drv_read(data, mask);
 
-	pr_debug("get_cur_val = %u\n", cmd.val);
+	pr_debug("get_cur_val = %u\n", val);
 
-	return cmd.val;
+	return val;
 }
 
 static unsigned int get_cur_freq_on_cpu(unsigned int cpu)
@@ -416,7 +412,7 @@ static int acpi_cpufreq_target(struct cp
 {
 	struct acpi_cpufreq_data *data = policy->driver_data;
 	struct acpi_processor_performance *perf;
-	struct drv_cmd cmd;
+	const struct cpumask *mask;
 	unsigned int next_perf_state = 0; /* Index into perf table */
 	int result = 0;
 
@@ -438,37 +434,17 @@ static int acpi_cpufreq_target(struct cp
 		}
 	}
 
-	switch (data->cpu_feature) {
-	case SYSTEM_INTEL_MSR_CAPABLE:
-		cmd.type = SYSTEM_INTEL_MSR_CAPABLE;
-		cmd.addr.msr.reg = MSR_IA32_PERF_CTL;
-		cmd.val = (u32) perf->states[next_perf_state].control;
-		break;
-	case SYSTEM_AMD_MSR_CAPABLE:
-		cmd.type = SYSTEM_AMD_MSR_CAPABLE;
-		cmd.addr.msr.reg = MSR_AMD_PERF_CTL;
-		cmd.val = (u32) perf->states[next_perf_state].control;
-		break;
-	case SYSTEM_IO_CAPABLE:
-		cmd.type = SYSTEM_IO_CAPABLE;
-		cmd.addr.io.port = perf->control_register.address;
-		cmd.addr.io.bit_width = perf->control_register.bit_width;
-		cmd.val = (u32) perf->states[next_perf_state].control;
-		break;
-	default:
-		return -ENODEV;
-	}
-
-	/* cpufreq holds the hotplug lock, so we are safe from here on */
-	if (policy->shared_type != CPUFREQ_SHARED_TYPE_ANY)
-		cmd.mask = policy->cpus;
-	else
-		cmd.mask = cpumask_of(policy->cpu);
+	/*
+	 * The core won't allow CPUs to go away until the governor has been
+	 * stopped, so we can rely on the stability of policy->cpus.
+	 */
+	mask = policy->shared_type == CPUFREQ_SHARED_TYPE_ANY ?
+		cpumask_of(policy->cpu) : policy->cpus;
 
-	drv_write(&cmd);
+	drv_write(data, mask, perf->states[next_perf_state].control);
 
 	if (acpi_pstate_strict) {
-		if (!check_freqs(cmd.mask, data->freq_table[index].frequency,
+		if (!check_freqs(mask, data->freq_table[index].frequency,
 					data)) {
 			pr_debug("acpi_cpufreq_target failed (%d)\n",
 				policy->cpu);
@@ -738,15 +714,21 @@ static int acpi_cpufreq_cpu_init(struct
 		}
 		pr_debug("SYSTEM IO addr space\n");
 		data->cpu_feature = SYSTEM_IO_CAPABLE;
+		data->cpu_freq_read = cpu_freq_read_io;
+		data->cpu_freq_write = cpu_freq_write_io;
 		break;
 	case ACPI_ADR_SPACE_FIXED_HARDWARE:
 		pr_debug("HARDWARE addr space\n");
 		if (check_est_cpu(cpu)) {
 			data->cpu_feature = SYSTEM_INTEL_MSR_CAPABLE;
+			data->cpu_freq_read = cpu_freq_read_intel;
+			data->cpu_freq_write = cpu_freq_write_intel;
 			break;
 		}
 		if (check_amd_hwpstate_cpu(cpu)) {
 			data->cpu_feature = SYSTEM_AMD_MSR_CAPABLE;
+			data->cpu_freq_read = cpu_freq_read_amd;
+			data->cpu_freq_write = cpu_freq_write_amd;
 			break;
 		}
 		result = -ENODEV;

[toc] | [prev] | [next] | [standalone]


#1349817 — [PATCH v2 3/10] cpufreq: governor: New data type for management part of dbs_data

From"Rafael J. Wysocki" <rjw@rjwysocki.net>
Date2016-03-04 04:40 +0100
Subject[PATCH v2 3/10] cpufreq: governor: New data type for management part of dbs_data
Message-ID<r8OKS-B1-9@gated-at.bofh.it>
In reply to#1349813
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

In addition to fields representing governor tunables, struct dbs_data
contains some fields needed for the management of objects of that
type.  As it turns out, that part of struct dbs_data may be shared
with (future) governors that won't use the common code used by
"ondemand" and "conservative", so move it to a separate struct type
and modify the code using struct dbs_data to follow.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

Changes from the previous version:
- The new data type is called gov_attr_set now (instead of gov_tunables)
  and some variable names etc have been changed to follow.

---
 drivers/cpufreq/cpufreq_conservative.c |   25 +++++----
 drivers/cpufreq/cpufreq_governor.c     |   90 ++++++++++++++++++++-------------
 drivers/cpufreq/cpufreq_governor.h     |   35 +++++++-----
 drivers/cpufreq/cpufreq_ondemand.c     |   29 ++++++----
 4 files changed, 107 insertions(+), 72 deletions(-)

Index: linux-pm/drivers/cpufreq/cpufreq_governor.h
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_governor.h
+++ linux-pm/drivers/cpufreq/cpufreq_governor.h
@@ -41,6 +41,13 @@
 /* Ondemand Sampling types */
 enum {OD_NORMAL_SAMPLE, OD_SUB_SAMPLE};
 
+struct gov_attr_set {
+	struct kobject kobj;
+	struct list_head policy_list;
+	struct mutex update_lock;
+	int usage_count;
+};
+
 /*
  * Abbreviations:
  * dbs: used as a shortform for demand based switching It helps to keep variable
@@ -52,7 +59,7 @@ enum {OD_NORMAL_SAMPLE, OD_SUB_SAMPLE};
 
 /* Governor demand based switching data (per-policy or global). */
 struct dbs_data {
-	int usage_count;
+	struct gov_attr_set attr_set;
 	void *tuners;
 	unsigned int min_sampling_rate;
 	unsigned int ignore_nice_load;
@@ -60,37 +67,35 @@ struct dbs_data {
 	unsigned int sampling_down_factor;
 	unsigned int up_threshold;
 	unsigned int io_is_busy;
-
-	struct kobject kobj;
-	struct list_head policy_dbs_list;
-	/*
-	 * Protect concurrent updates to governor tunables from sysfs,
-	 * policy_dbs_list and usage_count.
-	 */
-	struct mutex mutex;
 };
 
+static inline struct dbs_data *to_dbs_data(struct gov_attr_set *attr_set)
+{
+	return container_of(attr_set, struct dbs_data, attr_set);
+}
+
 /* Governor's specific attributes */
-struct dbs_data;
 struct governor_attr {
 	struct attribute attr;
-	ssize_t (*show)(struct dbs_data *dbs_data, char *buf);
-	ssize_t (*store)(struct dbs_data *dbs_data, const char *buf,
+	ssize_t (*show)(struct gov_attr_set *attr_set, char *buf);
+	ssize_t (*store)(struct gov_attr_set *attr_set, const char *buf,
 			 size_t count);
 };
 
 #define gov_show_one(_gov, file_name)					\
 static ssize_t show_##file_name						\
-(struct dbs_data *dbs_data, char *buf)					\
+(struct gov_attr_set *attr_set, char *buf)				\
 {									\
+	struct dbs_data *dbs_data = to_dbs_data(attr_set);		\
 	struct _gov##_dbs_tuners *tuners = dbs_data->tuners;		\
 	return sprintf(buf, "%u\n", tuners->file_name);			\
 }
 
 #define gov_show_one_common(file_name)					\
 static ssize_t show_##file_name						\
-(struct dbs_data *dbs_data, char *buf)					\
+(struct gov_attr_set *attr_set, char *buf)				\
 {									\
+	struct dbs_data *dbs_data = to_dbs_data(attr_set);		\
 	return sprintf(buf, "%u\n", dbs_data->file_name);		\
 }
 
@@ -184,7 +189,7 @@ void od_register_powersave_bias_handler(
 		(struct cpufreq_policy *, unsigned int, unsigned int),
 		unsigned int powersave_bias);
 void od_unregister_powersave_bias_handler(void);
-ssize_t store_sampling_rate(struct dbs_data *dbs_data, const char *buf,
+ssize_t store_sampling_rate(struct gov_attr_set *attr_set, const char *buf,
 			    size_t count);
 void gov_update_cpu_data(struct dbs_data *dbs_data);
 #endif /* _CPUFREQ_GOVERNOR_H */
Index: linux-pm/drivers/cpufreq/cpufreq_governor.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_governor.c
+++ linux-pm/drivers/cpufreq/cpufreq_governor.c
@@ -43,9 +43,10 @@ static DEFINE_MUTEX(gov_dbs_data_mutex);
  * This must be called with dbs_data->mutex held, otherwise traversing
  * policy_dbs_list isn't safe.
  */
-ssize_t store_sampling_rate(struct dbs_data *dbs_data, const char *buf,
+ssize_t store_sampling_rate(struct gov_attr_set *attr_set, const char *buf,
 			    size_t count)
 {
+	struct dbs_data *dbs_data = to_dbs_data(attr_set);
 	struct policy_dbs_info *policy_dbs;
 	unsigned int rate;
 	int ret;
@@ -59,7 +60,7 @@ ssize_t store_sampling_rate(struct dbs_d
 	 * We are operating under dbs_data->mutex and so the list and its
 	 * entries can't be freed concurrently.
 	 */
-	list_for_each_entry(policy_dbs, &dbs_data->policy_dbs_list, list) {
+	list_for_each_entry(policy_dbs, &attr_set->policy_list, list) {
 		mutex_lock(&policy_dbs->timer_mutex);
 		/*
 		 * On 32-bit architectures this may race with the
@@ -96,7 +97,7 @@ void gov_update_cpu_data(struct dbs_data
 {
 	struct policy_dbs_info *policy_dbs;
 
-	list_for_each_entry(policy_dbs, &dbs_data->policy_dbs_list, list) {
+	list_for_each_entry(policy_dbs, &dbs_data->attr_set.policy_list, list) {
 		unsigned int j;
 
 		for_each_cpu(j, policy_dbs->policy->cpus) {
@@ -111,9 +112,9 @@ void gov_update_cpu_data(struct dbs_data
 }
 EXPORT_SYMBOL_GPL(gov_update_cpu_data);
 
-static inline struct dbs_data *to_dbs_data(struct kobject *kobj)
+static inline struct gov_attr_set *to_gov_attr_set(struct kobject *kobj)
 {
-	return container_of(kobj, struct dbs_data, kobj);
+	return container_of(kobj, struct gov_attr_set, kobj);
 }
 
 static inline struct governor_attr *to_gov_attr(struct attribute *attr)
@@ -124,25 +125,24 @@ static inline struct governor_attr *to_g
 static ssize_t governor_show(struct kobject *kobj, struct attribute *attr,
 			     char *buf)
 {
-	struct dbs_data *dbs_data = to_dbs_data(kobj);
 	struct governor_attr *gattr = to_gov_attr(attr);
 
-	return gattr->show(dbs_data, buf);
+	return gattr->show(to_gov_attr_set(kobj), buf);
 }
 
 static ssize_t governor_store(struct kobject *kobj, struct attribute *attr,
 			      const char *buf, size_t count)
 {
-	struct dbs_data *dbs_data = to_dbs_data(kobj);
+	struct gov_attr_set *attr_set = to_gov_attr_set(kobj);
 	struct governor_attr *gattr = to_gov_attr(attr);
 	int ret = -EBUSY;
 
-	mutex_lock(&dbs_data->mutex);
+	mutex_lock(&attr_set->update_lock);
 
-	if (dbs_data->usage_count)
-		ret = gattr->store(dbs_data, buf, count);
+	if (attr_set->usage_count)
+		ret = gattr->store(attr_set, buf, count);
 
-	mutex_unlock(&dbs_data->mutex);
+	mutex_unlock(&attr_set->update_lock);
 
 	return ret;
 }
@@ -424,6 +424,41 @@ static void free_policy_dbs_info(struct
 	gov->free(policy_dbs);
 }
 
+static void gov_attr_set_init(struct gov_attr_set *attr_set,
+			      struct list_head *list_node)
+{
+	INIT_LIST_HEAD(&attr_set->policy_list);
+	mutex_init(&attr_set->update_lock);
+	attr_set->usage_count = 1;
+	list_add(list_node, &attr_set->policy_list);
+}
+
+static void gov_attr_set_get(struct gov_attr_set *attr_set,
+			     struct list_head *list_node)
+{
+	mutex_lock(&attr_set->update_lock);
+	attr_set->usage_count++;
+	list_add(list_node, &attr_set->policy_list);
+	mutex_unlock(&attr_set->update_lock);
+}
+
+static unsigned int gov_attr_set_put(struct gov_attr_set *attr_set,
+				     struct list_head *list_node)
+{
+	unsigned int count;
+
+	mutex_lock(&attr_set->update_lock);
+	list_del(list_node);
+	count = --attr_set->usage_count;
+	mutex_unlock(&attr_set->update_lock);
+	if (count)
+		return count;
+
+	kobject_put(&attr_set->kobj);
+	mutex_destroy(&attr_set->update_lock);
+	return 0;
+}
+
 static int cpufreq_governor_init(struct cpufreq_policy *policy)
 {
 	struct dbs_governor *gov = dbs_governor_of(policy);
@@ -452,10 +487,7 @@ static int cpufreq_governor_init(struct
 		policy_dbs->dbs_data = dbs_data;
 		policy->governor_data = policy_dbs;
 
-		mutex_lock(&dbs_data->mutex);
-		dbs_data->usage_count++;
-		list_add(&policy_dbs->list, &dbs_data->policy_dbs_list);
-		mutex_unlock(&dbs_data->mutex);
+		gov_attr_set_get(&dbs_data->attr_set, &policy_dbs->list);
 		goto out;
 	}
 
@@ -465,8 +497,7 @@ static int cpufreq_governor_init(struct
 		goto free_policy_dbs_info;
 	}
 
-	INIT_LIST_HEAD(&dbs_data->policy_dbs_list);
-	mutex_init(&dbs_data->mutex);
+	gov_attr_set_init(&dbs_data->attr_set, &policy_dbs->list);
 
 	ret = gov->init(dbs_data, !policy->governor->initialized);
 	if (ret)
@@ -486,14 +517,11 @@ static int cpufreq_governor_init(struct
 	if (!have_governor_per_policy())
 		gov->gdbs_data = dbs_data;
 
-	policy->governor_data = policy_dbs;
-
 	policy_dbs->dbs_data = dbs_data;
-	dbs_data->usage_count = 1;
-	list_add(&policy_dbs->list, &dbs_data->policy_dbs_list);
+	policy->governor_data = policy_dbs;
 
 	gov->kobj_type.sysfs_ops = &governor_sysfs_ops;
-	ret = kobject_init_and_add(&dbs_data->kobj, &gov->kobj_type,
+	ret = kobject_init_and_add(&dbs_data->attr_set.kobj, &gov->kobj_type,
 				   get_governor_parent_kobj(policy),
 				   "%s", gov->gov.name);
 	if (!ret)
@@ -522,29 +550,21 @@ static int cpufreq_governor_exit(struct
 	struct dbs_governor *gov = dbs_governor_of(policy);
 	struct policy_dbs_info *policy_dbs = policy->governor_data;
 	struct dbs_data *dbs_data = policy_dbs->dbs_data;
-	int count;
+	unsigned int count;
 
 	/* Protect gov->gdbs_data against concurrent updates. */
 	mutex_lock(&gov_dbs_data_mutex);
 
-	mutex_lock(&dbs_data->mutex);
-	list_del(&policy_dbs->list);
-	count = --dbs_data->usage_count;
-	mutex_unlock(&dbs_data->mutex);
+	count = gov_attr_set_put(&dbs_data->attr_set, &policy_dbs->list);
 
-	if (!count) {
-		kobject_put(&dbs_data->kobj);
-
-		policy->governor_data = NULL;
+	policy->governor_data = NULL;
 
+	if (!count) {
 		if (!have_governor_per_policy())
 			gov->gdbs_data = NULL;
 
 		gov->exit(dbs_data, policy->governor->initialized == 1);
-		mutex_destroy(&dbs_data->mutex);
 		kfree(dbs_data);
-	} else {
-		policy->governor_data = NULL;
 	}
 
 	free_policy_dbs_info(policy_dbs, gov);
Index: linux-pm/drivers/cpufreq/cpufreq_ondemand.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_ondemand.c
+++ linux-pm/drivers/cpufreq/cpufreq_ondemand.c
@@ -207,9 +207,10 @@ static unsigned int od_dbs_timer(struct
 /************************** sysfs interface ************************/
 static struct dbs_governor od_dbs_gov;
 
-static ssize_t store_io_is_busy(struct dbs_data *dbs_data, const char *buf,
-		size_t count)
+static ssize_t store_io_is_busy(struct gov_attr_set *attr_set, const char *buf,
+				size_t count)
 {
+	struct dbs_data *dbs_data = to_dbs_data(attr_set);
 	unsigned int input;
 	int ret;
 
@@ -224,9 +225,10 @@ static ssize_t store_io_is_busy(struct d
 	return count;
 }
 
-static ssize_t store_up_threshold(struct dbs_data *dbs_data, const char *buf,
-		size_t count)
+static ssize_t store_up_threshold(struct gov_attr_set *attr_set,
+				  const char *buf, size_t count)
 {
+	struct dbs_data *dbs_data = to_dbs_data(attr_set);
 	unsigned int input;
 	int ret;
 	ret = sscanf(buf, "%u", &input);
@@ -240,9 +242,10 @@ static ssize_t store_up_threshold(struct
 	return count;
 }
 
-static ssize_t store_sampling_down_factor(struct dbs_data *dbs_data,
-		const char *buf, size_t count)
+static ssize_t store_sampling_down_factor(struct gov_attr_set *attr_set,
+					  const char *buf, size_t count)
 {
+	struct dbs_data *dbs_data = to_dbs_data(attr_set);
 	struct policy_dbs_info *policy_dbs;
 	unsigned int input;
 	int ret;
@@ -254,7 +257,7 @@ static ssize_t store_sampling_down_facto
 	dbs_data->sampling_down_factor = input;
 
 	/* Reset down sampling multiplier in case it was active */
-	list_for_each_entry(policy_dbs, &dbs_data->policy_dbs_list, list) {
+	list_for_each_entry(policy_dbs, &attr_set->policy_list, list) {
 		/*
 		 * Doing this without locking might lead to using different
 		 * rate_mult values in od_update() and od_dbs_timer().
@@ -267,9 +270,10 @@ static ssize_t store_sampling_down_facto
 	return count;
 }
 
-static ssize_t store_ignore_nice_load(struct dbs_data *dbs_data,
-		const char *buf, size_t count)
+static ssize_t store_ignore_nice_load(struct gov_attr_set *attr_set,
+				      const char *buf, size_t count)
 {
+	struct dbs_data *dbs_data = to_dbs_data(attr_set);
 	unsigned int input;
 	int ret;
 
@@ -291,9 +295,10 @@ static ssize_t store_ignore_nice_load(st
 	return count;
 }
 
-static ssize_t store_powersave_bias(struct dbs_data *dbs_data, const char *buf,
-		size_t count)
+static ssize_t store_powersave_bias(struct gov_attr_set *attr_set,
+				    const char *buf, size_t count)
 {
+	struct dbs_data *dbs_data = to_dbs_data(attr_set);
 	struct od_dbs_tuners *od_tuners = dbs_data->tuners;
 	struct policy_dbs_info *policy_dbs;
 	unsigned int input;
@@ -308,7 +313,7 @@ static ssize_t store_powersave_bias(stru
 
 	od_tuners->powersave_bias = input;
 
-	list_for_each_entry(policy_dbs, &dbs_data->policy_dbs_list, list)
+	list_for_each_entry(policy_dbs, &attr_set->policy_list, list)
 		ondemand_powersave_bias_init(policy_dbs->policy);
 
 	return count;
Index: linux-pm/drivers/cpufreq/cpufreq_conservative.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_conservative.c
+++ linux-pm/drivers/cpufreq/cpufreq_conservative.c
@@ -129,9 +129,10 @@ static struct notifier_block cs_cpufreq_
 /************************** sysfs interface ************************/
 static struct dbs_governor cs_dbs_gov;
 
-static ssize_t store_sampling_down_factor(struct dbs_data *dbs_data,
-		const char *buf, size_t count)
+static ssize_t store_sampling_down_factor(struct gov_attr_set *attr_set,
+					  const char *buf, size_t count)
 {
+	struct dbs_data *dbs_data = to_dbs_data(attr_set);
 	unsigned int input;
 	int ret;
 	ret = sscanf(buf, "%u", &input);
@@ -143,9 +144,10 @@ static ssize_t store_sampling_down_facto
 	return count;
 }
 
-static ssize_t store_up_threshold(struct dbs_data *dbs_data, const char *buf,
-		size_t count)
+static ssize_t store_up_threshold(struct gov_attr_set *attr_set,
+				  const char *buf, size_t count)
 {
+	struct dbs_data *dbs_data = to_dbs_data(attr_set);
 	struct cs_dbs_tuners *cs_tuners = dbs_data->tuners;
 	unsigned int input;
 	int ret;
@@ -158,9 +160,10 @@ static ssize_t store_up_threshold(struct
 	return count;
 }
 
-static ssize_t store_down_threshold(struct dbs_data *dbs_data, const char *buf,
-		size_t count)
+static ssize_t store_down_threshold(struct gov_attr_set *attr_set,
+				    const char *buf, size_t count)
 {
+	struct dbs_data *dbs_data = to_dbs_data(attr_set);
 	struct cs_dbs_tuners *cs_tuners = dbs_data->tuners;
 	unsigned int input;
 	int ret;
@@ -175,9 +178,10 @@ static ssize_t store_down_threshold(stru
 	return count;
 }
 
-static ssize_t store_ignore_nice_load(struct dbs_data *dbs_data,
-		const char *buf, size_t count)
+static ssize_t store_ignore_nice_load(struct gov_attr_set *attr_set,
+				      const char *buf, size_t count)
 {
+	struct dbs_data *dbs_data = to_dbs_data(attr_set);
 	unsigned int input;
 	int ret;
 
@@ -199,9 +203,10 @@ static ssize_t store_ignore_nice_load(st
 	return count;
 }
 
-static ssize_t store_freq_step(struct dbs_data *dbs_data, const char *buf,
-		size_t count)
+static ssize_t store_freq_step(struct gov_attr_set *attr_set, const char *buf,
+			       size_t count)
 {
+	struct dbs_data *dbs_data = to_dbs_data(attr_set);
 	struct cs_dbs_tuners *cs_tuners = dbs_data->tuners;
 	unsigned int input;
 	int ret;

[toc] | [prev] | [next] | [standalone]


#1349860 — Re: [PATCH v2 3/10] cpufreq: governor: New data type for management part of dbs_data

FromViresh Kumar <viresh.kumar@linaro.org>
Date2016-03-04 07:00 +0100
SubjectRe: [PATCH v2 3/10] cpufreq: governor: New data type for management part of dbs_data
Message-ID<r8QWl-20V-9@gated-at.bofh.it>
In reply to#1349817
On 04-03-16, 04:01, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> In addition to fields representing governor tunables, struct dbs_data
> contains some fields needed for the management of objects of that
> type.  As it turns out, that part of struct dbs_data may be shared
> with (future) governors that won't use the common code used by
> "ondemand" and "conservative", so move it to a separate struct type
> and modify the code using struct dbs_data to follow.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> 
> Changes from the previous version:
> - The new data type is called gov_attr_set now (instead of gov_tunables)
>   and some variable names etc have been changed to follow.

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

[toc] | [prev] | [next] | [standalone]


#1349819 — [PATCH v2 7/10] cpufreq: Rework the scheduler hooks for triggering updates

From"Rafael J. Wysocki" <rjw@rjwysocki.net>
Date2016-03-04 04:40 +0100
Subject[PATCH v2 7/10] cpufreq: Rework the scheduler hooks for triggering updates
Message-ID<r8OKS-B1-13@gated-at.bofh.it>
In reply to#1349813
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Commit fe7034338ba0 (cpufreq: Add mechanism for registering
utilization update callbacks) added cpufreq_update_util() to be
called by the scheduler (from the CFS part) on utilization updates.
The goal was to allow CFS to pass utilization information to cpufreq
and to trigger it to evaluate the frequency/voltage configuration
(P-state) of every CPU on a regular basis.

However, the last two arguments of that function are never used by
the current code, so CFS might simply call cpufreq_trigger_update()
instead of it (like the RT and DL sched classes).

For this reason, drop the last two arguments of cpufreq_update_util(),
rename it to cpufreq_trigger_update() and modify CFS to call it.

Moreover, since the utilization is not involved in that now, rename
data types, functions and variables related to cpufreq_trigger_update()
to reflect that (eg. struct update_util_data becomes struct
freq_update_hook and so on).

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

New patch.

Not strictly necessary, but I like the new names better. :-)

---
 drivers/cpufreq/cpufreq.c          |   52 +++++++++++++++++++++----------------
 drivers/cpufreq/cpufreq_governor.c |   25 ++++++++---------
 drivers/cpufreq/cpufreq_governor.h |    2 -
 drivers/cpufreq/intel_pstate.c     |   15 ++++------
 include/linux/cpufreq.h            |   32 ++--------------------
 kernel/sched/deadline.c            |    2 -
 kernel/sched/fair.c                |   13 +--------
 kernel/sched/rt.c                  |    2 -
 8 files changed, 58 insertions(+), 85 deletions(-)

Index: linux-pm/drivers/cpufreq/cpufreq.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq.c
+++ linux-pm/drivers/cpufreq/cpufreq.c
@@ -65,57 +65,65 @@ static struct cpufreq_driver *cpufreq_dr
 static DEFINE_PER_CPU(struct cpufreq_policy *, cpufreq_cpu_data);
 static DEFINE_RWLOCK(cpufreq_driver_lock);
 
-static DEFINE_PER_CPU(struct update_util_data *, cpufreq_update_util_data);
+static DEFINE_PER_CPU(struct freq_update_hook *, cpufreq_freq_update_hook);
 
 /**
- * cpufreq_set_update_util_data - Populate the CPU's update_util_data pointer.
+ * cpufreq_set_freq_update_hook - Populate the CPU's freq_update_hook pointer.
  * @cpu: The CPU to set the pointer for.
- * @data: New pointer value.
+ * @hook: New pointer value.
  *
- * Set and publish the update_util_data pointer for the given CPU.  That pointer
- * points to a struct update_util_data object containing a callback function
- * to call from cpufreq_update_util().  That function will be called from an RCU
- * read-side critical section, so it must not sleep.
+ * Set and publish the freq_update_hook pointer for the given CPU.  That pointer
+ * points to a struct freq_update_hook object containing a callback function
+ * to call from cpufreq_trigger_update().  That function will be called from
+ * an RCU read-side critical section, so it must not sleep.
  *
  * Callers must use RCU-sched callbacks to free any memory that might be
  * accessed via the old update_util_data pointer or invoke synchronize_sched()
  * right after this function to avoid use-after-free.
  */
-void cpufreq_set_update_util_data(int cpu, struct update_util_data *data)
+void cpufreq_set_freq_update_hook(int cpu, struct freq_update_hook *hook)
 {
-	if (WARN_ON(data && !data->func))
+	if (WARN_ON(hook && !hook->func))
 		return;
 
-	rcu_assign_pointer(per_cpu(cpufreq_update_util_data, cpu), data);
+	rcu_assign_pointer(per_cpu(cpufreq_freq_update_hook, cpu), hook);
 }
-EXPORT_SYMBOL_GPL(cpufreq_set_update_util_data);
+EXPORT_SYMBOL_GPL(cpufreq_set_freq_update_hook);
 
 /**
- * cpufreq_update_util - Take a note about CPU utilization changes.
+ * cpufreq_trigger_update - Trigger CPU performance state evaluation if needed.
  * @time: Current time.
- * @util: Current utilization.
- * @max: Utilization ceiling.
  *
- * This function is called by the scheduler on every invocation of
- * update_load_avg() on the CPU whose utilization is being updated.
+ * The way cpufreq is currently arranged requires it to evaluate the CPU
+ * performance state (frequency/voltage) on a regular basis.  To facilitate
+ * that, this function is called by update_load_avg() in CFS when executed for
+ * the current CPU's runqueue.
  *
- * It can only be called from RCU-sched read-side critical sections.
+ * However, this isn't sufficient to prevent the CPU from being stuck in a
+ * completely inadequate performance level for too long, because the calls
+ * from CFS will not be made if RT or deadline tasks are active all the time
+ * (or there are RT and DL tasks only).
+ *
+ * As a workaround for that issue, this function is called by the RT and DL
+ * sched classes to trigger extra cpufreq updates to prevent it from stalling,
+ * but that really is a band-aid.  Going forward it should be replaced with
+ * solutions targeted more specifically at RT and DL tasks.
  */
-void cpufreq_update_util(u64 time, unsigned long util, unsigned long max)
+void cpufreq_trigger_update(u64 time)
 {
-	struct update_util_data *data;
+	struct freq_update_hook *hook;
 
 #ifdef CONFIG_LOCKDEP
 	WARN_ON(debug_locks && !rcu_read_lock_sched_held());
 #endif
 
-	data = rcu_dereference_sched(*this_cpu_ptr(&cpufreq_update_util_data));
+	hook = rcu_dereference_sched(*this_cpu_ptr(&cpufreq_freq_update_hook));
 	/*
 	 * If this isn't inside of an RCU-sched read-side critical section, data
 	 * may become NULL after the check below.
 	 */
-	if (data)
-		data->func(data, time, util, max);
+	if (hook)
+		hook->func(hook, time);
 }
 
 /* Flag to suspend/resume CPUFreq governors */
Index: linux-pm/include/linux/cpufreq.h
===================================================================
--- linux-pm.orig/include/linux/cpufreq.h
+++ linux-pm/include/linux/cpufreq.h
@@ -146,35 +146,13 @@ static inline bool policy_is_shared(stru
 extern struct kobject *cpufreq_global_kobject;
 
 #ifdef CONFIG_CPU_FREQ
-void cpufreq_update_util(u64 time, unsigned long util, unsigned long max);
+void cpufreq_trigger_update(u64 time);
 
-/**
- * cpufreq_trigger_update - Trigger CPU performance state evaluation if needed.
- * @time: Current time.
- *
- * The way cpufreq is currently arranged requires it to evaluate the CPU
- * performance state (frequency/voltage) on a regular basis to prevent it from
- * being stuck in a completely inadequate performance level for too long.
- * That is not guaranteed to happen if the updates are only triggered from CFS,
- * though, because they may not be coming in if RT or deadline tasks are active
- * all the time (or there are RT and DL tasks only).
- *
- * As a workaround for that issue, this function is called by the RT and DL
- * sched classes to trigger extra cpufreq updates to prevent it from stalling,
- * but that really is a band-aid.  Going forward it should be replaced with
- * solutions targeted more specifically at RT and DL tasks.
- */
-static inline void cpufreq_trigger_update(u64 time)
-{
-	cpufreq_update_util(time, ULONG_MAX, 0);
-}
-
-struct update_util_data {
-	void (*func)(struct update_util_data *data,
-		     u64 time, unsigned long util, unsigned long max);
+struct freq_update_hook {
+	void (*func)(struct freq_update_hook *hook, u64 time);
 };
 
-void cpufreq_set_update_util_data(int cpu, struct update_util_data *data);
+void cpufreq_set_freq_update_hook(int cpu, struct freq_update_hook *hook);
 
 unsigned int cpufreq_get(unsigned int cpu);
 unsigned int cpufreq_quick_get(unsigned int cpu);
@@ -187,8 +165,6 @@ int cpufreq_update_policy(unsigned int c
 bool have_governor_per_policy(void);
 struct kobject *get_governor_parent_kobj(struct cpufreq_policy *policy);
 #else
-static inline void cpufreq_update_util(u64 time, unsigned long util,
-				       unsigned long max) {}
 static inline void cpufreq_trigger_update(u64 time) {}
 
 static inline unsigned int cpufreq_get(unsigned int cpu)
Index: linux-pm/drivers/cpufreq/cpufreq_governor.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_governor.c
+++ linux-pm/drivers/cpufreq/cpufreq_governor.c
@@ -62,10 +62,10 @@ ssize_t store_sampling_rate(struct dbs_d
 		mutex_lock(&policy_dbs->timer_mutex);
 		/*
 		 * On 32-bit architectures this may race with the
-		 * sample_delay_ns read in dbs_update_util_handler(), but that
+		 * sample_delay_ns read in dbs_freq_update_handler(), but that
 		 * really doesn't matter.  If the read returns a value that's
 		 * too big, the sample will be skipped, but the next invocation
-		 * of dbs_update_util_handler() (when the update has been
+		 * of dbs_freq_update_handler() (when the update has been
 		 * completed) will take a sample.
 		 *
 		 * If this runs in parallel with dbs_work_handler(), we may end
@@ -257,7 +257,7 @@ unsigned int dbs_update(struct cpufreq_p
 }
 EXPORT_SYMBOL_GPL(dbs_update);
 
-static void gov_set_update_util(struct policy_dbs_info *policy_dbs,
+static void gov_set_freq_update_hooks(struct policy_dbs_info *policy_dbs,
 				unsigned int delay_us)
 {
 	struct cpufreq_policy *policy = policy_dbs->policy;
@@ -269,16 +269,16 @@ static void gov_set_update_util(struct p
 	for_each_cpu(cpu, policy->cpus) {
 		struct cpu_dbs_info *cdbs = &per_cpu(cpu_dbs, cpu);
 
-		cpufreq_set_update_util_data(cpu, &cdbs->update_util);
+		cpufreq_set_freq_update_hook(cpu, &cdbs->update_hook);
 	}
 }
 
-static inline void gov_clear_update_util(struct cpufreq_policy *policy)
+static inline void gov_clear_freq_update_hooks(struct cpufreq_policy *policy)
 {
 	int i;
 
 	for_each_cpu(i, policy->cpus)
-		cpufreq_set_update_util_data(i, NULL);
+		cpufreq_set_freq_update_hook(i, NULL);
 
 	synchronize_sched();
 }
@@ -287,7 +287,7 @@ static void gov_cancel_work(struct cpufr
 {
 	struct policy_dbs_info *policy_dbs = policy->governor_data;
 
-	gov_clear_update_util(policy_dbs->policy);
+	gov_clear_freq_update_hooks(policy_dbs->policy);
 	irq_work_sync(&policy_dbs->irq_work);
 	cancel_work_sync(&policy_dbs->work);
 	atomic_set(&policy_dbs->work_count, 0);
@@ -331,10 +331,9 @@ static void dbs_irq_work(struct irq_work
 	schedule_work(&policy_dbs->work);
 }
 
-static void dbs_update_util_handler(struct update_util_data *data, u64 time,
-				    unsigned long util, unsigned long max)
+static void dbs_freq_update_handler(struct freq_update_hook *hook, u64 time)
 {
-	struct cpu_dbs_info *cdbs = container_of(data, struct cpu_dbs_info, update_util);
+	struct cpu_dbs_info *cdbs = container_of(hook, struct cpu_dbs_info, update_hook);
 	struct policy_dbs_info *policy_dbs = cdbs->policy_dbs;
 	u64 delta_ns, lst;
 
@@ -403,7 +402,7 @@ static struct policy_dbs_info *alloc_pol
 		struct cpu_dbs_info *j_cdbs = &per_cpu(cpu_dbs, j);
 
 		j_cdbs->policy_dbs = policy_dbs;
-		j_cdbs->update_util.func = dbs_update_util_handler;
+		j_cdbs->update_hook.func = dbs_freq_update_handler;
 	}
 	return policy_dbs;
 }
@@ -419,7 +418,7 @@ static void free_policy_dbs_info(struct
 		struct cpu_dbs_info *j_cdbs = &per_cpu(cpu_dbs, j);
 
 		j_cdbs->policy_dbs = NULL;
-		j_cdbs->update_util.func = NULL;
+		j_cdbs->update_hook.func = NULL;
 	}
 	gov->free(policy_dbs);
 }
@@ -586,7 +585,7 @@ static int cpufreq_governor_start(struct
 
 	gov->start(policy);
 
-	gov_set_update_util(policy_dbs, sampling_rate);
+	gov_set_freq_update_hooks(policy_dbs, sampling_rate);
 	return 0;
 }
 
Index: linux-pm/drivers/cpufreq/cpufreq_governor.h
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_governor.h
+++ linux-pm/drivers/cpufreq/cpufreq_governor.h
@@ -144,7 +144,7 @@ struct cpu_dbs_info {
 	 * wake-up from idle.
 	 */
 	unsigned int prev_load;
-	struct update_util_data update_util;
+	struct freq_update_hook update_hook;
 	struct policy_dbs_info *policy_dbs;
 };
 
Index: linux-pm/drivers/cpufreq/intel_pstate.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/intel_pstate.c
+++ linux-pm/drivers/cpufreq/intel_pstate.c
@@ -103,7 +103,7 @@ struct _pid {
 struct cpudata {
 	int cpu;
 
-	struct update_util_data update_util;
+	struct freq_update_hook update_hook;
 
 	struct pstate_data pstate;
 	struct vid_data vid;
@@ -1019,10 +1019,9 @@ static inline void intel_pstate_adjust_b
 		sample->freq);
 }
 
-static void intel_pstate_update_util(struct update_util_data *data, u64 time,
-				     unsigned long util, unsigned long max)
+static void intel_pstate_freq_update(struct freq_update_hook *hook, u64 time)
 {
-	struct cpudata *cpu = container_of(data, struct cpudata, update_util);
+	struct cpudata *cpu = container_of(hook, struct cpudata, update_hook);
 	u64 delta_ns = time - cpu->sample.time;
 
 	if ((s64)delta_ns >= pid_params.sample_rate_ns) {
@@ -1088,8 +1087,8 @@ static int intel_pstate_init_cpu(unsigne
 	intel_pstate_busy_pid_reset(cpu);
 	intel_pstate_sample(cpu, 0);
 
-	cpu->update_util.func = intel_pstate_update_util;
-	cpufreq_set_update_util_data(cpunum, &cpu->update_util);
+	cpu->update_hook.func = intel_pstate_freq_update;
+	cpufreq_set_freq_update_hook(cpunum, &cpu->update_hook);
 
 	pr_debug("intel_pstate: controlling: cpu %d\n", cpunum);
 
@@ -1173,7 +1172,7 @@ static void intel_pstate_stop_cpu(struct
 
 	pr_debug("intel_pstate: CPU %d exiting\n", cpu_num);
 
-	cpufreq_set_update_util_data(cpu_num, NULL);
+	cpufreq_set_freq_update_hook(cpu_num, NULL);
 	synchronize_sched();
 
 	if (hwp_active)
@@ -1441,7 +1440,7 @@ out:
 	get_online_cpus();
 	for_each_online_cpu(cpu) {
 		if (all_cpu_data[cpu]) {
-			cpufreq_set_update_util_data(cpu, NULL);
+			cpufreq_set_freq_update_hook(cpu, NULL);
 			synchronize_sched();
 			kfree(all_cpu_data[cpu]);
 		}
Index: linux-pm/kernel/sched/fair.c
===================================================================
--- linux-pm.orig/kernel/sched/fair.c
+++ linux-pm/kernel/sched/fair.c
@@ -2839,8 +2839,6 @@ static inline void update_load_avg(struc
 		update_tg_load_avg(cfs_rq, 0);
 
 	if (cpu == smp_processor_id() && &rq->cfs == cfs_rq) {
-		unsigned long max = rq->cpu_capacity_orig;
-
 		/*
 		 * There are a few boundary cases this might miss but it should
 		 * get called often enough that that should (hopefully) not be
@@ -2849,16 +2847,9 @@ static inline void update_load_avg(struc
 		 * the next tick/schedule should update.
 		 *
 		 * It will not get called when we go idle, because the idle
-		 * thread is a different class (!fair), nor will the utilization
-		 * number include things like RT tasks.
-		 *
-		 * As is, the util number is not freq-invariant (we'd have to
-		 * implement arch_scale_freq_capacity() for that).
-		 *
-		 * See cpu_util().
+		 * thread is a different class (!fair).
 		 */
-		cpufreq_update_util(rq_clock(rq),
-				    min(cfs_rq->avg.util_avg, max), max);
+		cpufreq_trigger_update(rq_clock(rq));
 	}
 }
 
Index: linux-pm/kernel/sched/deadline.c
===================================================================
--- linux-pm.orig/kernel/sched/deadline.c
+++ linux-pm/kernel/sched/deadline.c
@@ -726,7 +726,7 @@ static void update_curr_dl(struct rq *rq
 	if (!dl_task(curr) || !on_dl_rq(dl_se))
 		return;
 
-	/* Kick cpufreq (see the comment in linux/cpufreq.h). */
+	/* Kick cpufreq (see the comment in drivers/cpufreq/cpufreq.c). */
 	if (cpu_of(rq) == smp_processor_id())
 		cpufreq_trigger_update(rq_clock(rq));
 
Index: linux-pm/kernel/sched/rt.c
===================================================================
--- linux-pm.orig/kernel/sched/rt.c
+++ linux-pm/kernel/sched/rt.c
@@ -945,7 +945,7 @@ static void update_curr_rt(struct rq *rq
 	if (curr->sched_class != &rt_sched_class)
 		return;
 
-	/* Kick cpufreq (see the comment in linux/cpufreq.h). */
+	/* Kick cpufreq (see the comment in drivers/cpufreq/cpufreq.c). */
 	if (cpu_of(rq) == smp_processor_id())
 		cpufreq_trigger_update(rq_clock(rq));
 

[toc] | [prev] | [next] | [standalone]


#1349820 — [PATCH v2 4/10] cpufreq: governor: Move abstract gov_attr_set code to seperate file

From"Rafael J. Wysocki" <rjw@rjwysocki.net>
Date2016-03-04 04:40 +0100
Subject[PATCH v2 4/10] cpufreq: governor: Move abstract gov_attr_set code to seperate file
Message-ID<r8OKS-B1-17@gated-at.bofh.it>
In reply to#1349813
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Move abstract code related to struct gov_attr_set to a separate (new)
file so it can be shared with (future) goverernors that won't share
more code with "ondemand" and "conservative".

No intentional functional changes.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

Changes from the previous version:
- Different name of the new file.
- Different name of the new Kconfig symbol.

---
 drivers/cpufreq/Kconfig                     |    4 +
 drivers/cpufreq/Makefile                    |    1 
 drivers/cpufreq/cpufreq_governor.c          |   82 ---------------------------
 drivers/cpufreq/cpufreq_governor.h          |    6 ++
 drivers/cpufreq/cpufreq_governor_attr_set.c |   84 ++++++++++++++++++++++++++++
 5 files changed, 95 insertions(+), 82 deletions(-)

Index: linux-pm/drivers/cpufreq/Kconfig
===================================================================
--- linux-pm.orig/drivers/cpufreq/Kconfig
+++ linux-pm/drivers/cpufreq/Kconfig
@@ -18,7 +18,11 @@ config CPU_FREQ
 
 if CPU_FREQ
 
+config CPU_FREQ_GOV_ATTR_SET
+	bool
+
 config CPU_FREQ_GOV_COMMON
+	select CPU_FREQ_GOV_ATTR_SET
 	select IRQ_WORK
 	bool
 
Index: linux-pm/drivers/cpufreq/Makefile
===================================================================
--- linux-pm.orig/drivers/cpufreq/Makefile
+++ linux-pm/drivers/cpufreq/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_CPU_FREQ_GOV_USERSPACE)	+=
 obj-$(CONFIG_CPU_FREQ_GOV_ONDEMAND)	+= cpufreq_ondemand.o
 obj-$(CONFIG_CPU_FREQ_GOV_CONSERVATIVE)	+= cpufreq_conservative.o
 obj-$(CONFIG_CPU_FREQ_GOV_COMMON)		+= cpufreq_governor.o
+obj-$(CONFIG_CPU_FREQ_GOV_ATTR_SET)	+= cpufreq_governor_attr_set.o
 
 obj-$(CONFIG_CPUFREQ_DT)		+= cpufreq-dt.o
 
Index: linux-pm/drivers/cpufreq/cpufreq_governor.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_governor.c
+++ linux-pm/drivers/cpufreq/cpufreq_governor.c
@@ -112,53 +112,6 @@ void gov_update_cpu_data(struct dbs_data
 }
 EXPORT_SYMBOL_GPL(gov_update_cpu_data);
 
-static inline struct gov_attr_set *to_gov_attr_set(struct kobject *kobj)
-{
-	return container_of(kobj, struct gov_attr_set, kobj);
-}
-
-static inline struct governor_attr *to_gov_attr(struct attribute *attr)
-{
-	return container_of(attr, struct governor_attr, attr);
-}
-
-static ssize_t governor_show(struct kobject *kobj, struct attribute *attr,
-			     char *buf)
-{
-	struct governor_attr *gattr = to_gov_attr(attr);
-
-	return gattr->show(to_gov_attr_set(kobj), buf);
-}
-
-static ssize_t governor_store(struct kobject *kobj, struct attribute *attr,
-			      const char *buf, size_t count)
-{
-	struct gov_attr_set *attr_set = to_gov_attr_set(kobj);
-	struct governor_attr *gattr = to_gov_attr(attr);
-	int ret = -EBUSY;
-
-	mutex_lock(&attr_set->update_lock);
-
-	if (attr_set->usage_count)
-		ret = gattr->store(attr_set, buf, count);
-
-	mutex_unlock(&attr_set->update_lock);
-
-	return ret;
-}
-
-/*
- * Sysfs Ops for accessing governor attributes.
- *
- * All show/store invocations for governor specific sysfs attributes, will first
- * call the below show/store callbacks and the attribute specific callback will
- * be called from within it.
- */
-static const struct sysfs_ops governor_sysfs_ops = {
-	.show	= governor_show,
-	.store	= governor_store,
-};
-
 unsigned int dbs_update(struct cpufreq_policy *policy)
 {
 	struct policy_dbs_info *policy_dbs = policy->governor_data;
@@ -424,41 +377,6 @@ static void free_policy_dbs_info(struct
 	gov->free(policy_dbs);
 }
 
-static void gov_attr_set_init(struct gov_attr_set *attr_set,
-			      struct list_head *list_node)
-{
-	INIT_LIST_HEAD(&attr_set->policy_list);
-	mutex_init(&attr_set->update_lock);
-	attr_set->usage_count = 1;
-	list_add(list_node, &attr_set->policy_list);
-}
-
-static void gov_attr_set_get(struct gov_attr_set *attr_set,
-			     struct list_head *list_node)
-{
-	mutex_lock(&attr_set->update_lock);
-	attr_set->usage_count++;
-	list_add(list_node, &attr_set->policy_list);
-	mutex_unlock(&attr_set->update_lock);
-}
-
-static unsigned int gov_attr_set_put(struct gov_attr_set *attr_set,
-				     struct list_head *list_node)
-{
-	unsigned int count;
-
-	mutex_lock(&attr_set->update_lock);
-	list_del(list_node);
-	count = --attr_set->usage_count;
-	mutex_unlock(&attr_set->update_lock);
-	if (count)
-		return count;
-
-	kobject_put(&attr_set->kobj);
-	mutex_destroy(&attr_set->update_lock);
-	return 0;
-}
-
 static int cpufreq_governor_init(struct cpufreq_policy *policy)
 {
 	struct dbs_governor *gov = dbs_governor_of(policy);
Index: linux-pm/drivers/cpufreq/cpufreq_governor.h
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_governor.h
+++ linux-pm/drivers/cpufreq/cpufreq_governor.h
@@ -48,6 +48,12 @@ struct gov_attr_set {
 	int usage_count;
 };
 
+extern const struct sysfs_ops governor_sysfs_ops;
+
+void gov_attr_set_init(struct gov_attr_set *attr_set, struct list_head *list_node);
+void gov_attr_set_get(struct gov_attr_set *attr_set, struct list_head *list_node);
+unsigned int gov_attr_set_put(struct gov_attr_set *attr_set, struct list_head *list_node);
+
 /*
  * Abbreviations:
  * dbs: used as a shortform for demand based switching It helps to keep variable
Index: linux-pm/drivers/cpufreq/cpufreq_governor_attr_set.c
===================================================================
--- /dev/null
+++ linux-pm/drivers/cpufreq/cpufreq_governor_attr_set.c
@@ -0,0 +1,84 @@
+/*
+ * Abstract code for CPUFreq governor tunable sysfs attributes.
+ *
+ * Copyright (C) 2016, Intel Corporation
+ * Author: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include "cpufreq_governor.h"
+
+static inline struct gov_attr_set *to_gov_attr_set(struct kobject *kobj)
+{
+	return container_of(kobj, struct gov_attr_set, kobj);
+}
+
+static inline struct governor_attr *to_gov_attr(struct attribute *attr)
+{
+	return container_of(attr, struct governor_attr, attr);
+}
+
+static ssize_t governor_show(struct kobject *kobj, struct attribute *attr,
+			     char *buf)
+{
+	struct governor_attr *gattr = to_gov_attr(attr);
+
+	return gattr->show(to_gov_attr_set(kobj), buf);
+}
+
+static ssize_t governor_store(struct kobject *kobj, struct attribute *attr,
+			      const char *buf, size_t count)
+{
+	struct gov_attr_set *attr_set = to_gov_attr_set(kobj);
+	struct governor_attr *gattr = to_gov_attr(attr);
+	int ret;
+
+	mutex_lock(&attr_set->update_lock);
+	ret = attr_set->usage_count ? gattr->store(attr_set, buf, count) : -EBUSY;
+	mutex_unlock(&attr_set->update_lock);
+	return ret;
+}
+
+const struct sysfs_ops governor_sysfs_ops = {
+	.show	= governor_show,
+	.store	= governor_store,
+};
+EXPORT_SYMBOL_GPL(governor_sysfs_ops);
+
+void gov_attr_set_init(struct gov_attr_set *attr_set, struct list_head *list_node)
+{
+	INIT_LIST_HEAD(&attr_set->policy_list);
+	mutex_init(&attr_set->update_lock);
+	attr_set->usage_count = 1;
+	list_add(list_node, &attr_set->policy_list);
+}
+EXPORT_SYMBOL_GPL(gov_attr_set_init);
+
+void gov_attr_set_get(struct gov_attr_set *attr_set, struct list_head *list_node)
+{
+	mutex_lock(&attr_set->update_lock);
+	attr_set->usage_count++;
+	list_add(list_node, &attr_set->policy_list);
+	mutex_unlock(&attr_set->update_lock);
+}
+EXPORT_SYMBOL_GPL(gov_attr_set_get);
+
+unsigned int gov_attr_set_put(struct gov_attr_set *attr_set, struct list_head *list_node)
+{
+	unsigned int count;
+
+	mutex_lock(&attr_set->update_lock);
+	list_del(list_node);
+	count = --attr_set->usage_count;
+	mutex_unlock(&attr_set->update_lock);
+	if (count)
+		return count;
+
+	kobject_put(&attr_set->kobj);
+	mutex_destroy(&attr_set->update_lock);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(gov_attr_set_put);

[toc] | [prev] | [next] | [standalone]


#1349866 — Re: [PATCH v2 4/10] cpufreq: governor: Move abstract gov_attr_set code to seperate file

FromViresh Kumar <viresh.kumar@linaro.org>
Date2016-03-04 07:00 +0100
SubjectRe: [PATCH v2 4/10] cpufreq: governor: Move abstract gov_attr_set code to seperate file
Message-ID<r8QWm-20V-19@gated-at.bofh.it>
In reply to#1349820
On 04-03-16, 04:03, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Move abstract code related to struct gov_attr_set to a separate (new)
> file so it can be shared with (future) goverernors that won't share
> more code with "ondemand" and "conservative".
> 
> No intentional functional changes.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> 
> Changes from the previous version:
> - Different name of the new file.
> - Different name of the new Kconfig symbol.

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

[toc] | [prev] | [next] | [standalone]


#1349821 — [PATCH v2 8/10] cpufreq: Move scheduler-related code to the sched directory

From"Rafael J. Wysocki" <rjw@rjwysocki.net>
Date2016-03-04 04:40 +0100
Subject[PATCH v2 8/10] cpufreq: Move scheduler-related code to the sched directory
Message-ID<r8OKS-B1-23@gated-at.bofh.it>
In reply to#1349813
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Create cpufreq.c under kernel/sched/ and move the cpufreq code
related to the scheduler to that file.  Also move the headers
related to that code from cpufreq.h to sched.h.

No functional changes.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

New patch.

---
 drivers/cpufreq/cpufreq.c          |   61 ------------------------------
 drivers/cpufreq/cpufreq_governor.c |    1 
 drivers/cpufreq/intel_pstate.c     |    1 
 include/linux/cpufreq.h            |   10 -----
 include/linux/sched.h              |   12 ++++++
 kernel/sched/Makefile              |    1 
 kernel/sched/cpufreq.c             |   73 +++++++++++++++++++++++++++++++++++++
 7 files changed, 88 insertions(+), 71 deletions(-)

Index: linux-pm/drivers/cpufreq/cpufreq.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq.c
+++ linux-pm/drivers/cpufreq/cpufreq.c
@@ -65,67 +65,6 @@ static struct cpufreq_driver *cpufreq_dr
 static DEFINE_PER_CPU(struct cpufreq_policy *, cpufreq_cpu_data);
 static DEFINE_RWLOCK(cpufreq_driver_lock);
 
-static DEFINE_PER_CPU(struct freq_update_hook *, cpufreq_freq_update_hook);
-
-/**
- * cpufreq_set_freq_update_hook - Populate the CPU's freq_update_hook pointer.
- * @cpu: The CPU to set the pointer for.
- * @hook: New pointer value.
- *
- * Set and publish the freq_update_hook pointer for the given CPU.  That pointer
- * points to a struct freq_update_hook object containing a callback function
- * to call from cpufreq_trigger_update().  That function will be called from
- * an RCU read-side critical section, so it must not sleep.
- *
- * Callers must use RCU-sched callbacks to free any memory that might be
- * accessed via the old update_util_data pointer or invoke synchronize_sched()
- * right after this function to avoid use-after-free.
- */
-void cpufreq_set_freq_update_hook(int cpu, struct freq_update_hook *hook)
-{
-	if (WARN_ON(hook && !hook->func))
-		return;
-
-	rcu_assign_pointer(per_cpu(cpufreq_freq_update_hook, cpu), hook);
-}
-EXPORT_SYMBOL_GPL(cpufreq_set_freq_update_hook);
-
-/**
- * cpufreq_trigger_update - Trigger CPU performance state evaluation if needed.
- * @time: Current time.
- *
- * The way cpufreq is currently arranged requires it to evaluate the CPU
- * performance state (frequency/voltage) on a regular basis.  To facilitate
- * that, this function is called by update_load_avg() in CFS when executed for
- * the current CPU's runqueue.
- *
- * However, this isn't sufficient to prevent the CPU from being stuck in a
- * completely inadequate performance level for too long, because the calls
- * from CFS will not be made if RT or deadline tasks are active all the time
- * (or there are RT and DL tasks only).
- *
- * As a workaround for that issue, this function is called by the RT and DL
- * sched classes to trigger extra cpufreq updates to prevent it from stalling,
- * but that really is a band-aid.  Going forward it should be replaced with
- * solutions targeted more specifically at RT and DL tasks.
- */
-void cpufreq_trigger_update(u64 time)
-{
-	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, data
-	 * may become NULL after the check below.
-	 */
-	if (hook)
-		hook->func(hook, time);
-}
-
 /* Flag to suspend/resume CPUFreq governors */
 static bool cpufreq_suspended;
 
Index: linux-pm/drivers/cpufreq/cpufreq_governor.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_governor.c
+++ linux-pm/drivers/cpufreq/cpufreq_governor.c
@@ -18,6 +18,7 @@
 
 #include <linux/export.h>
 #include <linux/kernel_stat.h>
+#include <linux/sched.h>
 #include <linux/slab.h>
 
 #include "cpufreq_governor.h"
Index: linux-pm/drivers/cpufreq/intel_pstate.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/intel_pstate.c
+++ linux-pm/drivers/cpufreq/intel_pstate.c
@@ -21,6 +21,7 @@
 #include <linux/list.h>
 #include <linux/cpu.h>
 #include <linux/cpufreq.h>
+#include <linux/sched.h>
 #include <linux/sysfs.h>
 #include <linux/types.h>
 #include <linux/fs.h>
Index: linux-pm/include/linux/cpufreq.h
===================================================================
--- linux-pm.orig/include/linux/cpufreq.h
+++ linux-pm/include/linux/cpufreq.h
@@ -146,14 +146,6 @@ static inline bool policy_is_shared(stru
 extern struct kobject *cpufreq_global_kobject;
 
 #ifdef CONFIG_CPU_FREQ
-void cpufreq_trigger_update(u64 time);
-
-struct freq_update_hook {
-	void (*func)(struct freq_update_hook *hook, u64 time);
-};
-
-void cpufreq_set_freq_update_hook(int cpu, struct freq_update_hook *hook);
-
 unsigned int cpufreq_get(unsigned int cpu);
 unsigned int cpufreq_quick_get(unsigned int cpu);
 unsigned int cpufreq_quick_get_max(unsigned int cpu);
@@ -165,8 +157,6 @@ int cpufreq_update_policy(unsigned int c
 bool have_governor_per_policy(void);
 struct kobject *get_governor_parent_kobj(struct cpufreq_policy *policy);
 #else
-static inline void cpufreq_trigger_update(u64 time) {}
-
 static inline unsigned int cpufreq_get(unsigned int cpu)
 {
 	return 0;
Index: linux-pm/include/linux/sched.h
===================================================================
--- linux-pm.orig/include/linux/sched.h
+++ linux-pm/include/linux/sched.h
@@ -2362,6 +2362,18 @@ extern u64 scheduler_tick_max_deferment(
 static inline bool sched_can_stop_tick(void) { return false; }
 #endif
 
+#ifdef CONFIG_CPU_FREQ
+void cpufreq_trigger_update(u64 time);
+
+struct freq_update_hook {
+	void (*func)(struct freq_update_hook *hook, u64 time);
+};
+
+void cpufreq_set_freq_update_hook(int cpu, struct freq_update_hook *hook);
+#else
+static inline void cpufreq_trigger_update(u64 time) {}
+#endif
+
 #ifdef CONFIG_SCHED_AUTOGROUP
 extern void sched_autogroup_create_attach(struct task_struct *p);
 extern void sched_autogroup_detach(struct task_struct *p);
Index: linux-pm/kernel/sched/Makefile
===================================================================
--- linux-pm.orig/kernel/sched/Makefile
+++ linux-pm/kernel/sched/Makefile
@@ -19,3 +19,4 @@ obj-$(CONFIG_SCHED_AUTOGROUP) += auto_gr
 obj-$(CONFIG_SCHEDSTATS) += stats.o
 obj-$(CONFIG_SCHED_DEBUG) += debug.o
 obj-$(CONFIG_CGROUP_CPUACCT) += cpuacct.o
+obj-$(CONFIG_CPU_FREQ) += cpufreq.o
Index: linux-pm/kernel/sched/cpufreq.c
===================================================================
--- /dev/null
+++ linux-pm/kernel/sched/cpufreq.c
@@ -0,0 +1,73 @@
+/*
+ * Scheduler code and data structures related to cpufreq.
+ *
+ * Copyright (C) 2016, Intel Corporation
+ * Author: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/sched.h>
+
+static DEFINE_PER_CPU(struct freq_update_hook *, cpufreq_freq_update_hook);
+
+/**
+ * cpufreq_set_freq_update_hook - Populate the CPU's freq_update_hook pointer.
+ * @cpu: The CPU to set the pointer for.
+ * @hook: New pointer value.
+ *
+ * Set and publish the freq_update_hook pointer for the given CPU.  That pointer
+ * points to a struct freq_update_hook object containing a callback function
+ * to call from cpufreq_trigger_update().  That function will be called from
+ * an RCU read-side critical section, so it must not sleep.
+ *
+ * Callers must use RCU-sched callbacks to free any memory that might be
+ * accessed via the old update_util_data pointer or invoke synchronize_sched()
+ * right after this function to avoid use-after-free.
+ */
+void cpufreq_set_freq_update_hook(int cpu, struct freq_update_hook *hook)
+{
+	if (WARN_ON(hook && !hook->func))
+		return;
+
+	rcu_assign_pointer(per_cpu(cpufreq_freq_update_hook, cpu), hook);
+}
+EXPORT_SYMBOL_GPL(cpufreq_set_freq_update_hook);
+
+/**
+ * cpufreq_trigger_update - Trigger CPU performance state evaluation if needed.
+ * @time: Current time.
+ *
+ * The way cpufreq is currently arranged requires it to evaluate the CPU
+ * performance state (frequency/voltage) on a regular basis.  To facilitate
+ * that, this function is called by update_load_avg() in CFS when executed for
+ * the current CPU's runqueue.
+ *
+ * However, this isn't sufficient to prevent the CPU from being stuck in a
+ * completely inadequate performance level for too long, because the calls
+ * from CFS will not be made if RT or deadline tasks are active all the time
+ * (or there are RT and DL tasks only).
+ *
+ * As a workaround for that issue, this function is called by the RT and DL
+ * sched classes to trigger extra cpufreq updates to prevent it from stalling,
+ * but that really is a band-aid.  Going forward it should be replaced with
+ * solutions targeted more specifically at RT and DL tasks.
+ */
+void cpufreq_trigger_update(u64 time)
+{
+	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)
+		hook->func(hook, time);
+}

[toc] | [prev] | [next] | [standalone]


#1349822 — [PATCH v2 9/10] cpufreq: sched: Re-introduce cpufreq_update_util()

From"Rafael J. Wysocki" <rjw@rjwysocki.net>
Date2016-03-04 04:40 +0100
Subject[PATCH v2 9/10] cpufreq: sched: Re-introduce cpufreq_update_util()
Message-ID<r8OKS-B1-19@gated-at.bofh.it>
In reply to#1349813
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

A subsequent change set will introduce a new cpufreq governor using
CPU utilization information from the scheduler, so introduce
cpufreq_update_util() (again) to allow that information to be passed to
the new governor and make cpufreq_trigger_update() call it internally.

To that end, add a new ->update_util callback pointer to struct
freq_update_hook to be set by entities that want to use the util
and max arguments and make cpufreq_update_util() use that callback
if available or the ->func callback that only takes the time argument
otherwise.

In addition to that, arrange helpers to set/clear the utilization
update hooks in such a way that the full ->update_util callbacks
can only be set by code inside the kernel/sched/ directory.

Update the current users of cpufreq_set_freq_update_hook() to use
the new helpers.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

New patch.  Maybe slightly over the top, but at least it should be clear
who uses the util and max arguments and who doesn't use them after it.

---
 drivers/cpufreq/cpufreq_governor.c |   76 +++++++++++++--------------
 drivers/cpufreq/intel_pstate.c     |    8 +-
 include/linux/sched.h              |   10 +--
 kernel/sched/cpufreq.c             |  101 +++++++++++++++++++++++++++++--------
 kernel/sched/fair.c                |    8 ++
 kernel/sched/sched.h               |   16 +++++
 6 files changed, 150 insertions(+), 69 deletions(-)

Index: linux-pm/include/linux/sched.h
===================================================================
--- linux-pm.orig/include/linux/sched.h
+++ linux-pm/include/linux/sched.h
@@ -2363,15 +2363,15 @@ static inline bool sched_can_stop_tick(v
 #endif
 
 #ifdef CONFIG_CPU_FREQ
-void cpufreq_trigger_update(u64 time);
-
 struct freq_update_hook {
 	void (*func)(struct freq_update_hook *hook, u64 time);
+	void (*update_util)(struct freq_update_hook *hook, u64 time,
+			    unsigned long util, unsigned long max);
 };
 
-void cpufreq_set_freq_update_hook(int cpu, struct freq_update_hook *hook);
-#else
-static inline void cpufreq_trigger_update(u64 time) {}
+void cpufreq_set_freq_update_hook(int cpu, struct freq_update_hook *hook,
+			void (*func)(struct freq_update_hook *hook, u64 time));
+void cpufreq_clear_freq_update_hook(int cpu);
 #endif
 
 #ifdef CONFIG_SCHED_AUTOGROUP
Index: linux-pm/kernel/sched/cpufreq.c
===================================================================
--- linux-pm.orig/kernel/sched/cpufreq.c
+++ linux-pm/kernel/sched/cpufreq.c
@@ -9,12 +9,12 @@
  * published by the Free Software Foundation.
  */
 
-#include <linux/sched.h>
+#include "sched.h"
 
 static DEFINE_PER_CPU(struct freq_update_hook *, cpufreq_freq_update_hook);
 
 /**
- * cpufreq_set_freq_update_hook - Populate the CPU's freq_update_hook pointer.
+ * set_freq_update_hook - Populate the CPU's freq_update_hook pointer.
  * @cpu: The CPU to set the pointer for.
  * @hook: New pointer value.
  *
@@ -27,23 +27,96 @@ static DEFINE_PER_CPU(struct freq_update
  * accessed via the old update_util_data pointer or invoke synchronize_sched()
  * right after this function to avoid use-after-free.
  */
-void cpufreq_set_freq_update_hook(int cpu, struct freq_update_hook *hook)
+static void set_freq_update_hook(int cpu, struct freq_update_hook *hook)
 {
-	if (WARN_ON(hook && !hook->func))
+	rcu_assign_pointer(per_cpu(cpufreq_freq_update_hook, cpu), hook);
+}
+
+/**
+ * cpufreq_set_freq_update_hook - Set the CPU's frequency update callback.
+ * @cpu: The CPU to set the callback for.
+ * @hook: New freq_update_hook pointer value.
+ * @func: Callback function to use with the new hook.
+ */
+void cpufreq_set_freq_update_hook(int cpu, struct freq_update_hook *hook,
+			void (*func)(struct freq_update_hook *hook, u64 time))
+{
+	if (WARN_ON(!hook || !func))
 		return;
 
-	rcu_assign_pointer(per_cpu(cpufreq_freq_update_hook, cpu), hook);
+	hook->func = func;
+	set_freq_update_hook(cpu, hook);
 }
 EXPORT_SYMBOL_GPL(cpufreq_set_freq_update_hook);
 
 /**
+ * cpufreq_set_update_util_hook - Set the CPU's utilization update callback.
+ * @cpu: The CPU to set the callback for.
+ * @hook: New freq_update_hook pointer value.
+ * @update_util: Callback function to use with the new hook.
+ */
+void cpufreq_set_update_util_hook(int cpu, struct freq_update_hook *hook,
+		void (*update_util)(struct freq_update_hook *hook, u64 time,
+				    unsigned long util, unsigned long max))
+{
+	if (WARN_ON(!hook || !update_util))
+		return;
+
+	hook->update_util = update_util;
+	set_freq_update_hook(cpu, hook);
+}
+EXPORT_SYMBOL_GPL(cpufreq_set_update_util_hook);
+
+/**
+ * cpufreq_set_update_util_hook - Clear the CPU's freq_update_hook pointer.
+ * @cpu: The CPU to clear the pointer for.
+ */
+void cpufreq_clear_freq_update_hook(int cpu)
+{
+	set_freq_update_hook(cpu, NULL);
+}
+EXPORT_SYMBOL_GPL(cpufreq_clear_freq_update_hook);
+
+/**
+ * cpufreq_update_util - Take a note about CPU utilization changes.
+ * @time: Current time.
+ * @util: CPU utilization.
+ * @max: CPU capacity.
+ *
+ * This function is called on every invocation of update_load_avg() on the CPU
+ * whose utilization is being updated.
+ *
+ * It can only be called from RCU-sched read-side critical sections.
+ */
+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(*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);
+	}
+}
+
+/**
  * cpufreq_trigger_update - Trigger CPU performance state evaluation if needed.
  * @time: Current time.
  *
  * The way cpufreq is currently arranged requires it to evaluate the CPU
  * performance state (frequency/voltage) on a regular basis.  To facilitate
- * that, this function is called by update_load_avg() in CFS when executed for
- * the current CPU's runqueue.
+ * that, cpufreq_update_util() is called by update_load_avg() in CFS when
+ * executed for the current CPU's runqueue.
  *
  * However, this isn't sufficient to prevent the CPU from being stuck in a
  * completely inadequate performance level for too long, because the calls
@@ -57,17 +130,5 @@ EXPORT_SYMBOL_GPL(cpufreq_set_freq_updat
  */
 void cpufreq_trigger_update(u64 time)
 {
-	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)
-		hook->func(hook, time);
+	cpufreq_update_util(time, ULONG_MAX, 0);
 }
Index: linux-pm/kernel/sched/fair.c
===================================================================
--- linux-pm.orig/kernel/sched/fair.c
+++ linux-pm/kernel/sched/fair.c
@@ -2839,6 +2839,8 @@ static inline void update_load_avg(struc
 		update_tg_load_avg(cfs_rq, 0);
 
 	if (cpu == smp_processor_id() && &rq->cfs == cfs_rq) {
+		unsigned long max = rq->cpu_capacity_orig;
+
 		/*
 		 * There are a few boundary cases this might miss but it should
 		 * get called often enough that that should (hopefully) not be
@@ -2847,9 +2849,11 @@ static inline void update_load_avg(struc
 		 * the next tick/schedule should update.
 		 *
 		 * It will not get called when we go idle, because the idle
-		 * thread is a different class (!fair).
+		 * thread is a different class (!fair), nor will the utilization
+-		 * number include things like RT tasks.
 		 */
-		cpufreq_trigger_update(rq_clock(rq));
+		cpufreq_update_util(rq_clock(rq),
+				    min(cfs_rq->avg.util_avg, max), max);
 	}
 }
 
Index: linux-pm/kernel/sched/sched.h
===================================================================
--- linux-pm.orig/kernel/sched/sched.h
+++ linux-pm/kernel/sched/sched.h
@@ -1739,3 +1739,19 @@ static inline u64 irq_time_read(int cpu)
 }
 #endif /* CONFIG_64BIT */
 #endif /* CONFIG_IRQ_TIME_ACCOUNTING */
+
+#ifdef CONFIG_CPU_FREQ
+void cpufreq_update_util(u64 time, unsigned long util, unsigned long max);
+void cpufreq_trigger_update(u64 time);
+void cpufreq_set_update_util_hook(int cpu, struct freq_update_hook *hook,
+		void (*update_util)(struct freq_update_hook *hook, u64 time,
+				    unsigned long util, unsigned long max));
+static inline void cpufreq_clear_update_util_hook(int cpu)
+{
+	cpufreq_clear_freq_update_hook(cpu);
+}
+#else
+static inline void cpufreq_update_util(u64 time, unsigned long util,
+				       unsigned long max) {}
+static inline void cpufreq_trigger_update(u64 time) {}
+#endif /* CONFIG_CPU_FREQ */
Index: linux-pm/drivers/cpufreq/intel_pstate.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/intel_pstate.c
+++ linux-pm/drivers/cpufreq/intel_pstate.c
@@ -1088,8 +1088,8 @@ static int intel_pstate_init_cpu(unsigne
 	intel_pstate_busy_pid_reset(cpu);
 	intel_pstate_sample(cpu, 0);
 
-	cpu->update_hook.func = intel_pstate_freq_update;
-	cpufreq_set_freq_update_hook(cpunum, &cpu->update_hook);
+	cpufreq_set_freq_update_hook(cpunum, &cpu->update_hook,
+				     intel_pstate_freq_update);
 
 	pr_debug("intel_pstate: controlling: cpu %d\n", cpunum);
 
@@ -1173,7 +1173,7 @@ static void intel_pstate_stop_cpu(struct
 
 	pr_debug("intel_pstate: CPU %d exiting\n", cpu_num);
 
-	cpufreq_set_freq_update_hook(cpu_num, NULL);
+	cpufreq_clear_freq_update_hook(cpu_num);
 	synchronize_sched();
 
 	if (hwp_active)
@@ -1441,7 +1441,7 @@ out:
 	get_online_cpus();
 	for_each_online_cpu(cpu) {
 		if (all_cpu_data[cpu]) {
-			cpufreq_set_freq_update_hook(cpu, NULL);
+			cpufreq_clear_freq_update_hook(cpu);
 			synchronize_sched();
 			kfree(all_cpu_data[cpu]);
 		}
Index: linux-pm/drivers/cpufreq/cpufreq_governor.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_governor.c
+++ linux-pm/drivers/cpufreq/cpufreq_governor.c
@@ -211,43 +211,6 @@ unsigned int dbs_update(struct cpufreq_p
 }
 EXPORT_SYMBOL_GPL(dbs_update);
 
-static void gov_set_freq_update_hooks(struct policy_dbs_info *policy_dbs,
-				unsigned int delay_us)
-{
-	struct cpufreq_policy *policy = policy_dbs->policy;
-	int cpu;
-
-	gov_update_sample_delay(policy_dbs, delay_us);
-	policy_dbs->last_sample_time = 0;
-
-	for_each_cpu(cpu, policy->cpus) {
-		struct cpu_dbs_info *cdbs = &per_cpu(cpu_dbs, cpu);
-
-		cpufreq_set_freq_update_hook(cpu, &cdbs->update_hook);
-	}
-}
-
-static inline void gov_clear_freq_update_hooks(struct cpufreq_policy *policy)
-{
-	int i;
-
-	for_each_cpu(i, policy->cpus)
-		cpufreq_set_freq_update_hook(i, NULL);
-
-	synchronize_sched();
-}
-
-static void gov_cancel_work(struct cpufreq_policy *policy)
-{
-	struct policy_dbs_info *policy_dbs = policy->governor_data;
-
-	gov_clear_freq_update_hooks(policy_dbs->policy);
-	irq_work_sync(&policy_dbs->irq_work);
-	cancel_work_sync(&policy_dbs->work);
-	atomic_set(&policy_dbs->work_count, 0);
-	policy_dbs->work_in_progress = false;
-}
-
 static void dbs_work_handler(struct work_struct *work)
 {
 	struct policy_dbs_info *policy_dbs;
@@ -334,6 +297,44 @@ static void dbs_freq_update_handler(stru
 	irq_work_queue(&policy_dbs->irq_work);
 }
 
+static void gov_set_freq_update_hooks(struct policy_dbs_info *policy_dbs,
+				unsigned int delay_us)
+{
+	struct cpufreq_policy *policy = policy_dbs->policy;
+	int cpu;
+
+	gov_update_sample_delay(policy_dbs, delay_us);
+	policy_dbs->last_sample_time = 0;
+
+	for_each_cpu(cpu, policy->cpus) {
+		struct cpu_dbs_info *cdbs = &per_cpu(cpu_dbs, cpu);
+
+		cpufreq_set_freq_update_hook(cpu, &cdbs->update_hook,
+					     dbs_freq_update_handler);
+	}
+}
+
+static inline void gov_clear_freq_update_hooks(struct cpufreq_policy *policy)
+{
+	int i;
+
+	for_each_cpu(i, policy->cpus)
+		cpufreq_clear_freq_update_hook(i);
+
+	synchronize_sched();
+}
+
+static void gov_cancel_work(struct cpufreq_policy *policy)
+{
+	struct policy_dbs_info *policy_dbs = policy->governor_data;
+
+	gov_clear_freq_update_hooks(policy_dbs->policy);
+	irq_work_sync(&policy_dbs->irq_work);
+	cancel_work_sync(&policy_dbs->work);
+	atomic_set(&policy_dbs->work_count, 0);
+	policy_dbs->work_in_progress = false;
+}
+
 static struct policy_dbs_info *alloc_policy_dbs_info(struct cpufreq_policy *policy,
 						     struct dbs_governor *gov)
 {
@@ -356,7 +357,6 @@ static struct policy_dbs_info *alloc_pol
 		struct cpu_dbs_info *j_cdbs = &per_cpu(cpu_dbs, j);
 
 		j_cdbs->policy_dbs = policy_dbs;
-		j_cdbs->update_hook.func = dbs_freq_update_handler;
 	}
 	return policy_dbs;
 }

[toc] | [prev] | [standalone]


Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel


csiph-web