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 3 of 3 — ← Prev page 1 2 [3]

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

On 13/07/17 15:42, Peter Zijlstra wrote:
> On Thu, Jul 13, 2017 at 03:04:09PM +0100, Sudeep Holla wrote:
> 
>> The question is whether we *need* to know the completion of frequency
>> transition. What is the impact of absence of it ? I am considering
>> platforms which may take up to a ms or more to do the actual transition
>> in the firmware.
> 
> So on x86 we can recover from not knowing by means of the APERF/MPERF
> thing, which gives us the average effective frequency over the last
> period.
> 

Understood, and I agree we *must* head in that direction.

> If you lack that you need something to update the actual effective
> frequency.
> 
> Changing the effective frequency at request time might confuse things --
> esp. if the request might not be honoured at all or can take a
> significant time to complete. Not to mention that _IF_ you rely on the
> effective frequency to set other clocks things can come unstuck.
> 
> So unless you go the whole distance and do APERF/MPERF like things, I
> think it would be very good to have a notification of completion (and
> possibly a read-back of the effective frequency that is now set).
>

Yes I agree, but sadly/unfortunately the new SCMI specification I keep
referring makes these notification optional. We even have statistics
collected by the firmware to get the effective frequency, but again
optional and may get updated at slower rate than what we would expect as
they are typically running on slower processors. But IMO it's still
worth exploring the feasibility of fast switch on system with such
standard interface.

I completely agree with you on the old system which Linux has to deal
with I2C and other slow paths. I was under the assumption that we had
already eliminated the use of fast switch on such systems. When Dietmar
referred fast switching, I think he was referring to only systems with
std. firmware interface.

-- 
Regards,
Sudeep

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

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

On 12/07/17 10:27, Viresh Kumar wrote:
> On 12-07-17, 10:31, Peter Zijlstra wrote:
>> So the problem with the thread is two-fold; one the one hand we like the
>> scheduler to directly set frequency, but then we need to schedule a task
>> to change the frequency, which will change the frequency and around we
>> go.
>>
>> On the other hand, there's very nasty issues with PI. This thread would
>> have very high priority (otherwise the SCHED_DEADLINE stuff won't work)
>> but that then means this thread needs to boost the owner of the i2c
>> mutex. And that then creates a massive bandwidth accounting hole.
>>
>>
>> The advantage of using an interrupt driven state machine is that all
>> those issues go away.
>>
>> But yes, whichever way around you turn things, its crap. But given the
>> hardware its the best we can do.
> 
> Thanks for the explanation Peter.
> 
> IIUC, it will take more time to change the frequency eventually with
> the interrupt-driven state machine as there may be multiple bottom
> halves involved here, for supply, clk, etc, which would run at normal
> priorities now. And those were boosted currently due to the high
> priority sugov thread. And we are fine with that (from performance
> point of view) ?
> 
> Coming back to where we started from (where should we call
> arch_set_freq_scale() from ?).
> 
> I think we would still need some kind of synchronization between
> cpufreq core and the cpufreq drivers to make sure we don't start
> another freq change before the previous one is complete. Otherwise
> the cpufreq drivers would be required to have similar support with
> proper locking in place.
> 

Good point, but with firmware interface we are considering fro
fast-switch, the firmware can override the previous request if it's not
yet started. So I assume that's fine and expected ?

> And if the core is going to get notified about successful freq changes
> (which it should IMHO),

Is that mandatory for even fast-switching ?

-- 
Regards,
Sudeep

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

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

On 12/07/17 05:09, Viresh Kumar wrote:
> On 11-07-17, 16:06, Dietmar Eggemann wrote:
>> But in the meantime we're convinced that cpufreq_driver_fast_switch() is
>> not the right place to call arch_set_freq_scale() 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.
> 
> Yeah, I saw your discussion with Peter on #linux-rt IRC and TBH I wasn't aware
> that we are going to do fast switching that way. Just trying to get
> understanding of that idea a bit..
> 
> So we will do fast switching from scheduler's point of view, i.e. we wouldn't
> schedule a kthread to change the frequency. But the real hardware still can't do
> that without sleeping, like if we have I2C somewhere in between. AFAIU, we will
> still have some kind of *software* bottom half to do that work, isn't it? And it
> wouldn't be that we have pushed some instructions to the hardware, which it can
> do a bit later.
> 

No the platforms we are considering are only where a standard firmware
interface is provided and the firmware deals with all those I2C/PMIC crap.

> For example, the regulator may be accessed via I2C and we need to program that
> before changing the clock. So, it will be done by some software code only.
> 

Software but just not Linux OSPM but some firmware(remote processors
presumably, can't imagine on the same processor though)

-- 
Regards,
Sudeep

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

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

On 07-07-17, 17:01, Dietmar Eggemann wrote:
> 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);

This function gets called for-each-cpu-in-policy, so you don't need the first
argument. And the topology code already has max_freq, so not sure if you need
the last parameter as well, specially for the ARM solution.

-- 
viresh

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

#1682290 — [PATCH v2 04/10] arm: wire cpufreq input data for frequency-invariant accounting up to the arch

Define arch_set_freq_scale to be the arch_topology "driver" function
topology_set_freq_scale() to let FIE work correctly.

Cc: Russell King <linux@arm.linux.org.uk>
Cc: Juri Lelli <juri.lelli@arm.com>
Signed-off-by: Dietmar Eggemann <dietmar.eggemann@arm.com>
---
 arch/arm/include/asm/topology.h | 5 +++++
 arch/arm/kernel/topology.c      | 1 -
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/arm/include/asm/topology.h b/arch/arm/include/asm/topology.h
index 370f7a732900..ca05d1b90411 100644
--- a/arch/arm/include/asm/topology.h
+++ b/arch/arm/include/asm/topology.h
@@ -24,6 +24,11 @@ void init_cpu_topology(void);
 void store_cpu_topology(unsigned int cpuid);
 const struct cpumask *cpu_coregroup_mask(int cpu);
 
+#include <linux/arch_topology.h>
+
+/* Subscribe for input data for frequency-invariant load-tracking */
+#define arch_set_freq_scale topology_set_freq_scale
+
 #else
 
 static inline void init_cpu_topology(void) { }
diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
index bf949a763dbe..2c47a76c67b0 100644
--- a/arch/arm/kernel/topology.c
+++ b/arch/arm/kernel/topology.c
@@ -11,7 +11,6 @@
  * for more details.
  */
 
-#include <linux/arch_topology.h>
 #include <linux/cpu.h>
 #include <linux/cpufreq.h>
 #include <linux/cpumask.h>
-- 
2.11.0

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

#1682342 — Re: [PATCH v2 04/10] arm: wire cpufreq input data for frequency-invariant accounting up to the arch

On 06-07-17, 10:49, Dietmar Eggemann wrote:
> Define arch_set_freq_scale to be the arch_topology "driver" function
> topology_set_freq_scale() to let FIE work correctly.
> 
> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: Juri Lelli <juri.lelli@arm.com>
> Signed-off-by: Dietmar Eggemann <dietmar.eggemann@arm.com>
> ---
>  arch/arm/include/asm/topology.h | 5 +++++
>  arch/arm/kernel/topology.c      | 1 -
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/include/asm/topology.h b/arch/arm/include/asm/topology.h
> index 370f7a732900..ca05d1b90411 100644
> --- a/arch/arm/include/asm/topology.h
> +++ b/arch/arm/include/asm/topology.h
> @@ -24,6 +24,11 @@ void init_cpu_topology(void);
>  void store_cpu_topology(unsigned int cpuid);
>  const struct cpumask *cpu_coregroup_mask(int cpu);
>  
> +#include <linux/arch_topology.h>
> +
> +/* Subscribe for input data for frequency-invariant load-tracking */
> +#define arch_set_freq_scale topology_set_freq_scale
> +
>  #else
>  
>  static inline void init_cpu_topology(void) { }
> diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
> index bf949a763dbe..2c47a76c67b0 100644
> --- a/arch/arm/kernel/topology.c
> +++ b/arch/arm/kernel/topology.c
> @@ -11,7 +11,6 @@
>   * for more details.
>   */
>  
> -#include <linux/arch_topology.h>

Why is this diff part of this patch ?

-- 
viresh

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

#1684344 — Re: [PATCH v2 04/10] arm: wire cpufreq input data for frequency-invariant accounting up to the arch

On 06/07/17 11:42, Viresh Kumar wrote:
> On 06-07-17, 10:49, Dietmar Eggemann wrote:
>> Define arch_set_freq_scale to be the arch_topology "driver" function
>> topology_set_freq_scale() to let FIE work correctly.
>>
>> Cc: Russell King <linux@arm.linux.org.uk>
>> Cc: Juri Lelli <juri.lelli@arm.com>
>> Signed-off-by: Dietmar Eggemann <dietmar.eggemann@arm.com>
>> ---
>>  arch/arm/include/asm/topology.h | 5 +++++
>>  arch/arm/kernel/topology.c      | 1 -
>>  2 files changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/include/asm/topology.h b/arch/arm/include/asm/topology.h
>> index 370f7a732900..ca05d1b90411 100644
>> --- a/arch/arm/include/asm/topology.h
>> +++ b/arch/arm/include/asm/topology.h
>> @@ -24,6 +24,11 @@ void init_cpu_topology(void);
>>  void store_cpu_topology(unsigned int cpuid);
>>  const struct cpumask *cpu_coregroup_mask(int cpu);
>>  
>> +#include <linux/arch_topology.h>
>> +
>> +/* Subscribe for input data for frequency-invariant load-tracking */
>> +#define arch_set_freq_scale topology_set_freq_scale
>> +
>>  #else
>>  
>>  static inline void init_cpu_topology(void) { }
>> diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
>> index bf949a763dbe..2c47a76c67b0 100644
>> --- a/arch/arm/kernel/topology.c
>> +++ b/arch/arm/kernel/topology.c
>> @@ -11,7 +11,6 @@
>>   * for more details.
>>   */
>>  
>> -#include <linux/arch_topology.h>
> 
> Why is this diff part of this patch ?

Since 'arch/$ARCH/include/asm/topology.h' now includes
'include/linux/arch_topology.h' and 'arch/$ARCH/kernel/topology.c'
already includes 'arch/$ARCH/include/asm/topology.h' I thought it's a
good idea to get rid of this include here.

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

#1684801 — Re: [PATCH v2 04/10] arm: wire cpufreq input data for frequency-invariant accounting up to the arch

On 10-07-17, 16:13, Dietmar Eggemann wrote:
> Since 'arch/$ARCH/include/asm/topology.h' now includes
> 'include/linux/arch_topology.h' and 'arch/$ARCH/kernel/topology.c'
> already includes 'arch/$ARCH/include/asm/topology.h' I thought it's a
> good idea to get rid of this include here.

Ahh, makes sense.

-- 
viresh

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

#1682291 — [PATCH v2 09/10] arm64: wire cpu-invariant accounting support up to the task scheduler

Commit 8cd5601c5060 ("sched/fair: Convert arch_scale_cpu_capacity() from
weak function to #define") changed the wiring which now has to be done
by associating arch_scale_cpu_capacity with the actual implementation
provided by the architecture.

Define arch_scale_cpu_capacity to use the arch_topology "driver"
function topology_get_cpu_scale() for the task scheduler's cpu-invariant
accounting instead of the default arch_scale_cpu_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 4fd598b432e6..0dc81860cc0d 100644
--- a/arch/arm64/include/asm/topology.h
+++ b/arch/arm64/include/asm/topology.h
@@ -40,6 +40,9 @@ int pcibus_to_node(struct pci_bus *bus);
 /* Replace task scheduler's default frequency-invariant accounting */
 #define arch_scale_freq_capacity topology_get_freq_scale
 
+/* Replace task scheduler's default cpu-invariant accounting */
+#define arch_scale_cpu_capacity topology_get_cpu_scale
+
 #include <asm-generic/topology.h>
 
 #endif /* _ASM_ARM_TOPOLOGY_H */
-- 
2.11.0

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

#1682339 — Re: [PATCH v2 09/10] arm64: wire cpu-invariant accounting support up to the task scheduler

On 06-07-17, 10:49, Dietmar Eggemann wrote:
> Commit 8cd5601c5060 ("sched/fair: Convert arch_scale_cpu_capacity() from
> weak function to #define") changed the wiring which now has to be done
> by associating arch_scale_cpu_capacity with the actual implementation
> provided by the architecture.
> 
> Define arch_scale_cpu_capacity to use the arch_topology "driver"
> function topology_get_cpu_scale() for the task scheduler's cpu-invariant
> accounting instead of the default arch_scale_cpu_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 4fd598b432e6..0dc81860cc0d 100644
> --- a/arch/arm64/include/asm/topology.h
> +++ b/arch/arm64/include/asm/topology.h
> @@ -40,6 +40,9 @@ int pcibus_to_node(struct pci_bus *bus);
>  /* Replace task scheduler's default frequency-invariant accounting */
>  #define arch_scale_freq_capacity topology_get_freq_scale
>  
> +/* Replace task scheduler's default cpu-invariant accounting */
> +#define arch_scale_cpu_capacity topology_get_cpu_scale
> +
>  #include <asm-generic/topology.h>
>  
>  #endif /* _ASM_ARM_TOPOLOGY_H */

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

-- 
viresh

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

#1682296 — [PATCH v2 06/10] arm: wire cpu-invariant accounting support up to the task scheduler

Commit 8cd5601c5060 ("sched/fair: Convert arch_scale_cpu_capacity() from
weak function to #define") changed the wiring which now has to be done
by associating arch_scale_cpu_capacity with the actual implementation
provided by the architecture.

Define arch_scale_cpu_capacity to use the arch_topology "driver"
function topology_get_cpu_scale() for the task scheduler's cpu-invariant
accounting instead of the default arch_scale_cpu_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 57aebfa03e24..26d491c65955 100644
--- a/arch/arm/include/asm/topology.h
+++ b/arch/arm/include/asm/topology.h
@@ -32,6 +32,9 @@ const struct cpumask *cpu_coregroup_mask(int cpu);
 /* Replace task scheduler's default frequency-invariant accounting */
 #define arch_scale_freq_capacity topology_get_freq_scale
 
+/* Replace task scheduler's default cpu-invariant accounting */
+#define arch_scale_cpu_capacity topology_get_cpu_scale
+
 #else
 
 static inline void init_cpu_topology(void) { }
-- 
2.11.0

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

#1682341 — Re: [PATCH v2 06/10] arm: wire cpu-invariant accounting support up to the task scheduler

On 06-07-17, 10:49, Dietmar Eggemann wrote:
> Commit 8cd5601c5060 ("sched/fair: Convert arch_scale_cpu_capacity() from
> weak function to #define") changed the wiring which now has to be done
> by associating arch_scale_cpu_capacity with the actual implementation
> provided by the architecture.
> 
> Define arch_scale_cpu_capacity to use the arch_topology "driver"
> function topology_get_cpu_scale() for the task scheduler's cpu-invariant
> accounting instead of the default arch_scale_cpu_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 57aebfa03e24..26d491c65955 100644
> --- a/arch/arm/include/asm/topology.h
> +++ b/arch/arm/include/asm/topology.h
> @@ -32,6 +32,9 @@ const struct cpumask *cpu_coregroup_mask(int cpu);
>  /* Replace task scheduler's default frequency-invariant accounting */
>  #define arch_scale_freq_capacity topology_get_freq_scale
>  
> +/* Replace task scheduler's default cpu-invariant accounting */
> +#define arch_scale_cpu_capacity topology_get_cpu_scale
> +
>  #else
>  
>  static inline void init_cpu_topology(void) { }

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

-- 
viresh

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

#1682297 — [PATCH v2 01/10] drivers base/arch_topology: free cpumask cpus_to_visit

Free cpumask cpus_to_visit in case registering
init_cpu_capacity_notifier has failed or the parsing of the cpu
capacity-dmips-mhz property is done. The cpumask cpus_to_visit is
only used inside the notifier call init_cpu_capacity_callback.

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Juri Lelli <juri.lelli@arm.com>
Reported-by: Vincent Guittot <vincent.guittot@linaro.org>
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>
---
 drivers/base/arch_topology.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
index d1c33a85059e..f4832c662762 100644
--- a/drivers/base/arch_topology.c
+++ b/drivers/base/arch_topology.c
@@ -206,6 +206,8 @@ static struct notifier_block init_cpu_capacity_notifier = {
 
 static int __init register_cpufreq_notifier(void)
 {
+	int ret;
+
 	/*
 	 * on ACPI-based systems we need to use the default cpu capacity
 	 * until we have the necessary code to parse the cpu capacity, so
@@ -221,13 +223,19 @@ static int __init register_cpufreq_notifier(void)
 
 	cpumask_copy(cpus_to_visit, cpu_possible_mask);
 
-	return cpufreq_register_notifier(&init_cpu_capacity_notifier,
-					 CPUFREQ_POLICY_NOTIFIER);
+	ret = cpufreq_register_notifier(&init_cpu_capacity_notifier,
+					CPUFREQ_POLICY_NOTIFIER);
+
+	if (ret)
+		free_cpumask_var(cpus_to_visit);
+
+	return ret;
 }
 core_initcall(register_cpufreq_notifier);
 
 static void parsing_done_workfn(struct work_struct *work)
 {
+	free_cpumask_var(cpus_to_visit);
 	cpufreq_unregister_notifier(&init_cpu_capacity_notifier,
 					 CPUFREQ_POLICY_NOTIFIER);
 }
-- 
2.11.0

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

#1682318 — Re: [PATCH v2 01/10] drivers base/arch_topology: free cpumask cpus_to_visit

On 06-07-17, 10:49, Dietmar Eggemann wrote:
> Free cpumask cpus_to_visit in case registering
> init_cpu_capacity_notifier has failed or the parsing of the cpu
> capacity-dmips-mhz property is done. The cpumask cpus_to_visit is
> only used inside the notifier call init_cpu_capacity_callback.
> 
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Juri Lelli <juri.lelli@arm.com>
> Reported-by: Vincent Guittot <vincent.guittot@linaro.org>
> 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>
> ---
>  drivers/base/arch_topology.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
> index d1c33a85059e..f4832c662762 100644
> --- a/drivers/base/arch_topology.c
> +++ b/drivers/base/arch_topology.c
> @@ -206,6 +206,8 @@ static struct notifier_block init_cpu_capacity_notifier = {
>  
>  static int __init register_cpufreq_notifier(void)
>  {
> +	int ret;
> +
>  	/*
>  	 * on ACPI-based systems we need to use the default cpu capacity
>  	 * until we have the necessary code to parse the cpu capacity, so
> @@ -221,13 +223,19 @@ static int __init register_cpufreq_notifier(void)
>  
>  	cpumask_copy(cpus_to_visit, cpu_possible_mask);
>  
> -	return cpufreq_register_notifier(&init_cpu_capacity_notifier,
> -					 CPUFREQ_POLICY_NOTIFIER);
> +	ret = cpufreq_register_notifier(&init_cpu_capacity_notifier,
> +					CPUFREQ_POLICY_NOTIFIER);
> +
> +	if (ret)
> +		free_cpumask_var(cpus_to_visit);
> +
> +	return ret;
>  }
>  core_initcall(register_cpufreq_notifier);
>  
>  static void parsing_done_workfn(struct work_struct *work)
>  {
> +	free_cpumask_var(cpus_to_visit);
>  	cpufreq_unregister_notifier(&init_cpu_capacity_notifier,
>  					 CPUFREQ_POLICY_NOTIFIER);

As a general rule (and good coding practice), it is better to free resources
only after the users are gone. And so we should have changed the order here.
i.e. Unregister the notifier first and then free the cpumask.

And because of that we may end up crashing the kernel here.

Here is an example:

Consider that init_cpu_capacity_callback() is getting called concurrently on big
and LITTLE CPUs.


CPU0 (big)                            CPU4 (LITTLE)

                                      if (cap_parsing_failed || cap_parsing_done)
                                          return 0;

cap_parsing_done = true;
schedule_work(&parsing_done_work);

parsing_done_workfn(work)
  -> free_cpumask_var(cpus_to_visit);
  -> cpufreq_unregister_notifier()


                                      switch (val) {
                                          ...
                                          /* Touch cpus_to_visit and crash */


My assumption here is that the same notifier head can get called in parallel on
two CPUs as all I see there is a down_read() in __blocking_notifier_call_chain()
which shouldn't block parallel calls.

Maybe I am wrong :(

-- 
viresh

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

#1682354 — Re: [PATCH v2 01/10] drivers base/arch_topology: free cpumask cpus_to_visit

Hi Viresh,

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

[...]

> >  static void parsing_done_workfn(struct work_struct *work)
> >  {
> > +	free_cpumask_var(cpus_to_visit);
> >  	cpufreq_unregister_notifier(&init_cpu_capacity_notifier,
> >  					 CPUFREQ_POLICY_NOTIFIER);
> 
> As a general rule (and good coding practice), it is better to free resources
> only after the users are gone. And so we should have changed the order here.
> i.e. Unregister the notifier first and then free the cpumask.
> 
> And because of that we may end up crashing the kernel here.
> 
> Here is an example:
> 
> Consider that init_cpu_capacity_callback() is getting called concurrently on big
> and LITTLE CPUs.
> 
> 
> CPU0 (big)                            CPU4 (LITTLE)
> 
>                                       if (cap_parsing_failed || cap_parsing_done)
>                                           return 0;
> 

But, in this case the policy notifier for LITTLE cluster has not been
executed yet, so the domain's CPUs have not yet been cleared out from
cpus_to_visit. CPU0 won't see the mask as empty then, right?

> cap_parsing_done = true;
> schedule_work(&parsing_done_work);
> 
> parsing_done_workfn(work)
>   -> free_cpumask_var(cpus_to_visit);
>   -> cpufreq_unregister_notifier()
> 
> 
>                                       switch (val) {
>                                           ...
>                                           /* Touch cpus_to_visit and crash */
> 
> 
> My assumption here is that the same notifier head can get called in parallel on
> two CPUs as all I see there is a down_read() in __blocking_notifier_call_chain()
> which shouldn't block parallel calls.
> 

If that's the case I'm wondering however if we need explicit
synchronization though. Otherwise both threads can read the mask as
full, clear only their bits and not schedule the workfn?

But, can the policies be concurrently initialized? Or is the
initialization process serialized or the different domains?

Thanks,

- Juri

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

#1682361 — Re: [PATCH v2 01/10] drivers base/arch_topology: free cpumask cpus_to_visit

On 06-07-17, 11:59, Juri Lelli wrote:
> On 06/07/17 15:52, Viresh Kumar wrote:

> > CPU0 (big)                            CPU4 (LITTLE)
> > 
> >                                       if (cap_parsing_failed || cap_parsing_done)
> >                                           return 0;
> > 
> 
> But, in this case the policy notifier for LITTLE cluster has not been
> executed yet,

Not necessarily. The cpufreq notifier with CPUFREQ_NOTIFY event can get called
again and again (as soon as the policy is changed, for example min/max changed
from sysfs). And so it is possible that the LITTLE cpus are already cleared from
the mask.

> so the domain's CPUs have not yet been cleared out from
> cpus_to_visit. CPU0 won't see the mask as empty then, right?

And so it can.

> > cap_parsing_done = true;
> > schedule_work(&parsing_done_work);
> > 
> > parsing_done_workfn(work)
> >   -> free_cpumask_var(cpus_to_visit);
> >   -> cpufreq_unregister_notifier()
> > 
> > 
> >                                       switch (val) {
> >                                           ...
> >                                           /* Touch cpus_to_visit and crash */
> > 
> > 
> > My assumption here is that the same notifier head can get called in parallel on
> > two CPUs as all I see there is a down_read() in __blocking_notifier_call_chain()
> > which shouldn't block parallel calls.
> > 
> 
> If that's the case I'm wondering however if we need explicit
> synchronization though. Otherwise both threads can read the mask as
> full, clear only their bits and not schedule the workfn?

Maybe not as the policies are created one by one only, not concurrently.

> But, can the policies be concurrently initialized? Or is the
> initialization process serialized or the different domains?

There can be complex cases here. For example consider this.

Only the little CPUs are brought online at boot. Their policy is set and they
are cleared from the cpus_to_visit mask. Now we try to bring any big CPU online
and at the same time try changing min/max from sysfs for the LITTLE CPU policy.

The notifier may get called concurrently here I believe and cause the problem I
mentioned earlier.

-- 
viresh

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

#1683269 — Re: [PATCH v2 01/10] drivers base/arch_topology: free cpumask cpus_to_visit

On 06/07/17 12:15, Viresh Kumar wrote:
> On 06-07-17, 11:59, Juri Lelli wrote:
>> On 06/07/17 15:52, Viresh Kumar wrote:

[...]

>>
>> If that's the case I'm wondering however if we need explicit
>> synchronization though. Otherwise both threads can read the mask as
>> full, clear only their bits and not schedule the workfn?
> 
> Maybe not as the policies are created one by one only, not concurrently.
> 
>> But, can the policies be concurrently initialized? Or is the
>> initialization process serialized or the different domains?
> 
> There can be complex cases here. For example consider this.
> 
> Only the little CPUs are brought online at boot. Their policy is set and they
> are cleared from the cpus_to_visit mask. Now we try to bring any big CPU online
> and at the same time try changing min/max from sysfs for the LITTLE CPU policy.
> 
> The notifier may get called concurrently here I believe and cause the problem I
> mentioned earlier.
> 

I chatted with Juri and your proposed fix to do the unregister before
the free makes sense to us. Thanks for spotting this! I will change in
the next version.

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

#1682298 — [PATCH v2 10/10] drivers base/arch_topology: inline cpu- and frequency-invariant accounting

To speed up the cpu- and frequency-invariant accounting of the task
scheduler make sure that the CIE (topology_get_cpu_scale()) and FIE
(topology_get_freq_scale() get completely inlined into the task
scheduler consumer functions (e.g. __update_load_avg_se()).

This patch-set changes the interface for CIE and FIE from:

drivers/base/arch_topology.c:

 static DEFINE_PER_CPU(unsigned long, item);

 unsigned long topology_get_item_scale(...)
 {
 	return per_cpu(item, cpu)
 }

include/linux/arch_topology.h:

 unsigned long topology_get_item_scale(...);

to:

drivers/base/arch_topology.c:

 DEFINE_PER_CPU(unsigned long, item);

include/linux/arch_topology.h:

 DECLARE_PER_CPU(unsigned long, item);

 static inline
 unsigned long topology_get_item_scale(...)
 {
 	return per_cpu(item, cpu)
 }

An uplift in performance could be detected running the kernel with the
following test patch on top (on JUNO R0 (arm64)):

@@ -2812,10 +2812,18 @@ accumulate_sum(u64 delta, int cpu, struct sched_avg *sa,
	unsigned long scale_freq, scale_cpu;
	u32 contrib = (u32)delta; /* p == 0 -> delta < 1024 */
	u64 periods;
+       u64 t1, t2;
+
+       t1 = sched_clock_cpu(cpu);

	scale_freq = arch_scale_freq_capacity(NULL, cpu);
	scale_cpu = arch_scale_cpu_capacity(NULL, cpu);

+       t2 = sched_clock_cpu(cpu);
+
+       trace_printk("cpu=%d t1=%llu t2=%llu diff=%llu\n",
+                    cpu, t1, t2, t2 - t1);
+
	delta += sa->period_contrib;
	periods = delta / 1024; /* A period is * 1024us * (~1ms) */

The following test results (3 test runs each) have been obtained by
tracing this trace printk (diff=x) for Cortex A-53 (LITTLE) and Cortex
A-57 (big) cpus w/ (inline) and w/o (non-inline) this patch.

         mean  max  min

A-57 inline:

	119.6  300   60
	 96.8  280   60
	110.2  660   60

A-57 non-inline:

	142.8  460   80
	157.6  680   80
	153.4  720   80

A-53 inline:

	141.6  360  100
	118.8  500  100
	148.6  380  100

A-53 non-inline:

	293    840  120
	253.2  840  120
	299.6 1060  140

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  | 14 ++------------
 include/linux/arch_topology.h | 15 +++++++++++++--
 2 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
index 63fb3f945d21..b4481cff14bf 100644
--- a/drivers/base/arch_topology.c
+++ b/drivers/base/arch_topology.c
@@ -22,12 +22,7 @@
 #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);
-}
+DEFINE_PER_CPU(unsigned long, freq_scale) = SCHED_CAPACITY_SCALE;
 
 void topology_set_freq_scale(struct cpumask *cpus, unsigned long cur_freq,
 			     unsigned long max_freq)
@@ -43,12 +38,7 @@ void topology_set_freq_scale(struct cpumask *cpus, unsigned long cur_freq,
 
 
 static DEFINE_MUTEX(cpu_scale_mutex);
-static DEFINE_PER_CPU(unsigned long, cpu_scale) = SCHED_CAPACITY_SCALE;
-
-unsigned long topology_get_cpu_scale(struct sched_domain *sd, int cpu)
-{
-	return per_cpu(cpu_scale, cpu);
-}
+DEFINE_PER_CPU(unsigned long, cpu_scale) = SCHED_CAPACITY_SCALE;
 
 void topology_set_cpu_scale(unsigned int cpu, unsigned long capacity)
 {
diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h
index 168104d2d2cf..361e85a30151 100644
--- a/include/linux/arch_topology.h
+++ b/include/linux/arch_topology.h
@@ -11,12 +11,23 @@ void topology_normalize_cpu_scale(void);
 struct device_node;
 int topology_parse_cpu_capacity(struct device_node *cpu_node, int cpu);
 
+DECLARE_PER_CPU(unsigned long, cpu_scale);
+DECLARE_PER_CPU(unsigned long, freq_scale);
+
 struct sched_domain;
-unsigned long topology_get_cpu_scale(struct sched_domain *sd, int cpu);
+static inline
+unsigned long topology_get_cpu_scale(struct sched_domain *sd, int cpu)
+{
+	return per_cpu(cpu_scale, cpu);
+}
 
 void topology_set_cpu_scale(unsigned int cpu, unsigned long capacity);
 
-unsigned long topology_get_freq_scale(struct sched_domain *sd, int cpu);
+static inline
+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);
-- 
2.11.0

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

#1682352 — Re: [PATCH v2 10/10] drivers base/arch_topology: inline cpu- and frequency-invariant accounting

Sure this patch looks pretty useful, but ...

On 06-07-17, 10:49, Dietmar Eggemann wrote:
> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
> index 63fb3f945d21..b4481cff14bf 100644
> --- a/drivers/base/arch_topology.c
> +++ b/drivers/base/arch_topology.c
> @@ -22,12 +22,7 @@
>  #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);
> -}
> +DEFINE_PER_CPU(unsigned long, freq_scale) = SCHED_CAPACITY_SCALE;

... you just undo what you did earlier in this series, and that is somewhat
discouraged.

What about making this as the first patch of the series and move only the below
part to the header. And then you can add the above part to the right place in
the first attempt itself?

But maybe this is all okay :)

-- 
viresh

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

#1684342 — Re: [PATCH v2 10/10] drivers base/arch_topology: inline cpu- and frequency-invariant accounting

On 06/07/17 11:57, Viresh Kumar wrote:
> Sure this patch looks pretty useful, but ...
> 
> On 06-07-17, 10:49, Dietmar Eggemann wrote:
>> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
>> index 63fb3f945d21..b4481cff14bf 100644
>> --- a/drivers/base/arch_topology.c
>> +++ b/drivers/base/arch_topology.c
>> @@ -22,12 +22,7 @@
>>  #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);
>> -}
>> +DEFINE_PER_CPU(unsigned long, freq_scale) = SCHED_CAPACITY_SCALE;
> 
> ... you just undo what you did earlier in this series, and that is somewhat
> discouraged.
> 
> What about making this as the first patch of the series and move only the below
> part to the header. And then you can add the above part to the right place in
> the first attempt itself?
> 
> But maybe this is all okay :)

I just wanted to show people what we gain in completely inlining FIE and
CIE on ARM64 in the scheduler hot-path. But yes, with the next version I
want to fold this inlining into the actual FIE/CIE patch.

[toc] | [prev] | [standalone]

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

Back to top | Article view | linux.kernel

csiph-web