Re: [sched/fair] b4d95a034c: phoronix-test-suite.tiobench.RandomWrite.64MB.8.mb_s -26.3% regression

[Mel Gorman <mgorman@techsingularity.net>]

[-- Attachment #1: Type: text/plain, Size: 2511 bytes --]

On Sun, Nov 28, 2021 at 11:06:58PM +0800, kernel test robot wrote:
> 
> 
> Greeting,
> 
> FYI, we noticed a -26.3% regression of phoronix-test-suite.tiobench.RandomWrite.64MB.8.mb_s due to commit:
> 
> 
> commit: b4d95a034cffb1e4424874645549d3cac2de5c02 ("[PATCH 2/2] sched/fair: Adjust the allowed NUMA imbalance when SD_NUMA spans multiple LLCs")
> url: https://github.com/0day-ci/linux/commits/Mel-Gorman/Adjust-NUMA-imbalance-for-multiple-LLCs/20211125-232336
> base: https://git.kernel.org/cgit/linux/kernel/git/tip/tip.git 8c92606ab81086db00cbb73347d124b4eb169b7e
> 
> in testcase: phoronix-test-suite
> on test machine: 96 threads 2 sockets Intel(R) Xeon(R) Gold 6252 CPU @ 2.10GHz with 128G memory
> with following parameters:
> 
> 	test: tiobench-1.3.1
> 	option_a: Random Write
> 	option_b: 64MB
> 	option_c: 8
> 	cpufreq_governor: performance
> 	ucode: 0x5003006
> 
> test-description: The Phoronix Test Suite is the most comprehensive testing and benchmarking platform available that provides an extensible framework for which new tests can be easily added.
> test-url: http://www.phoronix-test-suite.com/
> 

Ok, while I'm surprised there is a difference with tiobench, there
definitely is a problem with the patch and a v3 is needed. Am queueing
up a test of v3 but the diff is

diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
index 9adeaa89ccb4..fee2930745ab 100644
--- a/kernel/sched/topology.c
+++ b/kernel/sched/topology.c
@@ -2244,25 +2244,21 @@ build_sched_domains(const struct cpumask *cpu_map, struct sched_domain_attr *att
 
 	/* Calculate allowed NUMA imbalance */
 	for_each_cpu(i, cpu_map) {
+		int imb_numa_nr = 0;
+
 		for (sd = *per_cpu_ptr(d.sd, i); sd; sd = sd->parent) {
 			struct sched_domain *child = sd->child;
 
-			if (!(sd->flags & SD_SHARE_PKG_RESOURCES) &&
+			if (!(sd->flags & SD_SHARE_PKG_RESOURCES) && child &&
 			    (child->flags & SD_SHARE_PKG_RESOURCES)) {
-				struct sched_domain *sd_numa = sd;
-				int imb_numa_nr, nr_groups;
+				int nr_groups;
 
 				nr_groups = sd->span_weight / child->span_weight;
-				imb_numa_nr = nr_groups / num_online_nodes();
-
-				while (sd_numa) {
-					if (sd_numa->flags & SD_NUMA) {
-						sd_numa->imb_numa_nr = imb_numa_nr;
-						break;
-					}
-					sd_numa = sd_numa->parent;
-				}
+				imb_numa_nr = max(1U, ((child->span_weight) >> 1) /
+						(nr_groups * num_online_nodes()));
 			}
+
+			sd->imb_numa_nr = imb_numa_nr;
 		}
 	}