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 EB806C4828F for ; Sat, 3 Feb 2024 04:40:30 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 51AC26B0082; Fri, 2 Feb 2024 23:40:30 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 4C9EF6B0083; Fri, 2 Feb 2024 23:40:30 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 391A56B0085; Fri, 2 Feb 2024 23:40:30 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 24E0E6B0082 for ; Fri, 2 Feb 2024 23:40:30 -0500 (EST) Received: from smtpin16.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id BFA541C0EE0 for ; Sat, 3 Feb 2024 04:40:29 +0000 (UTC) X-FDA: 81749241378.16.52039EC Received: from mail-ej1-f50.google.com (mail-ej1-f50.google.com [209.85.218.50]) by imf05.hostedemail.com (Postfix) with ESMTP id E88CA100005 for ; Sat, 3 Feb 2024 04:40:27 +0000 (UTC) Authentication-Results: imf05.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=Y024i4wG; spf=pass (imf05.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.50 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1706935228; a=rsa-sha256; cv=none; b=p1l2ABmHeVBrynBV96ql9azTMt9PFC5FYDOoSTmDUrBr1/9Gym942Ccm4kHKfP+IQx6hmA /2MU/b9JlLl4xXh0Zw3GlOx51tNNTWgX/igCZWxnWA/pRK58li9h8QUd8oW8gjhvgA7jnG QQPsAb8rwCKpe1ZPfOnopOIk/f1u6Kc= ARC-Authentication-Results: i=1; imf05.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=Y024i4wG; spf=pass (imf05.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.50 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1706935228; 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=bpXXG/EMRhEf0E4UMXfbSBAtkE3fSF2flaNq2YKC3mA=; b=vZNOIlQMRJ8/ytZOtLZcH+F8tfDCAGP3MmYgiYsYjlAq4FPJGvXGAPLuQXTQNKQBYUnD// h52DET8LttfofUmBPbUs5OIJHukcLTZOrujavpLeByHnjahE88c2AzC4Fg2BakhvUhYAOU EXpPk0rlp/2d3vzsy7Hn5I8Ak3VQto4= Received: by mail-ej1-f50.google.com with SMTP id a640c23a62f3a-a3122b70439so376685566b.3 for ; Fri, 02 Feb 2024 20:40:27 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1706935226; x=1707540026; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=bpXXG/EMRhEf0E4UMXfbSBAtkE3fSF2flaNq2YKC3mA=; b=Y024i4wGwaRS1PJLRCOLq7r/y6LYRPKs3DsvAfH3ZW7D3NTu0QZD8ybPL9dX3yLfvL WZMqkwqnUEfjZxSo1ndRn13QyFM/M+1QZGc+njPetQr67fuZSAjFhtB4hJqoaRMEEpW6 gKIDBDQvjt/mCW4OBS+WFrROmn6PIC5tLfEoMI/krcRxvj0D2Xu08QI5arrqvhK/QRJ6 4hcH0o14TApjd1BSg4CoixEw23mdua+CLca3rp/Z+mBmzMBm/Hm7C0jC/bXskpKIn+/U W3WOnhjYzqWKrScKuAy4E91zbbcULXcb0HK4j21KBkFBKsvho3hPEoLbtmIJHKYg2gah 3eTg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1706935226; x=1707540026; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=bpXXG/EMRhEf0E4UMXfbSBAtkE3fSF2flaNq2YKC3mA=; b=mLwqMjMpRRNM2ixcFGCH5z8XbFMttsYkZtdh5CiSUC8u6ew6SKTeBkh0UFOJYSZd/B eQ144+egLWYNb70YQAlvvkNxAiMYwh86GLBj2u4oUAR+TM2iLrRAF9p8TDtUvCmum3VH 8qL496A+Uefw+PQvTSvoa+KRD3HR7AmeJwLl8SQX1ZLSk261aMBxG/li4YIt9ZNBeDAm mER2vdkbqaWzSY9OPEJ8myQt+9WfCkc5CLvX4+o3LH7hQStBGPAjAXQ9FERYmTg8ZYu+ 01fXqKt722Aar5XQgCKsUWBLQ4xgX5gCQ0s/EiUsl/XS2cWdJXeR25VuaDxThcAtU4XG vJmg== X-Gm-Message-State: AOJu0YzFP6G2NSos87ZSDgPMxAXmI83hpVGWw3PXVQzIbaoMJtdqIQck DLFXGxG/LUjaz7zn5Q8WUb/0i1KXIKKaCKviHl19RX/MuTE60ntj5Alhpxl0UfLyvvfGoNlFkMn eEQeYiIecqWR4Nhipo6LN8/IbdyZAmGNYYl5t X-Google-Smtp-Source: AGHT+IEOqG40Y65sLEOSy86hePJBYD/7doxLjCm+pORd3Q1Ko/44+czAM1deYOmfsGNTWOU4rUzQvXxGBiJdk9yWGT4= X-Received: by 2002:a17:906:5ad3:b0:a36:8d18:4c25 with SMTP id x19-20020a1709065ad300b00a368d184c25mr6140159ejs.16.1706935226165; Fri, 02 Feb 2024 20:40:26 -0800 (PST) MIME-Version: 1.0 References: <20240203003414.1067730-1-yosryahmed@google.com> In-Reply-To: From: Yosry Ahmed Date: Fri, 2 Feb 2024 20:39:49 -0800 Message-ID: Subject: Re: [PATCH mm-hotfixes-unstable] mm: memcg: fix struct memcg_vmstats_percpu size and alignment To: Shakeel Butt Cc: Andrew Morton , Johannes Weiner , Michal Hocko , Roman Gushchin , Muchun Song , cgroups@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Greg Thelen Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam08 X-Rspamd-Queue-Id: E88CA100005 X-Stat-Signature: q6jgbr7z48wwxyme385ggquwhyk4oo87 X-Rspam-User: X-HE-Tag: 1706935227-583175 X-HE-Meta: U2FsdGVkX1+3UAqW0Bb3ncydcweKBqq9YZwBn9oJp4L+vkdltbUfMB1ssNe6JJbIVoobXUMGCMHd2ggqsB3WIrSJ5fuWoJlYlG1pRVcg0gieisQI2ndYqw9AAPo5oYjHSf31VfQKQKNV+4G6bpCgzF3aTBn+1TosNz9p0Dg8ki4j2UMyCUrILWowSWgbvWU93f4Q747EUjcb22sz+SnvcrAKJTYE6NUWD/3KSrgGOk0xzjxZoyvAwfCOoigIPaHOmZXrbouCSqge3yN09jmIUNuB/yjg+TiImZh+YXU+QcBMufEfXkWoelEMkfuLTRV3oj4/Og2JCcwtB0ctyoY9ZnnkzKtg1Xy31eAIkpa8/eW60nkBKPmcajc0Wf/vTiySjmZ5fnr7PRKJ8ET3cWa67zZy7WV5KcZpjQ8/oNNb/i5PMKyiZgmfjtZyf6vKQC+AYS1eNad8dgFeBbaZZ0prp7Kif/tOdbtr4Z8POKIzT9rZzey/KMjlzUbebto/6eaQAiERlt5h3Sb4/f0WE3D+Ptge27U7sIzUTumLqHywUjhachnCQRIDRqMs+VyaAncvowuUNSnWdE6t8C0TYV2VAM8ATngH21dpwrznMyG865neYiUifijsDy9+gzM6mYb1+Vc9I0CusyzaI2LoGBBTulY2pIhIiPizWGyc74Le+x64hgNXTawQH5foRQ2EPQIjfl2yIjGAhsI7DxOVf6i3PjQ+ct9OXBcbxLq4DoyROSGkmvgDsVwN/gNEd00SyzxmodTz3ubPomRCne+Zyz1+lHJ9pn2zUwJcxHf34f9RPRqvZKIYMGdNyQwNpZ8jZla+SxxlLANRsl1vEVC67cUR8Iz6TqLVATZRS+0SiHZRIBnrZUlexY5KqJj0/O7tE5k/u2+ZmI4+ospJzCrsRYqOUD2dpEilZVEPY1yKPXPhSURXwDNAMDZ8c9ay78dK3B33eKXqTKRVmBfLaIA7eE8 nBVOGYlI y8U2mcas3MlC+kp6TlRu2WFRpio2Riq2D3MxE2+TXImrgi3tvYm+7XHCCjtWxb9a0rR/Fa6d+goJPNrwhGAVl8HPB6b59mvAu+CagUMJremC4hdLGK3hZZrxextEZxmlJ/zb0fPCe1emCre5LuAsQxcv1XQtwQ9V4eBYGb5l2EXgGqSm45MRHMDbOSSb2FFOlkx6Rdnng/1L4Tt8RJYsOFeNNFIYSLD6jF9te/5ZZx9PqT+2WhBe5R4VgYgN5Khv65u6UpfDDlQ+Y0lmo2y6Y0LekGaIa1lgMI+cP5A37qInrz//0ahppsAQGuDYJsaARnNkn+rGNUI0fflg= 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: List-Subscribe: List-Unsubscribe: On Fri, Feb 2, 2024 at 8:34=E2=80=AFPM Shakeel Butt w= rote: > > On Fri, Feb 2, 2024 at 8:23=E2=80=AFPM Yosry Ahmed wrote: > > > > On Fri, Feb 2, 2024 at 8:13=E2=80=AFPM Shakeel Butt wrote: > > > > > > On Fri, Feb 2, 2024 at 4:34=E2=80=AFPM Yosry Ahmed wrote: > > > > > > > > Commit da10d7e140196 ("mm: memcg: optimize parent iteration in > > > > memcg_rstat_updated()") added two additional pointers to the end of > > > > struct memcg_vmstats_percpu with CACHELINE_PADDING to put them in a > > > > separate cacheline. This caused the struct size to increase from 12= 00 to > > > > 1280 on my config (80 extra bytes instead of 16). > > > > > > > > Upon revisiting, the relevant struct members do not need to be on a > > > > separate cacheline, they just need to fit in a single one. This is = a > > > > percpu struct, so there shouldn't be any contention on that cacheli= ne > > > > anyway. Move the members to the beginning of the struct and cacheal= ign > > > > the first member. Add a comment about the members that need to fit > > > > together in a cacheline. > > > > > > > > The struct size is now 1216 on my config with this change. > > > > > > > > Fixes: da10d7e140196 ("mm: memcg: optimize parent iteration in memc= g_rstat_updated()") > > > > Reported-by: Greg Thelen > > > > Signed-off-by: Yosry Ahmed > > > > --- > > > > mm/memcontrol.c | 19 +++++++++---------- > > > > 1 file changed, 9 insertions(+), 10 deletions(-) > > > > > > > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > > > > index d9ca0fdbe4ab0..09f09f37e397e 100644 > > > > --- a/mm/memcontrol.c > > > > +++ b/mm/memcontrol.c > > > > @@ -621,6 +621,15 @@ static inline int memcg_events_index(enum vm_e= vent_item idx) > > > > } > > > > > > > > struct memcg_vmstats_percpu { > > > > + /* Stats updates since the last flush */ > > > > + unsigned int stats_updates ____cacheline= _aligned; > > > > > > Why do you need ____cacheline_aligned here? From what I understand fo= r > > > the previous patch you want stats_updates, parent and vmstats on the > > > same cacheline, right? > > > > Yes. I am trying to ensure that stats_updates sits at the beginning of > > a cacheline to ensure they all fit in one cacheline. Is this > > implicitly guaranteed somehow? > > > > > > > > I would say just remove the CACHELINE_PADDING() from the previous > > > patch and we are good. > > > > IIUC, without CACHELINE_PADDING(), they may end up on different cache > > lines, depending on the size of the arrays before them in the struct > > (which depends on several configs). Am I misunderstanding? > > > > Yeah keeping them at the start will be better. Move > ____cacheline_aligned to the end of the struct definition like: > > struct memcg_vmstats_percpu { > ... > } ____cacheline_aligned; Will send a v2 shortly, thanks. I honestly didn't know what the difference was, but both gave me the same results. Is using ____cacheline_aligned with the first member the same as using it at the end of the definition?