linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Joel Fernandes <joelaf@google.com>
To: linux-kernel@vger.kernel.org
Cc: Joel Fernandes <joelaf@google.com>,
	"Rafael J . Wysocki" <rjw@rjwysocki.net>,
	Viresh Kumar <viresh.kumar@linaro.org>,
	Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	"Cc: Srinivas Pandruvada" <srinivas.pandruvada@linux.intel.com>,
	"Cc: Len Brown" <lenb@kernel.org>,
	"Cc: Juri Lelli" <juri.lelli@arm.com>,
	"Cc: Patrick Bellasi" <patrick.bellasi@arm.com>,
	"Cc: Steve Muckle" <smuckle@google.com>,
	"Cc: Brendan Jackman" <brendan.jackman@arm.com>,
	"Cc: Chris Redpath" <Chris.Redpath@arm.com>,
	"Cc: Atish Patra" <atish.patra@oracle.com>,
	"Cc: Dietmar Eggemann" <dietmar.eggemann@arm.com>,
	"Cc: Vincent Guittot" <vincent.guittot@linaro.org>,
	"Cc: Morten Ramussen" <morten.rasmussen@arm.com>,
	"Cc: Frederic Weisbecker" <fweisbec@gmail.com>,
	"Cc: Thomas Gleixner" <tglx@linutronix.de>,
	"Cc: EAS Dev" <eas-dev@lists.linaro.org>,
	"Cc: Android Kernel" <kernel-team@android.com>
Subject: [PATCH RFC 2/5] sched/fair: Skip frequency update if CPU about to idle
Date: Sat, 28 Oct 2017 02:59:38 -0700	[thread overview]
Message-ID: <20171028095941.4773-3-joelaf@google.com> (raw)
In-Reply-To: <20171028095941.4773-1-joelaf@google.com>

Updating CPU frequency on last dequeue of a CPU is useless. Because the
utilization since CPU came out of idle can increase till the last dequeue, this
means we are requesting for a higher frequency before entering idle which is
not very meaningful or useful. It causes unwanted wakeups of the schedutil
governor kthread in slow-switch systems resulting in large number of wake ups
that could have been avoided. In an Android application playing music where the
music app's thread wakes up and sleeps periodically on an Android device, its
seen that the frequency increases slightly on the dequeue and is reduced when
the task wakes up again. This oscillation continues between 300Mhz and 350Mhz,
and while the task is running, its at 300MHz the whole time. This is pointless.
Adding to that, these are unnecessary wake ups. Infact most of the time when
the sugov thread wakes up, all the CPUs are idle - so it can hurt power by
disturbing the cluster when it is idling.

This patch prevents a frequency update on the last dequeue. With this the
number of schedutil governor thread wake ups are reduces more than 2 times
(1389 -> 527).

Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
Cc: Viresh Kumar <viresh.kumar@linaro.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Joel Fernandes <joelaf@google.com>
---
 kernel/sched/fair.c  | 25 ++++++++++++++++++++++---
 kernel/sched/sched.h |  1 +
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index f97693fe8b6e..4c06e52935d3 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -3725,6 +3725,7 @@ static void detach_entity_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *s
 #define UPDATE_TG	0x1
 #define SKIP_AGE_LOAD	0x2
 #define DO_ATTACH	0x4
+#define SKIP_CPUFREQ	0x8
 
 /* Update task and its cfs_rq load average */
 static inline void update_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
@@ -3741,7 +3742,7 @@ static inline void update_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *s
 	if (se->avg.last_update_time && !(flags & SKIP_AGE_LOAD))
 		__update_load_avg_se(now, cpu, cfs_rq, se);
 
-	decayed  = update_cfs_rq_load_avg(now, cfs_rq, true);
+	decayed  = update_cfs_rq_load_avg(now, cfs_rq, !(flags & SKIP_CPUFREQ));
 	decayed |= propagate_entity_load_avg(se);
 
 	if (!se->avg.last_update_time && (flags & DO_ATTACH)) {
@@ -3839,6 +3840,7 @@ update_cfs_rq_load_avg(u64 now, struct cfs_rq *cfs_rq, bool update_freq)
 #define UPDATE_TG	0x0
 #define SKIP_AGE_LOAD	0x0
 #define DO_ATTACH	0x0
+#define SKIP_CPUFREQ	0x0
 
 static inline void update_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se, int not_used1)
 {
@@ -4060,6 +4062,8 @@ static __always_inline void return_cfs_rq_runtime(struct cfs_rq *cfs_rq);
 static void
 dequeue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
 {
+	int update_flags;
+
 	/*
 	 * Update run-time statistics of the 'current'.
 	 */
@@ -4073,7 +4077,12 @@ dequeue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
 	 *   - For group entity, update its weight to reflect the new share
 	 *     of its group cfs_rq.
 	 */
-	update_load_avg(cfs_rq, se, UPDATE_TG);
+	update_flags = UPDATE_TG;
+
+	if (flags & DEQUEUE_IDLE)
+		update_flags |= SKIP_CPUFREQ;
+
+	update_load_avg(cfs_rq, se, update_flags);
 	dequeue_runnable_load_avg(cfs_rq, se);
 
 	update_stats_dequeue(cfs_rq, se, flags);
@@ -5220,6 +5229,9 @@ static void dequeue_task_fair(struct rq *rq, struct task_struct *p, int flags)
 	struct sched_entity *se = &p->se;
 	int task_sleep = flags & DEQUEUE_SLEEP;
 
+	if (task_sleep && rq->nr_running == 1)
+		flags |= DEQUEUE_IDLE;
+
 	for_each_sched_entity(se) {
 		cfs_rq = cfs_rq_of(se);
 		dequeue_entity(cfs_rq, se, flags);
@@ -5250,13 +5262,20 @@ static void dequeue_task_fair(struct rq *rq, struct task_struct *p, int flags)
 	}
 
 	for_each_sched_entity(se) {
+		int update_flags;
+
 		cfs_rq = cfs_rq_of(se);
 		cfs_rq->h_nr_running--;
 
 		if (cfs_rq_throttled(cfs_rq))
 			break;
 
-		update_load_avg(cfs_rq, se, UPDATE_TG);
+		update_flags = UPDATE_TG;
+
+		if (flags & DEQUEUE_IDLE)
+			update_flags |= SKIP_CPUFREQ;
+
+		update_load_avg(cfs_rq, se, update_flags);
 		update_cfs_group(se);
 	}
 
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 8aa24b41f652..68f5cd102744 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1394,6 +1394,7 @@ extern const u32 sched_prio_to_wmult[40];
 #define DEQUEUE_SAVE		0x02 /* matches ENQUEUE_RESTORE */
 #define DEQUEUE_MOVE		0x04 /* matches ENQUEUE_MOVE */
 #define DEQUEUE_NOCLOCK		0x08 /* matches ENQUEUE_NOCLOCK */
+#define DEQUEUE_IDLE		0x10
 
 #define ENQUEUE_WAKEUP		0x01
 #define ENQUEUE_RESTORE		0x02
-- 
2.15.0.rc2.357.g7e34df9404-goog

  parent reply	other threads:[~2017-10-28 10:00 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-28  9:59 [PATCH RFC 0/5] sched and cpufreq fixes/cleanups Joel Fernandes
2017-10-28  9:59 ` [PATCH RFC 1/5] Revert "sched/fair: Drop always true parameter of update_cfs_rq_load_avg()" Joel Fernandes
2017-10-28  9:59 ` Joel Fernandes [this message]
2017-10-30 12:07   ` [PATCH RFC 2/5] sched/fair: Skip frequency update if CPU about to idle Viresh Kumar
2017-10-30 19:02     ` Joel Fernandes
2017-11-01 19:35       ` Steve Muckle
2017-11-04  5:44         ` Joel Fernandes
2017-11-06  8:00           ` Vincent Guittot
2017-11-06  9:29             ` Uladzislau Rezki
2017-11-08  5:11             ` Joel Fernandes
2017-10-28  9:59 ` [PATCH RFC 3/5] cpufreq: schedutil: Use idle_calls counter of the remote CPU Joel Fernandes
2017-10-30  9:18   ` Viresh Kumar
2017-10-28  9:59 ` [PATCH RFC 4/5] sched/fair: Correct obsolete comment about cpufreq_update_util Joel Fernandes
2017-10-30  9:22   ` Viresh Kumar
2017-10-30 19:16     ` Joel Fernandes
2017-10-28  9:59 ` [PATCH RFC 5/5] sched/fair: remove impossible condition from find_idlest_group_cpu Joel Fernandes
2017-10-30 15:41   ` Brendan Jackman
2017-10-30 16:00   ` Vincent Guittot
2017-10-30 16:19     ` Joel Fernandes
2017-10-30 16:26       ` Vincent Guittot

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=20171028095941.4773-3-joelaf@google.com \
    --to=joelaf@google.com \
    --cc=Chris.Redpath@arm.com \
    --cc=atish.patra@oracle.com \
    --cc=brendan.jackman@arm.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=eas-dev@lists.linaro.org \
    --cc=fweisbec@gmail.com \
    --cc=juri.lelli@arm.com \
    --cc=kernel-team@android.com \
    --cc=lenb@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=morten.rasmussen@arm.com \
    --cc=patrick.bellasi@arm.com \
    --cc=peterz@infradead.org \
    --cc=rjw@rjwysocki.net \
    --cc=smuckle@google.com \
    --cc=srinivas.pandruvada@linux.intel.com \
    --cc=tglx@linutronix.de \
    --cc=vincent.guittot@linaro.org \
    --cc=viresh.kumar@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).