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 0E76AC10F1A for ; Tue, 7 May 2024 15:54:43 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 8293D6B0089; Tue, 7 May 2024 11:54:43 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 7B2E96B0092; Tue, 7 May 2024 11:54:43 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 62CB36B0093; Tue, 7 May 2024 11:54:43 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 40FA26B0089 for ; Tue, 7 May 2024 11:54:43 -0400 (EDT) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id DA93480EA1 for ; Tue, 7 May 2024 15:54:42 +0000 (UTC) X-FDA: 82092047604.24.BB222A9 Received: from mail-ej1-f54.google.com (mail-ej1-f54.google.com [209.85.218.54]) by imf02.hostedemail.com (Postfix) with ESMTP id 0FC888000E for ; Tue, 7 May 2024 15:54:40 +0000 (UTC) Authentication-Results: imf02.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=1oAlsqBM; spf=pass (imf02.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.54 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=1715097281; 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=+//lctqNApZOsedm3CHZikHUh063xmeaaMM6XmCk0O8=; b=PzZEdUL+HhHE/FrCPKUJWlVpbAriTlwvdESIAEAUHpOgCYhoBErL2l86oiHZw0LdIp4msV 1Qz6Bozm32ItmnANxE0rXMexT2gk1l+rLToPHfUb99r9oZ0rBBW27pnQgZd49qoQuuCb7u 2575NYWrGi6qig+2fQLl7SLzuHTu4uI= ARC-Authentication-Results: i=1; imf02.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=1oAlsqBM; spf=pass (imf02.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.54 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=1715097281; a=rsa-sha256; cv=none; b=CgYEDi7Ga2BVeKQudtpOyeRKwOuZxHaa950XVSA7OYqAe0b2U9twn+MiZmArzgorEePyMX 95MMz3NtL4s9g+BqfQbtoXZ6peLlPeUWF/PfWvYdf8aIC515MmcXom97BdRTK/tYRg73od wndCCrZONM1TjxkeucRV2/bkcSpf6sM= Received: by mail-ej1-f54.google.com with SMTP id a640c23a62f3a-a59c5c9c6aeso639216666b.2 for ; Tue, 07 May 2024 08:54:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1715097279; x=1715702079; 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=+//lctqNApZOsedm3CHZikHUh063xmeaaMM6XmCk0O8=; b=1oAlsqBMhR/m1wx2yZKz/0QSvMH5ATOiabS+9R/VFB/4AyZQoCh1RLRcv62DO8fO8b e8UwqfQR3SiNYHaS1seRMbXj3uiIQUqmMhGLYxOGfcGDId4IjtAaNwZ7lOW9YUGf2lrB smdVo1jMXK/XlxJ6fL0QlIijslDERS1D7mQAv04yuc0SX9X3PPj1VDuj/wGw2cGplZ9o qlTQF/VdYr+K5eef3brCni25AcB5QnCfcC7ZCMq4Thk0cDpYPv1TK7DEVjKTdlaoh1sT RNj3gK0AqO1VmFOvXnRP2NxVF068vmzMAKpHMRyrsWp+ldep0PgCmNrX20LYz8izmzLm lBQQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1715097279; x=1715702079; 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=+//lctqNApZOsedm3CHZikHUh063xmeaaMM6XmCk0O8=; b=VetnQv4kqlg+piRtM9qJX7p+wxRFDecrUWorIiqdZcEhQowp4ZrlVj9IdTs1NSlj8c iP+5cWdAxL3Cz/sjUqUD2YeRkMO+Suf2cEaPU7WfGb1PMw8Y8Fz1KMLPToZlsQcNXgQa uEZZDEztdA/+EnXQDsxxR7eVio6awftzdsfzR3aGiPHAnf6pBTiCFsrfejGGPly1H2yq a5KcMJ8BKvfy2EWtzG4NZUGW4LV4NhUDosBW6E96FWlAxhPo1icTyNCfrWP6teTJ2G3c XaOBXTjLMa66T/Owvf02C70BP288fBidVOFUbMuoPwn1wW4M9QuFvihr8uanKs2pdjuu JkVw== X-Forwarded-Encrypted: i=1; AJvYcCWt714Kf2zoNS29qLf5ymiQ64CMahwvvOI7PRLraWCOLtFoi3fWmzy3T+pWuyd+xRc6fQDBK0OHo9SqRyxhmeC81u0= X-Gm-Message-State: AOJu0YzJ3YsgZEtRB7C5I6xKML8565spSGs6oFZhzmNsiapRHeTwZIpX NcGAoMd/MoxKouePoUu7fqA6T81WzDHwVo2E2O5+ks5Deh8Efk7Y0TLS2puXh1wzylE0ULvO8qY b06E+5hszXTLHFCnkSK9dyCwJ5bKCMiLz+9r3 X-Google-Smtp-Source: AGHT+IFxN2iMkSNNChJnnCrpQyYGZeo+zP3RyqjYIB5+hyDPmeiBMPxXA+yp70ayfL9q+R57TS3RzGHxOJgdbXrGQCQ= X-Received: by 2002:a17:906:1d0e:b0:a59:c28a:7eb6 with SMTP id n14-20020a1709061d0e00b00a59c28a7eb6mr4883867ejh.24.1715097279321; Tue, 07 May 2024 08:54:39 -0700 (PDT) MIME-Version: 1.0 References: <20240506211333.346605-1-yosryahmed@google.com> <1a408ed1-7e81-457e-a205-db274b4d6b78@redhat.com> In-Reply-To: <1a408ed1-7e81-457e-a205-db274b4d6b78@redhat.com> From: Yosry Ahmed Date: Tue, 7 May 2024 08:54:01 -0700 Message-ID: Subject: Re: [PATCH mm-unstable] mm: rmap: abstract updating per-node and per-memcg stats To: David Hildenbrand Cc: Andrew Morton , linux-mm@kvack.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Stat-Signature: yn51sxskf8fk4m48fzifq7xp7qgt8zi8 X-Rspam-User: X-Rspamd-Queue-Id: 0FC888000E X-Rspamd-Server: rspam05 X-HE-Tag: 1715097280-773423 X-HE-Meta: U2FsdGVkX19wNkhM/RcllKVHkVW/6/ktehKaSJR0QCHxVwFwbkboYBd8ZwkdCQZgv+mBS/zSQMkNBYTMb1Xq64ZSHCJs2JNK0uaGn/mbLK0p5KYjQD8jNhlcISpDzq0wcepkMjhUYhplbjodLv0zKKhN8lIVWXQ+vEhju/hICH0j8nUZ85848Kk3ZFGjinH3kCAQVidrciTyXqNZeRWe+VnPTP/hNNo75eikzCxTNQbSqCHD/fhrowB4iVjOlrbw2RFw3C3FvA0izMdvV0OiqJk3irLBjHoQ2zEtj0meWKBixM+M6MNX0B0lpZwv7KWcUWndTPHPW5+Do6geWAn4jWe2LC1ajobL4jMnJTRxSiO903aFdhALZEqJO57SA+pOuo6892l2BLyEv0gkKNqK/WTGJxUVDyV67FZCNtiutjoaUElobVGlfPilkXlKQY6oxemU26qC7e74vc8jDREdGrfg1XrcrAlSsl+OCVUZBhVqr/jO43o/VgHqszO3ET/xeNunIgaCzFnKzCExYPI5FT+2kO8a40t6J9iWCHYIySImNb6hSlOreXrtUpDxTP2AfDFoY5qolZVyIMPUvJEAM6BLSmlsYQkXcv0KAtdqVcJAR+1ejLHLREf/dA3iEHLgPVO3FV1aOBpEq4eWycKjVhAhkdlLXsuAsIxkscDcV2eKGwpfmwbFhLZLz3kGuzba9eijWrkgBKCm2SHHU7H5jCI0D5DbRHgsEc23l1VTUy5GdLvjn4FzSRztqMbxbx4kAm67aLFXz36HT5ikbYCvXykH+xLNf8z/z79il9M3ZLaT/QtlGugr19bKmQushzy/C5VboZsE/Z99d4Igu3eXTU8eer/pSQaUKx1ukcU7yjRyDaoDE5IAEV0ZbP4CHD8BBXQN4oY6JC5iP4WxCNcLv5yah7qQF/WTmY85dRuA+u+dyLVGgZYQevut2Hmyuo2gGzGReauvycxWCoyMnVb OIlNKSGy yEDSaDFGHVbvXusu/KY5ruYcDC/bNnap4D+y7eqWs3p/zqfW1MN831bKeK1cMXsD2MUM3nwqJHm4qHISBFSZaJGtP3Gy6aLSeRu2fri0AAGTFs8gSNuozUhqx+CAIR8xLIqZ9UFIKHVlVxs0gkQqYJo7zPJbgjZSkQRL7a23evxYv3kpyExAQyd3Uhp8I3VH1YX6Mj/T/c5FRTUAIxCchRiqSKLv9EklXcRwdjfSYBBuy20As/qu1uXhrbPlg3q4kZNbmRd0+22nT5GnzmZVMG/uvm+jUcvv1NnDKng4PdGsoPDQIuESTiOCr0bzbjKxNmznVSlNZSe3uoP0= 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 Tue, May 7, 2024 at 1:52=E2=80=AFAM David Hildenbrand = wrote: > > On 06.05.24 23:13, Yosry Ahmed wrote: > > A lot of intricacies go into updating the stats when adding or removing > > mappings: which stat index to use and which function. Abstract this awa= y > > into a new static helper in rmap.c, __folio_mod_stat(). > > > > This adds an unnecessary call to folio_test_anon() in > > __folio_add_anon_rmap() and __folio_add_file_rmap(). However, the folio > > struct should already be in the cache at this point, so it shouldn't > > cause any noticeable overhead. > > Depending on the inlining, we might have more branches that could be avoi= ded > (especially in folio_add_new_anon_rmap()). > > [the rmap code is more performance-sensitive and relevant than you might = think] I thought about making the helper __always_inline. Would that be better? > > > > > No functional change intended. > > > > Signed-off-by: Yosry Ahmed > > --- > > > > This applies on top of "mm: do not update memcg stats for > > NR_{FILE/SHMEM}_PMDMAPPED": > > https://lore.kernel.org/lkml/20240506192924.271999-1-yosryahmed@google.= com/ > > > > David, I was on the fence about adding a Suggested-by here. You did > > suggest adding a helper, but the one with the extra folio_test_anon() > > was my idea and I didn't want to blame it on you. So I'll leave this up > > to you :) > > :) fair enough! It's a clear improvement to readability. > > [...] > > > > - if (nr_pmdmapped) { > > - /* NR_{FILE/SHMEM}_PMDMAPPED are not maintained per-memcg= */ > > - if (folio_test_anon(folio)) > > - __lruvec_stat_mod_folio(folio, NR_ANON_THPS, -nr_= pmdmapped); > > - else > > - __mod_node_page_state(pgdat, > > - folio_test_swapbacked(folio) ? > > - NR_SHMEM_PMDMAPPED : NR_FILE_PMDM= APPED, > > - -nr_pmdmapped); > > - } > > if (nr) { > > - idx =3D folio_test_anon(folio) ? NR_ANON_MAPPED : NR_FILE= _MAPPED; > > - __lruvec_stat_mod_folio(folio, idx, -nr); > > - > > > We can now even do: > > diff --git a/mm/rmap.c b/mm/rmap.c > index 9ed995da4709..7a147195e512 100644 > --- a/mm/rmap.c > +++ b/mm/rmap.c > @@ -1555,18 +1555,17 @@ static __always_inline void __folio_remove_rmap(s= truct folio *folio, > break; > } > > - if (nr) { > - /* > - * Queue anon large folio for deferred split if at least = one > - * page of the folio is unmapped and at least one page > - * is still mapped. > - * > - * Check partially_mapped first to ensure it is a large f= olio. > - */ > - if (folio_test_anon(folio) && partially_mapped && > - list_empty(&folio->_deferred_list)) > - deferred_split_folio(folio); > - } > + /* > + * Queue anon large folio for deferred split if at least one > + * page of the folio is unmapped and at least one page > + * is still mapped. > + * > + * Check partially_mapped first to ensure it is a large folio. > + */ > + if (folio_test_anon(folio) && partially_mapped && > + list_empty(&folio->_deferred_list)) > + deferred_split_folio(folio); > + Dumb question: why is it okay to remove the 'if (nr)' condition here? It seems to me by looking at the code in case RMAP_LEVEL_PMD that it is possible for partially_mapped to be true while nr =3D=3D 0. Is this practically impossible for some reason, or is adding the folio to the deferred split queue okay either way? > __folio_mod_stat(folio, nr, nr_pmdmapped); > > /* > > > Which will help some of my upcoming patches. > > Feel free to include that in a v2, otherwise I'll include it in an upcomi= ng > patch series. > > > Reviewed-by: David Hildenbrand Thanks!