linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* bltk-game regressions on snb laptop
@ 2013-04-16  7:14 Alex Shi
  2013-04-17 16:34 ` Rafael J. Wysocki
  2013-04-17 16:46 ` Viresh Kumar
  0 siblings, 2 replies; 17+ messages in thread
From: Alex Shi @ 2013-04-16  7:14 UTC (permalink / raw)
  To: Linux PM list, Brown, Len, Wysocki, Rafael J, Arjan van de Ven,
	LKP ML, Viresh Kumar

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.

-- 
Thanks Alex

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: bltk-game regressions on snb laptop
  2013-04-16  7:14 bltk-game regressions on snb laptop Alex Shi
@ 2013-04-17 16:34 ` Rafael J. Wysocki
  2013-04-18  0:23   ` Alex Shi
  2013-04-17 16:46 ` Viresh Kumar
  1 sibling, 1 reply; 17+ messages in thread
From: Rafael J. Wysocki @ 2013-04-17 16:34 UTC (permalink / raw)
  To: Alex Shi
  Cc: Linux PM list, Brown, Len, Wysocki, Rafael J, Arjan van de Ven,
	LKP ML, Viresh Kumar

On Tuesday, April 16, 2013 03:14:31 PM Alex Shi wrote:
> 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.

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

Rafael


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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: bltk-game regressions on snb laptop
  2013-04-16  7:14 bltk-game regressions on snb laptop Alex Shi
  2013-04-17 16:34 ` Rafael J. Wysocki
@ 2013-04-17 16:46 ` Viresh Kumar
  2013-04-18  0:48   ` Alex Shi
  1 sibling, 1 reply; 17+ messages in thread
From: Viresh Kumar @ 2013-04-17 16:46 UTC (permalink / raw)
  To: Alex Shi
  Cc: Linux PM list, Brown, Len, Wysocki, Rafael J, Arjan van de Ven,
	LKP ML

On 16 April 2013 12:44, Alex Shi <alex.shi@intel.com> wrote:
> 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.

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.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: bltk-game regressions on snb laptop
  2013-04-17 16:34 ` Rafael J. Wysocki
@ 2013-04-18  0:23   ` Alex Shi
  0 siblings, 0 replies; 17+ messages in thread
From: Alex Shi @ 2013-04-18  0:23 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linux PM list, Brown, Len, Wysocki, Rafael J, Arjan van de Ven,
	LKP ML, Viresh Kumar

On 04/18/2013 12:34 AM, Rafael J. Wysocki wrote:
> On Tuesday, April 16, 2013 03:14:31 PM Alex Shi wrote:
>> 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.
> 
> 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.
> 
> Rafael
> 
> 


-- 
Thanks Alex

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: bltk-game regressions on snb laptop
  2013-04-17 16:46 ` Viresh Kumar
@ 2013-04-18  0:48   ` Alex Shi
  2013-04-18  4:26     ` Viresh Kumar
  0 siblings, 1 reply; 17+ messages in thread
From: Alex Shi @ 2013-04-18  0:48 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Linux PM list, Brown, Len, Wysocki, Rafael J, Arjan van de Ven,
	LKP ML

On 04/18/2013 12:46 AM, Viresh Kumar wrote:
> On 16 April 2013 12:44, Alex Shi <alex.shi@intel.com> wrote:
>> 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.
> 
> 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.

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


-- 
Thanks Alex

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: bltk-game regressions on snb laptop
  2013-04-18  0:48   ` Alex Shi
@ 2013-04-18  4:26     ` Viresh Kumar
  2013-04-18  4:36       ` Viresh Kumar
  2013-04-18  5:16       ` Alex Shi
  0 siblings, 2 replies; 17+ messages in thread
From: Viresh Kumar @ 2013-04-18  4:26 UTC (permalink / raw)
  To: Alex Shi
  Cc: Linux PM list, Brown, Len, Wysocki, Rafael J, Arjan van de Ven,
	LKP ML

On 18 April 2013 06:18, Alex Shi <alex.shi@intel.com> wrote:
> On 04/18/2013 12:46 AM, Viresh Kumar wrote:
>> On 16 April 2013 12:44, Alex Shi <alex.shi@intel.com> 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?

> 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);

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

> 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.

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.

> 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

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

> 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.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: bltk-game regressions on snb laptop
  2013-04-18  4:26     ` Viresh Kumar
@ 2013-04-18  4:36       ` Viresh Kumar
  2013-04-18  5:24         ` Alex Shi
  2013-04-18  5:16       ` Alex Shi
  1 sibling, 1 reply; 17+ messages in thread
From: Viresh Kumar @ 2013-04-18  4:36 UTC (permalink / raw)
  To: Alex Shi
  Cc: Linux PM list, Brown, Len, Wysocki, Rafael J, Arjan van de Ven,
	LKP ML

On 18 April 2013 09:56, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 18 April 2013 06:18, Alex Shi <alex.shi@intel.com> wrote:

>> 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
>
> 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.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: bltk-game regressions on snb laptop
  2013-04-18  4:26     ` Viresh Kumar
  2013-04-18  4:36       ` Viresh Kumar
@ 2013-04-18  5:16       ` Alex Shi
  2013-04-18  5:37         ` Viresh Kumar
  1 sibling, 1 reply; 17+ messages in thread
From: Alex Shi @ 2013-04-18  5:16 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Linux PM list, Brown, Len, Wysocki, Rafael J, Arjan van de Ven,
	LKP ML

On 04/18/2013 12:26 PM, Viresh Kumar wrote:
> On 18 April 2013 06:18, Alex Shi <alex.shi@intel.com> wrote:
>> On 04/18/2013 12:46 AM, Viresh Kumar wrote:
>>> On 16 April 2013 12:44, Alex Shi <alex.shi@intel.com> 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.

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.

> 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.
> 
>> 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);
> 
> 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. :)
> 
>>
> 
>> 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.
> 


-- 
Thanks Alex

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: bltk-game regressions on snb laptop
  2013-04-18  4:36       ` Viresh Kumar
@ 2013-04-18  5:24         ` Alex Shi
  0 siblings, 0 replies; 17+ messages in thread
From: Alex Shi @ 2013-04-18  5:24 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Linux PM list, Brown, Len, Wysocki, Rafael J, Arjan van de Ven,
	LKP ML

On 04/18/2013 12:36 PM, Viresh Kumar wrote:
>>> >> 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.
> 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. :)

-- 
Thanks Alex

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: bltk-game regressions on snb laptop
  2013-04-18  5:16       ` Alex Shi
@ 2013-04-18  5:37         ` Viresh Kumar
  2013-04-18  6:16           ` Alex Shi
  2013-04-18  8:54           ` Alex Shi
  0 siblings, 2 replies; 17+ messages in thread
From: Viresh Kumar @ 2013-04-18  5:37 UTC (permalink / raw)
  To: Alex Shi
  Cc: Linux PM list, Brown, Len, Wysocki, Rafael J, Arjan van de Ven,
	LKP ML

On 18 April 2013 10:46, Alex Shi <alex.shi@intel.com> wrote:
> 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.

>> 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.

> 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?

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: bltk-game regressions on snb laptop
  2013-04-18  5:37         ` Viresh Kumar
@ 2013-04-18  6:16           ` Alex Shi
  2013-04-18  8:54           ` Alex Shi
  1 sibling, 0 replies; 17+ messages in thread
From: Alex Shi @ 2013-04-18  6:16 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Linux PM list, Brown, Len, Wysocki, Rafael J, Arjan van de Ven,
	LKP ML

On 04/18/2013 01:37 PM, Viresh Kumar wrote:
>>> >> 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.
> 
>> > 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 



-- 
Thanks Alex

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: bltk-game regressions on snb laptop
  2013-04-18  5:37         ` Viresh Kumar
  2013-04-18  6:16           ` Alex Shi
@ 2013-04-18  8:54           ` Alex Shi
  2013-04-18 10:01             ` Viresh Kumar
  1 sibling, 1 reply; 17+ messages in thread
From: Alex Shi @ 2013-04-18  8:54 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Linux PM list, Brown, Len, Wysocki, Rafael J, Arjan van de Ven,
	LKP ML

On 04/18/2013 01:37 PM, Viresh Kumar wrote:
>>> 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.
> 

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.


-- 
Thanks Alex

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: bltk-game regressions on snb laptop
  2013-04-18  8:54           ` Alex Shi
@ 2013-04-18 10:01             ` Viresh Kumar
  2013-04-19 10:00               ` Alex Shi
  0 siblings, 1 reply; 17+ messages in thread
From: Viresh Kumar @ 2013-04-18 10:01 UTC (permalink / raw)
  To: Alex Shi
  Cc: Linux PM list, Brown, Len, Wysocki, Rafael J, Arjan van de Ven,
	LKP ML

On 18 April 2013 14:24, Alex Shi <alex.shi@intel.com> wrote:
> On 04/18/2013 01:37 PM, Viresh Kumar wrote:
>>>> 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.
>>
>
> 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.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: bltk-game regressions on snb laptop
  2013-04-18 10:01             ` Viresh Kumar
@ 2013-04-19 10:00               ` Alex Shi
  2013-04-19 12:23                 ` Viresh Kumar
  0 siblings, 1 reply; 17+ messages in thread
From: Alex Shi @ 2013-04-19 10:00 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Linux PM list, Brown, Len, Wysocki, Rafael J, Arjan van de Ven,
	LKP ML

On 04/18/2013 06:01 PM, Viresh Kumar wrote:
> On 18 April 2013 14:24, Alex Shi <alex.shi@intel.com> wrote:
>> On 04/18/2013 01:37 PM, Viresh Kumar wrote:
>>>>> 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.
>>>
>>
>> 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

> 
> This has got lots of other details too.
> 


-- 
Thanks
    Alex

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: bltk-game regressions on snb laptop
  2013-04-19 10:00               ` Alex Shi
@ 2013-04-19 12:23                 ` Viresh Kumar
  2013-04-19 14:43                   ` Alex Shi
  2013-04-19 15:44                   ` Arjan van de Ven
  0 siblings, 2 replies; 17+ messages in thread
From: Viresh Kumar @ 2013-04-19 12:23 UTC (permalink / raw)
  To: Alex Shi
  Cc: Linux PM list, Brown, Len, Wysocki, Rafael J, Arjan van de Ven,
	LKP ML

On 19 April 2013 15:30, Alex Shi <alex.shi@intel.com> wrote:
> 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

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?

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: bltk-game regressions on snb laptop
  2013-04-19 12:23                 ` Viresh Kumar
@ 2013-04-19 14:43                   ` Alex Shi
  2013-04-19 15:44                   ` Arjan van de Ven
  1 sibling, 0 replies; 17+ messages in thread
From: Alex Shi @ 2013-04-19 14:43 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Linux PM list, Brown, Len, Wysocki, Rafael J, Arjan van de Ven,
	LKP ML

On 04/19/2013 08:23 PM, Viresh Kumar wrote:
> 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.

-- 
Thanks
    Alex

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: bltk-game regressions on snb laptop
  2013-04-19 12:23                 ` Viresh Kumar
  2013-04-19 14:43                   ` Alex Shi
@ 2013-04-19 15:44                   ` Arjan van de Ven
  1 sibling, 0 replies; 17+ messages in thread
From: Arjan van de Ven @ 2013-04-19 15:44 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Alex Shi, Linux PM list, Brown, Len, Wysocki, Rafael J, LKP ML


>
> 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.



^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2013-04-19 15:44 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-16  7:14 bltk-game regressions on snb laptop Alex Shi
2013-04-17 16:34 ` Rafael J. Wysocki
2013-04-18  0:23   ` Alex Shi
2013-04-17 16:46 ` Viresh Kumar
2013-04-18  0:48   ` Alex Shi
2013-04-18  4:26     ` Viresh Kumar
2013-04-18  4:36       ` Viresh Kumar
2013-04-18  5:24         ` Alex Shi
2013-04-18  5:16       ` Alex Shi
2013-04-18  5:37         ` Viresh Kumar
2013-04-18  6:16           ` Alex Shi
2013-04-18  8:54           ` Alex Shi
2013-04-18 10:01             ` Viresh Kumar
2013-04-19 10:00               ` Alex Shi
2013-04-19 12:23                 ` Viresh Kumar
2013-04-19 14:43                   ` Alex Shi
2013-04-19 15:44                   ` Arjan van de Ven

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).