bltk-game regressions on snb laptop

DISCUSSION Alex Shi 4 participants 17 messages Apr 16, 2013 – Apr 19, 2013

Share & tools

Alex Shi

author

2013-04-16 07:15 UTC #0 lore raw

LKP found a performance and performance/watt regression on commit aa77a52764a92216b61, acpi-cpufreq: Don't set policy->related_cpus from .init.

The commit removed the related_cpus setting, plus Our laptop has no coordinate type setting in BIOS. So the related_cpus is only include the cpu self, then the policy->cpus are impacted and only has self too.

With ondemand governor, the bad commit cause bltk-game benchmark drop to 18fps from 50fps, the performance/watt value also dropped a lot. bltk-game runs 9 thread on the 4core*HT laptop.

As Arjan and Len mentioned, the commit is correct in logical, policy->cpus should only include the cpu self.
So I don't know where is the problem, maybe ondemand or some place others. Anyway, I just report the issue to you.

Signature · 1 line

Thanks Alex

Rafael J. Wysocki

2013-04-17 16:26 UTC #1 lore raw

Quoted 8 lines · Alex Shi

LKP found a performance and performance/watt regression on commit aa77a52764a92216b61, acpi-cpufreq: Don't set policy->related_cpus from .init.
The commit removed the related_cpus setting, plus Our laptop has no coordinate type setting in BIOS. So the related_cpus is only include the cpu self, then the policy->cpus are impacted and only has self too.

Quoted 8 lines

With ondemand governor, the bad commit cause bltk-game benchmark drop to 18fps from 50fps, the performance/watt value also dropped a lot. bltk-game runs 9 thread on the 4core*HT laptop.
As Arjan and Len mentioned, the commit is correct in logical, policy->cpus should only include the cpu self.
So I don't know where is the problem, maybe ondemand or some place others. Anyway, I just report the issue to you.

Have you verified that reverting the commit in question causes the problem to go away?

Rafael

Signature · 2 lines

I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Viresh Kumar

2013-04-17 16:46 UTC #3 lore raw

In reply to Alex Shi

Quoted 8 lines · Alex Shi

LKP found a performance and performance/watt regression on commit aa77a52764a92216b61, acpi-cpufreq: Don't set policy->related_cpus from .init.
The commit removed the related_cpus setting, plus Our laptop has no coordinate type setting in BIOS. So the related_cpus is only include the cpu self, then the policy->cpus are impacted and only has self too.

Quoted 8 lines

With ondemand governor, the bad commit cause bltk-game benchmark drop to 18fps from 50fps, the performance/watt value also dropped a lot. bltk-game runs 9 thread on the 4core*HT laptop.
As Arjan and Len mentioned, the commit is correct in logical, policy->cpus should only include the cpu self.
So I don't know where is the problem, maybe ondemand or some place others. Anyway, I just report the issue to you.

I really don't believe the commit you are pointing to should have any impact on performance. Because before i changed definitions of affected and related cpus, related_cpus was just not used at all (leaving some hotplug cases to identify the last governor).. And so even if we set related_cpus from driver earlier, it shouldn't be doing anything.

And now after it is being removed from acpi driver, i believe situation should still
be the same.

Alex Shi

author

2013-04-18 00:24 UTC #2 lore raw

In reply to Rafael J. Wysocki

Quoted 13 lines · Rafael J. Wysocki

Quoted 12 lines · Alex Shi
LKP found a performance and performance/watt regression on commit aa77a52764a92216b61, acpi-cpufreq: Don't set policy->related_cpus from .init.
The commit removed the related_cpus setting, plus Our laptop has no coordinate type setting in BIOS. So the related_cpus is only include the cpu self, then the policy->cpus are impacted and only has self too.
With ondemand governor, the bad commit cause bltk-game benchmark drop to 18fps from 50fps, the performance/watt value also dropped a lot. bltk-game runs 9 thread on the 4core*HT laptop.

Quoted 7 lines

Quoted 4 lines
As Arjan and Len mentioned, the commit is correct in logical, policy->cpus should only include the cpu self.
So I don't know where is the problem, maybe ondemand or some place others. Anyway, I just report the issue to you.
Have you verified that reverting the commit in question causes the problem to go away?

yes, after revert, the fps is back to about 50fps from 18fps.

Quoted 4 lines

Rafael

Signature · 1 line

Thanks Alex

Alex Shi

author

2013-04-18 00:49 UTC #4 lore raw

In reply to Viresh Kumar

Quoted 18 lines · Viresh Kumar

Quoted 8 lines · Alex Shi
LKP found a performance and performance/watt regression on commit aa77a52764a92216b61, acpi-cpufreq: Don't set policy->related_cpus from .init.
The commit removed the related_cpus setting, plus Our laptop has no coordinate type setting in BIOS. So the related_cpus is only include the cpu self, then the policy->cpus are impacted and only has self too.
Quoted 8 lines
With ondemand governor, the bad commit cause bltk-game benchmark drop to 18fps from 50fps, the performance/watt value also dropped a lot. bltk-game runs 9 thread on the 4core*HT laptop.
As Arjan and Len mentioned, the commit is correct in logical, policy->cpus should only include the cpu self.
So I don't know where is the problem, maybe ondemand or some place others. Anyway, I just report the issue to you.

Quoted 5 lines

I really don't believe the commit you are pointing to should have any impact on performance. Because before i changed definitions of affected and related cpus, related_cpus was just not used at all (leaving some hotplug cases to identify the last governor).. And so even if we set related_cpus from driver earlier, it shouldn't be doing anything.

Hi, Viresh, correct me if I am wrong. :)

the affected_cpus value changed from 3.9-rc1, then we get the good performance, and dropped after this commit. I have reverted this patch, then the performance recovered.

this patch remove the unconditionally related_cpus set:
@@ -730,7 +730,6 @@ static int acpi_cpufreq_cpu_init(struct cpufreq_policy *policy)

            policy->shared_type == CPUFREQ_SHARED_TYPE_ANY) {
                cpumask_copy(policy->cpus, perf->shared_cpu_map);
        }

- cpumask_copy(policy->related_cpus, perf->shared_cpu_map);

that cause path change in cpufreq_add_dev(),
#ifdef CONFIG_HOTPLUG_CPU

 873         /* Check if this cpu was hot-unplugged earlier and has siblings */
 874         spin_lock_irqsave(&cpufreq_driver_lock, flags);
 875         for_each_online_cpu(sibling) {

876 struct cpufreq_policy *cp = per_cpu(cpufreq_cpu_data, sibling);
877 if (cp && cpumask_test_cpu(cpu, cp->related_cpus)) {
878 spin_unlock_irqrestore(&cpufreq_driver_lock, flags);
879 return cpufreq_add_policy_cpu(cpu, sibling, dev);
880 }
881 }

cpufreq_add_policy_cpu() has no chance to run for other cpus, since they are not in cp->related_cpus, line 877.

The bltk-game use cpu to decode video, it has 9 load varied threads. The following is the typical snapshot of asked cpufreq:
with your patch
[alexs@lkp-sb01 ~]$ cat /sys/.../cpu*/cpufreq/scaling_cur_freq 2201000
800000
800000
800000
800000
800000
2201000
800000

Wihtout your patch
[alexs@lkp-sb01 ~]$ cat /sys/.../cpu*/cpufreq/scaling_cur_freq 2201000
2201000
2201000
2201000
2201000
2201000
2201000
2201000

Our p-state should be hardware coordinate, so affected_cpus is right on your patch.

Quoted 5 lines

And now after it is being removed from acpi driver, i believe situation should still
be the same.

Signature · 1 line

Thanks Alex

Viresh Kumar

2013-04-18 04:26 UTC #5 lore raw

Quoted 7 lines · Alex Shi

Quoted 1 line · Viresh Kumar
On 16 April 2013 12:44, Alex Shi <[email protected]> wrote:
Hi, Viresh, correct me if I am wrong. :)
the affected_cpus value changed from 3.9-rc1, then we get the good performance, and dropped after this commit. I have reverted this patch, then the performance recovered.

What's the fps value you got with v3.8? Current values should be similar to that.
In 3.8 related_cpus weren't used by any code path leaving:

        /* Set governor before ->init, so that driver could check it */
#ifdef CONFIG_HOTPLUG_CPU
        for_each_online_cpu(sibling) {
                struct cpufreq_policy *cp = per_cpu(cpufreq_cpu_data, sibling);
                if (cp && cp->governor &&
                    (cpumask_test_cpu(cpu, cp->related_cpus))) {
                        policy->governor = cp->governor;
                        found = 1;
                        break;
                }
        }

#endif

And this doesn't have any performance impact AFAICT.

Now starting 3.9-rc1 meaning and usage of affected/related cpus is changed. I never understood why people actually introduced related_cpus earlier? What's the difference b/w hardware and software coordinated cpus..

About your system: Can you give output of cpufreq-info for v3.8 and v3.9-latestrc?
Your cpus share a clock line or not?

Review static int acpi_cpufreq_cpu_init(struct cpufreq_policy *policy) · line 733

Quoted 6 lines

this patch remove the unconditionally related_cpus set:

(quoted diff) +0 -1

@@ -730,7 +730,6 @@ static int acpi_cpufreq_cpu_init(struct cpufreq_policy *policy)
            policy->shared_type == CPUFREQ_SHARED_TYPE_ANY) {
                cpumask_copy(policy->cpus, perf->shared_cpu_map);
        }
-       cpumask_copy(policy->related_cpus, perf->shared_cpu_map);

Because setting related cpus now didn't had a meaning and people were facing regressions due to it.

Quoted 12 lines

(quoted diff) +0 -0
 that cause path change in cpufreq_add_dev(),
 #ifdef CONFIG_HOTPLUG_CPU
 873         /* Check if this cpu was hot-unplugged earlier and has siblings */
874 spin_lock_irqsave(&cpufreq_driver_lock, flags);
875 for_each_online_cpu(sibling) {
876 struct cpufreq_policy *cp = per_cpu(cpufreq_cpu_data, sibling);
877 if (cp && cpumask_test_cpu(cpu, cp->related_cpus)) {
878 spin_unlock_irqrestore(&cpufreq_driver_lock, flags);
879 return cpufreq_add_policy_cpu(cpu, sibling, dev);
880 }
881 }

Quoted 2 lines

cpufreq_add_policy_cpu() has no chance to run for other cpus, since they are not in cp->related_cpus, line 877.

This is new code added by one of my commits and wasn't there in v3.8. Now we set related cpus from affected cpus at ->init() time and that is used here in this code. This is for cpus sharing clock line.

Quoted 13 lines

The bltk-game use cpu to decode video, it has 9 load varied threads. The following is the typical snapshot of asked cpufreq:
with your patch
[alexs@lkp-sb01 ~]$ cat /sys/.../cpu*/cpufreq/scaling_cur_freq 2201000
800000
800000
800000
800000
800000
2201000
800000

Quoted 10 lines

Wihtout your patch
[alexs@lkp-sb01 ~]$ cat /sys/.../cpu*/cpufreq/scaling_cur_freq 2201000
2201000
2201000
2201000
2201000
2201000
2201000
2201000

I think something wrong is happening without my patch as setting related_cpus alone was wrong.

Quoted 1 line

Our p-state should be hardware coordinate, so affected_cpus is right on your patch.

Didn't get that.. Your cpus share clock line? If so, they should be set in policy->cpus by your driver and same would be reflected in related_cpus by cpufreq core.

Viresh Kumar

2013-04-18 04:36 UTC #6 lore raw

Quoted 26 lines · Viresh Kumar

Quoted 13 lines · Alex Shi
The bltk-game use cpu to decode video, it has 9 load varied threads. The following is the typical snapshot of asked cpufreq:
with your patch
[alexs@lkp-sb01 ~]$ cat /sys/.../cpu*/cpufreq/scaling_cur_freq 2201000
800000
800000
800000
800000
800000
2201000
800000
Quoted 10 lines
Wihtout your patch
[alexs@lkp-sb01 ~]$ cat /sys/.../cpu*/cpufreq/scaling_cur_freq 2201000
2201000
2201000
2201000
2201000
2201000
2201000
2201000

Quoted 2 lines

I think something wrong is happening without my patch as setting related_cpus alone was wrong.

BTW, it looks many of your cpus are staying at higher or max freq and that's why you are getting better values. And this happened unintentionally due to a bug in setting related_cpus which my patch fixed.

Alex Shi

author

2013-04-18 05:17 UTC #8 lore raw

In reply to Viresh Kumar

Quoted 4 lines · Viresh Kumar

Quoted 2 lines · Alex Shi
Quoted 1 line · Viresh Kumar
On 16 April 2013 12:44, Alex Shi <[email protected]> wrote:

Quoted 8 lines

Quoted 4 lines
Hi, Viresh, correct me if I am wrong. :)
the affected_cpus value changed from 3.9-rc1, then we get the good performance, and dropped after this commit. I have reverted this patch, then the performance recovered.
What's the fps value you got with v3.8? Current values should be similar to that.

Yes. current value is similar to 3.8: 18fps, compare to 3.9-rc1, that is also bad.
I mean the good performance started from 3.9-rc1 kernel.

Quoted 3 lines

About your system: Can you give output of cpufreq-info for v3.8 and v3.9-latestrc?
Your cpus share a clock line or not?

Yes our cpu shares a clock line. So the cpufreq-info is same. SNB CPU p-state is per cpu package, and hardware will coordinate the software asking, then decide which cpufreq should be.

Quoted 10 lines

Review static int acpi_cpufreq_cpu_init(struct cpufreq_policy *policy) · line 733
Quoted 7 lines
this patch remove the unconditionally related_cpus set:
(quoted diff) +0 -1
@@ -730,7 +730,6 @@ static int acpi_cpufreq_cpu_init(struct cpufreq_policy *policy)
            policy->shared_type == CPUFREQ_SHARED_TYPE_ANY) {
                cpumask_copy(policy->cpus, perf->shared_cpu_map);
        }
-       cpumask_copy(policy->related_cpus, perf->shared_cpu_map);
Because setting related cpus now didn't had a meaning and people were facing regressions due to it.

I know. So, I reported this issue without asking for revert. :)

Quoted 8 lines

Quoted 4 lines
Our p-state should be hardware coordinate, so affected_cpus is right on your patch.
Didn't get that.. Your cpus share clock line? If so, they should be set in policy->cpus by your driver and same would be reflected in related_cpus by cpufreq core.

That's sth I am also confused. Yes, that cpu share a clock line. But Arjan and Len both said affected_cpus should only include the cpu self, because cpu will do hardware coordination for them.

Quoted 1 line

Signature · 1 line

Thanks Alex

Alex Shi

author

2013-04-18 05:25 UTC #7 lore raw

In reply to Viresh Kumar

Quoted 11 lines · Viresh Kumar

Quoted 11 lines
Quoted 11 lines
Quoted 10 lines
Quoted 10 lines
Wihtout your patch
[alexs@lkp-sb01 ~]$ cat /sys/.../cpu*/cpufreq/scaling_cur_freq 2201000
2201000
2201000
2201000
2201000
2201000
2201000
2201000

Quoted 5 lines

Quoted 2 lines
Quoted 2 lines
I think something wrong is happening without my patch as setting related_cpus alone was wrong.
BTW, it looks many of your cpus are staying at higher or max freq and that's why you are getting better values. And this happened unintentionally due to a bug in setting related_cpus which my patch fixed.

Anyway, that make 'ondemand' governor get quite better performance and perf/watt. :)

Signature · 1 line

Thanks Alex

Viresh Kumar

2013-04-18 05:37 UTC #9 lore raw

In reply to Alex Shi

Quoted 5 lines · Alex Shi

On 04/18/2013 12:26 PM, Viresh Kumar wrote:
Yes. current value is similar to 3.8: 18fps, compare to 3.9-rc1, that is also bad.
I mean the good performance started from 3.9-rc1 kernel.

Hmm... that means a bug gave you better fps, because cpus are kept at higher freqs... But 3.8 is the last stable release and so current version is the best for all of us. And we don't have to revert the patch we were talking about.

In case there is a scope of making performance better then we can do it with a more relevant change.

Quoted 7 lines

Quoted 3 lines
About your system: Can you give output of cpufreq-info for v3.8 and v3.9-latestrc?
Your cpus share a clock line or not?
Yes our cpu shares a clock line. So the cpufreq-info is same. SNB CPU p-state is per cpu package, and hardware will coordinate the software asking, then decide which cpufreq should be.

Can you paste cpufreq-info output, that will give us a better picture of your system.

Quoted 3 lines

That's sth I am also confused. Yes, that cpu share a clock line. But Arjan and Len both said affected_cpus should only include the cpu self, because cpu will do hardware coordination for them.

Why? we need a reason for that.... Do they mean Linux shouldn't be aware of any requirement of freq sync? But why?

Alex Shi

author

2013-04-18 06:17 UTC #10 lore raw

Quoted 9 lines · Viresh Kumar

Quoted 7 lines
Quoted 7 lines
Quoted 3 lines
Quoted 3 lines
About your system: Can you give output of cpufreq-info for v3.8 and v3.9-latestrc?
Your cpus share a clock line or not?
Yes our cpu shares a clock line. So the cpufreq-info is same. SNB CPU p-state is per cpu package, and hardware will coordinate the software asking, then decide which cpufreq should be.
Can you paste cpufreq-info output, that will give us a better picture of your system.

will give you turbostat info later.

Quoted 6 lines

Quoted 4 lines
Quoted 4 lines
That's sth I am also confused. Yes, that cpu share a clock line. But Arjan and Len both said affected_cpus should only include the cpu self, because cpu will do hardware coordination for them.
Why? we need a reason for that.... Do they mean Linux shouldn't be aware of any requirement of freq sync? But why?

Nope.
acpi_processor_preregister_performance() will read ACPI table and get the p-state coordinate type. I think that is the agreement between kernel and hardware.

Maybe the ondemand governor doesn't fit well with the laptop's p-state internal logical. So, there is intel p-state driver for SNB, and it use different internal governor.

acpi_processor_preregister_performance():
715 if (pdomain->coord_type == DOMAIN_COORD_TYPE_SW_ALL)
716 pr->performance->shared_type = CPUFREQ_SHARED_TYPE_ALL;
717 else if (pdomain->coord_type == DOMAIN_COORD_TYPE_HW_ALL)
718 pr->performance->shared_type = CPUFREQ_SHARED_TYPE_HW;
719 else if (pdomain->coord_type == DOMAIN_COORD_TYPE_SW_ANY)
720 pr->performance->shared_type = CPUFREQ_SHARED_TYPE_ANY;
721

Signature · 1 line

Thanks Alex

Alex Shi

author

2013-04-18 08:54 UTC #11 lore raw

In reply to Viresh Kumar

Quoted 10 lines · Viresh Kumar

Quoted 7 lines
Quoted 7 lines
About your system: Can you give output of cpufreq-info for v3.8 and
Quoted 2 lines
Quoted 2 lines
v3.9-latestrc?
Your cpus share a clock line or not?
Yes our cpu shares a clock line. So the cpufreq-info is same. SNB CPU p-state is per cpu package, and hardware will coordinate the software asking, then decide which cpufreq should be.
Can you paste cpufreq-info output, that will give us a better picture of your system.

at the most of time with your patch cpufreq-info is 0.8GHz, without the patch, the cpufreq is varied from 1.0 to 2.8 Ghz.

Signature · 1 line

Thanks Alex

Viresh Kumar

2013-04-18 10:01 UTC #12 lore raw

Quoted 8 lines · Alex Shi

Quoted 7 lines · Viresh Kumar
Quoted 7 lines
Quoted 7 lines
About your system: Can you give output of cpufreq-info for v3.8 and
Quoted 2 lines
Quoted 2 lines
v3.9-latestrc?
Your cpus share a clock line or not?
Yes our cpu shares a clock line. So the cpufreq-info is same. SNB CPU p-state is per cpu package, and hardware will coordinate the software asking, then decide which cpufreq should be.

Quoted 6 lines

Quoted 3 lines
Can you paste cpufreq-info output, that will give us a better picture of your system.
at the most of time with your patch cpufreq-info is 0.8GHz, without the patch, the cpufreq is varied from 1.0 to 2.8 Ghz.

I need full output of following command:

$ cpufreq-info

This has got lots of other details too.

Alex Shi

author

2013-04-19 10:00 UTC #13 lore raw

Quoted 13 lines · Viresh Kumar

Quoted 12 lines · Alex Shi
Quoted 10 lines · Viresh Kumar
Quoted 7 lines
Quoted 7 lines
About your system: Can you give output of cpufreq-info for v3.8 and
Quoted 2 lines
Quoted 2 lines
v3.9-latestrc?
Your cpus share a clock line or not?
Yes our cpu shares a clock line. So the cpufreq-info is same. SNB CPU p-state is per cpu package, and hardware will coordinate the software asking, then decide which cpufreq should be.
Can you paste cpufreq-info output, that will give us a better picture of your system.

Quoted 6 lines

Quoted 2 lines
at the most of time with your patch cpufreq-info is 0.8GHz, without the patch, the cpufreq is varied from 1.0 to 2.8 Ghz.
I need full output of following command:
$ cpufreq-info

Sorry for replay late.

the following is cpufreq-info output at 3.9-rc4:
During bltk-game testing, most of time the cpu freq is 2.2G
============
[root@lkp-sb01 cpufreq]#cpupower frequency-info
analyzing CPU 0:
driver: acpi-cpufreq
CPUs which run at the same hardware frequency: 0 1 2 3 4 5 6 7 CPUs which need to have their frequency coordinated by software: 0 1 2
3 4 5 6 7
maximum transition latency: 10.0 us.
hardware limits: 800 MHz - 2.20 GHz
available frequency steps: 2.20 GHz, 2.20 GHz, 2.00 GHz, 1.90 GHz,
1.80 GHz, 1.70 GHz, 1.60 GHz, 1.50 GHz, 1.40 GHz, 1.30 GHz, 1.20 GHz, 1.10 GHz, 1000 MHz, 900 MHz, 800 MHz
available cpufreq governors: ondemand, userspace, performance current policy: frequency should be within 800 MHz and 2.20 GHz.
The governor "ondemand" may decide which speed to use within this range.
current CPU frequency is 2.20 GHz (asserted by call to hardware). boost state support:
Supported: no
Active: no

And this is the 3.9-rc5 kernel, with your patch:
During testing, most of time cpufreq is 0.8Ghz
============
[root@lkp-sb01 ~]#cpupower frequency-info
analyzing CPU 0:
driver: acpi-cpufreq
CPUs which run at the same hardware frequency: 0
CPUs which need to have their frequency coordinated by software: 0 maximum transition latency: 10.0 us.
hardware limits: 800 MHz - 2.20 GHz
available frequency steps: 2.20 GHz, 2.20 GHz, 2.00 GHz, 1.90 GHz,
1.80 GHz, 1.70 GHz, 1.60 GHz, 1.50 GHz, 1.40 GHz, 1.30 GHz, 1.20 GHz, 1.10 GHz, 1000 MHz, 900 MHz, 800 MHz
available cpufreq governors: ondemand, userspace, performance current policy: frequency should be within 800 MHz and 2.20 GHz.
The governor "ondemand" may decide which speed to use within this range.
current CPU frequency is 800 MHz (asserted by call to hardware). boost state support:
Supported: no
Active: no

Quoted 3 lines

This has got lots of other details too.

Signature · 2 lines

Thanks
    Alex

Viresh Kumar

2013-04-19 12:23 UTC #14 lore raw

Quoted 36 lines · Alex Shi

the following is cpufreq-info output at 3.9-rc4:
During bltk-game testing, most of time the cpu freq is 2.2G
============
[root@lkp-sb01 cpufreq]#cpupower frequency-info
analyzing CPU 0:
driver: acpi-cpufreq
CPUs which run at the same hardware frequency: 0 1 2 3 4 5 6 7 CPUs which need to have their frequency coordinated by software: 0 1 2
3 4 5 6 7
maximum transition latency: 10.0 us.
hardware limits: 800 MHz - 2.20 GHz
available frequency steps: 2.20 GHz, 2.20 GHz, 2.00 GHz, 1.90 GHz,
1.80 GHz, 1.70 GHz, 1.60 GHz, 1.50 GHz, 1.40 GHz, 1.30 GHz, 1.20 GHz, 1.10 GHz, 1000 MHz, 900 MHz, 800 MHz
available cpufreq governors: ondemand, userspace, performance current policy: frequency should be within 800 MHz and 2.20 GHz.
The governor "ondemand" may decide which speed to use within this range.
current CPU frequency is 2.20 GHz (asserted by call to hardware). boost state support:
Supported: no
Active: no
And this is the 3.9-rc5 kernel, with your patch:
During testing, most of time cpufreq is 0.8Ghz
============
[root@lkp-sb01 ~]#cpupower frequency-info
analyzing CPU 0:
driver: acpi-cpufreq
CPUs which run at the same hardware frequency: 0
CPUs which need to have their frequency coordinated by software: 0 maximum transition latency: 10.0 us.
hardware limits: 800 MHz - 2.20 GHz
available frequency steps: 2.20 GHz, 2.20 GHz, 2.00 GHz, 1.90 GHz,
1.80 GHz, 1.70 GHz, 1.60 GHz, 1.50 GHz, 1.40 GHz, 1.30 GHz, 1.20 GHz, 1.10 GHz, 1000 MHz, 900 MHz, 800 MHz

Quoted 8 lines

available cpufreq governors: ondemand, userspace, performance current policy: frequency should be within 800 MHz and 2.20 GHz.
The governor "ondemand" may decide which speed to use within this range.
current CPU frequency is 800 MHz (asserted by call to hardware). boost state support:
Supported: no
Active: no

So you get better performance without my patch because we don't allocate any struct cpufreq_policy for any of the cpus leaving first one. And so only manage freq change for it and all other cpus stay at max power..

So, we clearly need to know why don't we want to have all cpus set in policy->cpus, when they actually share clock line?

Alex Shi

author

2013-04-19 14:43 UTC #15 lore raw

Quoted 6 lines · Viresh Kumar

So you get better performance without my patch because we don't allocate any struct cpufreq_policy for any of the cpus leaving first one. And so only manage freq change for it and all other cpus stay at max power..
So, we clearly need to know why don't we want to have all cpus set in policy->cpus, when they actually share clock line?

AFAIK, That because our p-state is HW coordinated. For further info, Maybe Arjan and Len can explain more.

Signature · 2 lines

Thanks
    Alex

Arjan van de Ven

2013-04-19 15:44 UTC #16 lore raw

In reply to Viresh Kumar

Quoted 9 lines

So you get better performance without my patch because we don't allocate any struct cpufreq_policy for any of the cpus leaving first one. And so only manage freq change for it and all other cpus stay at max power..
So, we clearly need to know why don't we want to have all cpus set in policy->cpus, when they actually share clock line?

right conclusion, wrong solution ;-)

Alex should know by now (we told him this internally quite a few times) that this indeed is happening. But the answer is not to try to make the software know that things share a clock line, the answer is that the current code is correct.

(on x86, the way shared voltage/frequency domains work is that the current clock value is the maximum of what the individual *non idle* cpu's asked for. If the cpu asking for the highest value goes idle, all others will drop their frequency to the value the next highest is asking for, etc. Trying to do this kind of thing in software is backwards and actually ends up burning a lot of power)

on the 'regression'; of COURSE things might go slower once you stop acting like the "performance" governor. now, ondemand is pretty dire for this generation of hardware, and that's why we have asked Alex repeatedly to actually use the driver Intel has gotten upstream for this generation of hardware instead. I do not know why he has not done that instead of trying to get a bug put back into the kernel.