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 C3CC2C25B4F for ; Tue, 7 May 2024 18:42:49 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 3EA1D6B0093; Tue, 7 May 2024 14:42:49 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 39ACC6B0099; Tue, 7 May 2024 14:42:49 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 289106B009A; Tue, 7 May 2024 14:42:49 -0400 (EDT) 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 058FF6B0093 for ; Tue, 7 May 2024 14:42:48 -0400 (EDT) Received: from smtpin08.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id AACD31A0C37 for ; Tue, 7 May 2024 18:42:48 +0000 (UTC) X-FDA: 82092471216.08.BD56B7F Received: from mail-ej1-f47.google.com (mail-ej1-f47.google.com [209.85.218.47]) by imf23.hostedemail.com (Postfix) with ESMTP id D42AC14000C for ; Tue, 7 May 2024 18:42:46 +0000 (UTC) Authentication-Results: imf23.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=PHK2Q6Uw; spf=pass (imf23.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.47 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=1715107366; 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=HBJwzDuq7a/1GG57UNlyVV56KYXXK+eUCnGXBNvuYsY=; b=1gno2Ek1er/j24EnH4EJE35B40yLn5GIngeRB2dOobQq8tJKmDb6YFEHAeP298BUeCVfU9 VznvIkVE4E5IJap9zWHZKlYEHAKsVClv9R4XsNf7EQwHJPruTjKtOpiAMyZnV4FG1VRwYB z538hmYMC5JRPTJZzCdgYMSzqPiTEYE= ARC-Authentication-Results: i=1; imf23.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=PHK2Q6Uw; spf=pass (imf23.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.47 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=1715107366; a=rsa-sha256; cv=none; b=yio8+Y1tgV+OUGL6qAUwsys0BxjG+kdX/rSYHfsNCtq2F+1pMe2+vCDVob7msaXwoWtQKt uonEAFsQ+gOb8fhYqKl2KtcTq3592IJZGkWkOsifehAoHfklNHKXiy4vNyecN+ipg/MEcP t7A/C6Oy4O1JznF59i2z0xr5WywadTQ= Received: by mail-ej1-f47.google.com with SMTP id a640c23a62f3a-a599af16934so881397266b.1 for ; Tue, 07 May 2024 11:42:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1715107365; x=1715712165; 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=HBJwzDuq7a/1GG57UNlyVV56KYXXK+eUCnGXBNvuYsY=; b=PHK2Q6UwaSi2H3aAc+2crrbYGW3MlpqsA/sZX8ZWprqx/4ek5SCOHOF17Cd28DBZmf NGJUAuIvvKB26C6FAAaLLAbMUjuhzUmPV1zRm9voPVdog0+VhUcj7z338W6I8ucbLUT8 U0GW/o7FnWSrEeZsDjiR+WeU9qIMH/uIb3EMPTpIKLUz0XumisdMFjOjqRGbDOp1YAFP ey0aPaJKLNhUJUeA9kA+r1R2AiqD0TpjQwF+kMI7GcFKMqgeXWxnQhx9sT2yybqpmvBJ 5tlJdqaTRkzM6TEGkq1me+L234rjZIt2wjKLP2DpijpVmkX3+crkJiJWEK3qguGnuFqZ cDag== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1715107365; x=1715712165; 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=HBJwzDuq7a/1GG57UNlyVV56KYXXK+eUCnGXBNvuYsY=; b=eI1zd49+ols4aqHcXp6DcIDNQNh4bkqy+g526FVvT7mH8/WkWcoDpU3XclaKIECM6v G/kbsKht3+4R4qRffwshkbrmYgitz6vIU427zAmA03ZuzuHCQUqUDJcIFHUiEFXvHl78 BSS0Z70N2yWugg8k8CG8mVbMU11MvxQbiBo7Y5pGdgH8j4apsVwU14hDl8wJTfh9iLry AfDQ0FdLsq8KU8SFEeO6wJ6++MeB7C0ke/tA5+OhDk6K68m+2K/0JzeNWNAloKY+kpRx kP1yCgdYNoVh5zQ1WOwsrQ80ZU5WaHL5WNXc8byKBgN9aC2cxbsY6+8HcInvWIPFDu3t v06w== X-Forwarded-Encrypted: i=1; AJvYcCVrHXKp5MdEpvHgU3ODZffYOEjMeWzlrPJibROSUfeTVVdhHJ9UGA7kvxP5hUNfZbl8D98Ka7RFVm56luJsoKKBxus= X-Gm-Message-State: AOJu0Yx70/ILFsDW99ci4vSIFQ0Jttso3DJfJiNqeTKygTdlloDDCVuk DBm3oU2bPAY0AcRLCyGyYFltxzXAT7zptZAX6l2PSSQrtU81zsSI9qjVE/soZ6hYFr8QzBwUyUP BL6Ag6XF6yVvuI/YLRmz4Ys+puNaVWO0vFv5e4V4OjH5DhsOTQPsi X-Google-Smtp-Source: AGHT+IE0eddGWRUD/u6sVgPGqrSIh1excx/dLF1miP1H89kcgRzA7f5C9SOOgwaanvTdkG1D2pEBaaReidGYG63gAIQ= X-Received: by 2002:a17:906:64c:b0:a59:c090:7781 with SMTP id a640c23a62f3a-a59fb94a633mr17613866b.3.1715107365190; Tue, 07 May 2024 11:42:45 -0700 (PDT) MIME-Version: 1.0 References: <20240506211333.346605-1-yosryahmed@google.com> <1a408ed1-7e81-457e-a205-db274b4d6b78@redhat.com> <42a18f08-99a5-4bb0-8a1d-771549cac046@redhat.com> In-Reply-To: <42a18f08-99a5-4bb0-8a1d-771549cac046@redhat.com> From: Yosry Ahmed Date: Tue, 7 May 2024 11:42:07 -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-Rspamd-Queue-Id: D42AC14000C X-Rspam-User: X-Rspamd-Server: rspam04 X-Stat-Signature: 448ygzjqdp7urur3cz7kna4yddt3zg6e X-HE-Tag: 1715107366-855799 X-HE-Meta: U2FsdGVkX1/ADst9h5H/tEc2INtIxUbtvwnwTCvHUA1drt6nGYt874koo12khUwcsRW/uTbFH9W0NvnA5+Rj35qxJYXkK+UbG+2Dyi5sXL/0/G3sgP3Fr2PUi8u6J2bRNi+7LPyeG41TpJkflZFwOv/x1Q2UnOhZv/y4jJYm45UqM8gpS7xRHVz0grIzJRbl3qtnSFSRBX8CscMjMZR64oGig/Z9UBdLQurhZbWT6iSF+bWz+Ov+ZXrNd0i77LO2rRJ0DNIhWLx8oMuwTkO9d+CA9fhov6kCfZ/6x4kKWA3uSt0PP0L/HhMJ5bbqPxH26eK9MXkfH4F2zalIeiNY1hupwdvp9fmQkTuL8zvJVBGiY8X2eYwMCj8O1e+bYuZt+Q1Cn27IhWzZfkXMyKAg0PEKt5jwUzA7QtbQKtAtk1lWxFjUoHoefcdUjSufnqy5m1t1hq2966MU6/6fa9zDoXM1zzIg92LXyCE+4bIfgF/oO8HdqTOLzU1yefx7X3lYuYKnu8p7k5adhp42QDntYjSPqGWilHAQEhmY8KTxnEYPJuoIANATKGotIdOsjoNa6M8gwX1xFAMagUhrN+ilZeSv/DuK8J0R0P2pn7TJRb5WdHVFcvMUs4LLB2UCb78EUSxcbinhYZ0222w3MVBJV7D1We+FG8Dpih3aO05arKiNj4PSkN37esyj1duLDdGWF7ZRgSHGu5U1b00OEcSUwJfvzBb48Nki/sSvWMjUfo2b5K9BOkAzFUtMl6it1hhLJ9p4/IVQgqYzVcpXkLrEzuoGCXrqVhjUqsaYWWaCNATD6RL2988oBcPbAiKkfrPcASXqle2aVUpE+eXT0xtNAy5YsW8q1EGDZg8PU15EqKYLxnHAfqNkt/HzGOKeODSDR6/UTKvSVIK4TjOOIo5Va357okvuEKLZ6p6shaLRzxaeSLMDk13yxcbRLMokZQDSSonKuxmXyv/fXmgt14V JauFrwNb rTUZ/I8YO/hpUuzKalPeNCC8sgjY6iWy6o7Acs1z25j+hUOi1ZtAv7wtBQjzMferqyyMhEdqEZeCJ8VWB/gSId9z63y6YvNLhyvWLJEnjv8Rhxep7WeUiCMnKvkZ18qdlXcVaUWyLA9cK7fu2uwOMPVGh5SWzsBYrn7yZjKoZjrK6/W4zdyWJ4TuAjJjU4d9mcQI5lofB4rq4eI6yfK+A5qb8sYMHxtt8FdO4JMIjnkxuZzNb8J1TxbG8HA== 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 11:38=E2=80=AFAM David Hildenbrand wrote: > > On 07.05.24 17:54, Yosry Ahmed wrote: > > 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 removi= ng > >>> mappings: which stat index to use and which function. Abstract this a= way > >>> 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 fol= io > >>> 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 a= voided > >> (especially in folio_add_new_anon_rmap()). > >> > >> [the rmap code is more performance-sensitive and relevant than you mig= ht think] > > > > I thought about making the helper __always_inline. Would that be better= ? > > Let's leave it like that. I might do some actual measurements to see if > it makes a difference at all. That would be interesting to find out for sure. [..] > >>> > >>> - if (nr_pmdmapped) { > >>> - /* NR_{FILE/SHMEM}_PMDMAPPED are not maintained per-mem= cg */ > >>> - if (folio_test_anon(folio)) > >>> - __lruvec_stat_mod_folio(folio, NR_ANON_THPS, -n= r_pmdmapped); > >>> - else > >>> - __mod_node_page_state(pgdat, > >>> - folio_test_swapbacked(folio) ? > >>> - NR_SHMEM_PMDMAPPED : NR_FILE_PM= DMAPPED, > >>> - -nr_pmdmapped); > >>> - } > >>> if (nr) { > >>> - idx =3D folio_test_anon(folio) ? NR_ANON_MAPPED : NR_FI= LE_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_rma= p(struct folio *folio, > >> break; > >> } > >> > >> - if (nr) { > >> - /* > >> - * Queue anon large folio for deferred split if at lea= st one > >> - * page of the folio is unmapped and at least one page > >> - * is still mapped. > >> - * > >> - * Check partially_mapped first to ensure it is a larg= e folio. > >> - */ > >> - 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. > > Not a dumb question at all, and I cannot immediately tell if we might > have to move the "nr" check to the RMAP_LEVEL_PMD case (I feel like > we're good, but will have to double check). So let's keep it as is for > now and I'll perform that change separately. SGTM, thanks for checking and for the review. It appears to me that no changes are required here then :)