All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sai Gurrappadi <sgurrappadi@nvidia.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>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	Juri Lelli <Juri.Lelli@arm.com>,
	Patrick Bellasi <patrick.bellasi@arm.com>,
	Michael Turquette <mturquette@baylibre.com>,
	pboonstoppel@nvidia.com
Subject: Re: [PATCH 2/2] sched/fair: do not call cpufreq hook unless util changed
Date: Thu, 24 Mar 2016 16:47:21 -0700	[thread overview]
Message-ID: <56F47C89.9070201@nvidia.com> (raw)
In-Reply-To: <1458606068-7476-2-git-send-email-smuckle@linaro.org>

Hi Steve,

On 03/21/2016 05:21 PM, Steve Muckle wrote:
> There's no reason to call the cpufreq hook if the root cfs_rq
> utilization has not been modified.
> 
> Signed-off-by: Steve Muckle <smuckle@linaro.org>
> ---
>  kernel/sched/fair.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index d418deb04049..af58e826cffe 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -2826,20 +2826,21 @@ static inline int update_cfs_rq_load_avg(u64 now, struct cfs_rq *cfs_rq)
>  {
>  	struct sched_avg *sa = &cfs_rq->avg;
>  	struct rq *rq = rq_of(cfs_rq);
> -	int decayed, removed = 0;
> +	int decayed, removed_load = 0, removed_util = 0;
>  	int cpu = cpu_of(rq);
>  
>  	if (atomic_long_read(&cfs_rq->removed_load_avg)) {
>  		s64 r = atomic_long_xchg(&cfs_rq->removed_load_avg, 0);
>  		sa->load_avg = max_t(long, sa->load_avg - r, 0);
>  		sa->load_sum = max_t(s64, sa->load_sum - r * LOAD_AVG_MAX, 0);
> -		removed = 1;
> +		removed_load = 1;
>  	}
>  
>  	if (atomic_long_read(&cfs_rq->removed_util_avg)) {
>  		long r = atomic_long_xchg(&cfs_rq->removed_util_avg, 0);
>  		sa->util_avg = max_t(long, sa->util_avg - r, 0);
>  		sa->util_sum = max_t(s32, sa->util_sum - r * LOAD_AVG_MAX, 0);
> +		removed_util = 1;
>  	}
>  
>  	decayed = __update_load_avg(now, cpu, sa,
> @@ -2850,7 +2851,8 @@ static inline int update_cfs_rq_load_avg(u64 now, struct cfs_rq *cfs_rq)
>  	cfs_rq->load_last_update_time_copy = sa->last_update_time;
>  #endif
>  
> -	if (cpu == smp_processor_id() && &rq->cfs == cfs_rq) {
> +	if (cpu == smp_processor_id() && &rq->cfs == cfs_rq &&
> +	    (decayed || removed_util)) {
>  		unsigned long max = rq->cpu_capacity_orig;

Should this filtering instead happen on the governor side?

Even if the CFS load itself didn't change, we could have switched from an
RT/DL thread to a CFS thread so util would have to be updated from ULONG_MAX
to whatever CFS needs right?

Also now that CFS enqueue calls cpufreq_update_util, RT/DL requests could
potentially get overridden.

-Sai

WARNING: multiple messages have this Message-ID (diff)
From: Sai Gurrappadi <sgurrappadi@nvidia.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>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	Juri Lelli <Juri.Lelli@arm.com>,
	Patrick Bellasi <patrick.bellasi@arm.com>,
	Michael Turquette <mturquette@baylibre.com>,
	<pboonstoppel@nvidia.com>
Subject: Re: [PATCH 2/2] sched/fair: do not call cpufreq hook unless util changed
Date: Thu, 24 Mar 2016 16:47:21 -0700	[thread overview]
Message-ID: <56F47C89.9070201@nvidia.com> (raw)
In-Reply-To: <1458606068-7476-2-git-send-email-smuckle@linaro.org>

Hi Steve,

On 03/21/2016 05:21 PM, Steve Muckle wrote:
> There's no reason to call the cpufreq hook if the root cfs_rq
> utilization has not been modified.
> 
> Signed-off-by: Steve Muckle <smuckle@linaro.org>
> ---
>  kernel/sched/fair.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index d418deb04049..af58e826cffe 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -2826,20 +2826,21 @@ static inline int update_cfs_rq_load_avg(u64 now, struct cfs_rq *cfs_rq)
>  {
>  	struct sched_avg *sa = &cfs_rq->avg;
>  	struct rq *rq = rq_of(cfs_rq);
> -	int decayed, removed = 0;
> +	int decayed, removed_load = 0, removed_util = 0;
>  	int cpu = cpu_of(rq);
>  
>  	if (atomic_long_read(&cfs_rq->removed_load_avg)) {
>  		s64 r = atomic_long_xchg(&cfs_rq->removed_load_avg, 0);
>  		sa->load_avg = max_t(long, sa->load_avg - r, 0);
>  		sa->load_sum = max_t(s64, sa->load_sum - r * LOAD_AVG_MAX, 0);
> -		removed = 1;
> +		removed_load = 1;
>  	}
>  
>  	if (atomic_long_read(&cfs_rq->removed_util_avg)) {
>  		long r = atomic_long_xchg(&cfs_rq->removed_util_avg, 0);
>  		sa->util_avg = max_t(long, sa->util_avg - r, 0);
>  		sa->util_sum = max_t(s32, sa->util_sum - r * LOAD_AVG_MAX, 0);
> +		removed_util = 1;
>  	}
>  
>  	decayed = __update_load_avg(now, cpu, sa,
> @@ -2850,7 +2851,8 @@ static inline int update_cfs_rq_load_avg(u64 now, struct cfs_rq *cfs_rq)
>  	cfs_rq->load_last_update_time_copy = sa->last_update_time;
>  #endif
>  
> -	if (cpu == smp_processor_id() && &rq->cfs == cfs_rq) {
> +	if (cpu == smp_processor_id() && &rq->cfs == cfs_rq &&
> +	    (decayed || removed_util)) {
>  		unsigned long max = rq->cpu_capacity_orig;

Should this filtering instead happen on the governor side?

Even if the CFS load itself didn't change, we could have switched from an
RT/DL thread to a CFS thread so util would have to be updated from ULONG_MAX
to whatever CFS needs right?

Also now that CFS enqueue calls cpufreq_update_util, RT/DL requests could
potentially get overridden.

-Sai

  reply	other threads:[~2016-03-24 23:48 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 [this message]
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
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=56F47C89.9070201@nvidia.com \
    --to=sgurrappadi@nvidia.com \
    --cc=Juri.Lelli@arm.com \
    --cc=dietmar.eggemann@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=pboonstoppel@nvidia.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.