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 23C67CFA47E for ; Wed, 23 Oct 2024 21:38:04 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 2A9046B00A2; Wed, 23 Oct 2024 17:38:04 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 258EB6B00A4; Wed, 23 Oct 2024 17:38:04 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 120136B00A5; Wed, 23 Oct 2024 17:38:04 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id E88426B00A2 for ; Wed, 23 Oct 2024 17:38:03 -0400 (EDT) Received: from smtpin02.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id DC8CF40ED6 for ; Wed, 23 Oct 2024 21:37:53 +0000 (UTC) X-FDA: 82706179836.02.5FB96CB Received: from mail-ed1-f48.google.com (mail-ed1-f48.google.com [209.85.208.48]) by imf08.hostedemail.com (Postfix) with ESMTP id DBB6F160005 for ; Wed, 23 Oct 2024 21:37:49 +0000 (UTC) Authentication-Results: imf08.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=1xlxtBaU; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf08.hostedemail.com: domain of yosryahmed@google.com designates 209.85.208.48 as permitted sender) smtp.mailfrom=yosryahmed@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1729719443; a=rsa-sha256; cv=none; b=qVp0+n9jnNqUQ518s4kIqryRpmQsMsO4jsX3J+UMJOHjqDefphh+pdJJttKXMlO5hfKZzg rduFkJTxwkZPzBuG3RR5zTlgozjWKsH4MlhbZsgYVNHf9xGyaqoAzsHYQ4O1aD4Ic0HDsO mGJjg3rE53kDzAuPLES98X7KcAVAeOU= ARC-Authentication-Results: i=1; imf08.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=1xlxtBaU; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf08.hostedemail.com: domain of yosryahmed@google.com designates 209.85.208.48 as permitted sender) smtp.mailfrom=yosryahmed@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1729719443; 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=N7W0tu6mfaHUoou8fq3H8oaPBI6nhuNRuYpGlTyNA/w=; b=B9vBuw9xbs8gLCP+7fPDzRsUgJiM7uZCagQPPc7QqDw6ZIxvYd3H1Qy2pVD117ctyHA3/7 vjK84PfoqtBYK1nH3sPKPBrHKrX+yBbgqjpzW5cWLDq7JPCAXSKbpgiXSvvbxBL24W4wDK WgkRB1rmIzpIns9yJecWde6Ge5RwJo4= Received: by mail-ed1-f48.google.com with SMTP id 4fb4d7f45d1cf-5c9150f9ed4so311875a12.0 for ; Wed, 23 Oct 2024 14:38:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1729719480; x=1730324280; 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=N7W0tu6mfaHUoou8fq3H8oaPBI6nhuNRuYpGlTyNA/w=; b=1xlxtBaUm161QrUCYKpCvcgaz6YbteqsA+1cw+LDSoEI70ohA4Qr2v3Fpz29AEkb+O BcprKABvdJSeHgt0DG42S1Z4CaLmlNXEEyfUjwCu3/g4fT9ovAZlOMum3VY2G7xbngIq rrbvDQa5WNi5/OdttNrj6sv8WeidelB1lE9UsWZMkyRVLhT1zmB95zFO8W8sAAwrplQZ 9aoRiBuNnu5+jTnAj6ThquGoX13rUuECD1yIZjPAYUGr87uJgUQWJCDRFqukHeZHa0bh M1I6FarXd1r6XH+P4xxh706xob/8bN/MmB9AKmiEtzgjNAHtBlgJgv2+WXIm2NA8vg+b LQPw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1729719480; x=1730324280; 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=N7W0tu6mfaHUoou8fq3H8oaPBI6nhuNRuYpGlTyNA/w=; b=Wt7kTdWiQbM/8CDeJn5mvbxoSBbH081CxBlK7SQbVv+LVh1Vf8vbfira/Tp8QsvtjS YRFggirrEk1TWCTNGn/vUWXJmaKIRWz5Il3YhHnDTvIAazl1ldJK6Ftm9jV4DTXwxoWC 3WJrJXpOqye+/1io3A+kKsUhvEfUpio+l6ll/YkSKu+L9uMMpZ9ocKB6OY1VapYX5oyM khZMxkDALSV3sq72JqiUz6ZX7aANzd7qtlJtgEbGcvK6ITyusa4mwxj1lqC4DwjRVsHG eCQLg7TLsPpQWFgg7LDzPrssYeLRx000bHV/hurdmpQh2GIoqwIh8FbbhP6+ytEY5sqf 08mw== X-Forwarded-Encrypted: i=1; AJvYcCVvpiDtwu+IXEgJ3TluDInCLOD1xHF1tTiu45/VuL8aLqYPTJIxWpU2lIuzO2Azj1M9DvRrfJG8AQ==@kvack.org X-Gm-Message-State: AOJu0YwbbErsomGl85pKqbdjf2GUBpREY0kHre5FwbTO5H3riyPzX+Ub PLCyQXZSPJBMu5cKZCehTLHzK7RlP9oikXsrRViPTkaXK50XTJA5wEWRhe6HNKWYIVl+uU7TzbV cZRz4D6KhqNAKRcnb9oOi12qf7jw0Cb6rpgnJ X-Google-Smtp-Source: AGHT+IEkEsqYuEyoOEPEtrFPG8xRsxQ5sOvL9rvaBgxs2alYdsADn5R9r8cwT2t3P//Fjj3xTrRXB/KjbzzO6U9mYRg= X-Received: by 2002:a17:907:9455:b0:a99:f4fd:31c8 with SMTP id a640c23a62f3a-a9abf8778bcmr458459166b.22.1729719479816; Wed, 23 Oct 2024 14:37:59 -0700 (PDT) MIME-Version: 1.0 References: <20241023203433.1568323-1-joshua.hahnjy@gmail.com> In-Reply-To: From: Yosry Ahmed Date: Wed, 23 Oct 2024 14:37:23 -0700 Message-ID: Subject: Re: [PATCH v2 1/1] memcg/hugetlb: Adding hugeTLB counters to memcg To: Joshua Hahn Cc: hannes@cmpxchg.org, nphamcs@gmail.com, mhocko@kernel.org, shakeel.butt@linux.dev, muchun.song@linux.dev, lnyng@meta.com, akpm@linux-foundation.org, cgroups@vger.kernel.org, Linux-MM , Roman Gushchin Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Stat-Signature: en5ugzn9owf5y8t1b79k3uben7z7e887 X-Rspamd-Queue-Id: DBB6F160005 X-Rspamd-Server: rspam02 X-HE-Tag: 1729719469-365072 X-HE-Meta: U2FsdGVkX1/EU8X7luAEuvMqML8ujysx1VfqugyacLRA5BGnZnQDJ31Xukd+NFpKlgT4xTbUCu3IcJ4KCwh6GC8BezjkiFEWYtRswIv+tq4ovmpwscaKFsEektwDDIKnTSlqBHv6YURrhnxSALc0evMeF13tT8D4E5r3xn6j/bL8dOcGMBSKZaEBjpIO8oZrtgRe0UAwAk5+eLgQicwBdiAZlAyzQ4H7NnmDuQfArOA3SV9JCAH1sDUsP4VoIk82fJ4b5FqYtUpx2l2vEkaBuIksXm8MyqNtB4/cT13enm+kXXE3L1+o0hiiaxRo0fQWlHEIRbqFWRVY/NeUZE/+hOkbAmKUP8V1MoqVaAvr/vfcUmMKQZUDtVPDvVCISv6ksjuOdT6jaXCOsg4+xN69AEo1DJ1cbqlGLkxMlGmqTtocvitE1i09TNcA9PQ3vQqfhyQEy2/2wGnG3wjfR+U1aHbRX3+TrMijWnl9NrSzidPopvB6Uelta+NQKtFBjjqHdEBTvntwrWT+L/HFqOcTphOt87FEf3O2jzwnVjXIWML6eM7KzU6Gl2EqQFSjXAHq7H8OqtmsRBBxF4MN4INZb97TiatyEy3aOFuKfWo029Sp7phTT8NhhznUAmBT2qcZnA8qLcGjmnaCBPMkwK7j8lhAOSg3vZ44nKv8hfhul/caA/H+v9/y/RRxkZmHzMBy+QSkZXEvWEGZhgGCku2zcw1jFDuEVtkkgPUEymYdHpOnhUjF9zltw3tN0emOt1EaF1639zswpabwnEtZNY3+8FdTe48JMnrrww63NcOoAgioDj/qaKbI/5mymNqhzxmSzaL1vJz/aYoq6Hcmi0IZKyPoKjaFscJPAXZz4IjFzEkuMmELguYrJrk4/igpvV5LmBWIdNnK/Tcagd9LVTN9NYOH7lHPaX4s3DRBuaTr5oE4SZkPiHrG77N6mju+BEEKisyzkJXhHR6G3H0/Q93 L9L4dizo qeXRniEvybu/2CtHRgA64fkyqINV8Rzs8IP61jyyE7Mvj7YEE+MVNQf3UbQsToRBLOUWQawUSqZKA7KssAcvBccIa+CnyykvGeE3RAJCCKCrFG3vw+GiE8i17J7vR0IcqcMCGsHdR4968XfOSpK+bXFjBNrWCkGV0ftOV/hfuzWKohbqU1BgaoGzb9tVAdHjAFu2E99/Tp+0nBGAyMidE89NIoxD3i8eDwKS6p2/rRcKR4xffzPn7MhB7KiiaMKRtT+7VcbCI50lFscyqtOrv0Rkst/yapq5yWKIfkvRAnw1eTDIoMEyEOJ6JwQ== 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 Wed, Oct 23, 2024 at 2:32=E2=80=AFPM Joshua Hahn wrote: > > Hi Yosry, thank you for taking the time to review my patch. It seems > like I made a > lot of silly spelling / grammar / style mistakes in this patch, I'll > be more mindful of > these in the future. > > On Wed, Oct 23, 2024 at 5:15=E2=80=AFPM Yosry Ahmed wrote: > > > > Hi Joshua, > > [...] > > > diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h > > > index 17506e4a2835..d3ba49a974b2 100644 > > > --- a/include/linux/mmzone.h > > > +++ b/include/linux/mmzone.h > > > @@ -215,6 +215,9 @@ enum node_stat_item { > > > #ifdef CONFIG_NUMA_BALANCING > > > PGPROMOTE_SUCCESS, /* promote successfully */ > > > PGPROMOTE_CANDIDATE, /* candidate pages to promote */ > > > +#endif > > > +#ifdef CONFIG_HUGETLB_PAGE > > > + HUGETLB_B, > > > > Why '_B'? > > I added _B because I am measuring the statistics in bytes (not pages). > IIRC some stat items use _B at the end to denote that the unit is in byte= s, > so I put it at the end in the spirit of maintaining consistency. However,= if > is not the case, I will remove it in the next version. I think ~all the memcg stats are exported in bytes, the ones that have _B are also counted in bytes. I think in this case we are counting the stats in pages. See memcg_page_state_output_unit() and memcg_page_state_unit() for more det= ails. > > > [...] > > > #endif > > > /* PGDEMOTE_*: pages demoted */ > > > PGDEMOTE_KSWAPD, > > > diff --git a/mm/hugetlb.c b/mm/hugetlb.c > > > index 190fa05635f4..055bc91858e4 100644 > > > --- a/mm/hugetlb.c > > > +++ b/mm/hugetlb.c > > > @@ -1925,6 +1925,8 @@ void free_huge_folio(struct folio *folio) > > > pages_per_huge_page(h), folio); > > > hugetlb_cgroup_uncharge_folio_rsvd(hstate_index(h), > > > pages_per_huge_page(h), fol= io); > > > + if (cgrp_dfl_root.flags & CGRP_ROOT_MEMORY_HUGETLB_ACCOUNTING= ) > > > > I think we already have a couple of these checks and this patch adds a > > few more, perhaps we should add a helper at this point to improve > > readability? Maybe something like memcg_accounts_hugetlb()? > > [...] > > As far as I can tell, these checks only come up in mem_cgroup_hugetlb_try= _charge > and cgroup_show_options. Shakeel already requested a cleanup of the try c= harge > function in the v1 thread, so I think the best course of action is to > add the helper > function in this patch (series) and use that helper function in > another patch series to > clean up the remaining functions. Introducing the helper in this patch/series and using it in further cleanups makes sense to me. > > I'll be sure to add the other grammar / style changes that you > mentioned above in > v3 as well. Thank you again for your feedback! > > Joshua