Hey,
On Wed, Mar 24, 2010 at 09:28:59PM +0100, Jean-Christian wrote:
> I am running 2.6.32 SMP on a single Athlon 64 X2 (both cores share the same
> frequency).
>
> I observe that the convervative governor ignores the load of the first core
> when taking frequency increase decisions. On an idle system:
> Running "taskset 2 bash -c 'while ((1));do :;done'"
> results in an immediate frequency increase.
> Running "taskset 1 bash -c 'while ((1));do :;done'"
> never increases the frequency.
> Running "taskset 3 bash -c 'while ((1));do :;done'"
> increases the frequency after a random delay.
>
> With the ondemand governor the frequency increases immediately in all 3 cases.
>
> Looking at cpufreq_conservative.c, the load is computed for all CPUs but they
> are trashed and only the last one prevails:
> static void dbs_check_cpu(struct cpu_dbs_info_s *this_dbs_info)
> {
> unsigned int load = 0;
> ...
> for_each_cpu(j, policy->cpus) {
> unsigned int idle_time, wall_time;
> ...
> load = 100 * (wall_time - idle_time) / wall_time;
> }
> ...
> if (load > dbs_tuners_ins.up_threshold) {
> ...
>
> (same code in 2.6.34-rc2)
Nice catch.
From: Dominik Brodowski <linux@xxxxxxxxxxxxxxxxxxxx>
Date: Thu, 25 Mar 2010 18:13:20 +0100
Subject: [PATCH] cpufreq: use average load in conservative governor
Instead of using the load of the last CPU in a package, use the
average of all CPUs in a package.
Reported-by: Jean-Christian Goussard <jeanchristian.goussard@xxxxxx>
Signed-off-by: Dominik Brodowski <linux@xxxxxxxxxxxxxxxxxxxx>
diff --git a/drivers/cpufreq/cpufreq_conservative.c b/drivers/cpufreq/cpufreq_conservative.c
index 599a40b..e0be6bd 100644
--- a/drivers/cpufreq/cpufreq_conservative.c
+++ b/drivers/cpufreq/cpufreq_conservative.c
@@ -444,6 +444,7 @@ static struct attribute_group dbs_attr_group_old = {
static void dbs_check_cpu(struct cpu_dbs_info_s *this_dbs_info)
{
unsigned int load = 0;
+ unsigned int max_load = 0;
unsigned int freq_target;
struct cpufreq_policy *policy;
@@ -500,8 +501,9 @@ static void dbs_check_cpu(struct cpu_dbs_info_s *this_dbs_info)
if (unlikely(!wall_time || wall_time < idle_time))
continue;
- load = 100 * (wall_time - idle_time) / wall_time;
+ load += 100 * (wall_time - idle_time) / wall_time;
}
+ load = load / cpumask_weight(policy->cpus);
/*
* break out if we 'cannot' reduce the speed as the user might
@@ -511,7 +513,7 @@ static void dbs_check_cpu(struct cpu_dbs_info_s *this_dbs_info)
return;
/* Check for frequency increase */
- if (load > dbs_tuners_ins.up_threshold) {
+ if (max_load > dbs_tuners_ins.up_threshold) {
this_dbs_info->down_skip = 0;
/* if we are already at full speed then break out early */
@@ -538,7 +540,7 @@ static void dbs_check_cpu(struct cpu_dbs_info_s *this_dbs_info)
* can support the current CPU usage without triggering the up
* policy. To be safe, we focus 10 points under the threshold.
*/
- if (load < (dbs_tuners_ins.down_threshold - 10)) {
+ if (max_load < (dbs_tuners_ins.down_threshold - 10)) {
freq_target = (dbs_tuners_ins.freq_step * policy->max) / 100;
this_dbs_info->requested_freq -= freq_target;
--
To unsubscribe from this list: send the line "unsubscribe cpufreq" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
