All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dietmar Eggemann <dietmar.eggemann@arm.com>
To: Steve Muckle <steve.muckle@linaro.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>
Cc: linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Morten Rasmussen <morten.rasmussen@arm.com>,
	Juri Lelli <Juri.Lelli@arm.com>,
	Patrick Bellasi <patrick.bellasi@arm.com>,
	Michael Turquette <mturquette@baylibre.com>
Subject: Re: [PATCH 1/2] sched/fair: move cpufreq hook to update_cfs_rq_load_avg()
Date: Mon, 28 Mar 2016 20:30:46 +0200	[thread overview]
Message-ID: <56F97856.4040804@arm.com> (raw)
In-Reply-To: <56F95D10.4070400@linaro.org>

On 03/28/2016 06:34 PM, Steve Muckle wrote:
> Hi Dietmar,
>
> On 03/28/2016 05:02 AM, Dietmar Eggemann wrote:
>> Hi Steve,
>>
>> these patches fall into the bucket of 'optimization of updating the
>> value only if the root cfs_rq util has changed' as discussed in '[PATCH
>> 5/8] sched/cpufreq: pass sched class into cpufreq_update_util' of Mike
>> T's current series '[PATCH 0/8] schedutil enhancements', right?
>
> I would say just the second patch is an optimization. The first and
> third patches cover additional paths in CFS where the hook should be
> called but currently is not, which I think is a correctness issue.

Not disagreeing here but I don't know if this level of accuracy is 
really needed. I mean we currently miss updates in 
enqueue_task_fair()->enqueue_entity()->enqueue_entity_load_avg() and 
idle_balance()/rebalance_domains()->update_blocked_averages() but there 
are plenty of call sides of update_load_avg(se, ...) with 
'&rq_of(cfs_rq_of(se))->cfs == cfs_rq_of(se)'.

The question for me is does schedutil work better with this new, more 
accurate signal? IMO, not receiving a bunch of consecutive 
cpufreq_update_util's w/ the same 'util' value is probably a good thing, 
unless we see the interaction with RT/DL class as mentioned by Sai. Here 
an agreement on the design for the 'capacity vote aggregation from 
CFS/RT/DL' would help to clarify.

>> I wonder if it makes sense to apply them before a proper 'capacity vote
>> aggregation from CFS/RT/DL' has been agreed upon?
>
> Getting the right call sites for the hook in CFS should be orthogonal to
> the sched class vote aggregation IMO.

Hopefully :-)

[...]

Cheers,

-- Dietmar

WARNING: multiple messages have this Message-ID (diff)
From: Dietmar Eggemann <dietmar.eggemann@arm.com>
To: Steve Muckle <steve.muckle@linaro.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>
Cc: <linux-kernel@vger.kernel.org>, <linux-pm@vger.kernel.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Morten Rasmussen <morten.rasmussen@arm.com>,
	Juri Lelli <Juri.Lelli@arm.com>,
	Patrick Bellasi <patrick.bellasi@arm.com>,
	Michael Turquette <mturquette@baylibre.com>
Subject: Re: [PATCH 1/2] sched/fair: move cpufreq hook to update_cfs_rq_load_avg()
Date: Mon, 28 Mar 2016 20:30:46 +0200	[thread overview]
Message-ID: <56F97856.4040804@arm.com> (raw)
In-Reply-To: <56F95D10.4070400@linaro.org>

On 03/28/2016 06:34 PM, Steve Muckle wrote:
> Hi Dietmar,
>
> On 03/28/2016 05:02 AM, Dietmar Eggemann wrote:
>> Hi Steve,
>>
>> these patches fall into the bucket of 'optimization of updating the
>> value only if the root cfs_rq util has changed' as discussed in '[PATCH
>> 5/8] sched/cpufreq: pass sched class into cpufreq_update_util' of Mike
>> T's current series '[PATCH 0/8] schedutil enhancements', right?
>
> I would say just the second patch is an optimization. The first and
> third patches cover additional paths in CFS where the hook should be
> called but currently is not, which I think is a correctness issue.

Not disagreeing here but I don't know if this level of accuracy is 
really needed. I mean we currently miss updates in 
enqueue_task_fair()->enqueue_entity()->enqueue_entity_load_avg() and 
idle_balance()/rebalance_domains()->update_blocked_averages() but there 
are plenty of call sides of update_load_avg(se, ...) with 
'&rq_of(cfs_rq_of(se))->cfs == cfs_rq_of(se)'.

The question for me is does schedutil work better with this new, more 
accurate signal? IMO, not receiving a bunch of consecutive 
cpufreq_update_util's w/ the same 'util' value is probably a good thing, 
unless we see the interaction with RT/DL class as mentioned by Sai. Here 
an agreement on the design for the 'capacity vote aggregation from 
CFS/RT/DL' would help to clarify.

>> I wonder if it makes sense to apply them before a proper 'capacity vote
>> aggregation from CFS/RT/DL' has been agreed upon?
>
> Getting the right call sites for the hook in CFS should be orthogonal to
> the sched class vote aggregation IMO.

Hopefully :-)

[...]

Cheers,

-- Dietmar

  reply	other threads:[~2016-03-28 20:05 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-22  0:21 [PATCH 1/2] sched/fair: move cpufreq hook to update_cfs_rq_load_avg() Steve Muckle
2016-03-22  0:21 ` [PATCH 2/2] sched/fair: do not call cpufreq hook unless util changed Steve Muckle
2016-03-24 23:47   ` Sai Gurrappadi
2016-03-24 23:47     ` Sai Gurrappadi
2016-03-25  1:01     ` Steve Muckle
2016-03-25 21:24       ` Sai Gurrappadi
2016-03-25 21:24         ` Sai Gurrappadi
2016-04-23 12:57   ` [tip:sched/core] sched/fair: Do " tip-bot for Steve Muckle
2016-03-28 12:02 ` [PATCH 1/2] sched/fair: move cpufreq hook to update_cfs_rq_load_avg() Dietmar Eggemann
2016-03-28 12:02   ` Dietmar Eggemann
2016-03-28 16:34   ` Steve Muckle
2016-03-28 18:30     ` Dietmar Eggemann [this message]
2016-03-28 18:30       ` Dietmar Eggemann
2016-03-28 19:38       ` Steve Muckle
2016-03-30 19:35         ` Peter Zijlstra
2016-03-31  1:42           ` Steve Muckle
2016-03-31  7:37             ` Peter Zijlstra
2016-03-31 21:26               ` Steve Muckle
2016-04-01  9:20                 ` Peter Zijlstra
2016-04-11 19:28                   ` Steve Muckle
2016-04-11 21:20                     ` Rafael J. Wysocki
2016-04-12 14:29                       ` Rafael J. Wysocki
2016-04-12 19:38                         ` Steve Muckle
2016-04-13 14:45                           ` Rafael J. Wysocki
2016-04-13 17:53                             ` Steve Muckle
2016-04-13 19:39                               ` Rafael J. Wysocki
2016-04-13  0:08                         ` Steve Muckle
2016-04-13  4:48                           ` Rafael J. Wysocki
2016-04-13 16:05                             ` Rafael J. Wysocki
2016-04-13 16:07                               ` Rafael J. Wysocki
2016-04-13 18:06                                 ` Steve Muckle
2016-04-13 19:50                                   ` Rafael J. Wysocki
2016-04-20  2:22                                     ` Steve Muckle
2016-03-31  9:27           ` Vincent Guittot
2016-03-31  9:34             ` Peter Zijlstra
2016-03-31  9:50               ` Vincent Guittot
2016-03-31 10:47                 ` Peter Zijlstra
2016-03-31 12:14                   ` Vincent Guittot
2016-03-31 12:34                     ` Peter Zijlstra
2016-03-31 12:50                       ` Vincent Guittot
2016-04-23 12:57 ` [tip:sched/core] sched/fair: Move " tip-bot for Steve Muckle

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=56F97856.4040804@arm.com \
    --to=dietmar.eggemann@arm.com \
    --cc=Juri.Lelli@arm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=morten.rasmussen@arm.com \
    --cc=mturquette@baylibre.com \
    --cc=patrick.bellasi@arm.com \
    --cc=peterz@infradead.org \
    --cc=rafael@kernel.org \
    --cc=steve.muckle@linaro.org \
    --cc=vincent.guittot@linaro.org \
    /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.