linux-kernel - Re: [LKP] [cpuidle] 259231a045: will-it-scale.per_process

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Fri, 3 Jan 2020 10:31:17 +0800
From:   Feng Tang <feng.tang@...el.com>
To:     kernel test robot <rong.a.chen@...el.com>,
        Marcelo Tosatti <mtosatti@...hat.com>,
        "Rafael J. Wysocki" <rafael.j.wysocki@...el.com>
Cc:     Stephen Rothwell <sfr@...b.auug.org.au>, lkp@...org,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [LKP] [cpuidle] 259231a045: will-it-scale.per_process_ops -12.6%
 regression

On Tue, Dec 31, 2019 at 01:59:23PM +0800, Feng Tang wrote:
> Hi Marcelo,
> 
> On Wed, Sep 18, 2019 at 10:13:34AM +0800, kernel test robot wrote:
> > Greeting,
> > 
> > FYI, we noticed a -12.6% regression of will-it-scale.per_process_ops due to commit:
> > 
> > 
> > commit: 259231a045616c4101d023a8f4dcc8379af265a6 ("cpuidle: add poll_limit_ns to cpuidle_device structure")
> > https://kernel.googlesource.com/pub/scm/linux/kernel/git/next/linux-next.git master
> 
> Any comments on this? We re-run the test for 5.5-rc1, and the regression remains.

Anyway, I found commit 259231a04 lost one "break" when moving
the original code, thus the semantics is changed to the last
enabled state's target_residency instead of the first enabled
one's.

I don't know if it's intentional, and I guess no, so here 
is a fix patch, please review, thanks

But even with this patch, the regression is still not recovered.

- Feng

>From cddd6b409e18ce97a8d7b851db4400396f71d857 Mon Sep 17 00:00:00 2001
From: Feng Tang <feng.tang@...el.com>
Date: Thu, 2 Jan 2020 16:58:31 +0800
Subject: [PATCH] cpuidle: Add back the lost break in cpuidle_poll_time

Commit c4cbb8b649b5 move the poll time calculation into a
new function cpuidle_poll_time(), during which one "break"
get lost, and the semantic is changed from the last enabled
state's target_residency instead of the first enabled one's.

So add it back.

Fixes: c4cbb8b649b5 "cpuidle: add poll_limit_ns to cpuidle_device structure"
Signed-off-by: Feng Tang <feng.tang@...el.com>
Cc: Marcelo Tosatti <mtosatti@...hat.com>
---
 drivers/cpuidle/cpuidle.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
index 0895b98..29d2d7a 100644
--- a/drivers/cpuidle/cpuidle.c
+++ b/drivers/cpuidle/cpuidle.c
@@ -384,6 +384,7 @@ u64 cpuidle_poll_time(struct cpuidle_driver *drv,
 			continue;
 
 		limit_ns = (u64)drv->states[i].target_residency * NSEC_PER_USEC;
+		break;
 	}
 
 	dev->poll_limit_ns = limit_ns;
-- 
2.7.4

> 
> Thanks,
> Feng
> 
> > 
> > in testcase: will-it-scale
> > on test machine: 288 threads Intel(R) Xeon Phi(TM) CPU 7295 @ 1.50GHz with 80G memory
> > with following parameters:
> > 
> > 	nr_task: 100%
> > 	mode: process
> > 	test: mmap1
> > 	cpufreq_governor: performance
> > 
> > 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
> > 
> > 
> > 
> > If you fix the issue, kindly add following tag
> > Reported-by: kernel test robot <rong.a.chen@...el.com>
> > 
> > 
> > Details are as below:
> > -------------------------------------------------------------------------------------------------->
> > 
> > 
> > To reproduce:
> > 
> >         git clone https://github.com/intel/lkp-tests.git
> >         cd lkp-tests
> >         bin/lkp install job.yaml  # job file is attached in this email
> >         bin/lkp run     job.yaml
> > 
> > =========================================================================================
> > compiler/cpufreq_governor/kconfig/mode/nr_task/rootfs/tbox_group/test/testcase:
> >   gcc-7/performance/x86_64-rhel-7.6/process/100%/debian-x86_64-2019-05-14.cgz/lkp-knm01/mmap1/will-it-scale
> > 
> > commit: 
> >   fa86ee90eb ("add cpuidle-haltpoll driver")
> >   259231a045 ("cpuidle: add poll_limit_ns to cpuidle_device structure")
> > 
> > fa86ee90eb111126 259231a045616c4101d023a8f4d 
> > ---------------- --------------------------- 
> >        fail:runs  %reproduction    fail:runs
> >            |             |             |    
> >            :4           25%           1:4     dmesg.WARNING:at#for_ip_swapgs_restore_regs_and_return_to_usermode/0x
> >          %stddev     %change         %stddev
> >              \          |                \  
> >       1611           -12.6%       1408        will-it-scale.per_process_ops
> >     464144           -12.6%     405580        will-it-scale.workload
> >       1581 ±  2%      +3.3%       1633        vmstat.system.cs