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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id B471AEB64D7 for ; Fri, 16 Jun 2023 18:53:56 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 440A26B0075; Fri, 16 Jun 2023 14:53:56 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 3F0868E0001; Fri, 16 Jun 2023 14:53:56 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2DE926B007B; Fri, 16 Jun 2023 14:53:56 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 1C2036B0075 for ; Fri, 16 Jun 2023 14:53:56 -0400 (EDT) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id D8595C0CEE for ; Fri, 16 Jun 2023 18:53:55 +0000 (UTC) X-FDA: 80909510430.26.D1F185A Received: from smtpout.efficios.com (smtpout.efficios.com [167.114.26.122]) by imf28.hostedemail.com (Postfix) with ESMTP id 7F708C0015 for ; Fri, 16 Jun 2023 18:53:52 +0000 (UTC) Authentication-Results: imf28.hostedemail.com; dkim=pass header.d=efficios.com header.s=smtpout1 header.b=Ypp9IPZE; spf=pass (imf28.hostedemail.com: domain of mathieu.desnoyers@efficios.com designates 167.114.26.122 as permitted sender) smtp.mailfrom=mathieu.desnoyers@efficios.com; dmarc=pass (policy=none) header.from=efficios.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1686941632; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=SRS/8duwiKKDMkKT+fOdFDCE0eigCN+x4lELtPgaro4=; b=K1qxaNk1JM/lRiMx5ayN7itfNUJV21c9fvDmpFXW3A3sPnpPPi4zyC253m5+eh1KyfcHNV gZ8OW6ahnuvDNqficKGzfaOWfWWAkHuqeBHymoMdCdivf0RmwLpIoIGorTTYIln97s77/J 27/frTX3Y3Cn2HhtobyHiMowpcnsOl0= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1686941632; a=rsa-sha256; cv=none; b=dwOhYQ+eUHjvy6hMpZEzxgB8ADJoFD57b0iwg14toNZl1HhdJzLMMCrzAVm3tIWbbsuKcr zYSYipw50ZzWBlrGypNsrdencVxIzo1aqzXQLNu0f5K6NyszFKRngpqq1yeb9oheB96pry 4HK3ywvU6mKH7BFRkw9TlVbelOCXzTc= ARC-Authentication-Results: i=1; imf28.hostedemail.com; dkim=pass header.d=efficios.com header.s=smtpout1 header.b=Ypp9IPZE; spf=pass (imf28.hostedemail.com: domain of mathieu.desnoyers@efficios.com designates 167.114.26.122 as permitted sender) smtp.mailfrom=mathieu.desnoyers@efficios.com; dmarc=pass (policy=none) header.from=efficios.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=efficios.com; s=smtpout1; t=1686941631; bh=VNT5xnOEGjWm0oQ2Ve65fcX3U8t0E0DrHtFvp2I4rpU=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=Ypp9IPZEVGqp4u7FDGXa2iHYNous6G9/mLXHipj0tNQjDggg/VmIUC4LTKUcqGMf2 kkpWMzYOwKRSFYVzoUN+f4OE1uAjl8d4HpQoG7qoc8ad//nrlMRAiA4PVAqs8hQjr7 6X8OGs0fQDtILv3sV/asT7eT87w1H729KzAZe0GR8v6r3Qoe/Rzk688/d5l/RBn00c ER5yXpaGO9/3P5yh7jrT2GgBu8NGG0XyCCdpnTJ6s62Tj90r8ZiDnhBScDS/5yZqDz ji3jIaFW/ekkUSFtbc+bsbi3V/uoqjDv3vz+Pw2jWCLNj/n5iCUURIfyR/YmSWol70 Z+2OXTiu7gPXg== Received: from [172.16.0.134] (192-222-143-198.qc.cable.ebox.net [192.222.143.198]) by smtpout.efficios.com (Postfix) with ESMTPSA id 4QjSxC0KyPz18gY; Fri, 16 Jun 2023 14:53:51 -0400 (EDT) Message-ID: <313c3bea-d710-a769-8cb7-0964614425a2@efficios.com> Date: Fri, 16 Jun 2023 14:54:09 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.12.0 Subject: Re: [PATCH] mm: Move mm_count into its own cache line Content-Language: en-US To: Andrew Morton Cc: linux-kernel@vger.kernel.org, kernel test robot , Aaron Lu , Olivier Dion , michael.christie@oracle.com, Feng Tang , John Hubbard , Jason Gunthorpe , Peter Xu , linux-mm@kvack.org, Peter Zijlstra References: <20230515143536.114960-1-mathieu.desnoyers@efficios.com> From: Mathieu Desnoyers In-Reply-To: <20230515143536.114960-1-mathieu.desnoyers@efficios.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Stat-Signature: ckqnuow6k3i1tisj7qn1199n5j4mk1w7 X-Rspamd-Server: rspam10 X-Rspamd-Queue-Id: 7F708C0015 X-Rspam-User: X-HE-Tag: 1686941632-373591 X-HE-Meta: U2FsdGVkX19Rj1um9B9UO8QYWR0s1JY7cQ+idbpFU+45gph+LrkXvQGrneDIlhirg9qXCzZG09x76r08mT5A4ES81xmPFOz2BDpnySZh1TFXAaH/Vi6lxOdambtbvS1gPNmtb93c9uRjuGyBEcR3HIRXYiKJQ8AT4EDtym88Dx06Sg2lDN9fztHm/LsL0GcVhucTDhp9Wt76aaxRPGYP87Sc/AuzKsM8rY29ATXD19vItdlLgKacuEmh/GdvJckBjSSP+4pfrbb55e39i8dvrdg2487Xnt9zHf+7IKqqTmxYcYi6+HgNtYM43mNi3cB97sP/YF8LfqYs7ZUZdwdVXeuQwOLtsRXPcRXe4RF/3nMI4UmYYNoZuf7VJzhda0NdZ0mUTPRJ2swfMM5er/GkxJGq2OiTBLGnXH6+VYpo+TofydrgtNjVTMQCGkPAVD9sR9P8C1qnbtqecJS3r8vRBiczhFHMopYd4bosZag2e9ddsg46sea+eixvWjWMlrlhYeRPRRCMXTcx2IdObYshX4kc7j2vs/fLMXavZXi0XB8ho2qmDv8IifK8JRAcbPsRa4DNMTQv8ekP6dCCoemtsUYQNVc00KeFVe/LY13+600/XShDg/awbeyyiGzGErtqmLFxcdb4SULf9ZIsuFbuajdSfjA0QI1hUEzCJpSCP4Qhebp42KWHZyxJL3dNGvAsF5rwbX08y5hwpRN7pzBKV3omHy/Qt/cmlqWr1sFkqI5Dqd1Gp0d1ECeGb3KfxlCqBYRxifo8BUHKX9zPu5jEmo0/RAf6Cz7ax1Y71rWcBIzSxrKVbiqa/AN0LPD7wYDeI0Z5vIWFdHG+Nm8/DIFBRvEyDDjBdCfZBHhWrLA+rJRx5vEZfwz5vPxJh3zH1lDVgU3g9L5g5CqeJcHg1PAH4GaXobVmOVDRDTtq8hlUheC8nVloVEoLZQ0cdDxKU1CmasNAdV3xrD/X0jQhMGR z+omJmD1 IhqlHSlqoWE0Bw5lpR1i9W2HS1REIctiPtORyekqbcLZ8jyFFRAD6SnAW0DGrZGVppcg2D07H7IChR7JcGF+6iDC9MRvmQ39sLEZZiQn/h5o2JHbjYjeTEXxiXQAWQrfLXWP39PAkcXIE/zXfVw0Qjug4H9+YAvKIoo9TpdmAaW8ScIaQgr9LA3GCtVNbgoIFnVduQ1R3TybZ3YoZlROCS0wc32T8LI0EjZpVdPeHoyC74NGBPD/m44ynRWWjPdac9uttbybV89zvSgPwdLyOKYdHvNWvMAa8Q9PWZEV1eNBvxtEr5VL0hJy/Zho+NijzRg9k3zQrQ+ooWxvW3P+eKL42EWvHlSbP3LYJU01mYP73fhYkxPlqz6e9+Pkyj9L9tS/mugrarbHc1rpebObjzscFhrJScOMAkVGv4y//WbpmCCgYg7EluDUAGqnTYkQiCyzuNjzzrrq8roh8QTrZANsFnVsVXDVk9gT/017lVBufNXgCZIVk9XSgk1Gq0JYx49BWgD0zWY7og+C0fk0WrbTBM9izvAIGzIyGEaut4IvE0GVf4peQJKMDKSHFJfKWDuj+YhH0bUxMG7PtcwPnmj0OSlP5trmectBVPgj+SigEY2ed23MoDyszz0JIfwO/fQrS 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: On 5/15/23 10:35, Mathieu Desnoyers wrote: > The mm_struct mm_count field is frequently updated by mmgrab/mmdrop > performed by context switch. This causes false-sharing for surrounding > mm_struct fields which are read-mostly. Hi Andrew, Given that this patch touches mm code, I think it should go through your tree. Nobody has voiced any objection for a month, Aaron Lu gave his Reviewed-by tag [1], and it solves measurable performance regressions. Would you be willing to pick it up ? Please let me know if something else is needed. Thanks! Mathieu [1] https://lore.kernel.org/lkml/20230516044050.GA315678@ziqianlu-desk2 > > This has been observed on a 2sockets/112core/224cpu Intel Sapphire > Rapids server running hackbench, and by the kernel test robot > will-it-scale testcase. > > Move the mm_count field into its own cache line to prevent false-sharing > with other mm_struct fields. > > Move mm_count to the first field of mm_struct to minimize the amount of > padding required: rather than adding padding before and after the > mm_count field, padding is only added after mm_count. > > Note that I noticed this odd comment in mm_struct: > > commit 2e3025434a6b ("mm: relocate 'write_protect_seq' in struct mm_struct") > > /* > * With some kernel config, the current mmap_lock's offset > * inside 'mm_struct' is at 0x120, which is very optimal, as > * its two hot fields 'count' and 'owner' sit in 2 different > * cachelines, and when mmap_lock is highly contended, both > * of the 2 fields will be accessed frequently, current layout > * will help to reduce cache bouncing. > * > * So please be careful with adding new fields before > * mmap_lock, which can easily push the 2 fields into one > * cacheline. > */ > struct rw_semaphore mmap_lock; > > This comment is rather odd for a few reasons: > > - It requires addition/removal of mm_struct fields to carefully consider > field alignment of _other_ fields, > - It expresses the wish to keep an "optimal" alignment for a specific > kernel config. > > I suspect that the author of this comment may want to revisit this topic > and perhaps introduce a split-struct approach for struct rw_semaphore, > if the need is to place various fields of this structure in different > cache lines. > > Fixes: 223baf9d17f2 ("sched: Fix performance regression introduced by mm_cid") > Fixes: af7f588d8f73 ("sched: Introduce per-memory-map concurrency ID") > Link: https://lore.kernel.org/lkml/7a0c1db1-103d-d518-ed96-1584a28fbf32@efficios.com > Reported-by: kernel test robot > Link: https://lore.kernel.org/oe-lkp/202305151017.27581d75-yujie.liu@intel.com > Signed-off-by: Mathieu Desnoyers > Cc: Peter Zijlstra > Cc: Aaron Lu > Cc: Olivier Dion > Cc: michael.christie@oracle.com > Cc: Andrew Morton > Cc: Feng Tang > Cc: John Hubbard > Cc: Jason Gunthorpe > Cc: Peter Xu > Cc: linux-mm@kvack.org > --- > include/linux/mm_types.h | 23 +++++++++++++++-------- > 1 file changed, 15 insertions(+), 8 deletions(-) > > diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h > index 306a3d1a0fa6..de10fc797c8e 100644 > --- a/include/linux/mm_types.h > +++ b/include/linux/mm_types.h > @@ -583,6 +583,21 @@ struct mm_cid { > struct kioctx_table; > struct mm_struct { > struct { > + /* > + * Fields which are often written to are placed in a separate > + * cache line. > + */ > + struct { > + /** > + * @mm_count: The number of references to &struct > + * mm_struct (@mm_users count as 1). > + * > + * Use mmgrab()/mmdrop() to modify. When this drops to > + * 0, the &struct mm_struct is freed. > + */ > + atomic_t mm_count; > + } ____cacheline_aligned_in_smp; > + > struct maple_tree mm_mt; > #ifdef CONFIG_MMU > unsigned long (*get_unmapped_area) (struct file *filp, > @@ -620,14 +635,6 @@ struct mm_struct { > */ > atomic_t mm_users; > > - /** > - * @mm_count: The number of references to &struct mm_struct > - * (@mm_users count as 1). > - * > - * Use mmgrab()/mmdrop() to modify. When this drops to 0, the > - * &struct mm_struct is freed. > - */ > - atomic_t mm_count; > #ifdef CONFIG_SCHED_MM_CID > /** > * @pcpu_cid: Per-cpu current cid. -- Mathieu Desnoyers EfficiOS Inc. https://www.efficios.com