linux-kernel - Re: [PATCH 6/6] cpufreq: Evaluate P1 before enter turbo mode

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: <20101223105730.GB18363@comet.dominikbrodowski.net>
Date:	Thu, 23 Dec 2010 11:57:30 +0100
From:	Dominik Brodowski <linux@...inikbrodowski.net>
To:	Youquan Song <youquan.song@...el.com>
Cc:	davej@...hat.com, cpufreq@...r.kernel.org, venki@...gle.com,
	arjan@...ux.intel.com, lenb@...nel.org, suresh.b.siddha@...el.com,
	kent.liu@...el.com, chaohong.guo@...el.com,
	linux-kernel@...r.kernel.org, linux-acpi@...r.kernel.org,
	Youquan Song <youquan.song@...ux.intel.com>
Subject: Re: [PATCH 6/6] cpufreq: Evaluate P1 before enter turbo mode

Hey,

On Thu, Dec 23, 2010 at 02:23:44PM +0800, Youquan Song wrote:
> diff --git a/drivers/cpufreq/freq_table.c b/drivers/cpufreq/freq_table.c
> index 0543221..d7dc010 100644
> --- a/drivers/cpufreq/freq_table.c
> +++ b/drivers/cpufreq/freq_table.c
> @@ -26,6 +26,7 @@ int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy,
>  {
>  	unsigned int min_freq = ~0;
>  	unsigned int max_freq = 0;
> +	unsigned int sec_max_freq = 0;
>  	unsigned int i;
>  
>  	for (i = 0; (table[i].frequency != CPUFREQ_TABLE_END); i++) {
> @@ -41,10 +42,18 @@ int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy,
>  			min_freq = freq;
>  		if (freq > max_freq)
>  			max_freq = freq;
> +		/* Find the second largest frequency */
> +		if ((freq < max_freq) && (freq > sec_max_freq))
> +			sec_max_freq = freq;
>  	}
>  
>  	policy->min = policy->cpuinfo.min_freq = min_freq;
>  	policy->max = policy->cpuinfo.max_freq = max_freq;
> +	/* Check CPU turbo mode enabled */
> +	if (max_freq - sec_max_freq == 1000)
> +		policy->sec_max = sec_max_freq;
> +	else
> +		policy->sec_max = max_freq;
>  
>  	if (policy->min == ~0)
>  		return -EINVAL;
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index c3e9de8..0087e56 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -92,6 +92,7 @@ struct cpufreq_policy {
>  
>  	unsigned int		min;    /* in kHz */
>  	unsigned int		max;    /* in kHz */
> +	unsigned int		sec_max; /* in kHz*/
>  	unsigned int		cur;    /* in kHz, only needed if cpufreq
>  					 * governors are used */
>          unsigned int		policy; /* see above */

NACK. First of all, why is it only a "turbo mode" if it's 1000 kHz
difference? Second, I don't like to put such an additional level into the
generic cpufreq code -- it just looks to be too chipset/CPU-specific. Third,
it isn't open to different turbo modes, e.g. if future CPUs offer a
"super-turbo", a "turbo" and a "semi-turbo" mode. Finally, if (certain) 
governors really really need to become aware of the individual
frequency steps -- something we avoided in the past -- we could extend
struct cpufreq_policy to optionally(!) contain a reference to the struct
cpufreq_frequency_table array. And add a "power_usage" parameter to that
struct, which could then be evaluated by the governor.

Best,
	Dominik
--
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/