From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.2 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4EFB6C433B4 for ; Wed, 28 Apr 2021 14:19:50 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id A8E9D6143E for ; Wed, 28 Apr 2021 14:19:49 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org A8E9D6143E Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id F0E506B0036; Wed, 28 Apr 2021 10:19:48 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id EBE0D6B006E; Wed, 28 Apr 2021 10:19:48 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D10F46B0070; Wed, 28 Apr 2021 10:19:48 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0067.hostedemail.com [216.40.44.67]) by kanga.kvack.org (Postfix) with ESMTP id B28416B0036 for ; Wed, 28 Apr 2021 10:19:48 -0400 (EDT) Received: from smtpin09.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 6C203349B for ; Wed, 28 Apr 2021 14:19:48 +0000 (UTC) X-FDA: 78081984456.09.69A970B Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by imf27.hostedemail.com (Postfix) with ESMTP id E8E9380192FF for ; Wed, 28 Apr 2021 14:19:24 +0000 (UTC) IronPort-SDR: p1iuP2IZCo5NOkxCCbZzC4qguzvjOnoArHkkJbFFgUaZB2TbhE+N5jjW7JOYNR+iqSp8vkB/sd fA93FiBJmFKQ== X-IronPort-AV: E=McAfee;i="6200,9189,9968"; a="196306009" X-IronPort-AV: E=Sophos;i="5.82,258,1613462400"; d="scan'208";a="196306009" Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 28 Apr 2021 07:19:38 -0700 IronPort-SDR: 8rUhjorvtxF/HGQdYw3usVMvrlVGmZL8pmmC65c8BhQFN/jD7jhOr1/4uCxWG20pSd78h6vqhw BH5BM4X2ANIg== X-IronPort-AV: E=Sophos;i="5.82,258,1613462400"; d="scan'208";a="619327674" Received: from xsang-optiplex-9020.sh.intel.com (HELO xsang-OptiPlex-9020) ([10.239.159.140]) by fmsmga005-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 28 Apr 2021 07:19:33 -0700 Date: Wed, 28 Apr 2021 22:36:40 +0800 From: Oliver Sang To: Alexey Gladkov Cc: "Eric W. Biederman" , Linus Torvalds , Alexey Gladkov , 0day robot , LKML , lkp@lists.01.org, "Huang, Ying" , Feng Tang , zhengjun.xing@intel.com, Kernel Hardening , Linux Containers , Linux-MM , Andrew Morton , Christian Brauner , Jann Horn , Jens Axboe , Kees Cook , Oleg Nesterov Subject: Re: 08ed4efad6: stress-ng.sigsegv.ops_per_sec -41.9% regression Message-ID: <20210428143008.GA19916@xsang-OptiPlex-9020> References: <7abe5ab608c61fc2363ba458bea21cf9a4a64588.1617814298.git.gladkov.alexey@gmail.com> <20210408083026.GE1696@xsang-OptiPlex-9020> <20210423024722.GA13968@xsang-OptiPlex-9020> <20210423074431.7ob6aqasome2zjbk@example.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20210423074431.7ob6aqasome2zjbk@example.org> User-Agent: Mutt/1.9.4 (2018-02-28) X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: E8E9380192FF X-Stat-Signature: 4bypqkfxrwk5rfzm53yt96farxo9bcd1 Received-SPF: none (intel.com>: No applicable sender policy available) receiver=imf27; identity=mailfrom; envelope-from=""; helo=mga14.intel.com; client-ip=192.55.52.115 X-HE-DKIM-Result: none/none X-HE-Tag: 1619619564-819000 Content-Transfer-Encoding: quoted-printable X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: hi, Alexey Gladkov, On Fri, Apr 23, 2021 at 09:44:31AM +0200, Alexey Gladkov wrote: > On Fri, Apr 23, 2021 at 10:47:22AM +0800, Oliver Sang wrote: > > hi, Eric, > >=20 > > On Thu, Apr 08, 2021 at 01:44:43PM -0500, Eric W. Biederman wrote: > > > Linus Torvalds writes: > > >=20 > > > > On Thu, Apr 8, 2021 at 1:32 AM kernel test robot wrote: > > > >> > > > >> FYI, we noticed a -41.9% regression of stress-ng.sigsegv.ops_per= _sec due to commit > > > >> 08ed4efad684 ("[PATCH v10 6/9] Reimplement RLIMIT_SIGPENDING on = top of ucounts") > > > > > > > > Ouch. > > >=20 > > > We were cautiously optimistic when no test problems showed up from > > > the last posting that there was nothing to look at here. > > >=20 > > > Unfortunately it looks like the bots just missed the last posting.=20 > >=20 > > this report is upon v10. do you have newer version which hope bot tes= t? >=20 > Yes. I posted a new version of this patch set. I would be very grateful= if > you could test it. >=20 > https://lore.kernel.org/lkml/cover.1619094428.git.legion@kernel.org/ > we tested this v11 version, and found the regression reduced to about 1.6= %. please be noted, according to our previous experience, the stress-ng is kind of sensitive testsuite, so we normally wouldn't report <3% regressio= n. =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D class/compiler/cpufreq_governor/disk/kconfig/nr_threads/rootfs/tbox_group= /test/testcase/testtime/ucode: interrupt/gcc-9/performance/1HDD/x86_64-rhel-8.3/100%/debian-10.4-x86_6= 4-20200603.cgz/lkp-ivb-2ep1/sigsegv/stress-ng/60s/0x42e commit: 00a58a6af1c4 ("Reimplement RLIMIT_MSGQUEUE on top of ucounts") 8932738fc10c ("Reimplement RLIMIT_SIGPENDING on top of ucounts") 00a58a6af1c473c5 8932738fc10c4398521892adfe6 ---------------- --------------------------- %stddev %change %stddev \ | \ 4.745e+08 -1.6% 4.669e+08 stress-ng.sigsegv.ops 7908964 -1.6% 7781343 stress-ng.sigsegv.ops_per_s= ec Below is some data of results from your new branch and base. b3ad8e1fa3fd8 ucounts: Set ucount_max to the largest positive value the t= ype can hold 7783421.61 7794441.59 7775793.52 7773683.6 7760744.1 775772= 0.33 8932738fc10c4 Reimplement RLIMIT_SIGPENDING on top of ucounts = 7755985.06 7780646.72 7783944.12 7809090.98 7798193.32 7760= 202.59 00a58a6af1c47 Reimplement RLIMIT_MSGQUEUE on top of ucounts = 7940474.72 7912442.26 7879195.61 7869803.63 7912693.69 7939= 175.48 e75074781f173 selftests/resctrl: Change a few printed messages = 7660254.5 7676124.45 7745330.79 7736754.88 7716834.93 76601= 43.13 87f1c20e2effd Documentation: kselftest: fix path to test module files = 7729609.16 7726906.92 7760819.26 06bd03a57f8c2 selftests/resctrl: Fix MBA/MBM results reporting format = 7692866.06 7730606.11 7681414.48 a38fd87484648 Linux 5.12-rc2 = 7724932.06=20 =20 > > please be noted, sorry to say, due to various reasons, it will be a > > big challenge for us to capture each version of a patch set. > >=20 > > e.g. we didn't make out a similar performance regression for > > v8/v9 version of this one.. > >=20 > > >=20 > > > So it seems we are finally pretty much at correct code in need > > > of performance tuning. > > >=20 > > > > I *think* this test may be testing "send so many signals that it > > > > triggers the signal queue overflow case". > > > > > > > > And I *think* that the performance degradation may be due to lots= of > > > > unnecessary allocations, because ity looks like that commit chang= es > > > > __sigqueue_alloc() to do > > > > > > > > struct sigqueue *q =3D kmem_cache_alloc(sigqueue_cachep, = flags); > > > > > > > > *before* checking the signal limit, and then if the signal limit = was > > > > exceeded, it will just be free'd instead. > > > > > > > > The old code would check the signal count against RLIMIT_SIGPENDI= NG > > > > *first*, and if there were m ore pending signals then it wouldn't= do > > > > anything at all (including not incrementing that expensive atomic > > > > count). > > >=20 > > > This is an interesting test in a lot of ways as it is testing the > > > synchronous signal delivery path caused by an exception. The test > > > is either executing *ptr =3D 0 (where ptr points to a read-only pag= e) > > > or it executes an x86 instruction that is excessively long. > > >=20 > > > I have found the code but I haven't figured out how it is being > > > called yet. The core loop is just: > > > for(;;) { > > > sigaction(SIGSEGV, &action, NULL); > > > sigaction(SIGILL, &action, NULL); > > > sigaction(SIGBUS, &action, NULL); > > >=20 > > > ret =3D sigsetjmp(jmp_env, 1); > > > if (done()) > > > break; > > > if (ret) { > > > /* verify signal */ > > > } else { > > > *ptr =3D 0; > > > } > > > } > > >=20 > > > Code like that fundamentally can not be multi-threaded. So the onl= y way > > > the sigpending limit is being hit is if there are more processes ru= nning > > > that code simultaneously than the size of the limit. > > >=20 > > > Further it looks like stress-ng pushes RLIMIT_SIGPENDING as high as= it > > > will go before the test starts. > > >=20 > > >=20 > > > > Also, the old code was very careful to only do the "get_user()" f= or > > > > the *first* signal it added to the queue, and do the "put_user()"= for > > > > when removing the last signal. Exactly because those atomics are = very > > > > expensive. > > > > > > > > The new code just does a lot of these atomics unconditionally. > > >=20 > > > Yes. That seems a likely culprit. > > >=20 > > > > I dunno. The profile data in there is a bit hard to read, but the= re's > > > > a lot more cachee misses, and a *lot* of node crossers: > > > > > > > >> 5961544 +190.4% 17314361 perf-stat.i.cache-= misses > > > >> 22107466 +119.2% 48457656 perf-stat.i.cache-= references > > > >> 163292 =C4=85 3% +4582.0% 7645410 perf-stat.i.n= ode-load-misses > > > >> 227388 =C4=85 2% +3708.8% 8660824 perf-stat.i.n= ode-loads > > > > > > > > and (probably as a result) average instruction costs have gone up= enormously: > > > > > > > >> 3.47 +66.8% 5.79 perf-stat.overall.= cpi > > > >> 22849 -65.6% 7866 perf-stat.overall.= cycles-between-cache-misses > > > > > > > > and it does seem to be at least partly about "put_ucounts()": > > > > > > > >> 0.00 +4.5 4.46 perf-profile.callt= race.cycles-pp.put_ucounts.__sigqueue_free.get_signal.arch_do_signal_or_r= estart.exit_to_user_mode_prepare > > > > > > > > and a lot of "get_ucounts()". > > > > > > > > But it may also be that the new "get sigpending" is just *so* muc= h > > > > more expensive than it used to be. > > >=20 > > > That too is possible. > > >=20 > > > That node-load-misses number does look like something is bouncing b= ack > > > and forth between the nodes a lot more. So I suspect stress-ng is > > > running multiple copies of the sigsegv test in different processes = at > > > once. > > >=20 > > >=20 > > >=20 > > > That really suggests cache line ping pong from get_ucounts and > > > incrementing sigpending. > > >=20 > > > It surprises me that obtaining the cache lines exclusively is > > > the dominant cost on this code path but obtaining two cache lines > > > exclusively instead of one cache cache line exclusively is consiste= nt > > > with a causing the exception delivery to take nearly twice as long. > > >=20 > > > For the optimization we only care about the leaf count so with a li= ttle > > > care we can restore the optimization. So that is probably the thin= g > > > to do here. The fewer changes to worry about the less likely to fi= nd > > > surprises. > > >=20 > > >=20 > > >=20 > > > That said for this specific case there is a lot of potential room f= or > > > improvement. As this is a per thread signal the code update sigpen= ding > > > in commit_cred and never worry about needing to pin the struct > > > user_struct or struct ucounts. As this is a synchronous signal we = could > > > skip the sigpending increment, skip the signal queue entirely, and > > > deliver the signal to user-space immediately. The removal of all c= ache > > > ping pongs might make it worth it. > > >=20 > > > There is also Thomas Gleixner's recent optimization to cache one > > > sigqueue entry per task to give more predictable behavior. That > > > would remove the cost of the allocation. > > >=20 > > > Eric > >=20 >=20 > --=20 > Rgrds, legion >=20