Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1660675 > unrolled thread

[PATCH 0/5] x86, cpufreq: consolidate APERF/MPERF calculation

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/5] x86, cpufreq: consolidate APERF/MPERF calculation Len Brown <lenb@kernel.org> - 2017-06-08 04:40 +0200
    [PATCH 2/5] x86: use common aperfmperf_khz_on_cpu() to calculate KHz using APERF/MPERF Len Brown <lenb@kernel.org> - 2017-06-08 04:40 +0200
      Re: [PATCH 2/5] x86: use common aperfmperf_khz_on_cpu() to calculate KHz using APERF/MPERF "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-06-17 02:40 +0200
        Re: [PATCH 2/5] x86: use common aperfmperf_khz_on_cpu() to calculate  KHz using APERF/MPERF Len Brown <lenb@kernel.org> - 2017-06-17 03:50 +0200
          Re: [PATCH 2/5] x86: use common aperfmperf_khz_on_cpu() to calculate KHz using APERF/MPERF "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-06-19 14:40 +0200
            Re: [PATCH 2/5] x86: use common aperfmperf_khz_on_cpu() to calculate KHz using APERF/MPERF "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-06-19 16:30 +0200
    [PATCH 3/5] intel_pstate: remove intel_pstate.get() Len Brown <lenb@kernel.org> - 2017-06-08 04:40 +0200
      Re: [PATCH 3/5] intel_pstate: remove intel_pstate.get() "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-06-17 02:10 +0200
        Re: [PATCH 3/5] intel_pstate: remove intel_pstate.get() Len Brown <lenb@kernel.org> - 2017-06-17 02:40 +0200
          Re: [PATCH 3/5] intel_pstate: remove intel_pstate.get() "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-06-17 02:50 +0200
            Re: [PATCH 3/5] intel_pstate: remove intel_pstate.get() Len Brown <lenb@kernel.org> - 2017-06-17 03:20 +0200
              Re: [PATCH 3/5] intel_pstate: remove intel_pstate.get() "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-06-17 03:30 +0200
                Re: [PATCH 3/5] intel_pstate: remove intel_pstate.get() "Rafael J. Wysocki" <rafael@kernel.org> - 2017-06-17 03:50 +0200
                  Re: [PATCH 3/5] intel_pstate: remove intel_pstate.get() Len Brown <lenb@kernel.org> - 2017-06-17 04:10 +0200
      Re: [PATCH 3/5] intel_pstate: remove intel_pstate.get() "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-06-17 02:40 +0200
    [PATCH 1/5] x86: do not use cpufreq_quick_get() for /proc/cpuinfo "cpu MHz" Len Brown <lenb@kernel.org> - 2017-06-08 04:50 +0200
    [PATCH 4/5] intel_pstate: skip scheduler hook when in "performance" mode. Len Brown <lenb@kernel.org> - 2017-06-08 04:50 +0200
      Re: [PATCH 4/5] intel_pstate: skip scheduler hook when in "performance" mode. "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-06-17 02:20 +0200
        Re: [PATCH 4/5] intel_pstate: skip scheduler hook when in  "performance" mode. Len Brown <lenb@kernel.org> - 2017-06-17 03:00 +0200
          Re: [PATCH 4/5] intel_pstate: skip scheduler hook when in "performance" mode. "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-06-17 03:20 +0200
            Re: [PATCH 4/5] intel_pstate: skip scheduler hook when in  "performance" mode. Len Brown <lenb@kernel.org> - 2017-06-17 03:40 +0200
            Re: [PATCH 4/5] intel_pstate: skip scheduler hook when in  "performance" mode. Len Brown <lenb@kernel.org> - 2017-06-17 03:40 +0200
    [PATCH 5/5] intel_pstate: delete scheduler hook in HWP mode Len Brown <lenb@kernel.org> - 2017-06-08 04:50 +0200
      Re: [PATCH 5/5] intel_pstate: delete scheduler hook in HWP mode "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-06-17 02:20 +0200
        Re: [PATCH 5/5] intel_pstate: delete scheduler hook in HWP mode Len Brown <lenb@kernel.org> - 2017-06-17 03:30 +0200

Page 2 of 2 — ← Prev page 1 [2]

#1668124 — Re: [PATCH 4/5] intel_pstate: skip scheduler hook when in "performance" mode.

sorry, that was a premature send...

>>> > What about update_turbo_pstate()?
>>> >
>>> > In theory MSR_IA32_MISC_ENABLE_TURBO_DISABLE can be set at any time, so
>>> > wouldn't that become problematic after this change?
>>>
>>> yes, the sysfs "no_turbo" attribute can be modified at any time, invoking
>>> update_turbo_state(), which will update MSR_IA32_MISC_ENABLE_TURBO_DISABLE
>>
>> If that was the only way it could change, I wouldn't worry about it, but what
>> about changes by BMCs and similar?  Are they not a concern?
>>
>>> But how is the presence or change in turbo related to the lack of a
>>> need to hook the scheduler callback in "performance" mode?  The hook
>>> literally does nothing in this case, except consume cycles, no?
>>
>> No.
>>
>> It actually sets the P-state to the current maximum (which admittedly is
>> excessive) exactly because the maximum may change on the fly in theory.
>
> There are 2 cases.
>
> If turbo was enabled and were we requesting max turbo
> and "somebody" disabled turbo in an MSR, then the HW would
> simply clip our excessive
request to what the hardware supports.

if turbo was disabled and we were requesting max non-turbo,
and "somebody" enabled turbo in the MSR,
then it is highly likely that current request is already sufficiently
high enough
to enable turbo.  "highly likely" = would work on 100% of the
machines I have ever seen, including those with mis-configured TAR.

ie. it should not matter.

>> If it can't change on the fly (or we don't care), we can do some more
>> simplifications there. :-)
>
> I do not think it is Linux's responsibility to monitor changes to MSRs
> such as Turbo enable/disable done behind its back by a BMC at run-time.
> (if this is even possible)
>
>
> --
> Len Brown, Intel Open Source Technology Center

[toc] | [prev] | [next] | [standalone]

#1668125 — Re: [PATCH 4/5] intel_pstate: skip scheduler hook when in "performance" mode.

On Fri, Jun 16, 2017 at 9:06 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> On Friday, June 16, 2017 08:52:53 PM Len Brown wrote:
>> On Fri, Jun 16, 2017 at 8:04 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>> > On Wednesday, June 07, 2017 07:39:15 PM Len Brown wrote:
>> >> From: Len Brown <len.brown@intel.com>
>> >>
>> >> When the governor is set to "performance", intel_pstate does not
>> >> need the scheduler hook for doing any calculations.  Under these
>> >> conditions, its only purpose is to continue to maintain
>> >> cpufreq/scaling_cur_freq.
>> >>
>> >> But the cpufreq/scaling_cur_freq sysfs attribute is now provided by
>> >> the x86 cpufreq core on all modern x86 systems, including
>> >> all systems supported by the intel_pstate driver.
>> >>
>> >> So in "performance" governor mode, the scheduler hook can be skipped.
>> >> This applies to both in Software and Hardware P-state control modes.
>> >>
>> >> Suggested-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
>> >> Signed-off-by: Len Brown <len.brown@intel.com>
>> >> ---
>> >>  drivers/cpufreq/intel_pstate.c | 4 ++--
>> >>  1 file changed, 2 insertions(+), 2 deletions(-)
>> >>
>> >> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
>> >> index 5d67780..0ff3a4b 100644
>> >> --- a/drivers/cpufreq/intel_pstate.c
>> >> +++ b/drivers/cpufreq/intel_pstate.c
>> >> @@ -2025,10 +2025,10 @@ static int intel_pstate_set_policy(struct cpufreq_policy *policy)
>> >>                */
>> >>               intel_pstate_clear_update_util_hook(policy->cpu);
>> >
>> > The statement above shouldn't be necessary any more after the change below.
>>
>> The policy can change at run time form something other than  performance
>> to performance, so we want to clear the hook in that case, no?
>
> Yes.
>
>> >>               intel_pstate_max_within_limits(cpu);
>> >> +     } else {
>> >> +             intel_pstate_set_update_util_hook(policy->cpu);
>> >>       }
>> >>
>> >> -     intel_pstate_set_update_util_hook(policy->cpu);
>> >> -
>> >>       if (hwp_active)
>> >>               intel_pstate_hwp_set(policy->cpu);
>> >>
>> >
>> > What about update_turbo_pstate()?
>> >
>> > In theory MSR_IA32_MISC_ENABLE_TURBO_DISABLE can be set at any time, so
>> > wouldn't that become problematic after this change?
>>
>> yes, the sysfs "no_turbo" attribute can be modified at any time, invoking
>> update_turbo_state(), which will update MSR_IA32_MISC_ENABLE_TURBO_DISABLE
>
> If that was the only way it could change, I wouldn't worry about it, but what
> about changes by BMCs and similar?  Are they not a concern?
>
>> But how is the presence or change in turbo related to the lack of a
>> need to hook the scheduler callback in "performance" mode?  The hook
>> literally does nothing in this case, except consume cycles, no?
>
> No.
>
> It actually sets the P-state to the current maximum (which admittedly is
> excessive) exactly because the maximum may change on the fly in theory.

There are 2 cases.

If turbo was enabled and were we requesting max turbo
and "somebody" disabled turbo in an MSR, then the HW would
simply clip our excessive req
> If it can't change on the fly (or we don't care), we can do some more
> simplifications there. :-)

I do not think it is Linux's responsibility to monitor changes to MSRs
such as Turbo enable/disable done behind its back by a BMC at run-time.
(if this is even possible)


-- 
Len Brown, Intel Open Source Technology Center

[toc] | [prev] | [next] | [standalone]

#1660690 — [PATCH 5/5] intel_pstate: delete scheduler hook in HWP mode

From: Len Brown <len.brown@intel.com>

The cpufreqa/scaling_cur_freq sysfs attribute is now provided by
the x86 cpufreq core on all modern x86 systems, including
all systems supported by the intel_pstate driver.

In HWP mode, maintaining that value was the sole purpose of
the scheduler hook, intel_pstate_update_util_hwp(),
so it can now be removed.

Signed-off-by: Len Brown <len.brown@intel.com>
---
 drivers/cpufreq/intel_pstate.c | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 0ff3a4b..718732b 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -1726,16 +1726,6 @@ static void intel_pstate_adjust_pstate(struct cpudata *cpu, int target_pstate)
 		fp_toint(cpu->iowait_boost * 100));
 }
 
-static void intel_pstate_update_util_hwp(struct update_util_data *data,
-					 u64 time, unsigned int flags)
-{
-	struct cpudata *cpu = container_of(data, struct cpudata, update_util);
-	u64 delta_ns = time - cpu->sample.time;
-
-	if ((s64)delta_ns >= INTEL_PSTATE_HWP_SAMPLING_INTERVAL)
-		intel_pstate_sample(cpu, time);
-}
-
 static void intel_pstate_update_util_pid(struct update_util_data *data,
 					 u64 time, unsigned int flags)
 {
@@ -1920,6 +1910,9 @@ static void intel_pstate_set_update_util_hook(unsigned int cpu_num)
 {
 	struct cpudata *cpu = all_cpu_data[cpu_num];
 
+	if (hwp_active)
+		return;
+
 	if (cpu->update_util_set)
 		return;
 
@@ -2543,7 +2536,6 @@ static int __init intel_pstate_init(void)
 		} else {
 			hwp_active++;
 			intel_pstate.attr = hwp_cpufreq_attrs;
-			pstate_funcs.update_util = intel_pstate_update_util_hwp;
 			goto hwp_cpu_matched;
 		}
 	} else {
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1668103 — Re: [PATCH 5/5] intel_pstate: delete scheduler hook in HWP mode

On Wednesday, June 07, 2017 07:39:16 PM Len Brown wrote:
> From: Len Brown <len.brown@intel.com>
> 
> The cpufreqa/scaling_cur_freq sysfs attribute is now provided by
> the x86 cpufreq core on all modern x86 systems, including
> all systems supported by the intel_pstate driver.

Not sure what you mean by "x86 cpufreq core"?

Besides, I'd reorder this change with respect to patch [4/5] as this
eliminates the hook entirely and then the "performance"-related change
would only affect non-HWP.

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]

#1668119 — Re: [PATCH 5/5] intel_pstate: delete scheduler hook in HWP mode

On Fri, Jun 16, 2017 at 8:09 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> On Wednesday, June 07, 2017 07:39:16 PM Len Brown wrote:
>> From: Len Brown <len.brown@intel.com>
>>
>> The cpufreqa/scaling_cur_freq sysfs attribute is now provided by
>> the x86 cpufreq core on all modern x86 systems, including
>> all systems supported by the intel_pstate driver.
>
> Not sure what you mean by "x86 cpufreq core"?

I refer to code that builds if (CONFIG_X86 && CONFIG_CPU_FREQ)

Since it was enough to provoke a comment form you, how about this wording?:

The cpufreq/scaling_cur_freq sysfs attribute is now provided by
shared x86 cpufreq code on modern x86 systems, including
all systems supported by the intel_pstate driver.

> Besides, I'd reorder this change with respect to patch [4/5] as this
> eliminates the hook entirely and then the "performance"-related change
> would only affect non-HWP.

I don't actually see a problem with either order,
but i'll send the refresh with the order you suggest.

thanks,
Len Brown, Intel Open Source Technology Center

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web