Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1366506 > unrolled thread
| Started by | Jörg Otte <jrg.otte@gmail.com> |
|---|---|
| First post | 2016-03-29 19:40 +0200 |
| Last post | 2016-03-30 21:00 +0200 |
| Articles | 20 on this page of 40 — 6 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [intel-pstate driver regression] processor frequency very high even if in idle Jörg Otte <jrg.otte@gmail.com> - 2016-03-29 19:40 +0200
Re: [intel-pstate driver regression] processor frequency very high even if in idle "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-03-29 23:40 +0200
Re: [intel-pstate driver regression] processor frequency very high even if in idle Jörg Otte <jrg.otte@gmail.com> - 2016-03-30 12:20 +0200
Re: [intel-pstate driver regression] processor frequency very high even if in idle "Rafael J. Wysocki" <rafael@kernel.org> - 2016-03-30 13:10 +0200
Re: [intel-pstate driver regression] processor frequency very high even if in idle Jörg Otte <jrg.otte@gmail.com> - 2016-03-30 17:30 +0200
Re: [intel-pstate driver regression] processor frequency very high even if in idle "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-03-30 20:40 +0200
Re: [intel-pstate driver regression] processor frequency very high even if in idle Jörg Otte <jrg.otte@gmail.com> - 2016-03-31 11:10 +0200
Re: [intel-pstate driver regression] processor frequency very high even if in idle "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-03-31 13:50 +0200
Re: [intel-pstate driver regression] processor frequency very high even if in idle Jörg Otte <jrg.otte@gmail.com> - 2016-03-31 17:30 +0200
Re: [intel-pstate driver regression] processor frequency very high even if in idle "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-03-31 17:50 +0200
Re: [intel-pstate driver regression] processor frequency very high even if in idle Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> - 2016-03-31 18:20 +0200
Re: [intel-pstate driver regression] processor frequency very high even if in idle Jörg Otte <jrg.otte@gmail.com> - 2016-03-31 19:30 +0200
Re: [intel-pstate driver regression] processor frequency very high even if in idle Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> - 2016-03-31 20:00 +0200
Re: [intel-pstate driver regression] processor frequency very high even if in idle "Rafael J. Wysocki" <rafael@kernel.org> - 2016-04-01 02:10 +0200
Re: [intel-pstate driver regression] processor frequency very high even if in idle Jörg Otte <jrg.otte@gmail.com> - 2016-04-01 11:50 +0200
Re: [intel-pstate driver regression] processor frequency very high even if in idle Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> - 2016-04-01 17:10 +0200
Re: [intel-pstate driver regression] processor frequency very high even if in idle "Rafael J. Wysocki" <rafael@kernel.org> - 2016-04-01 22:30 +0200
Re: [intel-pstate driver regression] processor frequency very high even if in idle "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-04-01 14:40 +0200
Re: [intel-pstate driver regression] processor frequency very high even if in idle Jörg Otte <jrg.otte@gmail.com> - 2016-04-01 16:10 +0200
Re: [intel-pstate driver regression] processor frequency very high even if in idle Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> - 2016-04-01 19:50 +0200
RE: [intel-pstate driver regression] processor frequency very high even if in idle "Doug Smythies" <dsmythies@telus.net> - 2016-04-01 20:40 +0200
Re: [intel-pstate driver regression] processor frequency very high even if in idle Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> - 2016-04-01 20:50 +0200
Re: [intel-pstate driver regression] processor frequency very high even if in idle "Rafael J. Wysocki" <rafael@kernel.org> - 2016-04-01 22:00 +0200
RE: [intel-pstate driver regression] processor frequency very high even if in idle "Doug Smythies" <dsmythies@telus.net> - 2016-04-02 01:40 +0200
Re: [intel-pstate driver regression] processor frequency very high even if in idle "Rafael J. Wysocki" <rafael@kernel.org> - 2016-04-02 02:30 +0200
Re: [intel-pstate driver regression] processor frequency very high even if in idle "Rafael J. Wysocki" <rafael@kernel.org> - 2016-04-01 22:30 +0200
RE: [intel-pstate driver regression] processor frequency very high even if in idle "Doug Smythies" <dsmythies@telus.net> - 2016-04-01 17:30 +0200
Re: [intel-pstate driver regression] processor frequency very high even if in idle Jörg Otte <jrg.otte@gmail.com> - 2016-04-01 18:50 +0200
Re: [intel-pstate driver regression] processor frequency very high even if in idle Jörg Otte <jrg.otte@gmail.com> - 2016-04-01 19:40 +0200
Re: [intel-pstate driver regression] processor frequency very high even if in idle "Pandruvada, Srinivas" <srinivas.pandruvada@intel.com> - 2016-03-30 17:40 +0200
Re: [intel-pstate driver regression] processor frequency very high even if in idle Jörg Otte <jrg.otte@gmail.com> - 2016-03-30 18:00 +0200
Re: [intel-pstate driver regression] processor frequency very high even if in idle Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> - 2016-03-30 21:00 +0200
Re: [intel-pstate driver regression] processor frequency very high even if in idle "Rafael J. Wysocki" <rafael@kernel.org> - 2016-03-30 22:20 +0200
Re: [intel-pstate driver regression] processor frequency very high even if in idle Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> - 2016-03-30 22:30 +0200
Re: [intel-pstate driver regression] processor frequency very high even if in idle Jörg Otte <jrg.otte@gmail.com> - 2016-03-31 11:30 +0200
RE: [intel-pstate driver regression] processor frequency very high even if in idle "Doug Smythies" <dsmythies@telus.net> - 2016-03-31 16:40 +0200
RE: [intel-pstate driver regression] processor frequency very high even if in idle "Doug Smythies" <dsmythies@telus.net> - 2016-03-31 17:20 +0200
Re: [intel-pstate driver regression] processor frequency very high even if in idle Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> - 2016-03-31 17:20 +0200
RE: [intel-pstate driver regression] processor frequency very high even if in idle "Doug Smythies" <dsmythies@telus.net> - 2016-04-01 09:30 +0200
RE: [intel-pstate driver regression] processor frequency very high even if in idle "Doug Smythies" <dsmythies@telus.net> - 2016-03-30 21:00 +0200
Page 1 of 2 [1] 2 Next page →
| From | Jörg Otte <jrg.otte@gmail.com> |
|---|---|
| Date | 2016-03-29 19:40 +0200 |
| Subject | Re: [intel-pstate driver regression] processor frequency very high even if in idle |
| Message-ID | <ri5Mt-14H-1@gated-at.bofh.it> |
2016-03-29 19:24 GMT+02:00 Jörg Otte <jrg.otte@gmail.com>: > in v4.5 and earlier intel-pstate downscaled idle processors (load > 0.1-0.2%) to minumum frequency, in my case 800MHz. > > Now in v4.6-rc1 the characteristic has dramatically changed. If in > idle the processor frequency is more or less a few MHz around 2500Mhz. > This is the maximum non turbo frequency. > > No difference between powersafe or performance governor. > > I currently use acpi_cpufreq which works as usual. > > Processor: > Intel(R) Core(TM) i5-4200M CPU @ 2.50GHz (family: 0x6, model: 0x3c, > stepping: 0x3) > > Last known good kernel is: 4.5.0-01127-g9256d5a > First known bad kernel is: 4.5.0-02535-g09fd671 > > There is > commit 277edba Merge tag 'pm+acpi-4.6-rc1-1' of > git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm > in between, which brought a few changes in intel_pstate. > > thanks, Jörg
[toc] | [next] | [standalone]
| From | "Rafael J. Wysocki" <rjw@rjwysocki.net> |
|---|---|
| Date | 2016-03-29 23:40 +0200 |
| Subject | Re: [intel-pstate driver regression] processor frequency very high even if in idle |
| Message-ID | <ri9wJ-3Fl-9@gated-at.bofh.it> |
| In reply to | #1366506 |
On Tuesday, March 29, 2016 07:32:27 PM Jörg Otte wrote: > 2016-03-29 19:24 GMT+02:00 Jörg Otte <jrg.otte@gmail.com>: > > in v4.5 and earlier intel-pstate downscaled idle processors (load > > 0.1-0.2%) to minumum frequency, in my case 800MHz. > > > > Now in v4.6-rc1 the characteristic has dramatically changed. If in > > idle the processor frequency is more or less a few MHz around 2500Mhz. > > This is the maximum non turbo frequency. > > > > No difference between powersafe or performance governor. > > > > I currently use acpi_cpufreq which works as usual. > > > > Processor: > > Intel(R) Core(TM) i5-4200M CPU @ 2.50GHz (family: 0x6, model: 0x3c, > > stepping: 0x3) > > > > Last known good kernel is: 4.5.0-01127-g9256d5a > > First known bad kernel is: 4.5.0-02535-g09fd671 > > > > There is > > commit 277edba Merge tag 'pm+acpi-4.6-rc1-1' of > > git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm > > in between, which brought a few changes in intel_pstate. Can you please check commit a4675fbc4a7a (cpufreq: intel_pstate: Replace timers with utilization update callbacks)? Thanks, Rafael
[toc] | [prev] | [next] | [standalone]
| From | Jörg Otte <jrg.otte@gmail.com> |
|---|---|
| Date | 2016-03-30 12:20 +0200 |
| Message-ID | <riloe-3NH-23@gated-at.bofh.it> |
| In reply to | #1366728 |
2016-03-29 23:34 GMT+02:00 Rafael J. Wysocki <rjw@rjwysocki.net>: > On Tuesday, March 29, 2016 07:32:27 PM Jörg Otte wrote: >> 2016-03-29 19:24 GMT+02:00 Jörg Otte <jrg.otte@gmail.com>: >> > in v4.5 and earlier intel-pstate downscaled idle processors (load >> > 0.1-0.2%) to minumum frequency, in my case 800MHz. >> > >> > Now in v4.6-rc1 the characteristic has dramatically changed. If in >> > idle the processor frequency is more or less a few MHz around 2500Mhz. >> > This is the maximum non turbo frequency. >> > >> > No difference between powersafe or performance governor. >> > >> > I currently use acpi_cpufreq which works as usual. >> > >> > Processor: >> > Intel(R) Core(TM) i5-4200M CPU @ 2.50GHz (family: 0x6, model: 0x3c, >> > stepping: 0x3) >> > >> > Last known good kernel is: 4.5.0-01127-g9256d5a >> > First known bad kernel is: 4.5.0-02535-g09fd671 >> > >> > There is >> > commit 277edba Merge tag 'pm+acpi-4.6-rc1-1' of >> > git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm >> > in between, which brought a few changes in intel_pstate. > > Can you please check commit a4675fbc4a7a (cpufreq: intel_pstate: Replace timers > with utilization update callbacks)? > Yes , this solved the problem for me. I had to resolve some conflicts myself when reverting that commit. Hard work :). Here is a 10-seconds trace of the used frequencies when in "desktop-idle": driver cpu0 cpu1 cpu2 cpu3 ------------------------------------- intel_pstate ( 800 928 941 1200) MHz load:( 0.2)% intel_pstate ( 800 928 1181 1800) MHz load:( 0.0)% intel_pstate ( 1675 1576 1347 800) MHz load:( 0.0)% intel_pstate ( 1198 1576 842 800) MHz load:( 0.5)% intel_pstate ( 800 1181 1113 1600) MHz load:( 0.0)% intel_pstate ( 808 1181 805 800) MHz load:( 0.5)% intel_pstate ( 844 1191 900 1082) MHz load:( 0.3)% intel_pstate ( 816 1191 800 800) MHz load:( 0.0)% intel_pstate ( 800 905 892 1082) MHz load:( 0.2)% intel_pstate ( 945 905 1340 800) MHz load:( 0.3)% Thanks, Jörg
[toc] | [prev] | [next] | [standalone]
| From | "Rafael J. Wysocki" <rafael@kernel.org> |
|---|---|
| Date | 2016-03-30 13:10 +0200 |
| Message-ID | <rimaD-4sj-27@gated-at.bofh.it> |
| In reply to | #1367073 |
On Wed, Mar 30, 2016 at 12:17 PM, Jörg Otte <jrg.otte@gmail.com> wrote: > 2016-03-29 23:34 GMT+02:00 Rafael J. Wysocki <rjw@rjwysocki.net>: >> On Tuesday, March 29, 2016 07:32:27 PM Jörg Otte wrote: >>> 2016-03-29 19:24 GMT+02:00 Jörg Otte <jrg.otte@gmail.com>: >>> > in v4.5 and earlier intel-pstate downscaled idle processors (load >>> > 0.1-0.2%) to minumum frequency, in my case 800MHz. >>> > >>> > Now in v4.6-rc1 the characteristic has dramatically changed. If in >>> > idle the processor frequency is more or less a few MHz around 2500Mhz. >>> > This is the maximum non turbo frequency. >>> > >>> > No difference between powersafe or performance governor. >>> > >>> > I currently use acpi_cpufreq which works as usual. >>> > >>> > Processor: >>> > Intel(R) Core(TM) i5-4200M CPU @ 2.50GHz (family: 0x6, model: 0x3c, >>> > stepping: 0x3) >>> > >>> > Last known good kernel is: 4.5.0-01127-g9256d5a >>> > First known bad kernel is: 4.5.0-02535-g09fd671 >>> > >>> > There is >>> > commit 277edba Merge tag 'pm+acpi-4.6-rc1-1' of >>> > git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm >>> > in between, which brought a few changes in intel_pstate. >> >> Can you please check commit a4675fbc4a7a (cpufreq: intel_pstate: Replace timers >> with utilization update callbacks)? >> > Yes , this solved the problem for me. > I had to resolve some conflicts myself when reverting that > commit. Hard work :). Thanks for doing this. Can you please post the revert patch you have used? > Here is a 10-seconds trace of the used frequencies when > in "desktop-idle": > > driver cpu0 cpu1 cpu2 cpu3 > ------------------------------------- > intel_pstate ( 800 928 941 1200) MHz load:( 0.2)% > intel_pstate ( 800 928 1181 1800) MHz load:( 0.0)% > intel_pstate ( 1675 1576 1347 800) MHz load:( 0.0)% > intel_pstate ( 1198 1576 842 800) MHz load:( 0.5)% > intel_pstate ( 800 1181 1113 1600) MHz load:( 0.0)% > intel_pstate ( 808 1181 805 800) MHz load:( 0.5)% > intel_pstate ( 844 1191 900 1082) MHz load:( 0.3)% > intel_pstate ( 816 1191 800 800) MHz load:( 0.0)% > intel_pstate ( 800 905 892 1082) MHz load:( 0.2)% > intel_pstate ( 945 905 1340 800) MHz load:( 0.3)% Please also run turbostat with and without your revert patch applied. Thanks, Rafael
[toc] | [prev] | [next] | [standalone]
| From | Jörg Otte <jrg.otte@gmail.com> |
|---|---|
| Date | 2016-03-30 17:30 +0200 |
| Message-ID | <riqed-7pH-9@gated-at.bofh.it> |
| In reply to | #1367106 |
[Multipart message — attachments visible in raw view] — view raw
2016-03-30 13:05 GMT+02:00 Rafael J. Wysocki <rafael@kernel.org>:
> On Wed, Mar 30, 2016 at 12:17 PM, Jörg Otte <jrg.otte@gmail.com> wrote:
>> 2016-03-29 23:34 GMT+02:00 Rafael J. Wysocki <rjw@rjwysocki.net>:
>>> On Tuesday, March 29, 2016 07:32:27 PM Jörg Otte wrote:
>>>> 2016-03-29 19:24 GMT+02:00 Jörg Otte <jrg.otte@gmail.com>:
>>>> > in v4.5 and earlier intel-pstate downscaled idle processors (load
>>>> > 0.1-0.2%) to minumum frequency, in my case 800MHz.
>>>> >
>>>> > Now in v4.6-rc1 the characteristic has dramatically changed. If in
>>>> > idle the processor frequency is more or less a few MHz around 2500Mhz.
>>>> > This is the maximum non turbo frequency.
>>>> >
>>>> > No difference between powersafe or performance governor.
>>>> >
>>>> > I currently use acpi_cpufreq which works as usual.
>>>> >
>>>> > Processor:
>>>> > Intel(R) Core(TM) i5-4200M CPU @ 2.50GHz (family: 0x6, model: 0x3c,
>>>> > stepping: 0x3)
>>>> >
>>>> > Last known good kernel is: 4.5.0-01127-g9256d5a
>>>> > First known bad kernel is: 4.5.0-02535-g09fd671
>>>> >
>>>> > There is
>>>> > commit 277edba Merge tag 'pm+acpi-4.6-rc1-1' of
>>>> > git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm
>>>> > in between, which brought a few changes in intel_pstate.
>>>
>>> Can you please check commit a4675fbc4a7a (cpufreq: intel_pstate: Replace timers
>>> with utilization update callbacks)?
>>>
>> Yes , this solved the problem for me.
>> I had to resolve some conflicts myself when reverting that
>> commit. Hard work :).
>
> Thanks for doing this. Can you please post the revert patch you have used?
>
The patch is on top of 4.5.0-02535-g09fd671.
I'm not sure what gmail is doing with spaces and tabs,
so I attach the revert patch.
>> Here is a 10-seconds trace of the used frequencies when
>> in "desktop-idle":
>>
>> driver cpu0 cpu1 cpu2 cpu3
>> -------------------------------------
>> intel_pstate ( 800 928 941 1200) MHz load:( 0.2)%
>> intel_pstate ( 800 928 1181 1800) MHz load:( 0.0)%
>> intel_pstate ( 1675 1576 1347 800) MHz load:( 0.0)%
>> intel_pstate ( 1198 1576 842 800) MHz load:( 0.5)%
>> intel_pstate ( 800 1181 1113 1600) MHz load:( 0.0)%
>> intel_pstate ( 808 1181 805 800) MHz load:( 0.5)%
>> intel_pstate ( 844 1191 900 1082) MHz load:( 0.3)%
>> intel_pstate ( 816 1191 800 800) MHz load:( 0.0)%
>> intel_pstate ( 800 905 892 1082) MHz load:( 0.2)%
>> intel_pstate ( 945 905 1340 800) MHz load:( 0.3)%
>
> Please also run turbostat with and without your revert patch applied.
>
turbostat without revert
Kernel: 4.5.0-02535-g09fd671
-----------------------------
CPUID(7): No-SGX
CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
- 13 0.53 2514 2495
0 14 0.55 2518 2495
1 8 0.33 2527 2495
2 15 0.60 2506 2495
3 16 0.62 2509 2495
turbostat after revert of commit a4675fbc4a7a
kernel: 4.5.0-reva4675fbc4a7a-02536-g77225b1
------------------------------
CPUID(7): No-SGX
CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
- 4 0.35 1142 2494
0 1 0.11 1016 2494
1 2 0.17 961 2494
2 10 0.82 1215 2494
3 3 0.29 1086 2494
CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
- 4 0.46 885 2494
0 1 0.12 889 2494
1 1 0.16 885 2494
2 10 1.15 883 2494
3 4 0.40 891 2494
[toc] | [prev] | [next] | [standalone]
| From | "Rafael J. Wysocki" <rjw@rjwysocki.net> |
|---|---|
| Date | 2016-03-30 20:40 +0200 |
| Subject | Re: [intel-pstate driver regression] processor frequency very high even if in idle |
| Message-ID | <ritc5-113-5@gated-at.bofh.it> |
| In reply to | #1367295 |
On Wednesday, March 30, 2016 05:29:18 PM Jörg Otte wrote:
> 2016-03-30 13:05 GMT+02:00 Rafael J. Wysocki <rafael@kernel.org>:
> > On Wed, Mar 30, 2016 at 12:17 PM, Jörg Otte <jrg.otte@gmail.com> wrote:
> >> 2016-03-29 23:34 GMT+02:00 Rafael J. Wysocki <rjw@rjwysocki.net>:
> >>> On Tuesday, March 29, 2016 07:32:27 PM Jörg Otte wrote:
> >>>> 2016-03-29 19:24 GMT+02:00 Jörg Otte <jrg.otte@gmail.com>:
> >>>> > in v4.5 and earlier intel-pstate downscaled idle processors (load
> >>>> > 0.1-0.2%) to minumum frequency, in my case 800MHz.
> >>>> >
> >>>> > Now in v4.6-rc1 the characteristic has dramatically changed. If in
> >>>> > idle the processor frequency is more or less a few MHz around 2500Mhz.
> >>>> > This is the maximum non turbo frequency.
> >>>> >
> >>>> > No difference between powersafe or performance governor.
> >>>> >
> >>>> > I currently use acpi_cpufreq which works as usual.
> >>>> >
> >>>> > Processor:
> >>>> > Intel(R) Core(TM) i5-4200M CPU @ 2.50GHz (family: 0x6, model: 0x3c,
> >>>> > stepping: 0x3)
> >>>> >
> >>>> > Last known good kernel is: 4.5.0-01127-g9256d5a
> >>>> > First known bad kernel is: 4.5.0-02535-g09fd671
> >>>> >
> >>>> > There is
> >>>> > commit 277edba Merge tag 'pm+acpi-4.6-rc1-1' of
> >>>> > git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm
> >>>> > in between, which brought a few changes in intel_pstate.
> >>>
> >>> Can you please check commit a4675fbc4a7a (cpufreq: intel_pstate: Replace timers
> >>> with utilization update callbacks)?
> >>>
> >> Yes , this solved the problem for me.
> >> I had to resolve some conflicts myself when reverting that
> >> commit. Hard work :).
> >
> > Thanks for doing this. Can you please post the revert patch you have used?
> >
>
> The patch is on top of 4.5.0-02535-g09fd671.
> I'm not sure what gmail is doing with spaces and tabs,
> so I attach the revert patch.
That worked, thanks!
> >> Here is a 10-seconds trace of the used frequencies when
> >> in "desktop-idle":
> >>
> >> driver cpu0 cpu1 cpu2 cpu3
> >> -------------------------------------
> >> intel_pstate ( 800 928 941 1200) MHz load:( 0.2)%
> >> intel_pstate ( 800 928 1181 1800) MHz load:( 0.0)%
> >> intel_pstate ( 1675 1576 1347 800) MHz load:( 0.0)%
> >> intel_pstate ( 1198 1576 842 800) MHz load:( 0.5)%
> >> intel_pstate ( 800 1181 1113 1600) MHz load:( 0.0)%
> >> intel_pstate ( 808 1181 805 800) MHz load:( 0.5)%
> >> intel_pstate ( 844 1191 900 1082) MHz load:( 0.3)%
> >> intel_pstate ( 816 1191 800 800) MHz load:( 0.0)%
> >> intel_pstate ( 800 905 892 1082) MHz load:( 0.2)%
> >> intel_pstate ( 945 905 1340 800) MHz load:( 0.3)%
> >
> > Please also run turbostat with and without your revert patch applied.
> >
>
> turbostat without revert
> Kernel: 4.5.0-02535-g09fd671
> -----------------------------
> CPUID(7): No-SGX
> CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
> - 13 0.53 2514 2495
> 0 14 0.55 2518 2495
> 1 8 0.33 2527 2495
> 2 15 0.60 2506 2495
> 3 16 0.62 2509 2495
>
> turbostat after revert of commit a4675fbc4a7a
> kernel: 4.5.0-reva4675fbc4a7a-02536-g77225b1
> ------------------------------
> CPUID(7): No-SGX
> CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
> - 4 0.35 1142 2494
> 0 1 0.11 1016 2494
> 1 2 0.17 961 2494
> 2 10 0.82 1215 2494
> 3 3 0.29 1086 2494
> CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
> - 4 0.46 885 2494
> 0 1 0.12 889 2494
> 1 1 0.16 885 2494
> 2 10 1.15 883 2494
> 3 4 0.40 891 2494
Clearly, there's something fishy here.
I've simplified your revert patch somewhat. Can you please test if the one
below still helps?
---
drivers/cpufreq/intel_pstate.c | 59 ++++++++++++++++++++++-------------------
1 file changed, 32 insertions(+), 27 deletions(-)
Index: linux-pm/drivers/cpufreq/intel_pstate.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/intel_pstate.c
+++ linux-pm/drivers/cpufreq/intel_pstate.c
@@ -71,7 +71,7 @@ struct sample {
u64 mperf;
u64 tsc;
int freq;
- u64 time;
+ ktime_t time;
};
struct pstate_data {
@@ -103,13 +103,13 @@ struct _pid {
struct cpudata {
int cpu;
- struct update_util_data update_util;
+ struct timer_list timer;
struct pstate_data pstate;
struct vid_data vid;
struct _pid pid;
- u64 last_sample_time;
+ ktime_t last_sample_time;
u64 prev_aperf;
u64 prev_mperf;
u64 prev_tsc;
@@ -882,7 +882,7 @@ static inline void intel_pstate_calc_bus
sample->core_pct_busy = (int32_t)core_pct;
}
-static inline bool intel_pstate_sample(struct cpudata *cpu, u64 time)
+static inline bool intel_pstate_sample(struct cpudata *cpu)
{
u64 aperf, mperf;
unsigned long flags;
@@ -899,7 +899,7 @@ static inline bool intel_pstate_sample(s
local_irq_restore(flags);
cpu->last_sample_time = cpu->sample.time;
- cpu->sample.time = time;
+ cpu->sample.time = ktime_get();
cpu->sample.aperf = aperf;
cpu->sample.mperf = mperf;
cpu->sample.tsc = tsc;
@@ -957,7 +957,7 @@ static inline int32_t get_target_pstate_
static inline int32_t get_target_pstate_use_performance(struct cpudata *cpu)
{
int32_t core_busy, max_pstate, current_pstate, sample_ratio;
- u64 duration_ns;
+ s64 duration_ns;
intel_pstate_calc_busy(cpu);
@@ -978,14 +978,15 @@ static inline int32_t get_target_pstate_
core_busy = mul_fp(core_busy, div_fp(max_pstate, current_pstate));
/*
- * Since our utilization update callback will not run unless we are
- * in C0, check if the actual elapsed time is significantly greater (3x)
- * than our sample interval. If it is, then we were idle for a long
- * enough period of time to adjust our busyness.
+ * Since we have a deferred timer, it will not fire unless
+ * we are in C0. So, determine if the actual elapsed time
+ * is significantly greater (3x) than our sample interval. If it
+ * is, then we were idle for a long enough period of time
+ * to adjust our busyness.
*/
- duration_ns = cpu->sample.time - cpu->last_sample_time;
- if ((s64)duration_ns > pid_params.sample_rate_ns * 3
- && cpu->last_sample_time > 0) {
+ duration_ns = ktime_to_ns(ktime_sub(cpu->sample.time,
+ cpu->last_sample_time));
+ if (duration_ns > pid_params.sample_rate_ns * 3) {
sample_ratio = div_fp(int_tofp(pid_params.sample_rate_ns),
int_tofp(duration_ns));
core_busy = mul_fp(core_busy, sample_ratio);
@@ -1032,17 +1033,19 @@ static inline void intel_pstate_adjust_b
get_avg_frequency(cpu));
}
-static void intel_pstate_update_util(struct update_util_data *data, u64 time,
- unsigned long util, unsigned long max)
+static void intel_pstate_timer_func(unsigned long __data)
{
- struct cpudata *cpu = container_of(data, struct cpudata, update_util);
- u64 delta_ns = time - cpu->sample.time;
+ struct cpudata *cpu = (struct cpudata *) __data;
+ bool sample_taken = intel_pstate_sample(cpu);
- if ((s64)delta_ns >= pid_params.sample_rate_ns) {
- bool sample_taken = intel_pstate_sample(cpu, time);
+ if (sample_taken) {
+ int delay;
- if (sample_taken && !hwp_active)
+ if (!hwp_active)
intel_pstate_adjust_busy_pstate(cpu);
+
+ delay = msecs_to_jiffies(pid_params.sample_rate_ms);
+ mod_timer_pinned(&cpu->timer, jiffies + delay);
}
}
@@ -1099,11 +1102,15 @@ static int intel_pstate_init_cpu(unsigne
intel_pstate_get_cpu_pstates(cpu);
+ init_timer_deferrable(&cpu->timer);
+ cpu->timer.data = (unsigned long)cpu;
+ cpu->timer.expires = jiffies + HZ/100;
+ cpu->timer.function = intel_pstate_timer_func;
+
intel_pstate_busy_pid_reset(cpu);
- intel_pstate_sample(cpu, 0);
+ intel_pstate_sample(cpu);
- cpu->update_util.func = intel_pstate_update_util;
- cpufreq_set_update_util_data(cpunum, &cpu->update_util);
+ add_timer_on(&cpu->timer, cpunum);
pr_debug("intel_pstate: controlling: cpu %d\n", cpunum);
@@ -1187,8 +1194,7 @@ static void intel_pstate_stop_cpu(struct
pr_debug("intel_pstate: CPU %d exiting\n", cpu_num);
- cpufreq_set_update_util_data(cpu_num, NULL);
- synchronize_sched();
+ del_timer_sync(&all_cpu_data[cpu_num]->timer);
if (hwp_active)
return;
@@ -1455,8 +1461,7 @@ out:
get_online_cpus();
for_each_online_cpu(cpu) {
if (all_cpu_data[cpu]) {
- cpufreq_set_update_util_data(cpu, NULL);
- synchronize_sched();
+ del_timer_sync(&all_cpu_data[cpu]->timer);
kfree(all_cpu_data[cpu]);
}
}
[toc] | [prev] | [next] | [standalone]
| From | Jörg Otte <jrg.otte@gmail.com> |
|---|---|
| Date | 2016-03-31 11:10 +0200 |
| Message-ID | <riGM3-2Es-31@gated-at.bofh.it> |
| In reply to | #1367514 |
2016-03-30 20:39 GMT+02:00 Rafael J. Wysocki <rjw@rjwysocki.net>:
> On Wednesday, March 30, 2016 05:29:18 PM Jörg Otte wrote:
>> 2016-03-30 13:05 GMT+02:00 Rafael J. Wysocki <rafael@kernel.org>:
>> > On Wed, Mar 30, 2016 at 12:17 PM, Jörg Otte <jrg.otte@gmail.com> wrote:
>> >> 2016-03-29 23:34 GMT+02:00 Rafael J. Wysocki <rjw@rjwysocki.net>:
>> >>> On Tuesday, March 29, 2016 07:32:27 PM Jörg Otte wrote:
>> >>>> 2016-03-29 19:24 GMT+02:00 Jörg Otte <jrg.otte@gmail.com>:
>> >>>> > in v4.5 and earlier intel-pstate downscaled idle processors (load
>> >>>> > 0.1-0.2%) to minumum frequency, in my case 800MHz.
>> >>>> >
>> >>>> > Now in v4.6-rc1 the characteristic has dramatically changed. If in
>> >>>> > idle the processor frequency is more or less a few MHz around 2500Mhz.
>> >>>> > This is the maximum non turbo frequency.
>> >>>> >
>> >>>> > No difference between powersafe or performance governor.
>> >>>> >
>> >>>> > I currently use acpi_cpufreq which works as usual.
>> >>>> >
>> >>>> > Processor:
>> >>>> > Intel(R) Core(TM) i5-4200M CPU @ 2.50GHz (family: 0x6, model: 0x3c,
>> >>>> > stepping: 0x3)
>> >>>> >
>> >>>> > Last known good kernel is: 4.5.0-01127-g9256d5a
>> >>>> > First known bad kernel is: 4.5.0-02535-g09fd671
>> >>>> >
>> >>>> > There is
>> >>>> > commit 277edba Merge tag 'pm+acpi-4.6-rc1-1' of
>> >>>> > git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm
>> >>>> > in between, which brought a few changes in intel_pstate.
>> >>>
>> >>> Can you please check commit a4675fbc4a7a (cpufreq: intel_pstate: Replace timers
>> >>> with utilization update callbacks)?
>> >>>
>> >> Yes , this solved the problem for me.
>> >> I had to resolve some conflicts myself when reverting that
>> >> commit. Hard work :).
>> >
>> > Thanks for doing this. Can you please post the revert patch you have used?
>> >
>>
>> The patch is on top of 4.5.0-02535-g09fd671.
>> I'm not sure what gmail is doing with spaces and tabs,
>> so I attach the revert patch.
>
> That worked, thanks!
>
>> >> Here is a 10-seconds trace of the used frequencies when
>> >> in "desktop-idle":
>> >>
>> >> driver cpu0 cpu1 cpu2 cpu3
>> >> -------------------------------------
>> >> intel_pstate ( 800 928 941 1200) MHz load:( 0.2)%
>> >> intel_pstate ( 800 928 1181 1800) MHz load:( 0.0)%
>> >> intel_pstate ( 1675 1576 1347 800) MHz load:( 0.0)%
>> >> intel_pstate ( 1198 1576 842 800) MHz load:( 0.5)%
>> >> intel_pstate ( 800 1181 1113 1600) MHz load:( 0.0)%
>> >> intel_pstate ( 808 1181 805 800) MHz load:( 0.5)%
>> >> intel_pstate ( 844 1191 900 1082) MHz load:( 0.3)%
>> >> intel_pstate ( 816 1191 800 800) MHz load:( 0.0)%
>> >> intel_pstate ( 800 905 892 1082) MHz load:( 0.2)%
>> >> intel_pstate ( 945 905 1340 800) MHz load:( 0.3)%
>> >
>> > Please also run turbostat with and without your revert patch applied.
>> >
>>
>> turbostat without revert
>> Kernel: 4.5.0-02535-g09fd671
>> -----------------------------
>> CPUID(7): No-SGX
>> CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
>> - 13 0.53 2514 2495
>> 0 14 0.55 2518 2495
>> 1 8 0.33 2527 2495
>> 2 15 0.60 2506 2495
>> 3 16 0.62 2509 2495
>>
>> turbostat after revert of commit a4675fbc4a7a
>> kernel: 4.5.0-reva4675fbc4a7a-02536-g77225b1
>> ------------------------------
>> CPUID(7): No-SGX
>> CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
>> - 4 0.35 1142 2494
>> 0 1 0.11 1016 2494
>> 1 2 0.17 961 2494
>> 2 10 0.82 1215 2494
>> 3 3 0.29 1086 2494
>> CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
>> - 4 0.46 885 2494
>> 0 1 0.12 889 2494
>> 1 1 0.16 885 2494
>> 2 10 1.15 883 2494
>> 3 4 0.40 891 2494
>
> Clearly, there's something fishy here.
>
> I've simplified your revert patch somewhat. Can you please test if the one
> below still helps?
>
> ---
> drivers/cpufreq/intel_pstate.c | 59 ++++++++++++++++++++++-------------------
> 1 file changed, 32 insertions(+), 27 deletions(-)
>
> Index: linux-pm/drivers/cpufreq/intel_pstate.c
> ===================================================================
> --- linux-pm.orig/drivers/cpufreq/intel_pstate.c
> +++ linux-pm/drivers/cpufreq/intel_pstate.c
> @@ -71,7 +71,7 @@ struct sample {
> u64 mperf;
> u64 tsc;
> int freq;
> - u64 time;
> + ktime_t time;
> };
>
> struct pstate_data {
> @@ -103,13 +103,13 @@ struct _pid {
> struct cpudata {
> int cpu;
>
> - struct update_util_data update_util;
> + struct timer_list timer;
>
> struct pstate_data pstate;
> struct vid_data vid;
> struct _pid pid;
>
> - u64 last_sample_time;
> + ktime_t last_sample_time;
> u64 prev_aperf;
> u64 prev_mperf;
> u64 prev_tsc;
> @@ -882,7 +882,7 @@ static inline void intel_pstate_calc_bus
> sample->core_pct_busy = (int32_t)core_pct;
> }
>
> -static inline bool intel_pstate_sample(struct cpudata *cpu, u64 time)
> +static inline bool intel_pstate_sample(struct cpudata *cpu)
> {
> u64 aperf, mperf;
> unsigned long flags;
> @@ -899,7 +899,7 @@ static inline bool intel_pstate_sample(s
> local_irq_restore(flags);
>
> cpu->last_sample_time = cpu->sample.time;
> - cpu->sample.time = time;
> + cpu->sample.time = ktime_get();
> cpu->sample.aperf = aperf;
> cpu->sample.mperf = mperf;
> cpu->sample.tsc = tsc;
> @@ -957,7 +957,7 @@ static inline int32_t get_target_pstate_
> static inline int32_t get_target_pstate_use_performance(struct cpudata *cpu)
> {
> int32_t core_busy, max_pstate, current_pstate, sample_ratio;
> - u64 duration_ns;
> + s64 duration_ns;
>
> intel_pstate_calc_busy(cpu);
>
> @@ -978,14 +978,15 @@ static inline int32_t get_target_pstate_
> core_busy = mul_fp(core_busy, div_fp(max_pstate, current_pstate));
>
> /*
> - * Since our utilization update callback will not run unless we are
> - * in C0, check if the actual elapsed time is significantly greater (3x)
> - * than our sample interval. If it is, then we were idle for a long
> - * enough period of time to adjust our busyness.
> + * Since we have a deferred timer, it will not fire unless
> + * we are in C0. So, determine if the actual elapsed time
> + * is significantly greater (3x) than our sample interval. If it
> + * is, then we were idle for a long enough period of time
> + * to adjust our busyness.
> */
> - duration_ns = cpu->sample.time - cpu->last_sample_time;
> - if ((s64)duration_ns > pid_params.sample_rate_ns * 3
> - && cpu->last_sample_time > 0) {
> + duration_ns = ktime_to_ns(ktime_sub(cpu->sample.time,
> + cpu->last_sample_time));
> + if (duration_ns > pid_params.sample_rate_ns * 3) {
> sample_ratio = div_fp(int_tofp(pid_params.sample_rate_ns),
> int_tofp(duration_ns));
> core_busy = mul_fp(core_busy, sample_ratio);
> @@ -1032,17 +1033,19 @@ static inline void intel_pstate_adjust_b
> get_avg_frequency(cpu));
> }
>
> -static void intel_pstate_update_util(struct update_util_data *data, u64 time,
> - unsigned long util, unsigned long max)
> +static void intel_pstate_timer_func(unsigned long __data)
> {
> - struct cpudata *cpu = container_of(data, struct cpudata, update_util);
> - u64 delta_ns = time - cpu->sample.time;
> + struct cpudata *cpu = (struct cpudata *) __data;
> + bool sample_taken = intel_pstate_sample(cpu);
>
> - if ((s64)delta_ns >= pid_params.sample_rate_ns) {
> - bool sample_taken = intel_pstate_sample(cpu, time);
> + if (sample_taken) {
> + int delay;
>
> - if (sample_taken && !hwp_active)
> + if (!hwp_active)
> intel_pstate_adjust_busy_pstate(cpu);
> +
> + delay = msecs_to_jiffies(pid_params.sample_rate_ms);
> + mod_timer_pinned(&cpu->timer, jiffies + delay);
> }
> }
>
> @@ -1099,11 +1102,15 @@ static int intel_pstate_init_cpu(unsigne
>
> intel_pstate_get_cpu_pstates(cpu);
>
> + init_timer_deferrable(&cpu->timer);
> + cpu->timer.data = (unsigned long)cpu;
> + cpu->timer.expires = jiffies + HZ/100;
> + cpu->timer.function = intel_pstate_timer_func;
> +
> intel_pstate_busy_pid_reset(cpu);
> - intel_pstate_sample(cpu, 0);
> + intel_pstate_sample(cpu);
>
> - cpu->update_util.func = intel_pstate_update_util;
> - cpufreq_set_update_util_data(cpunum, &cpu->update_util);
> + add_timer_on(&cpu->timer, cpunum);
>
> pr_debug("intel_pstate: controlling: cpu %d\n", cpunum);
>
> @@ -1187,8 +1194,7 @@ static void intel_pstate_stop_cpu(struct
>
> pr_debug("intel_pstate: CPU %d exiting\n", cpu_num);
>
> - cpufreq_set_update_util_data(cpu_num, NULL);
> - synchronize_sched();
> + del_timer_sync(&all_cpu_data[cpu_num]->timer);
>
> if (hwp_active)
> return;
> @@ -1455,8 +1461,7 @@ out:
> get_online_cpus();
> for_each_online_cpu(cpu) {
> if (all_cpu_data[cpu]) {
> - cpufreq_set_update_util_data(cpu, NULL);
> - synchronize_sched();
> + del_timer_sync(&all_cpu_data[cpu]->timer);
> kfree(all_cpu_data[cpu]);
> }
> }
>
Yes, works for me.
CPUID(7): No-SGX
CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
- 11 0.66 1682 2494
0 11 0.60 1856 2494
1 6 0.34 1898 2494
2 13 0.82 1628 2494
3 13 0.87 1528 2494
CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
- 6 0.58 963 2494
0 8 0.83 957 2494
1 1 0.08 984 2494
2 10 1.04 975 2494
3 3 0.35 934 2494
Thanks, Jörg
[toc] | [prev] | [next] | [standalone]
| From | "Rafael J. Wysocki" <rjw@rjwysocki.net> |
|---|---|
| Date | 2016-03-31 13:50 +0200 |
| Subject | Re: [intel-pstate driver regression] processor frequency very high even if in idle |
| Message-ID | <riJgS-4jD-15@gated-at.bofh.it> |
| In reply to | #1368034 |
On Thursday, March 31, 2016 11:05:56 AM Jörg Otte wrote:
[cut]
> >
>
> Yes, works for me.
>
> CPUID(7): No-SGX
> CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
> - 11 0.66 1682 2494
> 0 11 0.60 1856 2494
> 1 6 0.34 1898 2494
> 2 13 0.82 1628 2494
> 3 13 0.87 1528 2494
> CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
> - 6 0.58 963 2494
> 0 8 0.83 957 2494
> 1 1 0.08 984 2494
> 2 10 1.04 975 2494
> 3 3 0.35 934 2494
>
Great, thanks!
To me, the only area where things are really different before and after the
revert is the initialization, so that likely is when the problem triggers.
And sure enough, there is an initialization problem in intel_pstate.
Please test the patch below instead of the revert and let me know if it
makes any difference.
It (or equivalent) will need to be applied anyway, so we'll work on top of it
going forward, but also it may just be sufficient to address the problem you're
seeing.
---
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Subject: [PATCH] intel_pstate: Do not set utilization update hook too early
The utilization update hook in the intel_pstate driver is set too
early, as it only should be set after the policy has been fully
initialized by the core. That may cause intel_pstate_update_util()
to use incorrect data and put the CPUs into incorrect P-states as
a result.
To prevent that from happening, make intel_pstate_set_policy() set
the utilization update hook instead of intel_pstate_init_cpu() so
intel_pstate_update_util() only runs when all things have been
initialized as appropriate.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/cpufreq/intel_pstate.c | 27 +++++++++++++++++++--------
1 file changed, 19 insertions(+), 8 deletions(-)
Index: linux-pm/drivers/cpufreq/intel_pstate.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/intel_pstate.c
+++ linux-pm/drivers/cpufreq/intel_pstate.c
@@ -1103,7 +1103,6 @@ static int intel_pstate_init_cpu(unsigne
intel_pstate_sample(cpu, 0);
cpu->update_util.func = intel_pstate_update_util;
- cpufreq_set_update_util_data(cpunum, &cpu->update_util);
pr_debug("intel_pstate: controlling: cpu %d\n", cpunum);
@@ -1122,18 +1121,29 @@ static unsigned int intel_pstate_get(uns
return get_avg_frequency(cpu);
}
+static void intel_pstate_set_update_util_hook(unsigned int cpu)
+{
+ cpufreq_set_update_util_data(cpu, &all_cpu_data[cpu]->update_util);
+}
+
+static void intel_pstate_clear_update_util_hook(unsigned int cpu)
+{
+ cpufreq_set_update_util_data(cpu, NULL);
+ synchronize_sched();
+}
+
static int intel_pstate_set_policy(struct cpufreq_policy *policy)
{
if (!policy->cpuinfo.max_freq)
return -ENODEV;
+ intel_pstate_clear_update_util_hook(policy->cpu);
+
if (policy->policy == CPUFREQ_POLICY_PERFORMANCE &&
policy->max >= policy->cpuinfo.max_freq) {
pr_debug("intel_pstate: set performance\n");
limits = &performance_limits;
- if (hwp_active)
- intel_pstate_hwp_set(policy->cpus);
- return 0;
+ goto out;
}
pr_debug("intel_pstate: set powersave\n");
@@ -1163,6 +1173,9 @@ static int intel_pstate_set_policy(struc
limits->max_perf = div_fp(int_tofp(limits->max_perf_pct),
int_tofp(100));
+ out:
+ intel_pstate_set_update_util_hook(policy->cpu);
+
if (hwp_active)
intel_pstate_hwp_set(policy->cpus);
@@ -1187,8 +1200,7 @@ static void intel_pstate_stop_cpu(struct
pr_debug("intel_pstate: CPU %d exiting\n", cpu_num);
- cpufreq_set_update_util_data(cpu_num, NULL);
- synchronize_sched();
+ intel_pstate_set_update_util_hook(cpu_num);
if (hwp_active)
return;
@@ -1455,8 +1467,7 @@ out:
get_online_cpus();
for_each_online_cpu(cpu) {
if (all_cpu_data[cpu]) {
- cpufreq_set_update_util_data(cpu, NULL);
- synchronize_sched();
+ intel_pstate_clear_update_util_hook(cpu);
kfree(all_cpu_data[cpu]);
}
}
[toc] | [prev] | [next] | [standalone]
| From | Jörg Otte <jrg.otte@gmail.com> |
|---|---|
| Date | 2016-03-31 17:30 +0200 |
| Message-ID | <riMHL-75H-3@gated-at.bofh.it> |
| In reply to | #1368217 |
2016-03-31 13:42 GMT+02:00 Rafael J. Wysocki <rjw@rjwysocki.net>:
> On Thursday, March 31, 2016 11:05:56 AM Jörg Otte wrote:
>
> [cut]
>
>> >
>>
>> Yes, works for me.
>>
>> CPUID(7): No-SGX
>> CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
>> - 11 0.66 1682 2494
>> 0 11 0.60 1856 2494
>> 1 6 0.34 1898 2494
>> 2 13 0.82 1628 2494
>> 3 13 0.87 1528 2494
>> CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
>> - 6 0.58 963 2494
>> 0 8 0.83 957 2494
>> 1 1 0.08 984 2494
>> 2 10 1.04 975 2494
>> 3 3 0.35 934 2494
>>
>
> Great, thanks!
>
> To me, the only area where things are really different before and after the
> revert is the initialization, so that likely is when the problem triggers.
>
> And sure enough, there is an initialization problem in intel_pstate.
>
> Please test the patch below instead of the revert and let me know if it
> makes any difference.
>
> It (or equivalent) will need to be applied anyway, so we'll work on top of it
> going forward, but also it may just be sufficient to address the problem you're
> seeing.
>
> ---
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Subject: [PATCH] intel_pstate: Do not set utilization update hook too early
>
> The utilization update hook in the intel_pstate driver is set too
> early, as it only should be set after the policy has been fully
> initialized by the core. That may cause intel_pstate_update_util()
> to use incorrect data and put the CPUs into incorrect P-states as
> a result.
>
> To prevent that from happening, make intel_pstate_set_policy() set
> the utilization update hook instead of intel_pstate_init_cpu() so
> intel_pstate_update_util() only runs when all things have been
> initialized as appropriate.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> drivers/cpufreq/intel_pstate.c | 27 +++++++++++++++++++--------
> 1 file changed, 19 insertions(+), 8 deletions(-)
>
> Index: linux-pm/drivers/cpufreq/intel_pstate.c
> ===================================================================
> --- linux-pm.orig/drivers/cpufreq/intel_pstate.c
> +++ linux-pm/drivers/cpufreq/intel_pstate.c
> @@ -1103,7 +1103,6 @@ static int intel_pstate_init_cpu(unsigne
> intel_pstate_sample(cpu, 0);
>
> cpu->update_util.func = intel_pstate_update_util;
> - cpufreq_set_update_util_data(cpunum, &cpu->update_util);
>
> pr_debug("intel_pstate: controlling: cpu %d\n", cpunum);
>
> @@ -1122,18 +1121,29 @@ static unsigned int intel_pstate_get(uns
> return get_avg_frequency(cpu);
> }
>
> +static void intel_pstate_set_update_util_hook(unsigned int cpu)
> +{
> + cpufreq_set_update_util_data(cpu, &all_cpu_data[cpu]->update_util);
> +}
> +
> +static void intel_pstate_clear_update_util_hook(unsigned int cpu)
> +{
> + cpufreq_set_update_util_data(cpu, NULL);
> + synchronize_sched();
> +}
> +
> static int intel_pstate_set_policy(struct cpufreq_policy *policy)
> {
> if (!policy->cpuinfo.max_freq)
> return -ENODEV;
>
> + intel_pstate_clear_update_util_hook(policy->cpu);
> +
> if (policy->policy == CPUFREQ_POLICY_PERFORMANCE &&
> policy->max >= policy->cpuinfo.max_freq) {
> pr_debug("intel_pstate: set performance\n");
> limits = &performance_limits;
> - if (hwp_active)
> - intel_pstate_hwp_set(policy->cpus);
> - return 0;
> + goto out;
> }
>
> pr_debug("intel_pstate: set powersave\n");
> @@ -1163,6 +1173,9 @@ static int intel_pstate_set_policy(struc
> limits->max_perf = div_fp(int_tofp(limits->max_perf_pct),
> int_tofp(100));
>
> + out:
> + intel_pstate_set_update_util_hook(policy->cpu);
> +
> if (hwp_active)
> intel_pstate_hwp_set(policy->cpus);
>
> @@ -1187,8 +1200,7 @@ static void intel_pstate_stop_cpu(struct
>
> pr_debug("intel_pstate: CPU %d exiting\n", cpu_num);
>
> - cpufreq_set_update_util_data(cpu_num, NULL);
> - synchronize_sched();
> + intel_pstate_set_update_util_hook(cpu_num);
>
> if (hwp_active)
> return;
> @@ -1455,8 +1467,7 @@ out:
> get_online_cpus();
> for_each_online_cpu(cpu) {
> if (all_cpu_data[cpu]) {
> - cpufreq_set_update_util_data(cpu, NULL);
> - synchronize_sched();
> + intel_pstate_clear_update_util_hook(cpu);
> kfree(all_cpu_data[cpu]);
> }
> }
>
No, this patch doesn't help.
CPUID(7): No-SGX
CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
- 8 0.32 2507 2495
0 13 0.53 2505 2495
1 3 0.11 2523 2495
2 1 0.06 2555 2495
3 15 0.59 2500 2495
CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
- 8 0.33 2486 2495
0 12 0.50 2482 2495
1 5 0.22 2489 2495
2 1 0.04 2492 2495
3 15 0.59 2487 2495
Thanks, Jörg
[toc] | [prev] | [next] | [standalone]
| From | "Rafael J. Wysocki" <rjw@rjwysocki.net> |
|---|---|
| Date | 2016-03-31 17:50 +0200 |
| Subject | Re: [intel-pstate driver regression] processor frequency very high even if in idle |
| Message-ID | <riN18-7fd-11@gated-at.bofh.it> |
| In reply to | #1368402 |
On Thursday, March 31, 2016 05:25:18 PM Jörg Otte wrote:
> 2016-03-31 13:42 GMT+02:00 Rafael J. Wysocki <rjw@rjwysocki.net>:
> > On Thursday, March 31, 2016 11:05:56 AM Jörg Otte wrote:
> >
> > [cut]
> >
> >> >
> >>
> >> Yes, works for me.
> >>
> >> CPUID(7): No-SGX
> >> CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
> >> - 11 0.66 1682 2494
> >> 0 11 0.60 1856 2494
> >> 1 6 0.34 1898 2494
> >> 2 13 0.82 1628 2494
> >> 3 13 0.87 1528 2494
> >> CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
> >> - 6 0.58 963 2494
> >> 0 8 0.83 957 2494
> >> 1 1 0.08 984 2494
> >> 2 10 1.04 975 2494
> >> 3 3 0.35 934 2494
> >>
> >
[cut]
> >
>
> No, this patch doesn't help.
Well, more work to do then.
I've just noticed a bug in this patch, which is not relevant for the results,
but below is a new version.
> CPUID(7): No-SGX
> CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
> - 8 0.32 2507 2495
> 0 13 0.53 2505 2495
> 1 3 0.11 2523 2495
> 2 1 0.06 2555 2495
> 3 15 0.59 2500 2495
> CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
> - 8 0.33 2486 2495
> 0 12 0.50 2482 2495
> 1 5 0.22 2489 2495
> 2 1 0.04 2492 2495
> 3 15 0.59 2487 2495
Please apply the patch below and take a (1s or so) trace from the pstate_sample
tracepoint (under /sys/kernel/debug/tracing/events/power/ on my systems).
Then please apply the revert instead of it and take a trace from that tracepoint
again and send both of the traces to me.
---
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Subject: [PATCH] intel_pstate: Do not set utilization update hook too early
The utilization update hook in the intel_pstate driver is set too
early, as it only should be set after the policy has been fully
initialized by the core. That may cause intel_pstate_update_util()
to use incorrect data and put the CPUs into incorrect P-states as
a result.
To prevent that from happening, make intel_pstate_set_policy() set
the utilization update hook instead of intel_pstate_init_cpu() so
intel_pstate_update_util() only runs when all things have been
initialized as appropriate.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/cpufreq/intel_pstate.c | 27 +++++++++++++++++++--------
1 file changed, 19 insertions(+), 8 deletions(-)
Index: linux-pm/drivers/cpufreq/intel_pstate.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/intel_pstate.c
+++ linux-pm/drivers/cpufreq/intel_pstate.c
@@ -1103,7 +1103,6 @@ static int intel_pstate_init_cpu(unsigne
intel_pstate_sample(cpu, 0);
cpu->update_util.func = intel_pstate_update_util;
- cpufreq_set_update_util_data(cpunum, &cpu->update_util);
pr_debug("intel_pstate: controlling: cpu %d\n", cpunum);
@@ -1122,18 +1121,29 @@ static unsigned int intel_pstate_get(uns
return get_avg_frequency(cpu);
}
+static void intel_pstate_set_update_util_hook(unsigned int cpu)
+{
+ cpufreq_set_update_util_data(cpu, &all_cpu_data[cpu]->update_util);
+}
+
+static void intel_pstate_clear_update_util_hook(unsigned int cpu)
+{
+ cpufreq_set_update_util_data(cpu, NULL);
+ synchronize_sched();
+}
+
static int intel_pstate_set_policy(struct cpufreq_policy *policy)
{
if (!policy->cpuinfo.max_freq)
return -ENODEV;
+ intel_pstate_clear_update_util_hook(policy->cpu);
+
if (policy->policy == CPUFREQ_POLICY_PERFORMANCE &&
policy->max >= policy->cpuinfo.max_freq) {
pr_debug("intel_pstate: set performance\n");
limits = &performance_limits;
- if (hwp_active)
- intel_pstate_hwp_set(policy->cpus);
- return 0;
+ goto out;
}
pr_debug("intel_pstate: set powersave\n");
@@ -1163,6 +1173,9 @@ static int intel_pstate_set_policy(struc
limits->max_perf = div_fp(int_tofp(limits->max_perf_pct),
int_tofp(100));
+ out:
+ intel_pstate_set_update_util_hook(policy->cpu);
+
if (hwp_active)
intel_pstate_hwp_set(policy->cpus);
@@ -1187,8 +1200,7 @@ static void intel_pstate_stop_cpu(struct
pr_debug("intel_pstate: CPU %d exiting\n", cpu_num);
- cpufreq_set_update_util_data(cpu_num, NULL);
- synchronize_sched();
+ intel_pstate_clear_update_util_hook(cpu_num);
if (hwp_active)
return;
@@ -1455,8 +1467,7 @@ out:
get_online_cpus();
for_each_online_cpu(cpu) {
if (all_cpu_data[cpu]) {
- cpufreq_set_update_util_data(cpu, NULL);
- synchronize_sched();
+ intel_pstate_clear_update_util_hook(cpu);
kfree(all_cpu_data[cpu]);
}
}
[toc] | [prev] | [next] | [standalone]
| From | Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> |
|---|---|
| Date | 2016-03-31 18:20 +0200 |
| Message-ID | <riNua-7Ia-9@gated-at.bofh.it> |
| In reply to | #1368410 |
On Thu, 2016-03-31 at 17:43 +0200, Rafael J. Wysocki wrote:
> On Thursday, March 31, 2016 05:25:18 PM Jörg Otte wrote:
> > 2016-03-31 13:42 GMT+02:00 Rafael J. Wysocki <rjw@rjwysocki.net>:
> > > On Thursday, March 31, 2016 11:05:56 AM Jörg Otte wrote:
> > >
> > > [cut]
> > >
> > > > >
> > > >
> > > > Yes, works for me.
> > > >
> > > > CPUID(7): No-SGX
> > > > CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
> > > > - 11 0.66 1682 2494
> > > > 0 11 0.60 1856 2494
> > > > 1 6 0.34 1898 2494
> > > > 2 13 0.82 1628 2494
> > > > 3 13 0.87 1528 2494
> > > > CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
> > > > - 6 0.58 963 2494
> > > > 0 8 0.83 957 2494
> > > > 1 1 0.08 984 2494
> > > > 2 10 1.04 975 2494
> > > > 3 3 0.35 934 2494
> > > >
> > >
>
> [cut]
>
> > >
> >
> > No, this patch doesn't help.
>
> Well, more work to do then.
>
> I've just noticed a bug in this patch, which is not relevant for the
> results,
> but below is a new version.
>
> > CPUID(7): No-SGX
> > CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
> > - 8 0.32 2507 2495
> > 0 13 0.53 2505 2495
> > 1 3 0.11 2523 2495
> > 2 1 0.06 2555 2495
> > 3 15 0.59 2500 2495
> > CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
> > - 8 0.33 2486 2495
> > 0 12 0.50 2482 2495
> > 1 5 0.22 2489 2495
> > 2 1 0.04 2492 2495
> > 3 15 0.59 2487 2495
>
> Please apply the patch below and take a (1s or so) trace from the
> pstate_sample
> tracepoint (under /sys/kernel/debug/tracing/events/power/ on my
> systems).
>
Jorg,
If you want to know how to trace
# cd /sys/kernel/debug/tracing/
# echo 1 > events/power/pstate_sample/enable
# echo 1 > events/power/cpu_frequency/enable
# cat trace
> Then please apply the revert instead of it and take a trace from that
> tracepoint
> again and send both of the traces to me.
>
> ---
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Subject: [PATCH] intel_pstate: Do not set utilization update hook too
> early
>
> The utilization update hook in the intel_pstate driver is set too
> early, as it only should be set after the policy has been fully
> initialized by the core. That may cause intel_pstate_update_util()
> to use incorrect data and put the CPUs into incorrect P-states as
> a result.
>
> To prevent that from happening, make intel_pstate_set_policy() set
> the utilization update hook instead of intel_pstate_init_cpu() so
> intel_pstate_update_util() only runs when all things have been
> initialized as appropriate.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> drivers/cpufreq/intel_pstate.c | 27 +++++++++++++++++++--------
> 1 file changed, 19 insertions(+), 8 deletions(-)
>
> Index: linux-pm/drivers/cpufreq/intel_pstate.c
> ===================================================================
> --- linux-pm.orig/drivers/cpufreq/intel_pstate.c
> +++ linux-pm/drivers/cpufreq/intel_pstate.c
> @@ -1103,7 +1103,6 @@ static int intel_pstate_init_cpu(unsigne
> intel_pstate_sample(cpu, 0);
>
> cpu->update_util.func = intel_pstate_update_util;
> - cpufreq_set_update_util_data(cpunum, &cpu->update_util);
>
> pr_debug("intel_pstate: controlling: cpu %d\n", cpunum);
>
> @@ -1122,18 +1121,29 @@ static unsigned int intel_pstate_get(uns
> return get_avg_frequency(cpu);
> }
>
> +static void intel_pstate_set_update_util_hook(unsigned int cpu)
> +{
> + cpufreq_set_update_util_data(cpu, &all_cpu_data[cpu]-
> >update_util);
> +}
> +
> +static void intel_pstate_clear_update_util_hook(unsigned int cpu)
> +{
> + cpufreq_set_update_util_data(cpu, NULL);
> + synchronize_sched();
> +}
> +
> static int intel_pstate_set_policy(struct cpufreq_policy *policy)
> {
> if (!policy->cpuinfo.max_freq)
> return -ENODEV;
>
> + intel_pstate_clear_update_util_hook(policy->cpu);
> +
> if (policy->policy == CPUFREQ_POLICY_PERFORMANCE &&
> policy->max >= policy->cpuinfo.max_freq) {
> pr_debug("intel_pstate: set performance\n");
> limits = &performance_limits;
> - if (hwp_active)
> - intel_pstate_hwp_set(policy->cpus);
> - return 0;
> + goto out;
> }
>
> pr_debug("intel_pstate: set powersave\n");
> @@ -1163,6 +1173,9 @@ static int intel_pstate_set_policy(struc
> limits->max_perf = div_fp(int_tofp(limits->max_perf_pct),
> int_tofp(100));
>
> + out:
> + intel_pstate_set_update_util_hook(policy->cpu);
> +
> if (hwp_active)
> intel_pstate_hwp_set(policy->cpus);
>
> @@ -1187,8 +1200,7 @@ static void intel_pstate_stop_cpu(struct
>
> pr_debug("intel_pstate: CPU %d exiting\n", cpu_num);
>
> - cpufreq_set_update_util_data(cpu_num, NULL);
> - synchronize_sched();
> + intel_pstate_clear_update_util_hook(cpu_num);
>
> if (hwp_active)
> return;
> @@ -1455,8 +1467,7 @@ out:
> get_online_cpus();
> for_each_online_cpu(cpu) {
> if (all_cpu_data[cpu]) {
> - cpufreq_set_update_util_data(cpu, NULL);
> - synchronize_sched();
> + intel_pstate_clear_update_util_hook(cpu);
> kfree(all_cpu_data[cpu]);
> }
> }
>
[toc] | [prev] | [next] | [standalone]
| From | Jörg Otte <jrg.otte@gmail.com> |
|---|---|
| Date | 2016-03-31 19:30 +0200 |
| Message-ID | <riOzV-8pW-39@gated-at.bofh.it> |
| In reply to | #1368410 |
2016-03-31 17:43 GMT+02:00 Rafael J. Wysocki <rjw@rjwysocki.net>:
> On Thursday, March 31, 2016 05:25:18 PM Jörg Otte wrote:
>> 2016-03-31 13:42 GMT+02:00 Rafael J. Wysocki <rjw@rjwysocki.net>:
>> > On Thursday, March 31, 2016 11:05:56 AM Jörg Otte wrote:
>> >
>> > [cut]
>> >
>> >> >
>> >>
>> >> Yes, works for me.
>> >>
>> >> CPUID(7): No-SGX
>> >> CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
>> >> - 11 0.66 1682 2494
>> >> 0 11 0.60 1856 2494
>> >> 1 6 0.34 1898 2494
>> >> 2 13 0.82 1628 2494
>> >> 3 13 0.87 1528 2494
>> >> CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
>> >> - 6 0.58 963 2494
>> >> 0 8 0.83 957 2494
>> >> 1 1 0.08 984 2494
>> >> 2 10 1.04 975 2494
>> >> 3 3 0.35 934 2494
>> >>
>> >
>
> [cut]
>
>> >
>>
>> No, this patch doesn't help.
>
> Well, more work to do then.
>
> I've just noticed a bug in this patch, which is not relevant for the results,
> but below is a new version.
>
>> CPUID(7): No-SGX
>> CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
>> - 8 0.32 2507 2495
>> 0 13 0.53 2505 2495
>> 1 3 0.11 2523 2495
>> 2 1 0.06 2555 2495
>> 3 15 0.59 2500 2495
>> CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
>> - 8 0.33 2486 2495
>> 0 12 0.50 2482 2495
>> 1 5 0.22 2489 2495
>> 2 1 0.04 2492 2495
>> 3 15 0.59 2487 2495
>
> Please apply the patch below and take a (1s or so) trace from the pstate_sample
> tracepoint (under /sys/kernel/debug/tracing/events/power/ on my systems).
>
> Then please apply the revert instead of it and take a trace from that tracepoint
> again and send both of the traces to me.
>
> ---
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Subject: [PATCH] intel_pstate: Do not set utilization update hook too early
>
> The utilization update hook in the intel_pstate driver is set too
> early, as it only should be set after the policy has been fully
> initialized by the core. That may cause intel_pstate_update_util()
> to use incorrect data and put the CPUs into incorrect P-states as
> a result.
>
> To prevent that from happening, make intel_pstate_set_policy() set
> the utilization update hook instead of intel_pstate_init_cpu() so
> intel_pstate_update_util() only runs when all things have been
> initialized as appropriate.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> drivers/cpufreq/intel_pstate.c | 27 +++++++++++++++++++--------
> 1 file changed, 19 insertions(+), 8 deletions(-)
>
> Index: linux-pm/drivers/cpufreq/intel_pstate.c
> ===================================================================
> --- linux-pm.orig/drivers/cpufreq/intel_pstate.c
> +++ linux-pm/drivers/cpufreq/intel_pstate.c
> @@ -1103,7 +1103,6 @@ static int intel_pstate_init_cpu(unsigne
> intel_pstate_sample(cpu, 0);
>
> cpu->update_util.func = intel_pstate_update_util;
> - cpufreq_set_update_util_data(cpunum, &cpu->update_util);
>
> pr_debug("intel_pstate: controlling: cpu %d\n", cpunum);
>
> @@ -1122,18 +1121,29 @@ static unsigned int intel_pstate_get(uns
> return get_avg_frequency(cpu);
> }
>
> +static void intel_pstate_set_update_util_hook(unsigned int cpu)
> +{
> + cpufreq_set_update_util_data(cpu, &all_cpu_data[cpu]->update_util);
> +}
> +
> +static void intel_pstate_clear_update_util_hook(unsigned int cpu)
> +{
> + cpufreq_set_update_util_data(cpu, NULL);
> + synchronize_sched();
> +}
> +
> static int intel_pstate_set_policy(struct cpufreq_policy *policy)
> {
> if (!policy->cpuinfo.max_freq)
> return -ENODEV;
>
> + intel_pstate_clear_update_util_hook(policy->cpu);
> +
> if (policy->policy == CPUFREQ_POLICY_PERFORMANCE &&
> policy->max >= policy->cpuinfo.max_freq) {
> pr_debug("intel_pstate: set performance\n");
> limits = &performance_limits;
> - if (hwp_active)
> - intel_pstate_hwp_set(policy->cpus);
> - return 0;
> + goto out;
> }
>
> pr_debug("intel_pstate: set powersave\n");
> @@ -1163,6 +1173,9 @@ static int intel_pstate_set_policy(struc
> limits->max_perf = div_fp(int_tofp(limits->max_perf_pct),
> int_tofp(100));
>
> + out:
> + intel_pstate_set_update_util_hook(policy->cpu);
> +
> if (hwp_active)
> intel_pstate_hwp_set(policy->cpus);
>
> @@ -1187,8 +1200,7 @@ static void intel_pstate_stop_cpu(struct
>
> pr_debug("intel_pstate: CPU %d exiting\n", cpu_num);
>
> - cpufreq_set_update_util_data(cpu_num, NULL);
> - synchronize_sched();
> + intel_pstate_clear_update_util_hook(cpu_num);
>
> if (hwp_active)
> return;
> @@ -1455,8 +1467,7 @@ out:
> get_online_cpus();
> for_each_online_cpu(cpu) {
> if (all_cpu_data[cpu]) {
> - cpufreq_set_update_util_data(cpu, NULL);
> - synchronize_sched();
> + intel_pstate_clear_update_util_hook(cpu);
> kfree(all_cpu_data[cpu]);
> }
> }
>
OK, patch is applied.
After some configurations and compilations I'm there.
Under pstate_sample I see:
enable filter format id trigger
what to do now ? (never did tracing before)
Thanks, Jörg
[toc] | [prev] | [next] | [standalone]
| From | Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> |
|---|---|
| Date | 2016-03-31 20:00 +0200 |
| Message-ID | <riP2W-ac-17@gated-at.bofh.it> |
| In reply to | #1368471 |
On Thu, 2016-03-31 at 19:27 +0200, Jörg Otte wrote:
> 2016-03-31 17:43 GMT+02:00 Rafael J. Wysocki <rjw@rjwysocki.net>:
> > On Thursday, March 31, 2016 05:25:18 PM Jörg Otte wrote:
> > > 2016-03-31 13:42 GMT+02:00 Rafael J. Wysocki <rjw@rjwysocki.net>:
> > > > On Thursday, March 31, 2016 11:05:56 AM Jörg Otte wrote:
> > > >
> > > > [cut]
> > > >
> > > > > >
> > > > >
> > > > > Yes, works for me.
> > > > >
> > > > > CPUID(7): No-SGX
> > > > > CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
> > > > > - 11 0.66 1682 2494
> > > > > 0 11 0.60 1856 2494
> > > > > 1 6 0.34 1898 2494
> > > > > 2 13 0.82 1628 2494
> > > > > 3 13 0.87 1528 2494
> > > > > CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
> > > > > - 6 0.58 963 2494
> > > > > 0 8 0.83 957 2494
> > > > > 1 1 0.08 984 2494
> > > > > 2 10 1.04 975 2494
> > > > > 3 3 0.35 934 2494
> > > > >
> > > >
> >
> > [cut]
> >
> > > >
> > >
> > > No, this patch doesn't help.
> >
> > Well, more work to do then.
> >
> > I've just noticed a bug in this patch, which is not relevant for
> > the results,
> > but below is a new version.
> >
> > > CPUID(7): No-SGX
> > > CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
> > > - 8 0.32 2507 2495
> > > 0 13 0.53 2505 2495
> > > 1 3 0.11 2523 2495
> > > 2 1 0.06 2555 2495
> > > 3 15 0.59 2500 2495
> > > CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
> > > - 8 0.33 2486 2495
> > > 0 12 0.50 2482 2495
> > > 1 5 0.22 2489 2495
> > > 2 1 0.04 2492 2495
> > > 3 15 0.59 2487 2495
> >
> > Please apply the patch below and take a (1s or so) trace from the
> > pstate_sample
> > tracepoint (under /sys/kernel/debug/tracing/events/power/ on my
> > systems).
> >
> > Then please apply the revert instead of it and take a trace from
> > that tracepoint
> > again and send both of the traces to me.
> >
> > ---
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > Subject: [PATCH] intel_pstate: Do not set utilization update hook
> > too early
> >
> > The utilization update hook in the intel_pstate driver is set too
> > early, as it only should be set after the policy has been fully
> > initialized by the core. That may cause intel_pstate_update_util()
> > to use incorrect data and put the CPUs into incorrect P-states as
> > a result.
> >
> > To prevent that from happening, make intel_pstate_set_policy() set
> > the utilization update hook instead of intel_pstate_init_cpu() so
> > intel_pstate_update_util() only runs when all things have been
> > initialized as appropriate.
> >
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> > drivers/cpufreq/intel_pstate.c | 27 +++++++++++++++++++--------
> > 1 file changed, 19 insertions(+), 8 deletions(-)
> >
> > Index: linux-pm/drivers/cpufreq/intel_pstate.c
> > ===================================================================
> > --- linux-pm.orig/drivers/cpufreq/intel_pstate.c
> > +++ linux-pm/drivers/cpufreq/intel_pstate.c
> > @@ -1103,7 +1103,6 @@ static int intel_pstate_init_cpu(unsigne
> > intel_pstate_sample(cpu, 0);
> >
> > cpu->update_util.func = intel_pstate_update_util;
> > - cpufreq_set_update_util_data(cpunum, &cpu->update_util);
> >
> > pr_debug("intel_pstate: controlling: cpu %d\n", cpunum);
> >
> > @@ -1122,18 +1121,29 @@ static unsigned int intel_pstate_get(uns
> > return get_avg_frequency(cpu);
> > }
> >
> > +static void intel_pstate_set_update_util_hook(unsigned int cpu)
> > +{
> > + cpufreq_set_update_util_data(cpu, &all_cpu_data[cpu]-
> > >update_util);
> > +}
> > +
> > +static void intel_pstate_clear_update_util_hook(unsigned int cpu)
> > +{
> > + cpufreq_set_update_util_data(cpu, NULL);
> > + synchronize_sched();
> > +}
> > +
> > static int intel_pstate_set_policy(struct cpufreq_policy *policy)
> > {
> > if (!policy->cpuinfo.max_freq)
> > return -ENODEV;
> >
> > + intel_pstate_clear_update_util_hook(policy->cpu);
> > +
> > if (policy->policy == CPUFREQ_POLICY_PERFORMANCE &&
> > policy->max >= policy->cpuinfo.max_freq) {
> > pr_debug("intel_pstate: set performance\n");
> > limits = &performance_limits;
> > - if (hwp_active)
> > - intel_pstate_hwp_set(policy->cpus);
> > - return 0;
> > + goto out;
> > }
> >
> > pr_debug("intel_pstate: set powersave\n");
> > @@ -1163,6 +1173,9 @@ static int intel_pstate_set_policy(struc
> > limits->max_perf = div_fp(int_tofp(limits->max_perf_pct),
> > int_tofp(100));
> >
> > + out:
> > + intel_pstate_set_update_util_hook(policy->cpu);
> > +
> > if (hwp_active)
> > intel_pstate_hwp_set(policy->cpus);
> >
> > @@ -1187,8 +1200,7 @@ static void intel_pstate_stop_cpu(struct
> >
> > pr_debug("intel_pstate: CPU %d exiting\n", cpu_num);
> >
> > - cpufreq_set_update_util_data(cpu_num, NULL);
> > - synchronize_sched();
> > + intel_pstate_clear_update_util_hook(cpu_num);
> >
> > if (hwp_active)
> > return;
> > @@ -1455,8 +1467,7 @@ out:
> > get_online_cpus();
> > for_each_online_cpu(cpu) {
> > if (all_cpu_data[cpu]) {
> > - cpufreq_set_update_util_data(cpu, NULL);
> > - synchronize_sched();
> > + intel_pstate_clear_update_util_hook(cpu);
> > kfree(all_cpu_data[cpu]);
> > }
> > }
> >
>
> OK, patch is applied.
> After some configurations and compilations I'm there.
> Under pstate_sample I see:
> enable filter format id trigger
>
> what to do now ? (never did tracing before)'
# cd /sys/kernel/debug/tracing/
# echo 1 > events/power/pstate_sample/enable
# echo 1 > events/power/cpu_frequency/enable
# cat trace
Send us the trace file.
Also your kernel config doesn't have many modules, Is it a custom
configuration you do for your system?
Thanks,
Srinivas
>
> Thanks, Jörg
[toc] | [prev] | [next] | [standalone]
| From | "Rafael J. Wysocki" <rafael@kernel.org> |
|---|---|
| Date | 2016-04-01 02:10 +0200 |
| Message-ID | <riUP0-4r5-7@gated-at.bofh.it> |
| In reply to | #1368506 |
On Thu, Mar 31, 2016 at 7:55 PM, Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> wrote: > On Thu, 2016-03-31 at 19:27 +0200, Jörg Otte wrote: >> 2016-03-31 17:43 GMT+02:00 Rafael J. Wysocki <rjw@rjwysocki.net>: >> > On Thursday, March 31, 2016 05:25:18 PM Jörg Otte wrote: >> > > 2016-03-31 13:42 GMT+02:00 Rafael J. Wysocki <rjw@rjwysocki.net>: >> > > > On Thursday, March 31, 2016 11:05:56 AM Jörg Otte wrote: >> > > > [cut] >> >> OK, patch is applied. >> After some configurations and compilations I'm there. >> Under pstate_sample I see: >> enable filter format id trigger >> >> what to do now ? (never did tracing before)' > > # cd /sys/kernel/debug/tracing/ > # echo 1 > events/power/pstate_sample/enable > # echo 1 > events/power/cpu_frequency/enable > # cat trace > Send us the trace file. Here's what I do, for completeness: # echo 0 > /sys/kernel/debug/tracing/tracing_on # echo global > /sys/kernel/debug/tracing/trace_clock # echo nop > /sys/kernel/debug/tracing/current_tracer # echo 1000 > /sys/kernel/debug/tracing/buffer_size_kb # echo 1 > /sys/kernel/debug/tracing/events/power/pstate_sample/enable # echo "" > /sys/kernel/debug/tracing/trace # echo 1 > /sys/kernel/debug/tracing/tracing_on Then, (after a while) make a copy of the trace file.
[toc] | [prev] | [next] | [standalone]
| From | Jörg Otte <jrg.otte@gmail.com> |
|---|---|
| Date | 2016-04-01 11:50 +0200 |
| Message-ID | <rj3Sh-2oD-1@gated-at.bofh.it> |
| In reply to | #1368506 |
2016-03-31 19:55 GMT+02:00 Srinivas Pandruvada
<srinivas.pandruvada@linux.intel.com>:
> On Thu, 2016-03-31 at 19:27 +0200, Jörg Otte wrote:
>> 2016-03-31 17:43 GMT+02:00 Rafael J. Wysocki <rjw@rjwysocki.net>:
>> > On Thursday, March 31, 2016 05:25:18 PM Jörg Otte wrote:
>> > > 2016-03-31 13:42 GMT+02:00 Rafael J. Wysocki <rjw@rjwysocki.net>:
>> > > > On Thursday, March 31, 2016 11:05:56 AM Jörg Otte wrote:
>> > > >
>> > > > [cut]
>> > > >
>> > > > > >
>> > > > >
>> > > > > Yes, works for me.
>> > > > >
>> > > > > CPUID(7): No-SGX
>> > > > > CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
>> > > > > - 11 0.66 1682 2494
>> > > > > 0 11 0.60 1856 2494
>> > > > > 1 6 0.34 1898 2494
>> > > > > 2 13 0.82 1628 2494
>> > > > > 3 13 0.87 1528 2494
>> > > > > CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
>> > > > > - 6 0.58 963 2494
>> > > > > 0 8 0.83 957 2494
>> > > > > 1 1 0.08 984 2494
>> > > > > 2 10 1.04 975 2494
>> > > > > 3 3 0.35 934 2494
>> > > > >
>> > > >
>> >
>> > [cut]
>> >
>> > > >
>> > >
>> > > No, this patch doesn't help.
>> >
>> > Well, more work to do then.
>> >
>> > I've just noticed a bug in this patch, which is not relevant for
>> > the results,
>> > but below is a new version.
>> >
>> > > CPUID(7): No-SGX
>> > > CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
>> > > - 8 0.32 2507 2495
>> > > 0 13 0.53 2505 2495
>> > > 1 3 0.11 2523 2495
>> > > 2 1 0.06 2555 2495
>> > > 3 15 0.59 2500 2495
>> > > CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
>> > > - 8 0.33 2486 2495
>> > > 0 12 0.50 2482 2495
>> > > 1 5 0.22 2489 2495
>> > > 2 1 0.04 2492 2495
>> > > 3 15 0.59 2487 2495
>> >
>> > Please apply the patch below and take a (1s or so) trace from the
>> > pstate_sample
>> > tracepoint (under /sys/kernel/debug/tracing/events/power/ on my
>> > systems).
>> >
>> > Then please apply the revert instead of it and take a trace from
>> > that tracepoint
>> > again and send both of the traces to me.
>> >
>> > ---
>> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>> > Subject: [PATCH] intel_pstate: Do not set utilization update hook
>> > too early
>> >
>> > The utilization update hook in the intel_pstate driver is set too
>> > early, as it only should be set after the policy has been fully
>> > initialized by the core. That may cause intel_pstate_update_util()
>> > to use incorrect data and put the CPUs into incorrect P-states as
>> > a result.
>> >
>> > To prevent that from happening, make intel_pstate_set_policy() set
>> > the utilization update hook instead of intel_pstate_init_cpu() so
>> > intel_pstate_update_util() only runs when all things have been
>> > initialized as appropriate.
>> >
>> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>> > ---
>> > drivers/cpufreq/intel_pstate.c | 27 +++++++++++++++++++--------
>> > 1 file changed, 19 insertions(+), 8 deletions(-)
>> >
>> > Index: linux-pm/drivers/cpufreq/intel_pstate.c
>> > ===================================================================
>> > --- linux-pm.orig/drivers/cpufreq/intel_pstate.c
>> > +++ linux-pm/drivers/cpufreq/intel_pstate.c
>> > @@ -1103,7 +1103,6 @@ static int intel_pstate_init_cpu(unsigne
>> > intel_pstate_sample(cpu, 0);
>> >
>> > cpu->update_util.func = intel_pstate_update_util;
>> > - cpufreq_set_update_util_data(cpunum, &cpu->update_util);
>> >
>> > pr_debug("intel_pstate: controlling: cpu %d\n", cpunum);
>> >
>> > @@ -1122,18 +1121,29 @@ static unsigned int intel_pstate_get(uns
>> > return get_avg_frequency(cpu);
>> > }
>> >
>> > +static void intel_pstate_set_update_util_hook(unsigned int cpu)
>> > +{
>> > + cpufreq_set_update_util_data(cpu, &all_cpu_data[cpu]-
>> > >update_util);
>> > +}
>> > +
>> > +static void intel_pstate_clear_update_util_hook(unsigned int cpu)
>> > +{
>> > + cpufreq_set_update_util_data(cpu, NULL);
>> > + synchronize_sched();
>> > +}
>> > +
>> > static int intel_pstate_set_policy(struct cpufreq_policy *policy)
>> > {
>> > if (!policy->cpuinfo.max_freq)
>> > return -ENODEV;
>> >
>> > + intel_pstate_clear_update_util_hook(policy->cpu);
>> > +
>> > if (policy->policy == CPUFREQ_POLICY_PERFORMANCE &&
>> > policy->max >= policy->cpuinfo.max_freq) {
>> > pr_debug("intel_pstate: set performance\n");
>> > limits = &performance_limits;
>> > - if (hwp_active)
>> > - intel_pstate_hwp_set(policy->cpus);
>> > - return 0;
>> > + goto out;
>> > }
>> >
>> > pr_debug("intel_pstate: set powersave\n");
>> > @@ -1163,6 +1173,9 @@ static int intel_pstate_set_policy(struc
>> > limits->max_perf = div_fp(int_tofp(limits->max_perf_pct),
>> > int_tofp(100));
>> >
>> > + out:
>> > + intel_pstate_set_update_util_hook(policy->cpu);
>> > +
>> > if (hwp_active)
>> > intel_pstate_hwp_set(policy->cpus);
>> >
>> > @@ -1187,8 +1200,7 @@ static void intel_pstate_stop_cpu(struct
>> >
>> > pr_debug("intel_pstate: CPU %d exiting\n", cpu_num);
>> >
>> > - cpufreq_set_update_util_data(cpu_num, NULL);
>> > - synchronize_sched();
>> > + intel_pstate_clear_update_util_hook(cpu_num);
>> >
>> > if (hwp_active)
>> > return;
>> > @@ -1455,8 +1467,7 @@ out:
>> > get_online_cpus();
>> > for_each_online_cpu(cpu) {
>> > if (all_cpu_data[cpu]) {
>> > - cpufreq_set_update_util_data(cpu, NULL);
>> > - synchronize_sched();
>> > + intel_pstate_clear_update_util_hook(cpu);
>> > kfree(all_cpu_data[cpu]);
>> > }
>> > }
>> >
>>
>> OK, patch is applied.
>> After some configurations and compilations I'm there.
>> Under pstate_sample I see:
>> enable filter format id trigger
>>
>> what to do now ? (never did tracing before)'
>
> # cd /sys/kernel/debug/tracing/
> # echo 1 > events/power/pstate_sample/enable
> # echo 1 > events/power/cpu_frequency/enable
> # cat trace
> Send us the trace file.
>
> Also your kernel config doesn't have many modules, Is it a custom
> configuration you do for your system?
>
I compile a minimum kernel for my notebook. The hardware is fix and
will never change. So I don't need thousends of modules to compile.
Kbuild supports this with target "localmodconfig".
In the rare cases where I get new usb-hardware I add a new driver
and compile a new kernel which takes only a minute.
Thanks, Jörg
[toc] | [prev] | [next] | [standalone]
| From | Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> |
|---|---|
| Date | 2016-04-01 17:10 +0200 |
| Message-ID | <rj8RY-6cF-25@gated-at.bofh.it> |
| In reply to | #1369135 |
On Fri, 2016-04-01 at 11:42 +0200, Jörg Otte wrote: > 2016-03-31 19:55 GMT+02:00 Srinivas Pandruvada > <srinivas.pandruvada@linux.intel.com>: > > On Thu, 2016-03-31 at 19:27 +0200, Jörg Otte wrote: > > > 2016-03-31 17:43 GMT+02:00 Rafael J. Wysocki <rjw@rjwysocki.net>: > > > > On Thursday, March 31, 2016 05:25:18 PM Jörg Otte wrote: > > > > > 2016-03-31 13:42 GMT+02:00 Rafael J. Wysocki <rjw@rjwysocki.n > > > > > et>: > > > > > > On Thursday, March 31, 2016 11:05:56 AM Jörg Otte wrote: [cut] > I compile a minimum kernel for my notebook. The hardware is fix and > will never change. So I don't need thousends of modules to compile. > Kbuild supports this with target "localmodconfig". > In the rare cases where I get new usb-hardware I add a new driver > and compile a new kernel which takes only a minute. > With this minimum config, I am not able to properly run my laptop to reproduce. May be something odd in this config is triggering this issue, we are not going to idle on some CPUs. Thanks, Srinivas > Thanks, Jörg > -- > To unsubscribe from this list: send the line "unsubscribe linux-pm" > in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
[toc] | [prev] | [next] | [standalone]
| From | "Rafael J. Wysocki" <rafael@kernel.org> |
|---|---|
| Date | 2016-04-01 22:30 +0200 |
| Message-ID | <rjdRE-18Z-9@gated-at.bofh.it> |
| In reply to | #1369406 |
On Fri, Apr 1, 2016 at 5:05 PM, Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> wrote: > On Fri, 2016-04-01 at 11:42 +0200, Jörg Otte wrote: >> 2016-03-31 19:55 GMT+02:00 Srinivas Pandruvada >> <srinivas.pandruvada@linux.intel.com>: >> > On Thu, 2016-03-31 at 19:27 +0200, Jörg Otte wrote: >> > > 2016-03-31 17:43 GMT+02:00 Rafael J. Wysocki <rjw@rjwysocki.net>: >> > > > On Thursday, March 31, 2016 05:25:18 PM Jörg Otte wrote: >> > > > > 2016-03-31 13:42 GMT+02:00 Rafael J. Wysocki <rjw@rjwysocki.n >> > > > > et>: >> > > > > > On Thursday, March 31, 2016 11:05:56 AM Jörg Otte wrote: > [cut] > >> I compile a minimum kernel for my notebook. The hardware is fix and >> will never change. So I don't need thousends of modules to compile. >> Kbuild supports this with target "localmodconfig". >> In the rare cases where I get new usb-hardware I add a new driver >> and compile a new kernel which takes only a minute. >> > With this minimum config, I am not able to properly run my laptop to > reproduce. > May be something odd in this config is triggering this issue, we are > not going to idle on some CPUs. You can use ./scripts/diffconfig (in the kernel source tree) to find differences between your working config and the Jörg's one and try to flip the bits that may matter in your config.
[toc] | [prev] | [next] | [standalone]
| From | "Rafael J. Wysocki" <rjw@rjwysocki.net> |
|---|---|
| Date | 2016-04-01 14:40 +0200 |
| Subject | Re: [intel-pstate driver regression] processor frequency very high even if in idle |
| Message-ID | <rj6wO-4fI-19@gated-at.bofh.it> |
| In reply to | #1368410 |
On Friday, April 01, 2016 11:20:42 AM Jörg Otte wrote:
> 2016-03-31 17:43 GMT+02:00 Rafael J. Wysocki <rjw@rjwysocki.net>:
> > On Thursday, March 31, 2016 05:25:18 PM Jörg Otte wrote:
> >> 2016-03-31 13:42 GMT+02:00 Rafael J. Wysocki <rjw@rjwysocki.net>:
> >> > On Thursday, March 31, 2016 11:05:56 AM Jörg Otte wrote:
> >> >
> >> > [cut]
> >> >
> >> >> >
> >> >>
> >> >> Yes, works for me.
> >> >>
> >> >> CPUID(7): No-SGX
> >> >> CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
> >> >> - 11 0.66 1682 2494
> >> >> 0 11 0.60 1856 2494
> >> >> 1 6 0.34 1898 2494
> >> >> 2 13 0.82 1628 2494
> >> >> 3 13 0.87 1528 2494
> >> >> CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
> >> >> - 6 0.58 963 2494
> >> >> 0 8 0.83 957 2494
> >> >> 1 1 0.08 984 2494
> >> >> 2 10 1.04 975 2494
> >> >> 3 3 0.35 934 2494
> >> >>
> >> >
> >
> > [cut]
> >
> >> >
> >>
> >> No, this patch doesn't help.
> >
> > Well, more work to do then.
> >
> > I've just noticed a bug in this patch, which is not relevant for the results,
> > but below is a new version.
> >
> >> CPUID(7): No-SGX
> >> CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
> >> - 8 0.32 2507 2495
> >> 0 13 0.53 2505 2495
> >> 1 3 0.11 2523 2495
> >> 2 1 0.06 2555 2495
> >> 3 15 0.59 2500 2495
> >> CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
> >> - 8 0.33 2486 2495
> >> 0 12 0.50 2482 2495
> >> 1 5 0.22 2489 2495
> >> 2 1 0.04 2492 2495
> >> 3 15 0.59 2487 2495
> >
[cut]
>
> here they are.
>
Thanks!
First of all, the sampling mechanics works as expected in the failing case,
which is the most important thing I wanted to know. However, there are anomalies
in the failing case trace. The core_busy column is clearly suspicious and it
looks like CPUs 2 and 3 never really go idle. I guess we'll need to find out
why they don't go idle to get to the bottom of this, but it firmly falls into
the weird stuff territory already.
In the meantime, below is one more patch to test, on top of the previous one
(that is, https://patchwork.kernel.org/patch/8714401/).
Again, this is a change I'd like to make regardless, so it would be good to
know if anything more has to be done before we go further.
---
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Subject: [PATCH] intel_pstate: Avoid extra invocation of intel_pstate_sample()
The initialization of intel_pstate for a given CPU involves populating
the fields of its struct cpudata that represent the previous sample,
but currently that is done in a problematic way.
Namely, intel_pstate_init_cpu() makes an extra call to
intel_pstate_sample() so it reads the current register values that
will be used to populate the "previous sample" record during the
next invocation of intel_pstate_sample(). However, after commit
a4675fbc4a7a (cpufreq: intel_pstate: Replace timers with utilization
update callbacks) that doesn't work for last_sample_time, because
the time value is passed to intel_pstate_sample() as an argument now.
Passing 0 to it from intel_pstate_init_cpu() is problematic, because
that causes cpu->last_sample_time == 0 to be visible in
get_target_pstate_use_performance() (and hence the extra
cpu->last_sample_time > 0 check in there) and effectively allows
the first invocation of intel_pstate_sample() from
intel_pstate_update_util() to happen immediately after the
initialization which may lead to a significant "turn on"
effect in the governor algorithm.
To mitigate that issue, rework the initialization to avoid the
extra intel_pstate_sample() call from intel_pstate_init_cpu().
Instead, make intel_pstate_sample() return false if it has been
called with cpu->sample.time equal to zero, which will make
intel_pstate_update_util() skip the sample in that case, and
reset cpu->sample.time from intel_pstate_set_update_util_hook()
to make the algorithm start properly every time the hook is set.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/cpufreq/intel_pstate.c | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
Index: linux-pm/drivers/cpufreq/intel_pstate.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/intel_pstate.c
+++ linux-pm/drivers/cpufreq/intel_pstate.c
@@ -910,7 +910,14 @@ static inline bool intel_pstate_sample(s
cpu->prev_aperf = aperf;
cpu->prev_mperf = mperf;
cpu->prev_tsc = tsc;
- return true;
+ /*
+ * First time this function is invoked in a given cycle, all of the
+ * previous sample data fields are equal to zero or stale and they must
+ * be populated with meaningful numbers for things to work, so assume
+ * that sample.time will always be reset before setting the utilization
+ * update hook and make the caller skip the sample then.
+ */
+ return !!cpu->last_sample_time;
}
static inline int32_t get_avg_frequency(struct cpudata *cpu)
@@ -984,8 +991,7 @@ static inline int32_t get_target_pstate_
* enough period of time to adjust our busyness.
*/
duration_ns = cpu->sample.time - cpu->last_sample_time;
- if ((s64)duration_ns > pid_params.sample_rate_ns * 3
- && cpu->last_sample_time > 0) {
+ if ((s64)duration_ns > pid_params.sample_rate_ns * 3) {
sample_ratio = div_fp(int_tofp(pid_params.sample_rate_ns),
int_tofp(duration_ns));
core_busy = mul_fp(core_busy, sample_ratio);
@@ -1100,7 +1106,6 @@ static int intel_pstate_init_cpu(unsigne
intel_pstate_get_cpu_pstates(cpu);
intel_pstate_busy_pid_reset(cpu);
- intel_pstate_sample(cpu, 0);
cpu->update_util.func = intel_pstate_update_util;
@@ -1121,9 +1126,13 @@ static unsigned int intel_pstate_get(uns
return get_avg_frequency(cpu);
}
-static void intel_pstate_set_update_util_hook(unsigned int cpu)
+static void intel_pstate_set_update_util_hook(unsigned int cpu_num)
{
- cpufreq_set_update_util_data(cpu, &all_cpu_data[cpu]->update_util);
+ struct cpudata *cpu = all_cpu_data[cpu_num];
+
+ /* Prevent intel_pstate_update_util() from using stale data. */
+ cpu->sample.time = 0;
+ cpufreq_set_update_util_data(cpu_num, &cpu->update_util);
}
static void intel_pstate_clear_update_util_hook(unsigned int cpu)
[toc] | [prev] | [next] | [standalone]
| From | Jörg Otte <jrg.otte@gmail.com> |
|---|---|
| Date | 2016-04-01 16:10 +0200 |
| Message-ID | <rj7VT-5vb-11@gated-at.bofh.it> |
| In reply to | #1369258 |
[Multipart message — attachments visible in raw view] — view raw
2016-04-01 14:40 GMT+02:00 Rafael J. Wysocki <rjw@rjwysocki.net>:
> On Friday, April 01, 2016 11:20:42 AM Jörg Otte wrote:
>> 2016-03-31 17:43 GMT+02:00 Rafael J. Wysocki <rjw@rjwysocki.net>:
>> > On Thursday, March 31, 2016 05:25:18 PM Jörg Otte wrote:
>> >> 2016-03-31 13:42 GMT+02:00 Rafael J. Wysocki <rjw@rjwysocki.net>:
>> >> > On Thursday, March 31, 2016 11:05:56 AM Jörg Otte wrote:
>> >> >
>> >> > [cut]
>> >> >
>> >> >> >
>> >> >>
>> >> >> Yes, works for me.
>> >> >>
>> >> >> CPUID(7): No-SGX
>> >> >> CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
>> >> >> - 11 0.66 1682 2494
>> >> >> 0 11 0.60 1856 2494
>> >> >> 1 6 0.34 1898 2494
>> >> >> 2 13 0.82 1628 2494
>> >> >> 3 13 0.87 1528 2494
>> >> >> CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
>> >> >> - 6 0.58 963 2494
>> >> >> 0 8 0.83 957 2494
>> >> >> 1 1 0.08 984 2494
>> >> >> 2 10 1.04 975 2494
>> >> >> 3 3 0.35 934 2494
>> >> >>
>> >> >
>> >
>> > [cut]
>> >
>> >> >
>> >>
>> >> No, this patch doesn't help.
>> >
>> > Well, more work to do then.
>> >
>> > I've just noticed a bug in this patch, which is not relevant for the results,
>> > but below is a new version.
>> >
>> >> CPUID(7): No-SGX
>> >> CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
>> >> - 8 0.32 2507 2495
>> >> 0 13 0.53 2505 2495
>> >> 1 3 0.11 2523 2495
>> >> 2 1 0.06 2555 2495
>> >> 3 15 0.59 2500 2495
>> >> CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
>> >> - 8 0.33 2486 2495
>> >> 0 12 0.50 2482 2495
>> >> 1 5 0.22 2489 2495
>> >> 2 1 0.04 2492 2495
>> >> 3 15 0.59 2487 2495
>> >
>
> [cut]
>
>>
>> here they are.
>>
>
> Thanks!
>
> First of all, the sampling mechanics works as expected in the failing case,
> which is the most important thing I wanted to know. However, there are anomalies
> in the failing case trace. The core_busy column is clearly suspicious and it
> looks like CPUs 2 and 3 never really go idle. I guess we'll need to find out
> why they don't go idle to get to the bottom of this, but it firmly falls into
> the weird stuff territory already.
>
> In the meantime, below is one more patch to test, on top of the previous one
> (that is, https://patchwork.kernel.org/patch/8714401/).
>
> Again, this is a change I'd like to make regardless, so it would be good to
> know if anything more has to be done before we go further.
>
> ---
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Subject: [PATCH] intel_pstate: Avoid extra invocation of intel_pstate_sample()
>
> The initialization of intel_pstate for a given CPU involves populating
> the fields of its struct cpudata that represent the previous sample,
> but currently that is done in a problematic way.
>
> Namely, intel_pstate_init_cpu() makes an extra call to
> intel_pstate_sample() so it reads the current register values that
> will be used to populate the "previous sample" record during the
> next invocation of intel_pstate_sample(). However, after commit
> a4675fbc4a7a (cpufreq: intel_pstate: Replace timers with utilization
> update callbacks) that doesn't work for last_sample_time, because
> the time value is passed to intel_pstate_sample() as an argument now.
> Passing 0 to it from intel_pstate_init_cpu() is problematic, because
> that causes cpu->last_sample_time == 0 to be visible in
> get_target_pstate_use_performance() (and hence the extra
> cpu->last_sample_time > 0 check in there) and effectively allows
> the first invocation of intel_pstate_sample() from
> intel_pstate_update_util() to happen immediately after the
> initialization which may lead to a significant "turn on"
> effect in the governor algorithm.
>
> To mitigate that issue, rework the initialization to avoid the
> extra intel_pstate_sample() call from intel_pstate_init_cpu().
> Instead, make intel_pstate_sample() return false if it has been
> called with cpu->sample.time equal to zero, which will make
> intel_pstate_update_util() skip the sample in that case, and
> reset cpu->sample.time from intel_pstate_set_update_util_hook()
> to make the algorithm start properly every time the hook is set.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> drivers/cpufreq/intel_pstate.c | 21 +++++++++++++++------
> 1 file changed, 15 insertions(+), 6 deletions(-)
>
> Index: linux-pm/drivers/cpufreq/intel_pstate.c
> ===================================================================
> --- linux-pm.orig/drivers/cpufreq/intel_pstate.c
> +++ linux-pm/drivers/cpufreq/intel_pstate.c
> @@ -910,7 +910,14 @@ static inline bool intel_pstate_sample(s
> cpu->prev_aperf = aperf;
> cpu->prev_mperf = mperf;
> cpu->prev_tsc = tsc;
> - return true;
> + /*
> + * First time this function is invoked in a given cycle, all of the
> + * previous sample data fields are equal to zero or stale and they must
> + * be populated with meaningful numbers for things to work, so assume
> + * that sample.time will always be reset before setting the utilization
> + * update hook and make the caller skip the sample then.
> + */
> + return !!cpu->last_sample_time;
> }
>
> static inline int32_t get_avg_frequency(struct cpudata *cpu)
> @@ -984,8 +991,7 @@ static inline int32_t get_target_pstate_
> * enough period of time to adjust our busyness.
> */
> duration_ns = cpu->sample.time - cpu->last_sample_time;
> - if ((s64)duration_ns > pid_params.sample_rate_ns * 3
> - && cpu->last_sample_time > 0) {
> + if ((s64)duration_ns > pid_params.sample_rate_ns * 3) {
> sample_ratio = div_fp(int_tofp(pid_params.sample_rate_ns),
> int_tofp(duration_ns));
> core_busy = mul_fp(core_busy, sample_ratio);
> @@ -1100,7 +1106,6 @@ static int intel_pstate_init_cpu(unsigne
> intel_pstate_get_cpu_pstates(cpu);
>
> intel_pstate_busy_pid_reset(cpu);
> - intel_pstate_sample(cpu, 0);
>
> cpu->update_util.func = intel_pstate_update_util;
>
> @@ -1121,9 +1126,13 @@ static unsigned int intel_pstate_get(uns
> return get_avg_frequency(cpu);
> }
>
> -static void intel_pstate_set_update_util_hook(unsigned int cpu)
> +static void intel_pstate_set_update_util_hook(unsigned int cpu_num)
> {
> - cpufreq_set_update_util_data(cpu, &all_cpu_data[cpu]->update_util);
> + struct cpudata *cpu = all_cpu_data[cpu_num];
> +
> + /* Prevent intel_pstate_update_util() from using stale data. */
> + cpu->sample.time = 0;
> + cpufreq_set_update_util_data(cpu_num, &cpu->update_util);
> }
>
> static void intel_pstate_clear_update_util_hook(unsigned int cpu)
>
Done. Attached the tracer.
For me it looks like the previous one of the failing case.
Thanks, Jörg
[toc] | [prev] | [next] | [standalone]
| From | Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> |
|---|---|
| Date | 2016-04-01 19:50 +0200 |
| Message-ID | <rjbmN-7Po-5@gated-at.bofh.it> |
| In reply to | #1369340 |
On Fri, 2016-04-01 at 16:06 +0200, Jörg Otte wrote: > 2016-04-01 14:40 GMT+02:00 Rafael J. Wysocki <rjw@rjwysocki.net>: > > > > On Friday, April 01, 2016 11:20:42 AM Jörg Otte wrote: > > > > > > 2016-03-31 17:43 GMT+02:00 Rafael J. Wysocki <rjw@rjwysocki.net>: > > > > > > > > On Thursday, March 31, 2016 05:25:18 PM Jörg Otte wrote: > > > > > > > > > > 2016-03-31 13:42 GMT+02:00 Rafael J. Wysocki <rjw@rjwysocki.n > > > > > et>: > > > > > > > > > > > > On Thursday, March 31, 2016 11:05:56 AM Jörg Otte wrote: > > > > > > [Cut] > > > > > > > Done. Attached the tracer. > For me it looks like the previous one of the failing case. > The traces show that idle task is constantly running without sleep. The driver is processing samples for idle task for every 10ms and aperf/mperf are showing that we are always in turbo mode for idle task. Need to find out why idle task is not sleeping. Thanks, Srinivas > Thanks, Jörg
[toc] | [prev] | [next] | [standalone]
Page 1 of 2 [1] 2 Next page →
Back to top | Article view | linux.kernel
csiph-web