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=-15.2 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham 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 2D537C48BE6 for ; Mon, 14 Jun 2021 03:27:48 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id C33B36137D for ; Mon, 14 Jun 2021 03:27:47 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C33B36137D 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 2B3186B006C; Sun, 13 Jun 2021 23:27:47 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 2635C6B006E; Sun, 13 Jun 2021 23:27:47 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0DD8B6B0070; Sun, 13 Jun 2021 23:27:47 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0187.hostedemail.com [216.40.44.187]) by kanga.kvack.org (Postfix) with ESMTP id CEC6B6B006C for ; Sun, 13 Jun 2021 23:27:46 -0400 (EDT) Received: from smtpin01.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 5637C1E13 for ; Mon, 14 Jun 2021 03:27:46 +0000 (UTC) X-FDA: 78250894932.01.35C1A72 Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by imf22.hostedemail.com (Postfix) with ESMTP id 07F5CC0042C0 for ; Mon, 14 Jun 2021 03:27:35 +0000 (UTC) IronPort-SDR: koYJ/b2pW8MIBpgkzCqbwke3MbrdzVRg0r9aCLWrkXKhnQnWpufoKux4NAGVHbrx1NKMo93IYt K/RXVKnyHbGg== X-IronPort-AV: E=McAfee;i="6200,9189,10014"; a="205573123" X-IronPort-AV: E=Sophos;i="5.83,272,1616482800"; d="scan'208";a="205573123" Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 13 Jun 2021 20:27:42 -0700 IronPort-SDR: EjXik5Pp2tDTDfva1Kobz5H0HHfNfIo0XIblIn7buh4lDzbfCxOktC6k2znQvY+Uibbl9fiQaF XZWULawo/jwQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.83,272,1616482800"; d="scan'208";a="483947534" Received: from shbuild999.sh.intel.com (HELO localhost) ([10.239.147.94]) by orsmga001.jf.intel.com with ESMTP; 13 Jun 2021 20:27:39 -0700 Date: Mon, 14 Jun 2021 11:27:39 +0800 From: Feng Tang To: Jason Gunthorpe Cc: linux-mm@kvack.org, Andrew Morton , Linus Torvalds , kernel test robot , John Hubbard , linux-kernel@vger.kernel.org, lkp@lists.01.org, Peter Xu Subject: Re: [PATCH] mm: relocate 'write_protect_seq' in struct mm_struct Message-ID: <20210614032738.GA72794@shbuild999.sh.intel.com> References: <1623376482-92265-1-git-send-email-feng.tang@intel.com> <20210611170917.GW1002214@nvidia.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210611170917.GW1002214@nvidia.com> User-Agent: Mutt/1.5.24 (2015-08-30) Authentication-Results: imf22.hostedemail.com; dkim=none; spf=none (imf22.hostedemail.com: domain of feng.tang@intel.com has no SPF policy when checking 192.55.52.115) smtp.mailfrom=feng.tang@intel.com; dmarc=fail reason="No valid SPF, No valid DKIM" header.from=intel.com (policy=none) X-Stat-Signature: 7o58nack9jxddt8j6e8bfknpkwy8x8i5 X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 07F5CC0042C0 X-HE-Tag: 1623641255-454632 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 Jason, On Fri, Jun 11, 2021 at 02:09:17PM -0300, Jason Gunthorpe wrote: > On Fri, Jun 11, 2021 at 09:54:42AM +0800, Feng Tang wrote: > > 0day robot reported a 9.2% regression for will-it-scale mmap1 test > > case[1], caused by commit 57efa1fe5957 ("mm/gup: prevent gup_fast > > from racing with COW during fork"). > > > > Further debug shows the regression is due to that commit changes > > the offset of hot fields 'mmap_lock' inside structure 'mm_struct', > > thus some cache alignment changes. > > > > From the perf data, the contention for 'mmap_lock' is very severe > > and takes around 95% cpu cycles, and it is a rw_semaphore > > > > struct rw_semaphore { > > atomic_long_t count; /* 8 bytes */ > > atomic_long_t owner; /* 8 bytes */ > > struct optimistic_spin_queue osq; /* spinner MCS lock */ > > ... > > > > Before commit 57efa1fe5957 adds the 'write_protect_seq', it > > happens to have a very optimal cache alignment layout, as > > Linus explained: > > > > "and before the addition of the 'write_protect_seq' field, the > > mmap_sem was at offset 120 in 'struct mm_struct'. > > > > Which meant that count and owner were in two different cachelines, > > and then when you have contention and spend time in > > rwsem_down_write_slowpath(), this is probably *exactly* the kind > > of layout you want. > > > > Because first the rwsem_write_trylock() will do a cmpxchg on the > > first cacheline (for the optimistic fast-path), and then in the > > case of contention, rwsem_down_write_slowpath() will just access > > the second cacheline. > > > > Which is probably just optimal for a load that spends a lot of > > time contended - new waiters touch that first cacheline, and then > > they queue themselves up on the second cacheline." > > > > After the commit, the rw_semaphore is at offset 128, which means > > the 'count' and 'owner' fields are now in the same cacheline, > > and causes more cache bouncing. > > > > Currently there are 3 "#ifdef CONFIG_XXX" before 'mmap_lock' which > > will affect its offset: > > > > CONFIG_MMU > > CONFIG_MEMBARRIER > > CONFIG_HAVE_ARCH_COMPAT_MMAP_BASES > > > > The layout above is on 64 bits system with 0day's default kernel > > config (similar to RHEL-8.3's config), in which all these 3 options > > are 'y'. And the layout can vary with different kernel configs. > > > > Relayouting a structure is usually a double-edged sword, as sometimes > > it can helps one case, but hurt other cases. For this case, one > > solution is, as the newly added 'write_protect_seq' is a 4 bytes long > > seqcount_t (when CONFIG_DEBUG_LOCK_ALLOC=n), placing it into an > > existing 4 bytes hole in 'mm_struct' will not change other fields' > > alignment, while restoring the regression. > > > > [1]. https://lore.kernel.org/lkml/20210525031636.GB7744@xsang-OptiPlex-9020/ > > Reported-by: kernel test robot > > Signed-off-by: Feng Tang > > Reviewed-by: John Hubbard > > Cc: Jason Gunthorpe > > Cc: Linus Torvalds > > Cc: Peter Xu > > --- > > include/linux/mm_types.h | 27 ++++++++++++++++++++------- > > 1 file changed, 20 insertions(+), 7 deletions(-) > > It seems Ok to me, but didn't we earlier add the has_pinned which > would have changed the layout too? Are we chasing performance delta's > nobody cares about? Good point! I checked my email folder for 0day's reports, and haven't found a report related with Peter's commit 008cfe4418b3 ("mm: Introduce mm_struct.has_pinned) which adds 'has_pinned' field. Will run the same test for it and report back. > Still it is mechanically fine, so: > > Reviewed-by: Jason Gunthorpe Thanks for the review! - Feng > Jason