All of lore.kernel.org
 help / color / mirror / Atom feed
From: Valentin Schneider <valentin.schneider@arm.com>
To: linux-kernel@vger.kernel.org
Cc: Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@kernel.org>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	Morten Rasmussen <morten.rasmussen@arm.com>,
	Qais Yousef <qais.yousef@arm.com>,
	Quentin Perret <qperret@google.com>,
	Pavan Kondeti <pkondeti@codeaurora.org>,
	Rik van Riel <riel@surriel.com>
Subject: [PATCH 7/8] sched/fair: Attempt misfit active balance when migration_type != migrate_misfit
Date: Thu, 28 Jan 2021 18:31:40 +0000	[thread overview]
Message-ID: <20210128183141.28097-8-valentin.schneider@arm.com> (raw)
In-Reply-To: <20210128183141.28097-1-valentin.schneider@arm.com>

Consider a 4-CPU big.LITTLE system with CPUs 0-1 as LITTLEs and CPUs 2-3 as
bigs. The resulting sched_domain hierarchy is:

  DIE [          ]
  MC  [    ][    ]
       0  1  2  3

When running a multithreaded CPU-bound workload (i.e. 1 hog per CPU), the
expected behaviour is to have the about-to-idle big CPUs pull a hog from
the LITTLEs, since bigs will complete their work sooner than LITTLEs.

Further Consider a scenario where:
- CPU0 is idle (e.g. its hog got migrated to one of the big CPUs)
- CPU1 is currently executing a per-CPU kworker, preempting the CPU hog
- CPU2 and CPU3 are executing CPU-hogs

CPU0 goes through load_balance() at MC level, and tries to pick stuff from
CPU1, but:
- the hog can't be pulled, because it's task_hot()
- the kworker can't be pulled, because it's pinned to CPU1, which sets
  LBF_SOME_PINNED

This load balance attempts ends with no load pulled, LBF_SOME_PINNED set,
and as a consequence we set the imbalance flag of DIE's [0, 1]
sched_group_capacity.

Shortly after, CPU2 completes its work and is about to go idle. It goes
through the newidle_balance(), and we would really like it to active
balance the hog running on CPU1 (which is a misfit task). However,
sgc->imbalance is set for the LITTLE group at DIE level, so the group gets
classified as group_imbalanced rather than group_misfit_task.

Unlike group_misfit_task (via migrate_misfit), the active balance logic
doesn't have any specific case for group_imbalanced, so CPU2 ends up going
idle. We'll have to wait for a load balance on CPU0 or CPU1 to happen and
clear the imbalance flag, and then for another DIE-level load-balance on
CPU2 to happen to pull the task off of CPU1. That's several precious
milliseconds wasted down the drain.

Giving group_misfit_task a higher group_classify() priority than
group_imbalance doesn't seem like the right thing to do. Instead, make
need_active_balance() return true for any migration_type when the
destination CPU is idle and the source CPU has a misfit task.

While at it, add an sd_has_asym_cpucapacity() guard in
need_active_balance().

Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
---
 kernel/sched/fair.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 0ac2f876b86f..cba9f97d9beb 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -9557,9 +9557,22 @@ static int need_active_balance(struct lb_env *env)
 			return 1;
 	}
 
+	if (!sd_has_asym_cpucapacity(sd))
+		return 0;
+
 	if (env->migration_type == migrate_misfit)
 		return 1;
 
+	/*
+	 * If we failed to pull anything and the src_rq has a misfit task, but
+	 * the busiest group_type was higher than group_misfit_task, try to
+	 * go for a misfit active balance anyway.
+	 */
+	if ((env->idle != CPU_NOT_IDLE) &&
+	    env->src_rq->misfit_task_load &&
+	    cpu_capacity_greater(env->dst_cpu, env->src_cpu))
+		return 1;
+
 	return 0;
 }
 
-- 
2.27.0


  parent reply	other threads:[~2021-01-28 18:41 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-28 18:31 [PATCH 0/8] sched/fair: misfit task load-balance tweaks Valentin Schneider
2021-01-28 18:31 ` [PATCH 1/8] sched/fair: Clean up active balance nr_balance_failed trickery Valentin Schneider
2021-02-03 15:14   ` Qais Yousef
2021-02-03 18:42     ` Valentin Schneider
2021-02-04 15:05       ` Qais Yousef
2021-02-05 13:51   ` Vincent Guittot
2021-02-05 14:05     ` Valentin Schneider
2021-02-05 14:34       ` Vincent Guittot
2021-01-28 18:31 ` [PATCH 2/8] sched/fair: Add more sched_asym_cpucapacity static branch checks Valentin Schneider
2021-02-03 15:14   ` Qais Yousef
2021-02-09  8:42   ` Vincent Guittot
2021-01-28 18:31 ` [PATCH 3/8] sched/fair: Tweak misfit-related capacity checks Valentin Schneider
2021-02-03 15:15   ` Qais Yousef
2021-02-03 18:42     ` Valentin Schneider
2021-02-05 14:31   ` Vincent Guittot
2021-02-05 16:59     ` Valentin Schneider
2021-02-05 17:17       ` Vincent Guittot
2021-02-05 20:07         ` Valentin Schneider
2021-02-08 15:29           ` Vincent Guittot
2021-02-08 17:49             ` Valentin Schneider
2021-01-28 18:31 ` [PATCH 4/8] sched/fair: Use dst_cpu's capacity rather than group {min, max} capacity Valentin Schneider
2021-02-03 15:15   ` Qais Yousef
2021-01-28 18:31 ` [PATCH 5/8] sched/fair: Make check_misfit_status() only compare dynamic capacities Valentin Schneider
2021-02-03 15:15   ` Qais Yousef
2021-02-04 10:49     ` Dietmar Eggemann
2021-02-04 11:34       ` Valentin Schneider
2021-02-04 14:57         ` Dietmar Eggemann
2021-01-28 18:31 ` [PATCH 6/8] sched/fair: Filter out locally-unsolvable misfit imbalances Valentin Schneider
2021-02-03 15:16   ` Qais Yousef
2021-02-03 18:43     ` Valentin Schneider
2021-01-28 18:31 ` Valentin Schneider [this message]
2021-02-03 15:16   ` [PATCH 7/8] sched/fair: Attempt misfit active balance when migration_type != migrate_misfit Qais Yousef
2021-02-03 18:43     ` Valentin Schneider
2021-02-04 11:44       ` Dietmar Eggemann
2021-02-04 12:22         ` Valentin Schneider
2021-02-09  8:58   ` Vincent Guittot
2021-02-09 18:19     ` Valentin Schneider
2021-01-28 18:31 ` [PATCH 8/8] sched/fair: Relax task_hot() for misfit tasks Valentin Schneider
2021-02-03 15:17   ` Qais Yousef
2021-02-08 16:21   ` Vincent Guittot
2021-02-08 18:24     ` Valentin Schneider
2021-02-09  8:56       ` Vincent Guittot
2021-02-03 15:14 ` [PATCH 0/8] sched/fair: misfit task load-balance tweaks Qais Yousef
2021-02-03 18:43   ` Valentin Schneider
2021-02-04 12:03     ` Dietmar Eggemann
2021-02-04 12:36       ` Valentin Schneider

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=20210128183141.28097-8-valentin.schneider@arm.com \
    --to=valentin.schneider@arm.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=morten.rasmussen@arm.com \
    --cc=peterz@infradead.org \
    --cc=pkondeti@codeaurora.org \
    --cc=qais.yousef@arm.com \
    --cc=qperret@google.com \
    --cc=riel@surriel.com \
    --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.