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

Groups > linux.kernel > #1682283 > unrolled thread

[PATCH v2 00/10] arm, arm64: frequency- and cpu-invariant accounting support for task scheduler

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 00/10] arm, arm64: frequency- and cpu-invariant accounting support for task scheduler Dietmar Eggemann <dietmar.eggemann@arm.com> - 2017-07-06 12:00 +0200
    [PATCH v2 08/10] arm64: wire frequency-invariant accounting support up to the task scheduler Dietmar Eggemann <dietmar.eggemann@arm.com> - 2017-07-06 12:00 +0200
      Re: [PATCH v2 08/10] arm64: wire frequency-invariant accounting  support up to the task scheduler Viresh Kumar <viresh.kumar@linaro.org> - 2017-07-06 12:50 +0200
    [PATCH v2 03/10] drivers base/arch_topology: frequency-invariant load-tracking support Dietmar Eggemann <dietmar.eggemann@arm.com> - 2017-07-06 12:00 +0200
      Re: [PATCH v2 03/10] drivers base/arch_topology: frequency-invariant  load-tracking support Viresh Kumar <viresh.kumar@linaro.org> - 2017-07-06 12:50 +0200
        Re: [PATCH v2 03/10] drivers base/arch_topology: frequency-invariant  load-tracking support Dietmar Eggemann <dietmar.eggemann@arm.com> - 2017-07-07 19:00 +0200
    [PATCH v2 05/10] arm: wire frequency-invariant accounting support up to the task scheduler Dietmar Eggemann <dietmar.eggemann@arm.com> - 2017-07-06 12:00 +0200
      Re: [PATCH v2 05/10] arm: wire frequency-invariant accounting  support up to the task scheduler Viresh Kumar <viresh.kumar@linaro.org> - 2017-07-06 12:50 +0200
    [PATCH v2 02/10] cpufreq: provide data for frequency-invariant load-tracking support Dietmar Eggemann <dietmar.eggemann@arm.com> - 2017-07-06 12:00 +0200
      Re: [PATCH v2 02/10] cpufreq: provide data for frequency-invariant  load-tracking support Viresh Kumar <viresh.kumar@linaro.org> - 2017-07-06 12:50 +0200
        Re: [PATCH v2 02/10] cpufreq: provide data for frequency-invariant load-tracking support "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-07-07 00:50 +0200
        Re: [PATCH v2 02/10] cpufreq: provide data for frequency-invariant  load-tracking support Dietmar Eggemann <dietmar.eggemann@arm.com> - 2017-07-07 18:10 +0200
          Re: [PATCH v2 02/10] cpufreq: provide data for frequency-invariant  load-tracking support "Rafael J. Wysocki" <rafael@kernel.org> - 2017-07-07 18:20 +0200
            Re: [PATCH v2 02/10] cpufreq: provide data for frequency-invariant  load-tracking support Dietmar Eggemann <dietmar.eggemann@arm.com> - 2017-07-07 19:10 +0200
              Re: [PATCH v2 02/10] cpufreq: provide data for frequency-invariant load-tracking support "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-07-08 14:20 +0200
                Re: [PATCH v2 02/10] cpufreq: provide data for frequency-invariant  load-tracking support Viresh Kumar <viresh.kumar@linaro.org> - 2017-07-10 09:00 +0200
                  Re: [PATCH v2 02/10] cpufreq: provide data for frequency-invariant load-tracking support "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-07-10 15:00 +0200
                    Re: [PATCH v2 02/10] cpufreq: provide data for frequency-invariant  load-tracking support Viresh Kumar <viresh.kumar@linaro.org> - 2017-07-11 08:50 +0200
                      Re: [PATCH v2 02/10] cpufreq: provide data for frequency-invariant  load-tracking support Dietmar Eggemann <dietmar.eggemann@arm.com> - 2017-07-11 17:30 +0200
                        Re: [PATCH v2 02/10] cpufreq: provide data for frequency-invariant  load-tracking support Sudeep Holla <sudeep.holla@arm.com> - 2017-07-13 14:50 +0200
                          Re: [PATCH v2 02/10] cpufreq: provide data for frequency-invariant  load-tracking support Dietmar Eggemann <dietmar.eggemann@arm.com> - 2017-07-13 15:10 +0200
                            Re: [PATCH v2 02/10] cpufreq: provide data for frequency-invariant  load-tracking support Sudeep Holla <sudeep.holla@arm.com> - 2017-07-13 16:10 +0200
                Re: [PATCH v2 02/10] cpufreq: provide data for frequency-invariant  load-tracking support Peter Zijlstra <peterz@infradead.org> - 2017-07-10 11:40 +0200
                  Re: [PATCH v2 02/10] cpufreq: provide data for frequency-invariant  load-tracking support Viresh Kumar <viresh.kumar@linaro.org> - 2017-07-10 11:50 +0200
                    Re: [PATCH v2 02/10] cpufreq: provide data for frequency-invariant  load-tracking support Dietmar Eggemann <dietmar.eggemann@arm.com> - 2017-07-10 12:40 +0200
                Re: [PATCH v2 02/10] cpufreq: provide data for frequency-invariant  load-tracking support Dietmar Eggemann <dietmar.eggemann@arm.com> - 2017-07-10 14:10 +0200
                  Re: [PATCH v2 02/10] cpufreq: provide data for frequency-invariant  load-tracking support Viresh Kumar <viresh.kumar@linaro.org> - 2017-07-11 08:10 +0200
                    Re: [PATCH v2 02/10] cpufreq: provide data for frequency-invariant  load-tracking support Dietmar Eggemann <dietmar.eggemann@arm.com> - 2017-07-11 17:10 +0200
                      Re: [PATCH v2 02/10] cpufreq: provide data for frequency-invariant load-tracking support "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-07-11 17:10 +0200
                        Re: [PATCH v2 02/10] cpufreq: provide data for frequency-invariant  load-tracking support Dietmar Eggemann <dietmar.eggemann@arm.com> - 2017-07-11 17:20 +0200
                      Re: [PATCH v2 02/10] cpufreq: provide data for frequency-invariant  load-tracking support Viresh Kumar <viresh.kumar@linaro.org> - 2017-07-12 06:10 +0200
                        Re: [PATCH v2 02/10] cpufreq: provide data for frequency-invariant  load-tracking support Peter Zijlstra <peterz@infradead.org> - 2017-07-12 10:40 +0200
                          Re: [PATCH v2 02/10] cpufreq: provide data for frequency-invariant  load-tracking support Viresh Kumar <viresh.kumar@linaro.org> - 2017-07-12 11:30 +0200
                            Re: [PATCH v2 02/10] cpufreq: provide data for frequency-invariant  load-tracking support Peter Zijlstra <peterz@infradead.org> - 2017-07-12 13:20 +0200
                              Re: [PATCH v2 02/10] cpufreq: provide data for frequency-invariant load-tracking support "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-07-13 01:30 +0200
                                Re: [PATCH v2 02/10] cpufreq: provide data for frequency-invariant  load-tracking support Peter Zijlstra <peterz@infradead.org> - 2017-07-13 10:00 +0200
                                Re: [PATCH v2 02/10] cpufreq: provide data for frequency-invariant  load-tracking support Viresh Kumar <viresh.kumar@linaro.org> - 2017-07-13 10:50 +0200
                                  Re: [PATCH v2 02/10] cpufreq: provide data for frequency-invariant  load-tracking support Peter Zijlstra <peterz@infradead.org> - 2017-07-13 13:20 +0200
                              Re: [PATCH v2 02/10] cpufreq: provide data for frequency-invariant  load-tracking support Sudeep Holla <sudeep.holla@arm.com> - 2017-07-13 16:10 +0200
                                Re: [PATCH v2 02/10] cpufreq: provide data for frequency-invariant  load-tracking support Peter Zijlstra <peterz@infradead.org> - 2017-07-13 16:50 +0200
                                  Re: [PATCH v2 02/10] cpufreq: provide data for frequency-invariant  load-tracking support Sudeep Holla <sudeep.holla@arm.com> - 2017-07-13 17:10 +0200
                            Re: [PATCH v2 02/10] cpufreq: provide data for frequency-invariant  load-tracking support Sudeep Holla <sudeep.holla@arm.com> - 2017-07-13 15:00 +0200
                        Re: [PATCH v2 02/10] cpufreq: provide data for frequency-invariant  load-tracking support Sudeep Holla <sudeep.holla@arm.com> - 2017-07-13 14:50 +0200
          Re: [PATCH v2 02/10] cpufreq: provide data for frequency-invariant  load-tracking support Viresh Kumar <viresh.kumar@linaro.org> - 2017-07-10 08:50 +0200
    [PATCH v2 04/10] arm: wire cpufreq input data for frequency-invariant accounting up to the arch Dietmar Eggemann <dietmar.eggemann@arm.com> - 2017-07-06 12:00 +0200
      Re: [PATCH v2 04/10] arm: wire cpufreq input data for  frequency-invariant accounting up to the arch Viresh Kumar <viresh.kumar@linaro.org> - 2017-07-06 12:50 +0200
        Re: [PATCH v2 04/10] arm: wire cpufreq input data for  frequency-invariant accounting up to the arch Dietmar Eggemann <dietmar.eggemann@arm.com> - 2017-07-10 17:20 +0200
          Re: [PATCH v2 04/10] arm: wire cpufreq input data for  frequency-invariant accounting up to the arch Viresh Kumar <viresh.kumar@linaro.org> - 2017-07-11 08:40 +0200
    [PATCH v2 09/10] arm64: wire cpu-invariant accounting support up to the task scheduler Dietmar Eggemann <dietmar.eggemann@arm.com> - 2017-07-06 12:00 +0200
      Re: [PATCH v2 09/10] arm64: wire cpu-invariant accounting support up  to the task scheduler Viresh Kumar <viresh.kumar@linaro.org> - 2017-07-06 12:50 +0200
    [PATCH v2 06/10] arm: wire cpu-invariant accounting support up to the task scheduler Dietmar Eggemann <dietmar.eggemann@arm.com> - 2017-07-06 12:00 +0200
      Re: [PATCH v2 06/10] arm: wire cpu-invariant accounting support up  to the task scheduler Viresh Kumar <viresh.kumar@linaro.org> - 2017-07-06 12:50 +0200
    [PATCH v2 01/10] drivers base/arch_topology: free cpumask cpus_to_visit Dietmar Eggemann <dietmar.eggemann@arm.com> - 2017-07-06 12:00 +0200
      Re: [PATCH v2 01/10] drivers base/arch_topology: free cpumask  cpus_to_visit Viresh Kumar <viresh.kumar@linaro.org> - 2017-07-06 12:30 +0200
        Re: [PATCH v2 01/10] drivers base/arch_topology: free cpumask  cpus_to_visit Juri Lelli <juri.lelli@arm.com> - 2017-07-06 13:00 +0200
          Re: [PATCH v2 01/10] drivers base/arch_topology: free cpumask  cpus_to_visit Viresh Kumar <viresh.kumar@linaro.org> - 2017-07-06 13:20 +0200
            Re: [PATCH v2 01/10] drivers base/arch_topology: free cpumask  cpus_to_visit Dietmar Eggemann <dietmar.eggemann@arm.com> - 2017-07-07 18:00 +0200
    [PATCH v2 10/10] drivers base/arch_topology: inline cpu- and frequency-invariant accounting Dietmar Eggemann <dietmar.eggemann@arm.com> - 2017-07-06 12:00 +0200
      Re: [PATCH v2 10/10] drivers base/arch_topology: inline cpu- and  frequency-invariant accounting Viresh Kumar <viresh.kumar@linaro.org> - 2017-07-06 13:00 +0200
        Re: [PATCH v2 10/10] drivers base/arch_topology: inline cpu- and  frequency-invariant accounting Dietmar Eggemann <dietmar.eggemann@arm.com> - 2017-07-10 17:20 +0200

Page 1 of 3  [1] 2 3  Next page →

#1682283 — [PATCH v2 00/10] arm, arm64: frequency- and cpu-invariant accounting support for task scheduler

For a more accurate (i.e. frequency- and cpu-invariant) load-tracking
the task scheduler needs a frequency-scaling and on a heterogeneous
system a cpu-scaling correction factor.

This patch-set implements a Frequency Invariance Engine (FIE)
(topology_get_freq_scale()) in drivers/base/arch_topology.c to provide
a frequency-scaling correction factor.

During the v1 [1] review Viresh Kumar pointed out that such a FIE based
on cpufreq transition notifier will not work with future cpufreq
policies supporting fast frequency switching.
To include support for fast frequency switching policies the FIE
implementation has been changed. Whenever there is a frequency change
cpufreq now calls the arch specific function arch_set_freq_scale() which
has to be implemented by the architecture. In case the arch does not
specify this function FIE support is compiled out of cpufreq.
The advantage is that this would support fast frequency switching since
it does not rely on cpufreq transition (or policy) notifier anymore.

The Cpu Invariance Engine (CIE) (topology_get_cpu_scale()) providing a
cpu-scaling correction factor was already introduced by the "Fix issues
and factorize arm/arm64 capacity information code" patch-set [2].

This patch-set also enables the frequency- and cpu-invariant accounting
support. Enabling here means to associate (wire) the task scheduler
function name arch_scale_freq_capacity and arch_scale_cpu_capacity with
the FIE and CIE function names from drivers/base/arch_topology.c. This
replaces the task scheduler's default FIE and CIE in
kernel/sched/sched.h.

There is an additional patch [10/10] in v2 which allows inlining of the
FIE and CIE into the appropriate task scheduler functions.

+------------------------------+       +------------------------------+
|                              |       |                              |
| cpufreq:                     |       | arch:                        |
|                              |       |                              |
|      arch_set_freq_scale() +-----------> topology_set_freq_scale()  |
|                              |       |                              |
+------------------------------+       |                              |
                                       |                              |
+------------------------------+       |                              |
|                              |       |                              |
| task scheduler:              |       |                              |
|                              |       |                              |
| arch_scale_freq_capacity() +-----------> topology_get_freq_scale()  |
|                              |       |                              |
| arch_scale_cpu_capacity()  +-----------> topology_get_cpu_scale()   |
|                              |       |                              |
+------------------------------+       +------------------------------+

Patch high level description:

  [   01/10] Fix to free cpumask cpus_to_visit
  [   02/10] Let cpufreq provide current and max supported frequency for
  	     the set of related cpus
  [   03/10] Frequency Invariance Engine (FIE)
  [   04/10] Connect cpufreq input data to FIE on arm
  [05,06/10] Enable frequency- and cpu-invariant accounting support on
  	     arm
  [   07/10] Connect cpufreq input data to FIE on arm64
  [08,09/10] Enable frequency- and cpu-invariant accounting support on
  	     arm64
  [   10/10] Allow CIE and FIE inlining

Changes v1->v2:

  - Rebase on top of next-20170630
  - Add fixup patch to free cpumask cpus_to_visit [01/10]
  - Propose solution to support fast frequency switching [02-04,07/10]
  - Add patch to allow CIE and FIE inlining [10/10]

The patch-set is based on top of linux-next/master (tag: next-20170630)
and it is also available from:

  git://linux-arm.org/linux-de.git upstream/freq_and_cpu_inv_v2

It has been tested on TC2 (arm) and JUNO (arm64) by running a ramp-up
rt-app task pinned to a cpu with the ondemand cpufreq governor and
checking the load-tracking signals of this task.

[1] https://marc.info/?l=linux-kernel&m=149690865010019&w=2
[2] https://marc.info/?l=linux-kernel&m=149625018223002&w=2

Dietmar Eggemann (10):
  drivers base/arch_topology: free cpumask cpus_to_visit
  cpufreq: provide data for frequency-invariant load-tracking support
  drivers base/arch_topology: frequency-invariant load-tracking support
  arm: wire cpufreq input data for frequency-invariant accounting up to
    the arch
  arm: wire frequency-invariant accounting support up to the task
    scheduler
  arm: wire cpu-invariant accounting support up to the task scheduler
  arm64: wire cpufreq input data for frequency-invariant accounting up
    to the arch
  arm64: wire frequency-invariant accounting support up to the task
    scheduler
  arm64: wire cpu-invariant accounting support up to the task scheduler
  drivers base/arch_topology: inline cpu- and frequency-invariant
    accounting

 arch/arm/include/asm/topology.h   | 11 +++++++++++
 arch/arm/kernel/topology.c        |  1 -
 arch/arm64/include/asm/topology.h | 11 +++++++++++
 arch/arm64/kernel/topology.c      |  1 -
 drivers/base/arch_topology.c      | 30 ++++++++++++++++++++++++------
 drivers/cpufreq/cpufreq.c         | 26 ++++++++++++++++++++++++++
 include/linux/arch_topology.h     | 20 +++++++++++++++++++-
 7 files changed, 91 insertions(+), 9 deletions(-)

-- 
2.11.0

[toc] | [next] | [standalone]

#1682284 — [PATCH v2 08/10] arm64: wire frequency-invariant accounting support up to the task scheduler

Commit dfbca41f3479 ("sched: Optimize freq invariant accounting")
changed the wiring which now has to be done by associating
arch_scale_freq_capacity with the actual implementation provided
by the architecture.

Define arch_scale_freq_capacity to use the arch_topology "driver"
function topology_get_freq_scale() for the task scheduler's
frequency-invariant accounting instead of the default
arch_scale_freq_capacity() in kernel/sched/sched.h.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Juri Lelli <juri.lelli@arm.com>
Signed-off-by: Dietmar Eggemann <dietmar.eggemann@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Acked-by: Vincent Guittot <vincent.guittot@linaro.org>
Tested-by: Juri Lelli <juri.lelli@arm.com>
Reviewed-by: Juri Lelli <juri.lelli@arm.com>
---
 arch/arm64/include/asm/topology.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm64/include/asm/topology.h b/arch/arm64/include/asm/topology.h
index d9cbb289e295..4fd598b432e6 100644
--- a/arch/arm64/include/asm/topology.h
+++ b/arch/arm64/include/asm/topology.h
@@ -37,6 +37,9 @@ int pcibus_to_node(struct pci_bus *bus);
 /* Subscribe for input data for frequency-invariant load-tracking */
 #define arch_set_freq_scale topology_set_freq_scale
 
+/* Replace task scheduler's default frequency-invariant accounting */
+#define arch_scale_freq_capacity topology_get_freq_scale
+
 #include <asm-generic/topology.h>
 
 #endif /* _ASM_ARM_TOPOLOGY_H */
-- 
2.11.0

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

#1682348 — Re: [PATCH v2 08/10] arm64: wire frequency-invariant accounting support up to the task scheduler

On 06-07-17, 10:49, Dietmar Eggemann wrote:
> Commit dfbca41f3479 ("sched: Optimize freq invariant accounting")
> changed the wiring which now has to be done by associating
> arch_scale_freq_capacity with the actual implementation provided
> by the architecture.
> 
> Define arch_scale_freq_capacity to use the arch_topology "driver"
> function topology_get_freq_scale() for the task scheduler's
> frequency-invariant accounting instead of the default
> arch_scale_freq_capacity() in kernel/sched/sched.h.
> 
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Juri Lelli <juri.lelli@arm.com>
> Signed-off-by: Dietmar Eggemann <dietmar.eggemann@arm.com>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> Acked-by: Vincent Guittot <vincent.guittot@linaro.org>
> Tested-by: Juri Lelli <juri.lelli@arm.com>
> Reviewed-by: Juri Lelli <juri.lelli@arm.com>
> ---
>  arch/arm64/include/asm/topology.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/topology.h b/arch/arm64/include/asm/topology.h
> index d9cbb289e295..4fd598b432e6 100644
> --- a/arch/arm64/include/asm/topology.h
> +++ b/arch/arm64/include/asm/topology.h
> @@ -37,6 +37,9 @@ int pcibus_to_node(struct pci_bus *bus);
>  /* Subscribe for input data for frequency-invariant load-tracking */
>  #define arch_set_freq_scale topology_set_freq_scale
>  
> +/* Replace task scheduler's default frequency-invariant accounting */
> +#define arch_scale_freq_capacity topology_get_freq_scale
> +
>  #include <asm-generic/topology.h>
>  
>  #endif /* _ASM_ARM_TOPOLOGY_H */

Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

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

#1682285 — [PATCH v2 03/10] drivers base/arch_topology: frequency-invariant load-tracking support

Implements an arch-specific frequency-scaling function
topology_get_freq_scale() which provides the following frequency
scaling factor:

  current_freq(cpu) << SCHED_CAPACITY_SHIFT / max_supported_freq(cpu)

One possible consumer of this is the Per-Entity Load Tracking (PELT)
mechanism of the task scheduler.

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Juri Lelli <juri.lelli@arm.com>
Signed-off-by: Dietmar Eggemann <dietmar.eggemann@arm.com>
---
 drivers/base/arch_topology.c  | 20 ++++++++++++++++++++
 include/linux/arch_topology.h |  7 +++++++
 2 files changed, 27 insertions(+)

diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
index f4832c662762..63fb3f945d21 100644
--- a/drivers/base/arch_topology.c
+++ b/drivers/base/arch_topology.c
@@ -22,6 +22,26 @@
 #include <linux/string.h>
 #include <linux/sched/topology.h>
 
+static DEFINE_PER_CPU(unsigned long, freq_scale) = SCHED_CAPACITY_SCALE;
+
+unsigned long topology_get_freq_scale(struct sched_domain *sd, int cpu)
+{
+	return per_cpu(freq_scale, cpu);
+}
+
+void topology_set_freq_scale(struct cpumask *cpus, unsigned long cur_freq,
+			     unsigned long max_freq)
+{
+	unsigned long scale;
+	int i;
+
+	scale = (cur_freq << SCHED_CAPACITY_SHIFT) / max_freq;
+
+	for_each_cpu(i, cpus)
+		per_cpu(freq_scale, i) = scale;
+}
+
+
 static DEFINE_MUTEX(cpu_scale_mutex);
 static DEFINE_PER_CPU(unsigned long, cpu_scale) = SCHED_CAPACITY_SCALE;
 
diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h
index 9af3c174c03a..168104d2d2cf 100644
--- a/include/linux/arch_topology.h
+++ b/include/linux/arch_topology.h
@@ -4,6 +4,8 @@
 #ifndef _LINUX_ARCH_TOPOLOGY_H_
 #define _LINUX_ARCH_TOPOLOGY_H_
 
+#include <linux/cpumask.h>
+
 void topology_normalize_cpu_scale(void);
 
 struct device_node;
@@ -14,4 +16,9 @@ unsigned long topology_get_cpu_scale(struct sched_domain *sd, int cpu);
 
 void topology_set_cpu_scale(unsigned int cpu, unsigned long capacity);
 
+unsigned long topology_get_freq_scale(struct sched_domain *sd, int cpu);
+
+void topology_set_freq_scale(struct cpumask *cpus, unsigned long cur_freq,
+			     unsigned long max_freq);
+
 #endif /* _LINUX_ARCH_TOPOLOGY_H_ */
-- 
2.11.0

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

#1682338 — Re: [PATCH v2 03/10] drivers base/arch_topology: frequency-invariant load-tracking support

On 06-07-17, 10:49, Dietmar Eggemann wrote:
> Implements an arch-specific frequency-scaling function
> topology_get_freq_scale() which provides the following frequency
> scaling factor:
> 
>   current_freq(cpu) << SCHED_CAPACITY_SHIFT / max_supported_freq(cpu)
> 
> One possible consumer of this is the Per-Entity Load Tracking (PELT)
> mechanism of the task scheduler.
> 
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Juri Lelli <juri.lelli@arm.com>
> Signed-off-by: Dietmar Eggemann <dietmar.eggemann@arm.com>
> ---
>  drivers/base/arch_topology.c  | 20 ++++++++++++++++++++
>  include/linux/arch_topology.h |  7 +++++++
>  2 files changed, 27 insertions(+)
> 
> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
> index f4832c662762..63fb3f945d21 100644
> --- a/drivers/base/arch_topology.c
> +++ b/drivers/base/arch_topology.c
> @@ -22,6 +22,26 @@
>  #include <linux/string.h>
>  #include <linux/sched/topology.h>
>  
> +static DEFINE_PER_CPU(unsigned long, freq_scale) = SCHED_CAPACITY_SCALE;
> +
> +unsigned long topology_get_freq_scale(struct sched_domain *sd, int cpu)
> +{
> +	return per_cpu(freq_scale, cpu);
> +}
> +
> +void topology_set_freq_scale(struct cpumask *cpus, unsigned long cur_freq,
> +			     unsigned long max_freq)
> +{
> +	unsigned long scale;
> +	int i;
> +
> +	scale = (cur_freq << SCHED_CAPACITY_SHIFT) / max_freq;
> +
> +	for_each_cpu(i, cpus)
> +		per_cpu(freq_scale, i) = scale;
> +}
> +
> +
>  static DEFINE_MUTEX(cpu_scale_mutex);
>  static DEFINE_PER_CPU(unsigned long, cpu_scale) = SCHED_CAPACITY_SCALE;
>  
> diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h
> index 9af3c174c03a..168104d2d2cf 100644
> --- a/include/linux/arch_topology.h
> +++ b/include/linux/arch_topology.h
> @@ -4,6 +4,8 @@
>  #ifndef _LINUX_ARCH_TOPOLOGY_H_
>  #define _LINUX_ARCH_TOPOLOGY_H_
>  
> +#include <linux/cpumask.h>
> +

You don't need a full include here, instead following will work pretty well.

struct cpumask;

>  void topology_normalize_cpu_scale(void);
>  
>  struct device_node;
> @@ -14,4 +16,9 @@ unsigned long topology_get_cpu_scale(struct sched_domain *sd, int cpu);
>  
>  void topology_set_cpu_scale(unsigned int cpu, unsigned long capacity);
>  
> +unsigned long topology_get_freq_scale(struct sched_domain *sd, int cpu);
> +
> +void topology_set_freq_scale(struct cpumask *cpus, unsigned long cur_freq,
> +			     unsigned long max_freq);
> +
>  #endif /* _LINUX_ARCH_TOPOLOGY_H_ */

Apart from that:

Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

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

#1683313 — Re: [PATCH v2 03/10] drivers base/arch_topology: frequency-invariant load-tracking support

On 06/07/17 11:45, Viresh Kumar wrote:
> On 06-07-17, 10:49, Dietmar Eggemann wrote:

[...]

>> diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h
>> index 9af3c174c03a..168104d2d2cf 100644
>> --- a/include/linux/arch_topology.h
>> +++ b/include/linux/arch_topology.h
>> @@ -4,6 +4,8 @@
>>  #ifndef _LINUX_ARCH_TOPOLOGY_H_
>>  #define _LINUX_ARCH_TOPOLOGY_H_
>>  
>> +#include <linux/cpumask.h>
>> +
> 
> You don't need a full include here, instead following will work pretty well.
> 
> struct cpumask;

True. Forward declaration is sufficient here. Will change it.

[...]

> Apart from that:
> 
> Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>

Thanks!

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

#1682286 — [PATCH v2 05/10] arm: wire frequency-invariant accounting support up to the task scheduler

Commit dfbca41f3479 ("sched: Optimize freq invariant accounting")
changed the wiring which now has to be done by associating
arch_scale_freq_capacity with the actual implementation provided
by the architecture.

Define arch_scale_freq_capacity to use the arch_topology "driver"
function topology_get_freq_scale() for the task scheduler's
frequency-invariant accounting instead of the default
arch_scale_freq_capacity() in kernel/sched/sched.h.

Cc: Russell King <linux@arm.linux.org.uk>
Cc: Juri Lelli <juri.lelli@arm.com>
Signed-off-by: Dietmar Eggemann <dietmar.eggemann@arm.com>
Acked-by: Vincent Guittot <vincent.guittot@linaro.org>
Tested-by: Juri Lelli <juri.lelli@arm.com>
Reviewed-by: Juri Lelli <juri.lelli@arm.com>
---
 arch/arm/include/asm/topology.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/include/asm/topology.h b/arch/arm/include/asm/topology.h
index ca05d1b90411..57aebfa03e24 100644
--- a/arch/arm/include/asm/topology.h
+++ b/arch/arm/include/asm/topology.h
@@ -29,6 +29,9 @@ const struct cpumask *cpu_coregroup_mask(int cpu);
 /* Subscribe for input data for frequency-invariant load-tracking */
 #define arch_set_freq_scale topology_set_freq_scale
 
+/* Replace task scheduler's default frequency-invariant accounting */
+#define arch_scale_freq_capacity topology_get_freq_scale
+
 #else
 
 static inline void init_cpu_topology(void) { }
-- 
2.11.0

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

#1682344 — Re: [PATCH v2 05/10] arm: wire frequency-invariant accounting support up to the task scheduler

On 06-07-17, 10:49, Dietmar Eggemann wrote:
> Commit dfbca41f3479 ("sched: Optimize freq invariant accounting")
> changed the wiring which now has to be done by associating
> arch_scale_freq_capacity with the actual implementation provided
> by the architecture.
> 
> Define arch_scale_freq_capacity to use the arch_topology "driver"
> function topology_get_freq_scale() for the task scheduler's
> frequency-invariant accounting instead of the default
> arch_scale_freq_capacity() in kernel/sched/sched.h.
> 
> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: Juri Lelli <juri.lelli@arm.com>
> Signed-off-by: Dietmar Eggemann <dietmar.eggemann@arm.com>
> Acked-by: Vincent Guittot <vincent.guittot@linaro.org>
> Tested-by: Juri Lelli <juri.lelli@arm.com>
> Reviewed-by: Juri Lelli <juri.lelli@arm.com>
> ---
>  arch/arm/include/asm/topology.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/arm/include/asm/topology.h b/arch/arm/include/asm/topology.h
> index ca05d1b90411..57aebfa03e24 100644
> --- a/arch/arm/include/asm/topology.h
> +++ b/arch/arm/include/asm/topology.h
> @@ -29,6 +29,9 @@ const struct cpumask *cpu_coregroup_mask(int cpu);
>  /* Subscribe for input data for frequency-invariant load-tracking */
>  #define arch_set_freq_scale topology_set_freq_scale
>  
> +/* Replace task scheduler's default frequency-invariant accounting */
> +#define arch_scale_freq_capacity topology_get_freq_scale
> +
>  #else
>  
>  static inline void init_cpu_topology(void) { }

Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

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

#1682287 — [PATCH v2 02/10] cpufreq: provide data for frequency-invariant load-tracking support

A frequency-invariant load-tracking solution based on cpufreq transition
notifier will not work for future fast frequency switching policies.
That is why a different solution is presented with this patch.

Let cpufreq call the function arch_set_freq_scale() to pass the current
frequency, the max supported frequency and the cpumask of the related
cpus to a consumer (an arch) which defines arch_set_freq_scale().

The consumer has to associate arch_set_freq_scale with the name of its
own implementation foo_set_freq_scale() to overwrite the empty standard
definition in drivers/cpufreq/cpufreq.c.
An arch could do this in one of its arch-specific header files
(e.g. arch/$ARCH/include/asm/topology.h) which gets included in
drivers/cpufreq/cpufreq.c.

In case arch_set_freq_scale() is not defined (and because of the
pr_debug() drivers/cpufreq/cpufreq.c is not compiled with -DDEBUG) the
function cpufreq_set_freq_scale() gets compiled out.

Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
Cc: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Dietmar Eggemann <dietmar.eggemann@arm.com>
---
 drivers/cpufreq/cpufreq.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 9bf97a366029..a04c5886a5ce 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -347,6 +347,28 @@ static void __cpufreq_notify_transition(struct cpufreq_policy *policy,
 	}
 }
 
+/*********************************************************************
+ *           FREQUENCY INVARIANT CPU CAPACITY SUPPORT                *
+ *********************************************************************/
+
+#ifndef arch_set_freq_scale
+static void arch_set_freq_scale(struct cpumask *cpus, unsigned long cur_freq,
+				unsigned long max_freq)
+{}
+#endif
+
+static void cpufreq_set_freq_scale(struct cpufreq_policy *policy,
+				   struct cpufreq_freqs *freqs)
+{
+	unsigned long cur_freq = freqs ? freqs->new : policy->cur;
+	unsigned long max_freq = policy->cpuinfo.max_freq;
+
+	pr_debug("cpus %*pbl cur/cur max freq %lu/%lu kHz\n",
+		 cpumask_pr_args(policy->related_cpus), cur_freq, max_freq);
+
+	arch_set_freq_scale(policy->related_cpus, cur_freq, max_freq);
+}
+
 /**
  * cpufreq_notify_transition - call notifier chain and adjust_jiffies
  * on frequency transition.
@@ -405,6 +427,8 @@ void cpufreq_freq_transition_begin(struct cpufreq_policy *policy,
 
 	spin_unlock(&policy->transition_lock);
 
+	cpufreq_set_freq_scale(policy, freqs);
+
 	cpufreq_notify_transition(policy, freqs, CPUFREQ_PRECHANGE);
 }
 EXPORT_SYMBOL_GPL(cpufreq_freq_transition_begin);
@@ -2203,6 +2227,8 @@ static int cpufreq_set_policy(struct cpufreq_policy *policy,
 	blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
 			CPUFREQ_NOTIFY, new_policy);
 
+	cpufreq_set_freq_scale(new_policy, NULL);
+
 	policy->min = new_policy->min;
 	policy->max = new_policy->max;
 
-- 
2.11.0

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

#1682346 — Re: [PATCH v2 02/10] cpufreq: provide data for frequency-invariant load-tracking support

On 06-07-17, 10:49, Dietmar Eggemann wrote:
> A frequency-invariant load-tracking solution based on cpufreq transition
> notifier will not work for future fast frequency switching policies.
> That is why a different solution is presented with this patch.
> 
> Let cpufreq call the function arch_set_freq_scale() to pass the current
> frequency, the max supported frequency and the cpumask of the related
> cpus to a consumer (an arch) which defines arch_set_freq_scale().
> 
> The consumer has to associate arch_set_freq_scale with the name of its
> own implementation foo_set_freq_scale() to overwrite the empty standard
> definition in drivers/cpufreq/cpufreq.c.
> An arch could do this in one of its arch-specific header files
> (e.g. arch/$ARCH/include/asm/topology.h) which gets included in
> drivers/cpufreq/cpufreq.c.
> 
> In case arch_set_freq_scale() is not defined (and because of the
> pr_debug() drivers/cpufreq/cpufreq.c is not compiled with -DDEBUG)

The line within () needs to be improved to convey a clear message.

> the
> function cpufreq_set_freq_scale() gets compiled out.
> 
> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
> Cc: Viresh Kumar <viresh.kumar@linaro.org>
> Signed-off-by: Dietmar Eggemann <dietmar.eggemann@arm.com>
> ---
>  drivers/cpufreq/cpufreq.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 9bf97a366029..a04c5886a5ce 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -347,6 +347,28 @@ static void __cpufreq_notify_transition(struct cpufreq_policy *policy,
>  	}
>  }
>  
> +/*********************************************************************
> + *           FREQUENCY INVARIANT CPU CAPACITY SUPPORT                *
> + *********************************************************************/
> +
> +#ifndef arch_set_freq_scale
> +static void arch_set_freq_scale(struct cpumask *cpus, unsigned long cur_freq,
> +				unsigned long max_freq)
> +{}
> +#endif
> +
> +static void cpufreq_set_freq_scale(struct cpufreq_policy *policy,
> +				   struct cpufreq_freqs *freqs)
> +{
> +	unsigned long cur_freq = freqs ? freqs->new : policy->cur;
> +	unsigned long max_freq = policy->cpuinfo.max_freq;
> +
> +	pr_debug("cpus %*pbl cur/cur max freq %lu/%lu kHz\n",
> +		 cpumask_pr_args(policy->related_cpus), cur_freq, max_freq);
> +
> +	arch_set_freq_scale(policy->related_cpus, cur_freq, max_freq);

I am not sure why all these are required to be sent here and will come back to
it later on after going through other patches.

> +}
> +
>  /**
>   * cpufreq_notify_transition - call notifier chain and adjust_jiffies
>   * on frequency transition.
> @@ -405,6 +427,8 @@ void cpufreq_freq_transition_begin(struct cpufreq_policy *policy,
>  
>  	spin_unlock(&policy->transition_lock);
>  
> +	cpufreq_set_freq_scale(policy, freqs);
> +

Why do this before even changing the frequency ? We may fail while changing it.

IMHO, you should call this routine whenever we update policy->cur and that
happens regularly in __cpufreq_notify_transition() and few other places..

>  	cpufreq_notify_transition(policy, freqs, CPUFREQ_PRECHANGE);
>  }
>  EXPORT_SYMBOL_GPL(cpufreq_freq_transition_begin);
> @@ -2203,6 +2227,8 @@ static int cpufreq_set_policy(struct cpufreq_policy *policy,
>  	blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
>  			CPUFREQ_NOTIFY, new_policy);
>  
> +	cpufreq_set_freq_scale(new_policy, NULL);

Why added it here ? To get it initialized ? If yes, then we should do that in
cpufreq_online() where we first initialize policy->cur.

Apart from this, you also need to update this in the schedutil governor (if you
haven't done that in this series later) as that also updates policy->cur in the
fast path.

-- 
viresh

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

#1682783 — Re: [PATCH v2 02/10] cpufreq: provide data for frequency-invariant load-tracking support

On Thursday, July 06, 2017 04:10:27 PM Viresh Kumar wrote:
> On 06-07-17, 10:49, Dietmar Eggemann wrote:
> > A frequency-invariant load-tracking solution based on cpufreq transition
> > notifier will not work for future fast frequency switching policies.
> > That is why a different solution is presented with this patch.
> > 
> > Let cpufreq call the function arch_set_freq_scale() to pass the current
> > frequency, the max supported frequency and the cpumask of the related
> > cpus to a consumer (an arch) which defines arch_set_freq_scale().
> > 
> > The consumer has to associate arch_set_freq_scale with the name of its
> > own implementation foo_set_freq_scale() to overwrite the empty standard
> > definition in drivers/cpufreq/cpufreq.c.
> > An arch could do this in one of its arch-specific header files
> > (e.g. arch/$ARCH/include/asm/topology.h) which gets included in
> > drivers/cpufreq/cpufreq.c.
> > 
> > In case arch_set_freq_scale() is not defined (and because of the
> > pr_debug() drivers/cpufreq/cpufreq.c is not compiled with -DDEBUG)
> 
> The line within () needs to be improved to convey a clear message.
> 
> > the
> > function cpufreq_set_freq_scale() gets compiled out.
> > 
> > Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
> > Cc: Viresh Kumar <viresh.kumar@linaro.org>
> > Signed-off-by: Dietmar Eggemann <dietmar.eggemann@arm.com>
> > ---
> >  drivers/cpufreq/cpufreq.c | 26 ++++++++++++++++++++++++++
> >  1 file changed, 26 insertions(+)
> > 
> > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> > index 9bf97a366029..a04c5886a5ce 100644
> > --- a/drivers/cpufreq/cpufreq.c
> > +++ b/drivers/cpufreq/cpufreq.c
> > @@ -347,6 +347,28 @@ static void __cpufreq_notify_transition(struct cpufreq_policy *policy,
> >  	}
> >  }
> >  
> > +/*********************************************************************
> > + *           FREQUENCY INVARIANT CPU CAPACITY SUPPORT                *
> > + *********************************************************************/
> > +
> > +#ifndef arch_set_freq_scale
> > +static void arch_set_freq_scale(struct cpumask *cpus, unsigned long cur_freq,
> > +				unsigned long max_freq)
> > +{}
> > +#endif
> > +
> > +static void cpufreq_set_freq_scale(struct cpufreq_policy *policy,
> > +				   struct cpufreq_freqs *freqs)
> > +{
> > +	unsigned long cur_freq = freqs ? freqs->new : policy->cur;
> > +	unsigned long max_freq = policy->cpuinfo.max_freq;
> > +
> > +	pr_debug("cpus %*pbl cur/cur max freq %lu/%lu kHz\n",
> > +		 cpumask_pr_args(policy->related_cpus), cur_freq, max_freq);
> > +
> > +	arch_set_freq_scale(policy->related_cpus, cur_freq, max_freq);
> 
> I am not sure why all these are required to be sent here and will come back to
> it later on after going through other patches.
> 
> > +}
> > +
> >  /**
> >   * cpufreq_notify_transition - call notifier chain and adjust_jiffies
> >   * on frequency transition.
> > @@ -405,6 +427,8 @@ void cpufreq_freq_transition_begin(struct cpufreq_policy *policy,
> >  
> >  	spin_unlock(&policy->transition_lock);
> >  
> > +	cpufreq_set_freq_scale(policy, freqs);
> > +
> 
> Why do this before even changing the frequency ? We may fail while changing it.
> 
> IMHO, you should call this routine whenever we update policy->cur and that
> happens regularly in __cpufreq_notify_transition() and few other places..

There seems to be a general problem with doing this in the core with respect to
things like intel_pstate that use their own governor callbacks and don't invoke
cpufreq_freq_transition_begin() then.

Thanks,
Rafael

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

#1683276 — Re: [PATCH v2 02/10] cpufreq: provide data for frequency-invariant load-tracking support

On 06/07/17 11:40, Viresh Kumar wrote:
> On 06-07-17, 10:49, Dietmar Eggemann wrote:

[...]

>> In case arch_set_freq_scale() is not defined (and because of the
>> pr_debug() drivers/cpufreq/cpufreq.c is not compiled with -DDEBUG)
> 
> The line within () needs to be improved to convey a clear message.

Probably not needed anymore. See below.

[...]

>> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
>> index 9bf97a366029..a04c5886a5ce 100644
>> --- a/drivers/cpufreq/cpufreq.c
>> +++ b/drivers/cpufreq/cpufreq.c
>> @@ -347,6 +347,28 @@ static void __cpufreq_notify_transition(struct cpufreq_policy *policy,
>>  	}
>>  }
>>  
>> +/*********************************************************************
>> + *           FREQUENCY INVARIANT CPU CAPACITY SUPPORT                *
>> + *********************************************************************/
>> +
>> +#ifndef arch_set_freq_scale
>> +static void arch_set_freq_scale(struct cpumask *cpus, unsigned long cur_freq,
>> +				unsigned long max_freq)
>> +{}
>> +#endif
>> +
>> +static void cpufreq_set_freq_scale(struct cpufreq_policy *policy,
>> +				   struct cpufreq_freqs *freqs)
>> +{
>> +	unsigned long cur_freq = freqs ? freqs->new : policy->cur;
>> +	unsigned long max_freq = policy->cpuinfo.max_freq;
>> +
>> +	pr_debug("cpus %*pbl cur/cur max freq %lu/%lu kHz\n",
>> +		 cpumask_pr_args(policy->related_cpus), cur_freq, max_freq);
>> +
>> +	arch_set_freq_scale(policy->related_cpus, cur_freq, max_freq);
> 
> I am not sure why all these are required to be sent here and will come back to
> it later on after going through other patches.

See below.

>> +}
>> +
>>  /**
>>   * cpufreq_notify_transition - call notifier chain and adjust_jiffies
>>   * on frequency transition.
>> @@ -405,6 +427,8 @@ void cpufreq_freq_transition_begin(struct cpufreq_policy *policy,
>>  
>>  	spin_unlock(&policy->transition_lock);
>>  
>> +	cpufreq_set_freq_scale(policy, freqs);
>> +
> 
> Why do this before even changing the frequency ? We may fail while changing it.
> 
> IMHO, you should call this routine whenever we update policy->cur and that
> happens regularly in __cpufreq_notify_transition() and few other places..

See below.
 
>>  	cpufreq_notify_transition(policy, freqs, CPUFREQ_PRECHANGE);
>>  }
>>  EXPORT_SYMBOL_GPL(cpufreq_freq_transition_begin);
>> @@ -2203,6 +2227,8 @@ static int cpufreq_set_policy(struct cpufreq_policy *policy,
>>  	blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
>>  			CPUFREQ_NOTIFY, new_policy);
>>  
>> +	cpufreq_set_freq_scale(new_policy, NULL);
> 
> Why added it here ? To get it initialized ? If yes, then we should do that in
> cpufreq_online() where we first initialize policy->cur.

I agree. This can go away. Initialization is not really needed here. We initialize
the scale values to SCHED_CAPACITY_SCALE at boot-time. 

> Apart from this, you also need to update this in the schedutil governor (if you
> haven't done that in this series later) as that also updates policy->cur in the
> fast path.

So what about I call arch_set_freq_scale() in __cpufreq_notify_transition() in the
CPUFREQ_POSTCHANGE case for slow-switching and in cpufreq_driver_fast_switch() for
fast-switching?

Like this: (only slow-path slightly tested):

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 9bf97a366029..77c4d5e7a598 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -301,6 +301,12 @@ static void adjust_jiffies(unsigned long val, struct cpufreq_freqs *ci)
 #endif
 }
 
+#ifndef arch_set_freq_scale
+static void arch_set_freq_scale(struct cpumask *cpus, unsigned long cur_freq,
+                               unsigned long max_freq)
+{}
+#endif
+
 static void __cpufreq_notify_transition(struct cpufreq_policy *policy,
                struct cpufreq_freqs *freqs, unsigned int state)
 {
@@ -343,6 +349,8 @@ static void __cpufreq_notify_transition(struct cpufreq_policy *policy,
                                CPUFREQ_POSTCHANGE, freqs);
                if (likely(policy) && likely(policy->cpu == freqs->cpu))
                        policy->cur = freqs->new;
+               arch_set_freq_scale(policy->related_cpus, policy->cur,
+                                   policy->cpuinfo.max_freq);
                break;
        }
 }
@@ -1824,9 +1832,18 @@ EXPORT_SYMBOL(cpufreq_unregister_notifier);
 unsigned int cpufreq_driver_fast_switch(struct cpufreq_policy *policy,
                                        unsigned int target_freq)
 {
+       unsigned int ret;
+
        target_freq = clamp_val(target_freq, policy->min, policy->max);
 
-       return cpufreq_driver->fast_switch(policy, target_freq);
+       ret = cpufreq_driver->fast_switch(policy, target_freq);
+
+       if (ret != CPUFREQ_ENTRY_INVALID) {
+               arch_set_freq_scale(policy->related_cpus, target_freq,
+                                   policy->cpuinfo.max_freq);
+       }
+
+       return ret;
 }
 EXPORT_SYMBOL_GPL(cpufreq_driver_fast_switch);

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

#1683283 — Re: [PATCH v2 02/10] cpufreq: provide data for frequency-invariant load-tracking support

On Fri, Jul 7, 2017 at 6:01 PM, Dietmar Eggemann
<dietmar.eggemann@arm.com> wrote:
> On 06/07/17 11:40, Viresh Kumar wrote:
>> On 06-07-17, 10:49, Dietmar Eggemann wrote:
>
> [...]
>
>>> In case arch_set_freq_scale() is not defined (and because of the
>>> pr_debug() drivers/cpufreq/cpufreq.c is not compiled with -DDEBUG)
>>
>> The line within () needs to be improved to convey a clear message.
>
> Probably not needed anymore. See below.
>
> [...]
>
>>> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
>>> index 9bf97a366029..a04c5886a5ce 100644
>>> --- a/drivers/cpufreq/cpufreq.c
>>> +++ b/drivers/cpufreq/cpufreq.c
>>> @@ -347,6 +347,28 @@ static void __cpufreq_notify_transition(struct cpufreq_policy *policy,
>>>      }
>>>  }
>>>
>>> +/*********************************************************************
>>> + *           FREQUENCY INVARIANT CPU CAPACITY SUPPORT                *
>>> + *********************************************************************/
>>> +
>>> +#ifndef arch_set_freq_scale
>>> +static void arch_set_freq_scale(struct cpumask *cpus, unsigned long cur_freq,
>>> +                            unsigned long max_freq)
>>> +{}
>>> +#endif
>>> +
>>> +static void cpufreq_set_freq_scale(struct cpufreq_policy *policy,
>>> +                               struct cpufreq_freqs *freqs)
>>> +{
>>> +    unsigned long cur_freq = freqs ? freqs->new : policy->cur;
>>> +    unsigned long max_freq = policy->cpuinfo.max_freq;
>>> +
>>> +    pr_debug("cpus %*pbl cur/cur max freq %lu/%lu kHz\n",
>>> +             cpumask_pr_args(policy->related_cpus), cur_freq, max_freq);
>>> +
>>> +    arch_set_freq_scale(policy->related_cpus, cur_freq, max_freq);
>>
>> I am not sure why all these are required to be sent here and will come back to
>> it later on after going through other patches.
>
> See below.
>
>>> +}
>>> +
>>>  /**
>>>   * cpufreq_notify_transition - call notifier chain and adjust_jiffies
>>>   * on frequency transition.
>>> @@ -405,6 +427,8 @@ void cpufreq_freq_transition_begin(struct cpufreq_policy *policy,
>>>
>>>      spin_unlock(&policy->transition_lock);
>>>
>>> +    cpufreq_set_freq_scale(policy, freqs);
>>> +
>>
>> Why do this before even changing the frequency ? We may fail while changing it.
>>
>> IMHO, you should call this routine whenever we update policy->cur and that
>> happens regularly in __cpufreq_notify_transition() and few other places..
>
> See below.
>
>>>      cpufreq_notify_transition(policy, freqs, CPUFREQ_PRECHANGE);
>>>  }
>>>  EXPORT_SYMBOL_GPL(cpufreq_freq_transition_begin);
>>> @@ -2203,6 +2227,8 @@ static int cpufreq_set_policy(struct cpufreq_policy *policy,
>>>      blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
>>>                      CPUFREQ_NOTIFY, new_policy);
>>>
>>> +    cpufreq_set_freq_scale(new_policy, NULL);
>>
>> Why added it here ? To get it initialized ? If yes, then we should do that in
>> cpufreq_online() where we first initialize policy->cur.
>
> I agree. This can go away. Initialization is not really needed here. We initialize
> the scale values to SCHED_CAPACITY_SCALE at boot-time.
>
>> Apart from this, you also need to update this in the schedutil governor (if you
>> haven't done that in this series later) as that also updates policy->cur in the
>> fast path.
>
> So what about I call arch_set_freq_scale() in __cpufreq_notify_transition() in the
> CPUFREQ_POSTCHANGE case for slow-switching and in cpufreq_driver_fast_switch() for
> fast-switching?

Why don't you do this in drivers instead of in the core?

Ultimately, the driver knows what frequency it has requested, so why
can't it call arch_set_freq_scale()?

Thanks,
Rafael

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

#1683318 — Re: [PATCH v2 02/10] cpufreq: provide data for frequency-invariant load-tracking support

On 07/07/17 17:18, Rafael J. Wysocki wrote:
> On Fri, Jul 7, 2017 at 6:01 PM, Dietmar Eggemann
> <dietmar.eggemann@arm.com> wrote:
>> On 06/07/17 11:40, Viresh Kumar wrote:
>>> On 06-07-17, 10:49, Dietmar Eggemann wrote:

[...]

>> So what about I call arch_set_freq_scale() in __cpufreq_notify_transition() in the
>> CPUFREQ_POSTCHANGE case for slow-switching and in cpufreq_driver_fast_switch() for
>> fast-switching?
> 
> Why don't you do this in drivers instead of in the core?
> 
> Ultimately, the driver knows what frequency it has requested, so why
> can't it call arch_set_freq_scale()?

That's correct but for arm/arm64 we have a lot of different cpufreq
drivers to deal with. And doing this call to arch_set_freq_scale() once
in the cpufreq core will cover them all.

[...]

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

#1683594 — Re: [PATCH v2 02/10] cpufreq: provide data for frequency-invariant load-tracking support

On Friday, July 07, 2017 06:06:30 PM Dietmar Eggemann wrote:
> On 07/07/17 17:18, Rafael J. Wysocki wrote:
> > On Fri, Jul 7, 2017 at 6:01 PM, Dietmar Eggemann
> > <dietmar.eggemann@arm.com> wrote:
> >> On 06/07/17 11:40, Viresh Kumar wrote:
> >>> On 06-07-17, 10:49, Dietmar Eggemann wrote:
> 
> [...]
> 
> >> So what about I call arch_set_freq_scale() in __cpufreq_notify_transition() in the
> >> CPUFREQ_POSTCHANGE case for slow-switching and in cpufreq_driver_fast_switch() for
> >> fast-switching?
> > 
> > Why don't you do this in drivers instead of in the core?
> > 
> > Ultimately, the driver knows what frequency it has requested, so why
> > can't it call arch_set_freq_scale()?
> 
> That's correct but for arm/arm64 we have a lot of different cpufreq
> drivers to deal with. And doing this call to arch_set_freq_scale() once
> in the cpufreq core will cover them all.
> 
> [...]

I'm sort of wondering how many is "a lot" really.  For instance, do you really
want all of the existing ARM platforms to use the new stuff even though
it may regress things there in principle?

Anyway, if everyone agrees that doing it in the core is the way to go (Peter?),
why don't you introduce a __weak function for setting policy->cur and
override it from your arch so as to call arch_set_freq_scale() from there?

Thanks,
Rafael

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

#1683993 — Re: [PATCH v2 02/10] cpufreq: provide data for frequency-invariant load-tracking support

On 08-07-17, 14:09, Rafael J. Wysocki wrote:
> I'm sort of wondering how many is "a lot" really.  For instance, do you really
> want all of the existing ARM platforms to use the new stuff even though
> it may regress things there in principle?

That's a valid question and we must (maybe we already have) have a policy for
such changes. I thought that such changes (which are so closely bound to the
scheduler) must be at least done at the architecture level and not really at
platform level. And so doing it widely (like done in this patch) maybe the right
thing to do.

> Anyway, if everyone agrees that doing it in the core is the way to go (Peter?),
> why don't you introduce a __weak function for setting policy->cur and
> override it from your arch so as to call arch_set_freq_scale() from there?

I agree. I wanted to suggest that earlier but somehow forgot to mention this.

-- 
viresh

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

#1684210 — Re: [PATCH v2 02/10] cpufreq: provide data for frequency-invariant load-tracking support

On Monday, July 10, 2017 12:24:43 PM Viresh Kumar wrote:
> On 08-07-17, 14:09, Rafael J. Wysocki wrote:
> > I'm sort of wondering how many is "a lot" really.  For instance, do you really
> > want all of the existing ARM platforms to use the new stuff even though
> > it may regress things there in principle?
> 
> That's a valid question and we must (maybe we already have) have a policy for
> such changes.

I don't think it is a matter of policy, as it tends to vary from case to case.

What really matters is the reason to make the changes in this particular case.

If the reason if to help new systems to work better in the first place and the
old ones are affected just by the way, it may be better to avoid affecting them.

On the other hand, if the reason is to improve things for all the new and old
systems altogether, then sure let's do it this way.

> I thought that such changes (which are so closely bound to the
> scheduler) must be at least done at the architecture level and not really at
> platform level. And so doing it widely (like done in this patch) maybe the right
> thing to do.

This particular change is about a new feature, so making it in the core is OK
in two cases IMO: (a) when you actively want everyone to be affected by it and
(b) when the effect of it on the old systems should not be noticeable.

Thanks,
Rafael

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

#1684807 — Re: [PATCH v2 02/10] cpufreq: provide data for frequency-invariant load-tracking support

On 10-07-17, 14:46, Rafael J. Wysocki wrote:
> This particular change is about a new feature, so making it in the core is OK
> in two cases IMO: (a) when you actively want everyone to be affected by it and

IMO this change should be done for the whole ARM architecture. And if some
regression happens due to this, then we come back and solve it.

> (b) when the effect of it on the old systems should not be noticeable.

I am not sure about the effects of this on performance really.

@Dietmar: Any inputs for that ?

-- 
viresh

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

#1685134 — Re: [PATCH v2 02/10] cpufreq: provide data for frequency-invariant load-tracking support

On 11/07/17 07:39, Viresh Kumar wrote:
> On 10-07-17, 14:46, Rafael J. Wysocki wrote:
>> This particular change is about a new feature, so making it in the core is OK
>> in two cases IMO: (a) when you actively want everyone to be affected by it and
> 
> IMO this change should be done for the whole ARM architecture. And if some
> regression happens due to this, then we come back and solve it.
> 
>> (b) when the effect of it on the old systems should not be noticeable.
> 
> I am not sure about the effects of this on performance really.
> 
> @Dietmar: Any inputs for that ?

Like I said in the other email, since for (future)
arm/arm64 fast-switch driver, the return value of
cpufreq_driver->fast_switch() does not give us the information that the
frequency value did actually change, we have to implement
arch_set_freq_scale() in the driver.
This means that we probably only implement this in the subset of drivers
which will be used in platforms on which we want to have
frequency-invariant load-tracking.

A future aperf/mperf like counter FIE solution can give us arch-wide
support when those counters are available.

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

#1686513 — Re: [PATCH v2 02/10] cpufreq: provide data for frequency-invariant load-tracking support

On 11/07/17 16:21, Dietmar Eggemann wrote:
> On 11/07/17 07:39, Viresh Kumar wrote:
>> On 10-07-17, 14:46, Rafael J. Wysocki wrote:
>>> This particular change is about a new feature, so making it in the core is OK
>>> in two cases IMO: (a) when you actively want everyone to be affected by it and
>>
>> IMO this change should be done for the whole ARM architecture. And if some
>> regression happens due to this, then we come back and solve it.
>>
>>> (b) when the effect of it on the old systems should not be noticeable.
>>
>> I am not sure about the effects of this on performance really.
>>
>> @Dietmar: Any inputs for that ?
> 
> Like I said in the other email, since for (future)
> arm/arm64 fast-switch driver, the return value of
> cpufreq_driver->fast_switch() does not give us the information that the
> frequency value did actually change, we have to implement

I was under the impression that we strictly don't care about that
information when I started exploring the fast_switch with the standard
firmware interface on ARM platforms(until if and when ARM provides an
instruction to achieve that).

If f/w failed to change the frequency, will that be not corrected in the
next sample or instance. I would like to know the impact of absence of
such notifications.

> arch_set_freq_scale() in the driver.
> This means that we probably only implement this in the subset of drivers
> which will be used in platforms on which we want to have
> frequency-invariant load-tracking.
> 
> A future aperf/mperf like counter FIE solution can give us arch-wide
> support when those counters are available.
> 

Agreed.

-- 
Regards,
Sudeep

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

Page 1 of 3  [1] 2 3  Next page →

Back to top | Article view | linux.kernel

csiph-web