linux-kernel - Re: [PATCH] cpufreq: Make sure CPU is running on a freq from freq-table

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]

Date:	Thu, 21 Nov 2013 14:11:57 +0100
From:	"Rafael J. Wysocki" <rjw@...ysocki.net>
To:	Viresh Kumar <viresh.kumar@...aro.org>
Cc:	nm@...com, linaro-kernel@...ts.linaro.org, patches@...aro.org,
	cpufreq@...r.kernel.org, linux-pm@...r.kernel.org,
	linux-kernel@...r.kernel.org, shawn.guo@...aro.org, ceh@...com
Subject: Re: [PATCH] cpufreq: Make sure CPU is running on a freq from freq-table

On Thursday, November 21, 2013 12:39:02 PM Viresh Kumar wrote:
> Sometimes boot loaders set CPU frequency to a value outside of frequency table
> present with cpufreq core. In such cases CPU might be unstable if it has to run
> on that frequency for long duration of time and so its better to set it to a
> frequency which is specified in freq-table. This also makes cpufreq stats
> inconsistent as cpufreq-stats would fail to register because current frequency
> of CPU isn't found in freq-table.
> 
> Because we don't want this change to effect boot process badly, we go for the
> next freq which is >= policy->cur ('cur' must be set by now, otherwise we will
> end up setting freq to lowest of the table as 'cur' is initialized to zero).
> 
> In case where CPU is already running on one of the frequencies present in
> freq-table, this would turn into a dummy call as __cpufreq_driver_target() would
> return early.
> 
> Reported-by: Carlos Hernandez <ceh@...com>
> Reported-by: Nishanth Menon <nm@...com>
> Signed-off-by: Viresh Kumar <viresh.kumar@...aro.org>
> ---
> After lots of discussion with Nishanth and others, I feel something like this..
> 
> @Nishanth: Please see if this works for you and I hope we don't need any of
> these patches anymore:
> 
> - https://lkml.org/lkml/2013/11/15/569 : cpufreq: cpufreq-cpu0: Use a sane boot
> frequency when booting with a mismatched bootloader configuration
> - https://lkml.org/lkml/2013/11/15/503 : cpufreq: stats: Do not populate stats
>   when policy->cur has no exact match
> - https://lkml.org/lkml/2013/11/19/16 : cpufreq/stats: Add "unknown" frequency
>   field in stats tables
> 
>  drivers/cpufreq/cpufreq.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 02d534d..d55c843 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1038,6 +1038,32 @@ static int __cpufreq_add_dev(struct device *dev, struct subsys_interface *sif,
>  		}
>  	}
>  
> +	/*
> +	 * Sometimes boot loaders set CPU frequency to a value outside of
> +	 * frequency table present with cpufreq core. In such cases CPU might be
> +	 * unstable if it has to run on that frequency for long duration of time
> +	 * and so its better to set it to a frequency which is specified in
> +	 * freq-table. This also makes cpufreq stats inconsistent as
> +	 * cpufreq-stats would fail to register because current frequency of CPU
> +	 * isn't found in freq-table.
> +	 *
> +	 * Because we don't want this change to effect boot process badly, we go
> +	 * for the next freq which is >= policy->cur ('cur' must be set by now,
> +	 * otherwise we will end up setting freq to lowest of the table as 'cur'
> +	 * is initialized to zero).
> +	 *
> +	 * In case where CPU is already running on one of the frequencies
> +	 * present in freq-table, this would turn into a dummy call as
> +	 * __cpufreq_driver_target() would return early.
> +	 */
> +	if (has_target()) {
> +		ret = __cpufreq_driver_target(policy, policy->cur,
> +				CPUFREQ_RELATION_L);
> +		if (ret)
> +			pr_err("%s: Unable to set frequency from table: %d\n",
> +					__func__, ret);

Should we continue in that case?

> +	}
> +
>  	/* related cpus should atleast have policy->cpus */
>  	cpumask_or(policy->related_cpus, policy->related_cpus, policy->cpus);
>  
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/