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 C921FD5CCB9 for ; Wed, 30 Oct 2024 15:27:43 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 443F98D0002; Wed, 30 Oct 2024 11:27:43 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 3F3AC8D0001; Wed, 30 Oct 2024 11:27:43 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2946B8D0002; Wed, 30 Oct 2024 11:27:43 -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 075878D0001 for ; Wed, 30 Oct 2024 11:27:43 -0400 (EDT) Received: from smtpin03.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id ABBDC8056D for ; Wed, 30 Oct 2024 15:27:42 +0000 (UTC) X-FDA: 82730647104.03.C22FEAA Received: from mail-ed1-f53.google.com (mail-ed1-f53.google.com [209.85.208.53]) by imf19.hostedemail.com (Postfix) with ESMTP id 451CF1A0005 for ; Wed, 30 Oct 2024 15:27:08 +0000 (UTC) Authentication-Results: imf19.hostedemail.com; dkim=pass header.d=suse.com header.s=google header.b=DmVp1omP; dmarc=pass (policy=quarantine) header.from=suse.com; spf=pass (imf19.hostedemail.com: domain of mhocko@suse.com designates 209.85.208.53 as permitted sender) smtp.mailfrom=mhocko@suse.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1730302005; a=rsa-sha256; cv=none; b=wH73I/IjEii8CsOqrsAA46Or1Jusg5NSOk4XnwO2Piy9ZsHsr3zwekBHHcSf3nosIwoz3y tPW/EcAvMu/zIGFrHT1Nxsncn/Kz7Se0NxvacRtpdO8JTUgUBWZeks9gX+m3WPdCPxs6Dh GVFqp0OV4Nri203VxOd999aachy8TIg= ARC-Authentication-Results: i=1; imf19.hostedemail.com; dkim=pass header.d=suse.com header.s=google header.b=DmVp1omP; dmarc=pass (policy=quarantine) header.from=suse.com; spf=pass (imf19.hostedemail.com: domain of mhocko@suse.com designates 209.85.208.53 as permitted sender) smtp.mailfrom=mhocko@suse.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1730302005; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=WwB5DeVTS7vafGaY+6J9Yo9yIy438E4Wo0/qN3/NQSc=; b=JQ1RCOeG7HiLUEOIaig619/ZJEA2UZ5ApqEzzHBiPTNhkOGFx5+7xgXlbn/CLBR7jHgKTA DRgId8tbuDPFsouTN8tH1qtO9NS2q9t2w/tiB+e8faCDhb5tIHGUXPninrKCRUYvcQ2/tm sHvc9AtEwa/vouV1o7eJTtZK3rpdGCA= Received: by mail-ed1-f53.google.com with SMTP id 4fb4d7f45d1cf-5c9709c9b0cso21718a12.1 for ; Wed, 30 Oct 2024 08:27:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=google; t=1730302059; x=1730906859; darn=kvack.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=WwB5DeVTS7vafGaY+6J9Yo9yIy438E4Wo0/qN3/NQSc=; b=DmVp1omPrNne4ppDQ1pwYEq0d5Amcku1F+pjrkuFKX9JeulnnvlJWvhOSRZ3gGjSCz DPag0sPapIFRBzd65Ucy5KJR7e2ulbSP1y2LsfTewN7g6h+sAKic1j6Y6vNnDo7JUirh pBZGuNpSGUFe9GNzt5VcKkdlEDDGegv3GBm+zUlMEyxuvET0h6r2AnAPwXmiO+fA7GBe dx5XUesxd4FHVipdvYwhNFOqX2B/b6ffCImgfokiz70cJ6OG4AwnJY7CqoMuIjhfhYG2 iSYwGwdjhW4unqEfyK1BPH2zaNVg1pdg+3WgLGkl591v0AIeUbKzQPmIqwEzzh6i/Y1I dBgA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1730302059; x=1730906859; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=WwB5DeVTS7vafGaY+6J9Yo9yIy438E4Wo0/qN3/NQSc=; b=BqgU1uFJWyHBcNjIrgkmcSmim/tG5nFy+OdwKlY68KES+rk3IMoIi38Go2iMc2tGdp KT+L2K1aj4ImT6/elYs/sPQ4zVfeAFIozTp2fCiLN3HoiVh5QjMUW7XHtMr+++Bwy6IJ Bay7FhBca0Lzr83mByrqALmWJHBWaR/dVAjTlFFX1WCRwEil1PLvEhzrtbgCtdMV8hRT xf/lHxxEHTm1TDDYnZVR48Q3yyeIAyiE1rf4KtD2srIALXfGYVFKggEQGjZuAYoBWsIr o0o4mLx6uDo9clc7kMrRrJu0/CoqvQdrVhfhGlv/PLE0vCmVflUOXHYC/UerW6BIZPLu iCIg== X-Forwarded-Encrypted: i=1; AJvYcCUkxpQzwYRXfbNAersKa+3FgJiiYYprzJDCNqFyyczeieuxsLrN+j5Kin73plWO/+mzeeLH9Y1qRw==@kvack.org X-Gm-Message-State: AOJu0Yy+3+OgLEJetZZixcOpGr8ChVKAtc7KuZmP1DLvHl0KNyZyMnvk 7iqO8lfHbXcJMwYKyJ/5ydmYi/7obL0JESQuCXoQKu+Wba0d6o1Oo7bM4hSNRxY= X-Google-Smtp-Source: AGHT+IGmLSt0zTkktlnYGAfkGITlISBKJ2hsSa2xN9IdSwZmvdJA4vg8q68zy3/an3of/jLhkV7m6w== X-Received: by 2002:a05:6402:524f:b0:5c9:7f8b:4e3d with SMTP id 4fb4d7f45d1cf-5cd54af10dcmr2933577a12.23.1730302059107; Wed, 30 Oct 2024 08:27:39 -0700 (PDT) Received: from localhost (109-81-81-105.rct.o2.cz. [109.81.81.105]) by smtp.gmail.com with ESMTPSA id 4fb4d7f45d1cf-5cbb62c0cb4sm4835864a12.43.2024.10.30.08.27.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 30 Oct 2024 08:27:38 -0700 (PDT) Date: Wed, 30 Oct 2024 16:27:37 +0100 From: Michal Hocko To: Johannes Weiner Cc: Joshua Hahn , nphamcs@gmail.com, shakeel.butt@linux.dev, roman.gushchin@linux.dev, muchun.song@linux.dev, tj@kernel.org, lizefan.x@bytedance.com, mkoutny@suse.com, corbet@lwn.net, lnyng@meta.com, akpm@linux-foundation.org, cgroups@vger.kernel.org, linux-mm@kvack.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-team@meta.com Subject: Re: [PATCH v3 1/1] memcg/hugetlb: Adding hugeTLB counters to memcg Message-ID: References: <20241028210505.1950884-1-joshua.hahnjy@gmail.com> <20241030150102.GA706616@cmpxchg.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20241030150102.GA706616@cmpxchg.org> X-Rspam-User: X-Rspamd-Queue-Id: 451CF1A0005 X-Rspamd-Server: rspam01 X-Stat-Signature: 1b1ytud98ec8m64boh8hojo9xeip1hoy X-HE-Tag: 1730302028-162510 X-HE-Meta: U2FsdGVkX1+cZNx14j4/XGROnl+ZsdmbltajpOP1PKraiFXAjy8odHyINyQtO+OG+hg0REsvfUwmVjqFjOMyn0Fn0SMJ8tru6tP+KNLJH1HSyqZdj7h7MqpsI209uEH7a/A9Dwko0htNYUbkeBsfMcwiwXj6GZrZT/Pr9jN0iXAsi070SaoV+739sHMHP5Mr+yppVaFJtJ6gDJI4X7uPZ+7gqQRMYFapdnrvNbeJfTbl5NH5i0RAe6jMNdNBa9VG030DQmcblLZ6hDfVPbLDasswllvpKA1jQi4qGhYE9Al/8+2uDJSH7eCgH9Aa9pyPJO1cUHxaezSvymKFhye/3njdeqR5soLjEqa3m8D8vmauUvzZ+N/blph5T7qMmR4da2BmrF2Z3ByVfiXS11A81SwzqXMpcC+gXwwCqg1e6P1Aj6DYxz0w/mKD0UPynTzL2ovrX7dc3rDbZX95E80e+3tDA0jThceNXO0iaM+jwhnDrkb9dNzB0ppdQz+Dgzes3CQCv921biXyILpEXLVW8s7EjPYRg58wOnL4j84CAy5hfQ80foFhVD3V2o/vQChnVkb+ivKRo8evSsVT2aZO7FwJ1X2TtMFOu7YO4eISMDdR1eXSa0xqSrl2zvKaFwdo+FwdwtcHlwuV4dflxW8Avh0NHXOwU8hIkt98VcHZ+yeX5HyYEoeQNUM+TZ7s3UBdQE4YkVCiIylDhH3CIxAG0DNFC25CkAy/17qB6WBVTOZNcfp5B89Ifi++u5MkF2hLZO7XuLNMaKvuZjpEJqIIW5bqZZhyEAQs9JfuN6bdPPPVSDjRyh6Rf45XoccI8kZbaw6W80i2kO3eVMPMYquTX/bI9fSkTUAWoTzFWKVavhtneTmGmOfTylIe5f+rwKQW5SbW3sy0PIZFTEptE1/AoA4QbsvAfUvTXV3gUoRaw05uEc3E2/uNAP8vn+jNwjOjShDam9A1eQeYs6SvQ3D 2o+25/WN Dfm7v/AXrzT8zZjLcqta5Y+X72FrzIUvpfWMM6PhMm+e2XK5DfJ+8pWyr9/ioKLGz+dyAlNr9HI0f+g8R0EI0yhH78hz03s1rZGF9kmqQzYf40rvSOpFNMvAKq8mhr7l/wKS+QmvEajlvn+CRw17edcDCTb974NkZCvgzNv4x/fpVm4wp5+GJgJLh71QblVJXqRsxC6+e0yD2uwI6jvYaG6aXtlnc+4Pw2jxJ78ePpdIc/QyvEsUPllM2zh4jj+dhVCWI6+i4AqGPGrgi7LEdZzwxNFLZEgzyqjwqIpmqdhYNxhXwaz1atUS/2BVS5BXtDRYcYcEn4YehYHFzn82nt+G5/AuTBXfYr0rGC4BFEFsG73WoCXr6rIdTcar8JL7cnvq4mGu4RUcDWYLSUOH1CANXLw== 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 30-10-24 11:01:02, Johannes Weiner wrote: > On Wed, Oct 30, 2024 at 12:35:25PM +0100, Michal Hocko wrote: > > On Mon 28-10-24 14:05:05, Joshua Hahn wrote: > > [...] > > > Changelog > > > v3: > > > * Removed check for whether CGRP_ROOT_HUGETLB_ACCOUNTING is on, since > > > this check is already handled by lruvec_stat_mod (and doing the > > > check in hugetlb.c actually breaks the build if MEMCG is not > > > enabled. > > [...] > > > diff --git a/mm/hugetlb.c b/mm/hugetlb.c > > > index 190fa05635f4..fbb10e52d7ea 100644 > > > --- a/mm/hugetlb.c > > > +++ b/mm/hugetlb.c > > > @@ -1925,6 +1925,7 @@ 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), folio); > > > + lruvec_stat_mod_folio(folio, NR_HUGETLB, -pages_per_huge_page(h)); > > > mem_cgroup_uncharge(folio); > > > if (restore_reserve) > > > h->resv_huge_pages++; > > > @@ -3093,6 +3094,7 @@ struct folio *alloc_hugetlb_folio(struct vm_area_struct *vma, > > > > > > if (!memcg_charge_ret) > > > mem_cgroup_commit_charge(folio, memcg); > > > + lruvec_stat_mod_folio(folio, NR_HUGETLB, pages_per_huge_page(h)); > > > mem_cgroup_put(memcg); > > > > > > return folio; > > > > I do not see any specific checks for CGRP_ROOT_MEMORY_HUGETLB_ACCOUNTING > > in these paths. I guess you wanted to say that you rely on > > mem_cgroup_commit_charge setting memcg pointer which then __lruvec_stat_mod_folio > > relies on when updating stats. > > Yes, this is what Shakeel pointed out here: > > https://lore.kernel.org/lkml/il346o3nahawquum3t5rzcuuntkdpyahidpm2ctmdibj3td7pm@2aqirlm5hrdh/ It belongs to the changelog. > > I suspect this all is done because you want a global counter to be > > updated as well, right? Changelog doesn't say anything about that > > though. Why is this needed when /proc/meminfo already describes the > > global hugetlb usage? > > Sigh. > > vmstats is the preferred framework for cgroup stats. It makes stat > items consistent between global and cgroup. It provides a per-node > breakdown as well which is useful. It avoids proliferating > cgroup-specific hooks in generic MM code. > > It was a ton of work to integrate cgroup stats into vmstats and get > rid of all the memcg special casing everywhere. You were there for all > of it. We're not adding cgroup-specific stats unless unavoidable. > > Duplication doesn't matter, either. We have plenty of overlap between > vmstat and meminfo. By all means, send a follow-up patch to have the > meminfo one sourced from global_node_page_state(). > > But you know all this. It is not really important what _I_ do know. The commit log is not written for me. Joshua has greatly improved the motivation part. Yet implementation specifics are still lacking behind mostly left in Changelog part of the commit message which will be dropped along with the diffstat. > I'm having a hard time seeing the way you are, > and have been, engaging with this patch as good-faithed. Sorry to hear that but this hasn't been my intention. My main focus has been to put implicit assumptions into patch description. I really do not see reason for such a pushback TBH. -- Michal Hocko SUSE Labs