linux-kernel - Re: [RFC PATCH] cpufreq: powernv: Add fast

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJZ5v0iaGCi8X7Ko0aZL6CBgZ3i4=59cqFhzi1nBFumHL=huQw@mail.gmail.com>
Date:	Wed, 18 May 2016 23:22:16 +0200
From:	"Rafael J. Wysocki" <rafael@...nel.org>
To:	Shilpasri G Bhat <shilpa.bhat@...ux.vnet.ibm.com>
Cc:	"Rafael J. Wysocki" <rjw@...ysocki.net>,
	Viresh Kumar <viresh.kumar@...aro.org>,
	"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	"Gautham R. Shenoy" <ego@...ux.vnet.ibm.com>,
	shreyas@...ux.vnet.ibm.com, akshay.adiga@...ux.vnet.ibm.com,
	linuxppc-dev@...abs.org, Peter Zijlstra <peterz@...radead.org>
Subject: Re: [RFC PATCH] cpufreq: powernv: Add fast_switch callback

On Wed, May 18, 2016 at 2:53 PM, Shilpasri G Bhat
<shilpa.bhat@...ux.vnet.ibm.com> wrote:
> Add fast_switch driver callback to support frequency update in
> interrupt context while using schedutil governor. Changing frequency
> in interrupt context will remove the jitter on the workloads which can
> be seen when a kworker thread is used for the changing the frequency.
>
> Signed-off-by: Shilpasri G Bhat <shilpa.bhat@...ux.vnet.ibm.com>

This looks simple enough. :-)

A couple of comments, though.

> ---
>  drivers/cpufreq/powernv-cpufreq.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>
> diff --git a/drivers/cpufreq/powernv-cpufreq.c b/drivers/cpufreq/powernv-cpufreq.c
> index 54c4536..4553eb6 100644
> --- a/drivers/cpufreq/powernv-cpufreq.c
> +++ b/drivers/cpufreq/powernv-cpufreq.c
> @@ -678,6 +678,8 @@ static int powernv_cpufreq_cpu_init(struct cpufreq_policy *policy)
>         for (i = 0; i < threads_per_core; i++)
>                 cpumask_set_cpu(base + i, policy->cpus);
>
> +       policy->fast_switch_possible = true;
> +
>         kn = kernfs_find_and_get(policy->kobj.sd, throttle_attr_grp.name);
>         if (!kn) {
>                 int ret;
> @@ -854,6 +856,24 @@ static void powernv_cpufreq_stop_cpu(struct cpufreq_policy *policy)
>         del_timer_sync(&gpstates->timer);
>  }
>
> +static unsigned int powernv_fast_switch(struct cpufreq_policy *policy,
> +                                       unsigned int target_freq)
> +{
> +       int index;
> +       struct powernv_smp_call_data freq_data;
> +
> +       cpufreq_frequency_table_target(policy, policy->freq_table,
> +                                      target_freq,
> +                                      CPUFREQ_RELATION_C, &index);

According to the discussion I had with Peter some time ago, this
should be RELATION_L or you may end up using a frequency that's not
sufficient to meet a deadline somewhere.

Also cpufreq_frequency_table_target() is somewhat heavy-weight
especially if the table is known to be sorted (which I guess is the
case).

> +       if (index < 0 || index >= powernv_pstate_info.nr_pstates)
> +               return CPUFREQ_ENTRY_INVALID;
> +       freq_data.pstate_id = powernv_freqs[index].driver_data;
> +       freq_data.gpstate_id = powernv_freqs[index].driver_data;
> +       set_pstate(&freq_data);
> +
> +       return pstate_id_to_freq(-index);
> +}
> +
>  static struct cpufreq_driver powernv_cpufreq_driver = {
>         .name           = "powernv-cpufreq",
>         .flags          = CPUFREQ_CONST_LOOPS,
> @@ -861,6 +881,7 @@ static struct cpufreq_driver powernv_cpufreq_driver = {
>         .exit           = powernv_cpufreq_cpu_exit,
>         .verify         = cpufreq_generic_frequency_table_verify,
>         .target_index   = powernv_cpufreq_target_index,
> +       .fast_switch    = powernv_fast_switch,
>         .get            = powernv_cpufreq_get,
>         .stop_cpu       = powernv_cpufreq_stop_cpu,
>         .attr           = powernv_cpu_freq_attr,
> --