Messages in this thread | | | Date | Tue, 20 Oct 2020 09:14:22 -0400 (EDT) | From | Mathieu Desnoyers <> | Subject | Re: [LKP] Re: [sched] bdfcae1140: will-it-scale.per_thread_ops -37.0% regression |
| |
----- On Oct 19, 2020, at 11:24 PM, Xing Zhengjun zhengjun.xing@linux.intel.com wrote:
> On 10/7/2020 10:50 PM, Mathieu Desnoyers wrote: >> ----- On Oct 2, 2020, at 4:33 AM, Rong Chen rong.a.chen@intel.com wrote: >> >>> Greeting, >>> >>> FYI, we noticed a -37.0% regression of will-it-scale.per_thread_ops due to >>> commit: >>> >>> >>> commit: bdfcae11403e5099769a7c8dc3262e3c4193edef ("[RFC PATCH 2/3] sched: >>> membarrier: cover kthread_use_mm (v3)") >>> url: >>> https://github.com/0day-ci/linux/commits/Mathieu-Desnoyers/Membarrier-updates/20200925-012549 >>> base: https://git.kernel.org/cgit/linux/kernel/git/tip/tip.git >>> 848785df48835eefebe0c4eb5da7690690b0a8b7 >>> >>> in testcase: will-it-scale >>> on test machine: 104 threads Skylake with 192G memory >>> with following parameters: >>> >>> nr_task: 50% >>> mode: thread >>> test: context_switch1 >>> cpufreq_governor: performance >>> ucode: 0x2006906 >>> >>> test-description: Will It Scale takes a testcase and runs it from 1 through to n >>> parallel copies to see if the testcase will scale. It builds both a process and >>> threads based test in order to see any differences between the two. >>> test-url: https://github.com/antonblanchard/will-it-scale >>> >> >> Hi, >> >> I would like to report what I suspect is a random thread placement issue in the >> context_switch1 test used by the 0day bot when running on a machine with >> hyperthread >> enabled. >> >> AFAIU the test code uses hwloc for thread placement which should theoretically >> ensure >> that each thread is placed on same processing unit, core and numa node between >> runs. >> >> We can find the test code here: >> >> https://github.com/antonblanchard/will-it-scale/blob/master/tests/context_switch1.c >> >> And the main file containing thread setup is here: >> >> https://github.com/antonblanchard/will-it-scale/blob/master/main.c >> >> AFAIU, the test is started without the "-m" switch, which therefore affinitizes >> tasks on cores rather than on processing units (SMT threads). >> >> When testcase() creates the child thread with new_task(), it basically issues: >> >> pthread_create(&threads[nr_threads++], NULL, func, arg); >> >> passing a NULL pthread_attr_t, and not executing any pre_trampoline on the >> child. >> The pre_trampoline would have issued hwloc_set_thread_cpubind if it were >> executed on >> the child, but it's not. Therefore, we expect the cpu affinity mask of the >> parent to >> be copied on clone and used by the child. >> >> A quick test on a machine with hyperthreading enabled shows that the cpu >> affinity mask >> for the parent and child has two bits set: >> >> taskset -p 1868607 >> pid 1868607's current affinity mask: 10001 >> taskset -p 1868606 >> pid 1868606's current affinity mask: 10001 >> >> So AFAIU the placement of the parent and child will be random on either the same >> processing unit, or on separate processing units within the same core. >> >> I suspect this randomness can significantly affect the performance number >> between >> runs, and trigger unwarranted performance regression warnings. >> >> Thanks, >> >> Mathieu >> > Yes, the randomness may happen in some special cases. But in 0-day, we > test multi times (>=3), the report is the average number. > For this case, we test 4 times, it is stable, the wave is ± 2%. > So I don't think the -37.0% regression is caused by the randomness. > > 0/stats.json: "will-it-scale.per_thread_ops": 105228, > 1/stats.json: "will-it-scale.per_thread_ops": 100443, > 2/stats.json: "will-it-scale.per_thread_ops": 98786, > 3/stats.json: "will-it-scale.per_thread_ops": 102821, > > c2daff748f0ea954 bdfcae11403e5099769a7c8dc32 > ---------------- --------------------------- > %stddev %change %stddev > \ | \ > 161714 ± 2% -37.0% 101819 ± 2% will-it-scale.per_thread_ops
Arguing whether this specific instance of the test is indeed a performance regression or not is not relevant to this discussion.
What I am pointing out here is that the test needs fixing because it generates noise due to a random thread placement configuration. This issue is about whether we can trust the results of those tests as kernel maintainers.
So on one hand, you can fix the test. This is simple to do: make sure the thread affinity does not allow for this randomness on SMT.
But you seem to argue that the test does not need to be fixed, because the 0day infrastructure in which it runs will cover for this randomness. I really doubt about this.
If you indeed choose to argue that the test does not need fixing, then here is the statistical analysis I am looking for:
- With the 4 runs, what are the odds that the average result for one class significantly differs from the other class due to this randomness. It may be small, but it is certainly not zero, - Based on those odds, and on the number of performance regression tests performed by 0day each year, how frequently does 0day end up spamming kernel developers with random results because of this randomness ?
That being said, I would really find more productive that we work together on fixing the test rather than justifying why it can stay broken. Let me know if you have specific questions on how to fix the test, and I'll be happy to help out.
Thanks,
Mathieu
-- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com
| |