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

Groups > linux.kernel > #1677403 > unrolled thread

[PATCH V2 0/4] sched: cpufreq: Allow remote callbacks

Back to article view | Back to linux.kernel

Contents

  [PATCH V2 0/4] sched: cpufreq: Allow remote callbacks Viresh Kumar <viresh.kumar@linaro.org> - 2017-06-29 07:30 +0200
    [PATCH V2 3/4] intel_pstate: Ignore scheduler cpufreq callbacks on remote CPUs Viresh Kumar <viresh.kumar@linaro.org> - 2017-06-29 07:30 +0200
      Re: [PATCH V2 3/4] intel_pstate: Ignore scheduler cpufreq callbacks  on remote CPUs Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> - 2017-06-29 23:30 +0200
        Re: [PATCH V2 3/4] intel_pstate: Ignore scheduler cpufreq callbacks  on remote CPUs Viresh Kumar <viresh.kumar@linaro.org> - 2017-06-30 05:30 +0200
    Re: [PATCH V2 0/4] sched: cpufreq: Allow remote callbacks "Rafael J. Wysocki" <rafael@kernel.org> - 2017-06-29 22:40 +0200
      Re: [PATCH V2 0/4] sched: cpufreq: Allow remote callbacks Viresh Kumar <viresh.kumar@linaro.org> - 2017-06-30 05:30 +0200

#1677403 — [PATCH V2 0/4] sched: cpufreq: Allow remote callbacks

Hi,

Here is the second version of this series. The first [1] version was
sent several months back.

With Android UI and benchmarks the latency of cpufreq response to
certain scheduling events can become very critical. Currently, callbacks
into schedutil are only made from the scheduler if the target CPU of the
event is the same as the current CPU. This means there are certain
situations where a target CPU may not run schedutil for some time.

One testcase to show this behavior is where a task starts running on
CPU0, then a new task is also spawned on CPU0 by a task on CPU1. If the
system is configured such that new tasks should receive maximum demand
initially, this should result in CPU0 increasing frequency immediately.
Because of the above mentioned limitation though this does not occur.
This is verified using ftrace with the sample [2] application.

Maybe the ideal solution is to always allow remote callbacks but that
has its own challenges:

o There is no protection required for single CPU per policy case today,
  and adding any kind of locking there, to supply remote callbacks,
  isn't really a good idea.

o If is local CPU isn't part of the same cpufreq policy as the target
  CPU, then we wouldn't be able to do fast switching at all and have to
  use some kind of bottom half to schedule work on the target CPU to do
  real switching. That may be overkill as well.


Taking above challenges into consideration, this version proposes a much
simpler diff as compared to the first version.

This series only allows remote callbacks for target CPUs that share the
cpufreq policy with the local CPU. Locking is mostly in place everywhere
and we wouldn't be required to change a lot of things.

This series is tested with couple of usecases (Android: hackbench,
recentfling, galleryfling, vellamo, Ubuntu: hackbench) on ARM hikey
board (64 bit octa-core, single policy). Only galleryfling showed minor
improvements, while others didn't had much deviation.

The reason being that this patchset only targets a corner case, where
following are required to be true to improve performance and that
doesn't happen too often with these tests:

- Task is migrated to another CPU.
- The task has maximum demand initially, and should take the CPU to
  higher OPPs.
- And the target CPU doesn't call into schedutil until the next tick.


V1->V2:
- Don't support remote callbacks for unshared cpufreq policies.
- Don't support remote callbacks where local CPU isn't part of the
  target CPU's cpufreq policy.
- Dropped dvfs_possible_from_any_cpu flag.

--
viresh

[1] https://marc.info/?l=linux-pm&m=148906015927796&w=2
[2] http://pastebin.com/7LkMSRxE


Steve Muckle (1):
  intel_pstate: Ignore scheduler cpufreq callbacks on remote CPUs

Viresh Kumar (3):
  cpufreq: schedutil: Process remote callback for shared policies
  cpufreq: governor: Process remote callback for shared policies
  sched: cpufreq: Enable remote sched cpufreq callbacks

 drivers/cpufreq/cpufreq_governor.c |  4 ++++
 drivers/cpufreq/intel_pstate.c     |  3 +++
 include/linux/sched/cpufreq.h      |  1 +
 kernel/sched/cpufreq.c             |  1 +
 kernel/sched/cpufreq_schedutil.c   | 19 ++++++++++++++-----
 kernel/sched/deadline.c            |  2 +-
 kernel/sched/fair.c                |  8 +++++---
 kernel/sched/rt.c                  |  2 +-
 kernel/sched/sched.h               | 10 ++--------
 9 files changed, 32 insertions(+), 18 deletions(-)

-- 
2.13.0.71.gd7076ec9c9cb

[toc] | [next] | [standalone]

#1677404 — [PATCH V2 3/4] intel_pstate: Ignore scheduler cpufreq callbacks on remote CPUs

From: Steve Muckle <smuckle.linux@gmail.com>

In preparation for the scheduler cpufreq callback happening on remote
CPUs, check for this case in intel_pstate which currently requires the
callback run on the local CPU. Such callbacks are ignored for now.

Signed-off-by: Steve Muckle <smuckle.linux@gmail.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/intel_pstate.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 4ce501148790..7a2a8ee579ef 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -1755,6 +1755,9 @@ static void intel_pstate_update_util(struct update_util_data *data, u64 time,
 	struct cpudata *cpu = container_of(data, struct cpudata, update_util);
 	u64 delta_ns;
 
+	if (smp_processor_id() != data->cpu)
+		return;
+
 	if (flags & SCHED_CPUFREQ_IOWAIT) {
 		cpu->iowait_boost = int_tofp(1);
 	} else if (cpu->iowait_boost) {
-- 
2.13.0.71.gd7076ec9c9cb

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

#1678209 — Re: [PATCH V2 3/4] intel_pstate: Ignore scheduler cpufreq callbacks on remote CPUs

On Thu, 2017-06-29 at 10:56 +0530, Viresh Kumar wrote:
> From: Steve Muckle <smuckle.linux@gmail.com>
> 
> In preparation for the scheduler cpufreq callback happening on remote
> CPUs, check for this case in intel_pstate which currently requires
> the
> callback run on the local CPU. Such callbacks are ignored for now.
Is it possible that we miss a chance to calculate load periodically at
a predefined interval (10ms default), because the callback happened on
a different CPU?

Thanks,
Srinivas

> 
> Signed-off-by: Steve Muckle <smuckle.linux@gmail.com>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  drivers/cpufreq/intel_pstate.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/cpufreq/intel_pstate.c
> b/drivers/cpufreq/intel_pstate.c
> index 4ce501148790..7a2a8ee579ef 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -1755,6 +1755,9 @@ static void intel_pstate_update_util(struct
> update_util_data *data, u64 time,
>  	struct cpudata *cpu = container_of(data, struct cpudata,
> update_util);
>  	u64 delta_ns;
>  
> +	if (smp_processor_id() != data->cpu)
> +		return;
> +
>  	if (flags & SCHED_CPUFREQ_IOWAIT) {
>  		cpu->iowait_boost = int_tofp(1);
>  	} else if (cpu->iowait_boost) {

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

#1678431 — Re: [PATCH V2 3/4] intel_pstate: Ignore scheduler cpufreq callbacks on remote CPUs

On 29-06-17, 14:23, Srinivas Pandruvada wrote:
> On Thu, 2017-06-29 at 10:56 +0530, Viresh Kumar wrote:
> > From: Steve Muckle <smuckle.linux@gmail.com>
> > 
> > In preparation for the scheduler cpufreq callback happening on remote
> > CPUs, check for this case in intel_pstate which currently requires
> > the
> > callback run on the local CPU. Such callbacks are ignored for now.
> Is it possible that we miss a chance to calculate load periodically at
> a predefined interval (10ms default), because the callback happened on
> a different CPU?

We aren't updating cpu->sample.time for remote callbacks here, so no
we shouldn't miss anything.

-- 
viresh

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

#1678176

Hi,

On Thu, Jun 29, 2017 at 7:26 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> Hi,
>
> Here is the second version of this series. The first [1] version was
> sent several months back.
>
> With Android UI and benchmarks the latency of cpufreq response to
> certain scheduling events can become very critical. Currently, callbacks
> into schedutil are only made from the scheduler if the target CPU of the
> event is the same as the current CPU. This means there are certain
> situations where a target CPU may not run schedutil for some time.
>
> One testcase to show this behavior is where a task starts running on
> CPU0, then a new task is also spawned on CPU0 by a task on CPU1. If the
> system is configured such that new tasks should receive maximum demand
> initially, this should result in CPU0 increasing frequency immediately.
> Because of the above mentioned limitation though this does not occur.
> This is verified using ftrace with the sample [2] application.
>
> Maybe the ideal solution is to always allow remote callbacks but that
> has its own challenges:
>
> o There is no protection required for single CPU per policy case today,
>   and adding any kind of locking there, to supply remote callbacks,
>   isn't really a good idea.
>
> o If is local CPU isn't part of the same cpufreq policy as the target
>   CPU, then we wouldn't be able to do fast switching at all and have to
>   use some kind of bottom half to schedule work on the target CPU to do
>   real switching. That may be overkill as well.
>
>
> Taking above challenges into consideration, this version proposes a much
> simpler diff as compared to the first version.
>
> This series only allows remote callbacks for target CPUs that share the
> cpufreq policy with the local CPU. Locking is mostly in place everywhere
> and we wouldn't be required to change a lot of things.
>
> This series is tested with couple of usecases (Android: hackbench,
> recentfling, galleryfling, vellamo, Ubuntu: hackbench) on ARM hikey
> board (64 bit octa-core, single policy). Only galleryfling showed minor
> improvements, while others didn't had much deviation.
>
> The reason being that this patchset only targets a corner case, where
> following are required to be true to improve performance and that
> doesn't happen too often with these tests:
>
> - Task is migrated to another CPU.
> - The task has maximum demand initially, and should take the CPU to
>   higher OPPs.
> - And the target CPU doesn't call into schedutil until the next tick.
>
>
> V1->V2:
> - Don't support remote callbacks for unshared cpufreq policies.
> - Don't support remote callbacks where local CPU isn't part of the
>   target CPU's cpufreq policy.
> - Dropped dvfs_possible_from_any_cpu flag.

There is no way I could consider this for inclusion into 4.13, so I'm
not sure why you chose this specific timing.

Thanks,
Rafael

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

#1678430

On 29-06-17, 22:30, Rafael J. Wysocki wrote:
> There is no way I could consider this for inclusion into 4.13, so I'm
> not sure why you chose this specific timing.

Sure, I don't aim at getting it merged for 4.13. I sent it now to get
some early feedback, so that it is ready for inclusion for the 4.14
merge window.

Isn't it fine to send such patches anytime? Should I avoid sending
such changes around merge window ?

-- 
viresh

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web