All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Vincent Guittot <vincent.guittot@linaro.org>
To: Dietmar Eggemann <dietmar.eggemann@arm.com>
Cc: Morten Rasmussen <Morten.Rasmussen@arm.com>,
	Peter Zijlstra <peterz@infradead.org>,
	"mingo@redhat.com" <mingo@redhat.com>,
	Daniel Lezcano <daniel.lezcano@linaro.org>,
	Yuyang Du <yuyang.du@intel.com>,
	Michael Turquette <mturquette@baylibre.com>,
	"rjw@rjwysocki.net" <rjw@rjwysocki.net>,
	Juri Lelli <Juri.Lelli@arm.com>,
	Sai Charan Gurrappadi <sgurrappadi@nvidia.com>,
	"pang.xunlei@zte.com.cn" <pang.xunlei@zte.com.cn>,
	linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 2/6] sched/fair: Convert arch_scale_cpu_capacity() from weak function to #define
Date: Fri, 4 Sep 2015 09:26:42 +0200	[thread overview]
Message-ID: <CAKfTPtBnj9_oHoXEZFnYnsxb5a3KwPcVHYydRVQEzypkCbJakQ@mail.gmail.com> (raw)
In-Reply-To: <55E8A65F.2070903@arm.com>

On 3 September 2015 at 21:58, Dietmar Eggemann <dietmar.eggemann@arm.com> wrote:
> Hi Vincent,
>
> On 02/09/15 10:31, Vincent Guittot wrote:
>> Hi Morten,
>>
>> On 14 August 2015 at 18:23, Morten Rasmussen <morten.rasmussen@arm.com> wrote:
>>> Bring arch_scale_cpu_capacity() in line with the recent change of its
>>> arch_scale_freq_capacity() sibling in commit dfbca41f3479 ("sched:
>>> Optimize freq invariant accounting") from weak function to #define to
>>> allow inlining of the function.
>>>
>>> While at it, remove the ARCH_CAPACITY sched_feature as well. With the
>>> change to #define there isn't a straightforward way to allow runtime
>>> switch between an arch implementation and the default implementation of
>>> arch_scale_cpu_capacity() using sched_feature. The default was to use
>>> the arch-specific implementation, but only the arm architecture provides
>>> one and that is essentially equivalent to the default implementation.
>
> [...]
>
>>
>> So you change the way to declare arch_scale_cpu_capacity but i don't
>> see the update of the arm arch which declare a
>> arch_scale_cpu_capacity to reflect this change in your series.
>
> We were reluctant to do this because this functionality makes only sense
> for ARCH=arm big.Little systems w/ cortex-a{15|7} cores and only if the
> clock-frequency property is set in the dts file.

IMO, we should maintain the compatibility of current implementation
instead of breaking the link and creating a dead code.
Your proposal below fits the requirement

>
> Are you planning to push for a 'struct cpu_efficiency/clock-frequency
> property' solution for ARCH=arm64 as well?

I know that there has been some discussions aorund that but i didn't
follow the thread in details

>
> I'm asking because for ARCH=arm64 systems today (JUNO, Hi6220) we use the
> capacity value of the last entry of the capacity_state vector for the cores
> (e.g. cortex-a{57|53).

This is a struct of the eas feature ? Not sure that we should link the
definition of  the cpu capacity to an internal struct of a feature; DT
seems a better way to define it.
So if you want to revisit the way, we set the capacity of CPU for arm
and/or arm64, I'm fully open to the discussion but this should happen
in another thread than this one which has for only purpose the
alignment of the arch_scale_cpu_capacity interface declaration with
arch_scale_freq_capacity one.

So, with the patch below that updates the arm definition of
arch_scale_cpu_capacity, you can add my Acked-by: Vincent Guittot
<vincent.guittot@linaro.org> on this patch and the additional one
below

Regards,
Vincent

>
> To connect the cpu invariant engine (scale_cpu_capacity()
> [arch/arm/kernel/topology.c]) with the scheduler, something like this is
> missing:
>
> diff --git a/arch/arm/include/asm/topology.h b/arch/arm/include/asm/topology.h
> index 370f7a732900..17c6b3243196 100644
> --- a/arch/arm/include/asm/topology.h
> +++ b/arch/arm/include/asm/topology.h
> @@ -24,6 +24,10 @@ void init_cpu_topology(void);
>  void store_cpu_topology(unsigned int cpuid);
>  const struct cpumask *cpu_coregroup_mask(int cpu);
>
> +#define arch_scale_cpu_capacity scale_cpu_capacity
> +struct sched_domain;
> +extern unsigned long scale_cpu_capacity(struct sched_domain *sd, int cpu);
> +
>  #else
>
>  static inline void init_cpu_topology(void) { }
> diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
> index 08b7847bf912..907e0d2d9b82 100644
> --- a/arch/arm/kernel/topology.c
> +++ b/arch/arm/kernel/topology.c
> @@ -42,7 +42,7 @@
>   */
>  static DEFINE_PER_CPU(unsigned long, cpu_scale);
>
> -unsigned long arch_scale_cpu_capacity(struct sched_domain *sd, int cpu)
> +unsigned long scale_cpu_capacity(struct sched_domain *sd, int cpu)
>  {
>         return per_cpu(cpu_scale, cpu);
>  }
> @@ -166,7 +166,7 @@ static void update_cpu_capacity(unsigned int cpu)
>         set_capacity_scale(cpu, cpu_capacity(cpu) / middle_capacity);
>
>         pr_info("CPU%u: update cpu_capacity %lu\n",
> -               cpu, arch_scale_cpu_capacity(NULL, cpu));
> +               cpu, scale_cpu_capacity(NULL, cpu));
>  }
>
> -- Dietmar
>
> [...]
>

  reply	other threads:[~2015-09-04  7:27 UTC|newest]

Thread overview: 97+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-14 16:23 [PATCH 0/6] sched/fair: Compute capacity invariant load/utilization tracking Morten Rasmussen
2015-08-14 16:23 ` [PATCH 1/6] sched/fair: Make load tracking frequency scale-invariant Morten Rasmussen
2015-09-13 11:03   ` [tip:sched/core] " tip-bot for Dietmar Eggemann
2015-08-14 16:23 ` [PATCH 2/6] sched/fair: Convert arch_scale_cpu_capacity() from weak function to #define Morten Rasmussen
2015-09-02  9:31   ` Vincent Guittot
2015-09-02 12:41     ` Vincent Guittot
2015-09-03 19:58     ` Dietmar Eggemann
2015-09-04  7:26       ` Vincent Guittot [this message]
2015-09-07 13:25         ` Dietmar Eggemann
2015-09-11 13:21         ` Dietmar Eggemann
2015-09-11 14:45           ` Vincent Guittot
2015-09-13 11:03   ` [tip:sched/core] " tip-bot for Morten Rasmussen
2015-08-14 16:23 ` [PATCH 3/6] sched/fair: Make utilization tracking cpu scale-invariant Morten Rasmussen
2015-08-14 23:04   ` Dietmar Eggemann
2015-09-04  7:52     ` Vincent Guittot
2015-09-13 11:04     ` [tip:sched/core] sched/fair: Make utilization tracking CPU scale-invariant tip-bot for Dietmar Eggemann
2015-08-14 16:23 ` [PATCH 4/6] sched/fair: Name utilization related data and functions consistently Morten Rasmussen
2015-09-04  9:08   ` Vincent Guittot
2015-09-11 16:35     ` Dietmar Eggemann
2015-09-13 11:04   ` [tip:sched/core] " tip-bot for Dietmar Eggemann
2015-08-14 16:23 ` [PATCH 5/6] sched/fair: Get rid of scaling utilization by capacity_orig Morten Rasmussen
2015-09-03 23:51   ` Steve Muckle
2015-09-07 15:37     ` Dietmar Eggemann
2015-09-07 16:21       ` Vincent Guittot
2015-09-07 18:54         ` Dietmar Eggemann
2015-09-07 19:47           ` Peter Zijlstra
2015-09-08 12:47             ` Dietmar Eggemann
2015-09-08  7:22           ` Vincent Guittot
2015-09-08 12:26             ` Peter Zijlstra
2015-09-08 12:52               ` Peter Zijlstra
2015-09-08 14:06                 ` Vincent Guittot
2015-09-08 14:35                   ` Morten Rasmussen
2015-09-08 14:40                     ` Vincent Guittot
2015-09-08 14:31                 ` Morten Rasmussen
2015-09-08 15:33                   ` Peter Zijlstra
2015-09-09 22:23                     ` bsegall
2015-09-10 11:06                       ` Morten Rasmussen
2015-09-10 11:11                         ` Vincent Guittot
2015-09-10 12:10                           ` Morten Rasmussen
2015-09-11  0:50                             ` Yuyang Du
2015-09-10 17:23                         ` bsegall
2015-09-08 16:53                   ` Morten Rasmussen
2015-09-09  9:43                     ` Peter Zijlstra
2015-09-09  9:45                       ` Peter Zijlstra
2015-09-09 11:13                       ` Morten Rasmussen
2015-09-11 17:22                         ` Morten Rasmussen
2015-09-17  9:51                           ` Peter Zijlstra
2015-09-17 10:38                           ` Peter Zijlstra
2015-09-21  1:16                             ` Yuyang Du
2015-09-21 17:30                               ` bsegall
2015-09-21 23:39                                 ` Yuyang Du
2015-09-22 17:18                                   ` bsegall
2015-09-22 23:22                                     ` Yuyang Du
2015-09-23 16:54                                       ` bsegall
2015-09-24  0:22                                         ` Yuyang Du
2015-09-30 12:52                                     ` Peter Zijlstra
2015-09-11  7:46                     ` Leo Yan
2015-09-11 10:02                       ` Morten Rasmussen
2015-09-11 14:11                         ` Leo Yan
2015-09-09 19:07                 ` Yuyang Du
2015-09-10 10:06                   ` Peter Zijlstra
2015-09-08 13:39               ` Vincent Guittot
2015-09-08 14:10                 ` Peter Zijlstra
2015-09-08 15:17                   ` Vincent Guittot
2015-09-08 12:50             ` Dietmar Eggemann
2015-09-08 14:01               ` Vincent Guittot
2015-09-08 14:27                 ` Dietmar Eggemann
2015-09-09 20:15               ` Yuyang Du
2015-09-10 10:07                 ` Peter Zijlstra
2015-09-11  0:28                   ` Yuyang Du
2015-09-11 10:31                     ` Morten Rasmussen
2015-09-11 17:05                       ` bsegall
2015-09-11 18:24                         ` Yuyang Du
2015-09-14 17:36                           ` bsegall
2015-09-14 12:56                         ` Morten Rasmussen
2015-09-14 17:34                           ` bsegall
2015-09-14 22:56                             ` Yuyang Du
2015-09-15 17:11                               ` bsegall
2015-09-15 18:39                                 ` Yuyang Du
2015-09-16 17:06                                   ` bsegall
2015-09-17  2:31                                     ` Yuyang Du
2015-09-15  8:43                             ` Morten Rasmussen
2015-09-16 15:36                             ` Peter Zijlstra
2015-09-08 11:44           ` Peter Zijlstra
2015-09-13 11:04       ` [tip:sched/core] " tip-bot for Dietmar Eggemann
2015-08-14 16:23 ` [PATCH 6/6] sched/fair: Initialize task load and utilization before placing task on rq Morten Rasmussen
2015-09-13 11:05   ` [tip:sched/core] " tip-bot for Morten Rasmussen
2015-08-16 20:46 ` [PATCH 0/6] sched/fair: Compute capacity invariant load/utilization tracking Peter Zijlstra
2015-08-17 11:29   ` Morten Rasmussen
2015-08-17 11:48     ` Peter Zijlstra
2015-08-31  9:24 ` Peter Zijlstra
2015-09-02  9:51   ` Dietmar Eggemann
2015-09-07 12:42   ` Peter Zijlstra
2015-09-07 13:21     ` Peter Zijlstra
2015-09-07 13:23     ` Peter Zijlstra
2015-09-07 14:44     ` Dietmar Eggemann
2015-09-13 11:06       ` [tip:sched/core] sched/fair: Defer calling scaling functions tip-bot for Dietmar Eggemann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAKfTPtBnj9_oHoXEZFnYnsxb5a3KwPcVHYydRVQEzypkCbJakQ@mail.gmail.com \
    --to=vincent.guittot@linaro.org \
    --cc=Juri.Lelli@arm.com \
    --cc=Morten.Rasmussen@arm.com \
    --cc=daniel.lezcano@linaro.org \
    --cc=dietmar.eggemann@arm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=mturquette@baylibre.com \
    --cc=pang.xunlei@zte.com.cn \
    --cc=peterz@infradead.org \
    --cc=rjw@rjwysocki.net \
    --cc=sgurrappadi@nvidia.com \
    --cc=yuyang.du@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.