linux-kernel - Re: [PATCH 4/7] sched/fair: Clean up the logic in fix_small

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160503101225.GM3430@twins.programming.kicks-ass.net>
Date:	Tue, 3 May 2016 12:12:25 +0200
From:	Peter Zijlstra <peterz@...radead.org>
To:	Dietmar Eggemann <dietmar.eggemann@....com>
Cc:	linux-kernel@...r.kernel.org,
	Morten Rasmussen <morten.rasmussen@....com>
Subject: Re: [PATCH 4/7] sched/fair: Clean up the logic in
 fix_small_imbalance()

On Fri, Apr 29, 2016 at 08:32:41PM +0100, Dietmar Eggemann wrote:
> 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.

This appears to have started with:

  dd41f596cda0 ("sched: cfs core code")

which for unexplained reasons does:

-               if (max_load - this_load >= busiest_load_per_task * imbn) {
+               if (max_load - this_load + SCHED_LOAD_SCALE_FUZZ >=
+                                       busiest_load_per_task * imbn) {

And later patches (by me) change that FUZZ into a variable metric,
because a fixed fuzz like that didn't at all work for the small loads
that result from cgroup tasks.

Now fix_small_imbalance() always hurt my head; it originated in the
original sched_domain balancer from Nick which wasn't smpnice aware; and
lives on until today.

Its purpose is to determine if moving one task over is beneficial.
However over time -- and smpnice started this -- the idea of _one_ task
became quite muddled.

With the fine grained load accounting of today; does it even make sense
to ask this question? IOW. what does fix_small_imbalance() really gain
us -- other than a head-ache?