Re: [LKP] [sched/fair] 6c8116c914: stress-ng.mmapfork.ops_per_sec -38.0% regression

From: Xing Zhengjun
Date: Fri Jun 12 2020 - 03:59:38 EST

Hi,

I test the regression, it still existed in v5.7. If you have any fix for it, please send it to me, I can verify it. Thanks.

=========================================================================================
tbox_group/testcase/rootfs/kconfig/compiler/nr_threads/disk/sc_pid_max/testtime/class/cpufreq_governor/ucode:

lkp-bdw-ep6/stress-ng/debian-x86_64-20191114.cgz/x86_64-rhel-7.6/gcc-7/100%/1HDD/4194304/1s/scheduler/performance/0xb000038

commit:
e94f80f6c49020008e6fa0f3d4b806b8595d17d8
6c8116c914b65be5e4d6f66d69c8142eb0648c22
v5.7-rc3
v5.7

e94f80f6c4902000 6c8116c914b65be5e4d6f66d69c v5.7-rc3 v5.7
---------------- --------------------------- --------------------------- ---------------------------
%stddev %change %stddev %change %stddev %change %stddev
\ | \ | \ | \
21398 Â 7% +6.5% 22781 Â 2% -14.5% 18287 Â 4% -5.5% 20231 Â 14% stress-ng.clone.ops
819250 Â 5% -10.1% 736616 Â 8% +34.2% 1099410 Â 5% +41.2% 1156877 Â 3% stress-ng.futex.ops
818985 Â 5% -10.1% 736460 Â 8% +34.2% 1099487 Â 5% +41.2% 1156215 Â 3% stress-ng.futex.ops_per_sec
1551 Â 3% -3.4% 1498 Â 5% -9.5% 1404 Â 2% -4.6% 1480 Â 5% stress-ng.inotify.ops
1547 Â 3% -3.5% 1492 Â 5% -9.5% 1400 Â 2% -4.8% 1472 Â 5% stress-ng.inotify.ops_per_sec
11292 Â 8% -2.8% 10974 Â 8% +1.9% 11505 Â 13% -9.4% 10225 Â 6% stress-ng.kill.ops
28.20 Â 4% -35.4% 18.22 -33.5% 18.75 -33.4% 18.77 stress-ng.mmapfork.ops_per_sec
1932318 +1.5% 1961688 Â 2% -22.8% 1492231 Â 2% +4.0% 2010509 Â 3% stress-ng.softlockup.ops
1931679 Â 2% +1.5% 1961143 Â 2% -22.8% 1491939 Â 2% +4.0% 2009585 Â 3% stress-ng.softlockup.ops_per_sec
18607406 Â 6% -12.9% 16210450 Â 21% -12.7% 16238693 Â 14% -8.0% 17120880 Â 13% stress-ng.switch.ops
18604406 Â 6% -12.9% 16208270 Â 21% -12.7% 16237956 Â 14% -8.0% 17115273 Â 13% stress-ng.switch.ops_per_sec
2999012 Â 21% -10.1% 2696954 Â 22% -9.1% 2725653 Â 21% -88.5% 344447 Â 11% stress-ng.tee.ops_per_sec
7882 Â 3% -5.4% 7458 Â 4% -4.0% 7566 Â 4% -2.0% 7724 Â 3% stress-ng.vforkmany.ops
7804 Â 3% -5.2% 7400 Â 4% -3.8% 7504 Â 4% -2.0% 7647 Â 3% stress-ng.vforkmany.ops_per_sec
46745421 Â 3% -8.1% 42938569 Â 3% -7.8% 43078233 Â 3% -5.2% 44312072 Â 4% stress-ng.yield.ops
46734472 Â 3% -8.1% 42926316 Â 3% -7.8% 43067447 Â 3% -5.2% 44290338 Â 4% stress-ng.yield.ops_per_sec


On 4/27/2020 8:46 PM, Vincent Guittot wrote:
On Mon, 27 Apr 2020 at 13:35, Hillf Danton <hdanton@xxxxxxxx> wrote:


On Mon, 27 Apr 2020 11:03:58 +0200 Vincent Guittot wrote:
On Sun, 26 Apr 2020 at 14:42, Hillf Danton wrote:

On 4/21/2020 8:47 AM, kernel test robot wrote:

Greeting,

FYI, we noticed a 56.4% improvement of stress-ng.fifo.ops_per_sec due to commit:


commit: 6c8116c914b65be5e4d6f66d69c8142eb0648c22 ("sched/fair: Fix condition of avg_load calculation")
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git master

in testcase: stress-ng
on test machine: 88 threads Intel(R) Xeon(R) CPU E5-2699 v4 @ 2.20GHz with 128G memory
with following parameters:

nr_threads: 100%
disk: 1HDD
testtime: 1s
class: scheduler
cpufreq_governor: performance
ucode: 0xb000038
sc_pid_max: 4194304


We need to handle group_fully_busy in a different way from
group_overloaded as task push does not help grow load balance
in the former case.

Have you tested this patch for the UC above ? Do you have figures ?

No I am looking for a box of 88 threads. Likely to get access to it in
as early as three weeks.

--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -8744,30 +8744,20 @@ find_idlest_group(struct sched_domain *s

switch (local_sgs.group_type) {
case group_overloaded:
- case group_fully_busy:
- /*
- * When comparing groups across NUMA domains, it's possible for
- * the local domain to be very lightly loaded relative to the
- * remote domains but "imbalance" skews the comparison making
- * remote CPUs look much more favourable. When considering
- * cross-domain, add imbalance to the load on the remote node
- * and consider staying local.
- */
-
- if ((sd->flags & SD_NUMA) &&
- ((idlest_sgs.avg_load + imbalance) >= local_sgs.avg_load))
+ if (100 * local_sgs.avg_load <= sd->imbalance_pct * (idlest_sgs.avg_load + imbalance))
+ return idlest;

So you have completely removed the NUMA special case without explaining why.

That was for the local domain that is lightly loaded, as the comment says,
it now is overloaded.

The load value is not linked to the overloaded state of the group as
you can be overloaded but still have a low load especially with cgroup

That's also why there are 2 type of comparison:
an absolute comparison for low load value
and a proportional comparison for normal/high value


And you have also removed the tests for small load.

It is a heuristic I want to avoid. It can be replaced with the load of the
task in question as best effort.

Could you explain the rationale behind all these changes ?

Also keep in mind that the current version provide +58% improvement
for stress-ng.fifo

Yes it's great. I'm on the minor one.

+ if (local_sgs.avg_load > idlest_sgs.avg_load + imbalance)
+ return idlest;
+ else
return NULL;

+ case group_fully_busy:
/*
- * If the local group is less loaded than the selected
- * idlest group don't try and push any tasks.
+ * Pushing task to the idlest group will make the target group
+ * overloaded, leaving the local group that is overloaded fully busy,
+ * thus we earn nothing except for the exchange of group types.

For this case both local and idlest are fully busy and in this case
one will become overloaded so you must compare the load to be fair in
the spread of load

It may be sooner than thought that the newly overloaded group is looking to
push task out, and we'll see a task ping-pong if it happens.


--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -8683,15 +8683,11 @@ find_idlest_group(struct sched_domain *s
struct sched_group *idlest = NULL, *local = NULL, *group = sd->groups;
struct sg_lb_stats local_sgs, tmp_sgs;
struct sg_lb_stats *sgs;
- unsigned long imbalance;
struct sg_lb_stats idlest_sgs = {
.avg_load = UINT_MAX,
.group_type = group_overloaded,
};

- imbalance = scale_load_down(NICE_0_LOAD) *
- (sd->imbalance_pct-100) / 100;
-
do {
int local_group;

@@ -8743,31 +8739,26 @@ find_idlest_group(struct sched_domain *s
return idlest;

switch (local_sgs.group_type) {
- case group_overloaded:
case group_fully_busy:
- /*
- * When comparing groups across NUMA domains, it's possible for
- * the local domain to be very lightly loaded relative to the
- * remote domains but "imbalance" skews the comparison making
- * remote CPUs look much more favourable. When considering
- * cross-domain, add imbalance to the load on the remote node
- * and consider staying local.
- */
-
- if ((sd->flags & SD_NUMA) &&
- ((idlest_sgs.avg_load + imbalance) >= local_sgs.avg_load))
- return NULL;
-
- /*
- * If the local group is less loaded than the selected
- * idlest group don't try and push any tasks.
- */
- if (idlest_sgs.avg_load >= (local_sgs.avg_load + imbalance))
- return NULL;
-
- if (100 * local_sgs.avg_load <= sd->imbalance_pct * idlest_sgs.avg_load)
- return NULL;
- break;
+ return NULL;
+ case group_overloaded:
+ /* No push if balanced */
+ if (100 * local_sgs.avg_load > sd->imbalance_pct *
+ idlest_sgs.avg_load) {
+ unsigned long avg_load;
+
+ avg_load = task_h_load(p) + idlest_sgs.group_load;
+ avg_load = (avg_load * SCHED_CAPACITY_SCALE) /
+ idlest_sgs.group_capacity;
+
+ if (100 * local_sgs.avg_load <= sd->imbalance_pct *
+ avg_load)
+ return idlest;
+
+ if (local_sgs.avg_load > avg_load)
+ return idlest;
+ }
+ return NULL;

case group_imbalanced:
case group_asym_packing:


--
Zhengjun Xing