mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Dietmar Eggemann <dietmar.eggemann@arm.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: linux-kernel@vger.kernel.org,
	Morten Rasmussen <morten.rasmussen@arm.com>
Subject: [PATCH 4/7] sched/fair: Clean up the logic in fix_small_imbalance()
Date: Fri, 29 Apr 2016 20:32:41 +0100	[thread overview]
Message-ID: <1461958364-675-5-git-send-email-dietmar.eggemann@arm.com> (raw)
In-Reply-To: <1461958364-675-1-git-send-email-dietmar.eggemann@arm.com>

Avoid the need to add scaled_busy_load_per_task on both sides of the if
condition to determine whether imbalance has to be set to
busiest->load_per_task or not.

The imbn variable was introduced with commit 2dd73a4f09be ("[PATCH]
sched: implement smpnice") and the original if condition was

  if (max_load - this_load >= busiest_load_per_task * imbn)

which over time changed into the current version where
scaled_busy_load_per_task is to be found on both sides of
the if condition.

Signed-off-by: Dietmar Eggemann <dietmar.eggemann@arm.com>
---

The original smpnice implementation sets imbalance to the avg task
load of the busiest sched group (sg) if the difference between the avg 
load of the busiest sg and the local sg is greater or equal 2 times
(imbn) the avg task load of the busiest sg if there is no task running
in the local sg or the avg task load of the busiest sg is smaller or
equal the avg task load of the local sg. Otherwise the imbn factor is
lowered to 1.

imbn set to 2 makes sense when all the tasks have the same priority so
in case there are n tasks on local sd there have to be n+2 tasks on the 
busiest sg to give load balance the chance to pull over one task.

imbn set to 1 makes sense when the avg task load of the busiest sg is
greater than the one of the local sg since there could be at least one 
task with a smaller load on the busiest sg which can be pulled over to
the local sg.

The current version lowered imbn effectively by one, so we use 1 instead
of 2 in case the avg task load of sg_busiest isn't greater than the one 
of sg_local and 0 instead of 1 in the other case. This behaviour is not in
sync with the explanation above on why we set imbn to certain values.

 kernel/sched/fair.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index c066574cff04..dc4828bbe50d 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -6915,7 +6915,7 @@ static inline
 void fix_small_imbalance(struct lb_env *env, struct sd_lb_stats *sds)
 {
 	unsigned long tmp, capa_now = 0, capa_move = 0;
-	unsigned int imbn = 2;
+	unsigned int imbn = 1;
 	unsigned long scaled_busy_load_per_task;
 	struct sg_lb_stats *local, *busiest;
 
@@ -6925,13 +6925,13 @@ void fix_small_imbalance(struct lb_env *env, struct sd_lb_stats *sds)
 	if (!local->sum_nr_running)
 		local->load_per_task = cpu_avg_load_per_task(env->dst_cpu);
 	else if (busiest->load_per_task > local->load_per_task)
-		imbn = 1;
+		imbn = 0;
 
 	scaled_busy_load_per_task =
 		(busiest->load_per_task * SCHED_CAPACITY_SCALE) /
 		busiest->group_capacity;
 
-	if (busiest->avg_load + scaled_busy_load_per_task >=
+	if (busiest->avg_load >=
 	    local->avg_load + (scaled_busy_load_per_task * imbn)) {
 		env->imbalance = busiest->load_per_task;
 		return;
-- 
1.9.1

  parent reply	other threads:[~2016-04-29 19:33 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-29 19:32 [PATCH 0/7] sched/fair: fixes and cleanups Dietmar Eggemann
2016-04-29 19:32 ` [PATCH 1/7] sched/fair: Remove remaining power aware scheduling comments Dietmar Eggemann
2016-05-05  9:42   ` [tip:sched/core] sched/fair: Remove stale " tip-bot for Dietmar Eggemann
2016-04-29 19:32 ` [PATCH 2/7] sched/fair: Fix comment in calculate_imbalance() Dietmar Eggemann
2016-05-05  9:42   ` [tip:sched/core] " tip-bot for Dietmar Eggemann
2016-04-29 19:32 ` [PATCH 3/7] sched/fair: Correct unit of load_above_capacity Dietmar Eggemann
2016-05-03 10:52   ` Peter Zijlstra
2016-05-03 14:56     ` Morten Rasmussen
2016-05-12 10:31   ` [tip:sched/core] " tip-bot for Morten Rasmussen
2016-05-12 21:48     ` Yuyang Du
2016-05-13  8:22       ` Vincent Guittot
2016-05-19 15:36         ` Morten Rasmussen
2016-05-20  8:17           ` Vincent Guittot
2016-05-23 20:24           ` Yuyang Du
2016-05-30 22:35             ` Yuyang Du
2016-04-29 19:32 ` Dietmar Eggemann [this message]
2016-05-03 10:12   ` [PATCH 4/7] sched/fair: Clean up the logic in fix_small_imbalance() Peter Zijlstra
2016-05-03 16:53     ` Dietmar Eggemann
2016-04-29 19:32 ` [PATCH 5/7] sched/fair: Remove cpu_avg_load_per_task() Dietmar Eggemann
2016-04-29 19:32 ` [PATCH 6/7] sched/fair: Reorder code in update_sd_lb_stats() Dietmar Eggemann
2016-04-29 19:32 ` [PATCH 7/7] sched/fair: Use group_cfs_rq(se) instead of se->my_q Dietmar Eggemann

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=1461958364-675-5-git-send-email-dietmar.eggemann@arm.com \
    --to=dietmar.eggemann@arm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=morten.rasmussen@arm.com \
    --cc=peterz@infradead.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

all inboxes | Powered by JetHome®