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 66A76C3271E for ; Mon, 8 Jul 2024 11:36:46 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id DF8506B0083; Mon, 8 Jul 2024 07:36:45 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id DA6BC6B0088; Mon, 8 Jul 2024 07:36:45 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C48356B0089; Mon, 8 Jul 2024 07:36:45 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id A568F6B0083 for ; Mon, 8 Jul 2024 07:36:45 -0400 (EDT) Received: from smtpin15.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 60521161255 for ; Mon, 8 Jul 2024 11:36:45 +0000 (UTC) X-FDA: 82316383170.15.5D6BB03 Received: from mail-ua1-f43.google.com (mail-ua1-f43.google.com [209.85.222.43]) by imf08.hostedemail.com (Postfix) with ESMTP id 919EA16000F for ; Mon, 8 Jul 2024 11:36:43 +0000 (UTC) Authentication-Results: imf08.hostedemail.com; dkim=none; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=kernel.org (policy=none); spf=pass (imf08.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.222.43 as permitted sender) smtp.mailfrom=21cnbao@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1720438570; 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; bh=Mcw6UNrBJv8rAqjjTUhPR1XEJKbzTv66sPWbmXvxFww=; b=2ekszQu6Ix5EH8ZFQH2fU1RpjAbZFKE5DXbpuRxupZarT1+sDk3WPSA3RSCJX/CmjTD8t4 /w5qAGCm2yNM5Q2BSEIjccwdiKiaHxx4rVsVXhZPeW/3EVaWeFRpDXt09w20GqcknQ2IDM 7UT33UFgWDOqwsSg2rjgHW6unZLviQU= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1720438570; a=rsa-sha256; cv=none; b=POwuMsFSKW4UxJfh/OXK1P9d98nZmZxPy4qqjq1fqZ8v3tHvk8BbmKx0ofu31DepomOIPf s+F6OwJSbnDlckjfQcFIYGj9ajIGOqrzfvDiEmeFh1wZyotU12nRcp7AN2zBkDtRdhGXxF T18Dp7Z1S9S/FEmSRVKkqzwIPliFfQs= ARC-Authentication-Results: i=1; imf08.hostedemail.com; dkim=none; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=kernel.org (policy=none); spf=pass (imf08.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.222.43 as permitted sender) smtp.mailfrom=21cnbao@gmail.com Received: by mail-ua1-f43.google.com with SMTP id a1e0cc1a2514c-81055d6f256so346688241.0 for ; Mon, 08 Jul 2024 04:36:43 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1720438602; x=1721043402; 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=Mcw6UNrBJv8rAqjjTUhPR1XEJKbzTv66sPWbmXvxFww=; b=qxgOVLKKDTVYzS68YWC7WBMB0ayqMy0uWQu9GBj4chGZCDnRO+d4bwR9bwYk9O1vg7 +9Pk/8JvlxGPC4443qUlnMLip8KN5IDpiRO2RTEy/mNckbhx5YHXjp+Ai3x7JE8BjGVD KDPd3Iv2orYATyVzGAN/SaBR98bV2Ju+3ruDpJWV9plUnhUe6ioUiwVixT2PCPnPPyGh HskrOmIIlGUYteo8XZLEOYDP2jAG7wAbRf/vQ1PL8YtJ3lYpDrD6jr7H4c5UKb9k2Oai IlS524EkqUH6mV1JUINWWZjAQn+h7NMp8tCuqDbax1q4ohAB6QuIs8+b18AJ1dllBqaY UFZQ== X-Forwarded-Encrypted: i=1; AJvYcCXVI1YiaMzviCaug2baDXYQ/0TbEOh6lL3MnrEkx4L98GWg0rJD8s1e/JWk8IVXyWrYMUdu6X8nQacwfLaNO/7XLA4= X-Gm-Message-State: AOJu0YzG3n1VIsT47qfUu6mVb6YyF3eBMgXJznvZocFR6QxTRgazRbA+ GNC2Zqn7Y/J8hbbfhVELbaSUJbtf/1jWExv81/4vbD8tYTgV3iWw7UGK2DXADOM3CJxj8GKQgug QnTakjeUz7+6iUi7hfnXxQIuY570= X-Google-Smtp-Source: AGHT+IHAhIdUk9D9MXk/PrnUS4YRbkVtUJ0XMVFt2+q9PpbWqoAja6N72rk/4Gy6UkQoFGKnVETbzpwi51GmbcjCiFA= X-Received: by 2002:a05:6102:d7:b0:48f:df86:db3 with SMTP id ada2fe7eead31-48fee64f3b0mr11713010137.3.1720438602575; Mon, 08 Jul 2024 04:36:42 -0700 (PDT) MIME-Version: 1.0 References: <20240708112445.2690631-1-ryan.roberts@arm.com> In-Reply-To: <20240708112445.2690631-1-ryan.roberts@arm.com> From: Barry Song Date: Mon, 8 Jul 2024 23:36:31 +1200 Message-ID: Subject: Re: [PATCH v1] mm: shmem: Rename mTHP shmem counters To: Ryan Roberts Cc: Andrew Morton , Hugh Dickins , Jonathan Corbet , David Hildenbrand , Baolin Wang , Lance Yang , Matthew Wilcox , Zi Yan , Daniel Gomez , linux-kernel@vger.kernel.org, linux-mm@kvack.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 919EA16000F X-Stat-Signature: cfzkdyzntg4zqj9jtnip3n3jyyqcyg6p X-Rspam-User: X-HE-Tag: 1720438603-160762 X-HE-Meta: U2FsdGVkX1/9X6ayWtKZ0poqhQm5fuYishWoiNP3QP37zunexkGKXebQ1t98bW2dyZiB0Vd0wlBqPH9Y7KAv2GZJl15VYeuWe1UNMExUr4JOq6oLiKrkwhhTWGXqB9W72rBbazWjf/9hqZG1t79oTyFQoCi4URzuSosVjPpOr+pWirazVv71J9fpX98Vt2lZqpBfI+y7IkZkaYgGTEPzqk5xLPWKaYPoh7fzCVz1okyZk58AsIr3srlopcshc/Yn9QM3NYK9h009mtTUQ3WkSjj3RAm31v7w6N4mUW6bU6E8KNd/yLR9m9BKQAWcnZR2JQkfVo+fXlJxK+zuJWTkeCcdF/L8Llgf4gw4TL3UQ9u9Q/0pVEGDkyf3Ilz1cdrNBkcWikNxd4XfpCJl8r4Gt8NDSDeLcj2gKbPM2dv7gG4HMS18tvvX8Dbd9tqVXe+Onw+twHPO/PMI8UlzhSVv7DUF7MHBlnvEM5n2zEPYhKvgIWD+xPLYfIQ6vwhvDpXlnnp64mR2cIQzzXjb6BRGYLKnEnyoFH9e02VXh82wQ+XFapIruBeZJKdX/LsNMELxVtUYZDsgVXMR/bGdpsa316k1/Ilb1URS6NenFA8lYWNmAVmrmPB0iyQqHrkZgkn6NwRx4BRqNYbnK7EX8JdMMi2Lf9meOzYGAG8h7SaUGSq5cDUl2TOkwetghD3BTLMoUiQGJWvz2b9AkAKpTLHNoFgIYrsRKyeaKm4+1PjE2i0MdgGrlyHtqE9nDHLrm3k6+f93l7VjLEszYacOlo+iaHaBglw+H/4uucGiFWb+mK7X6kZEm5YO7ooPYUksTH0frR1GWvdVxaiWuDenz9v6Li4TNoynH4kwAyslkFngcnRBt1SxQW6Zj8Xr5C+o0KBkzVCCPSqKXHVna8thgnCHR6CsObiCoVVmhnVBNXWbB8HNoLHgufuk6NBAysidOORxbb/oKjoz8Xq3O729HAK eUQX2/Lf gzv+weCY+YBj+eyg3j5NqEooVX08WyDhajnG1uHGu0fzv0+rKtpQlzxFj8dRXrQQM2siEbK+iFla1YD1vuwRvSfHtQvZlSu4xZbRi1tVA5JpNYDbuYLXFpnDs3Ly5KUtf7w+xoLo37CBchBV6wz8TCjVBU2uETCMJgVVAW19ybyQOH0R02wQEdRiTfZxNkhNryJpDdlT/yj3tqHHtNhbFfSPYjMa/pSLJ3cO91HzxIQvV2iaQWXZFGdQDY47nbhu94/NOBMbJPyDpqx7VvVP/6Pnmov1W4z4WVJm1iiLZghpZn/LyqYSM2FJyWKTSwCYwDDgVBw1mCNGgPuhDVAQ+6KPXXAoJPJ+JaCF1ec9oDcjcp/WF2VGOsv/Tj0I3+NjYXt4Zoy4ANwxziD9+gMsPhROo8q3wIC+AXU+i 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 Mon, Jul 8, 2024 at 11:24=E2=80=AFPM Ryan Roberts = wrote: > > The legacy PMD-sized THP counters at /proc/vmstat include > thp_file_alloc, thp_file_fallback and thp_file_fallback_charge, which > rather confusingly refer to shmem THP and do not include any other types > of file pages. This is inconsistent since in most other places in the > kernel, THP counters are explicitly separated for anon, shmem and file > flavours. However, we are stuck with it since it constitutes a user ABI. > > Recently, commit 66f44583f9b6 ("mm: shmem: add mTHP counters for > anonymous shmem") added equivalent mTHP stats for shmem, keeping the > same "file_" prefix in the names. But in future, we may want to add > extra stats to cover actual file pages, at which point, it would all > become very confusing. > > So let's take the opportunity to rename these new counters "shmem_" > before the change makes it upstream and the ABI becomes immutable. Personally, I think this approach is much clearer. However, I recall we discussed this before [1], and it seems that inconsistency is a concern? [1] https://lore.kernel.org/linux-mm/05d0096e4ec3e572d1d52d33a31a661321ac15= 51.1713755580.git.baolin.wang@linux.alibaba.com/ > > Signed-off-by: Ryan Roberts > --- > > Hi All, > > Applies on top of today's mm-unstable (2073cda629a4) and tested with mm > selftests; no regressions observed. > > The backstory here is that I'd like to introduce some counters for regula= r file > folio allocations to observe how often large folio allocation succeeds, b= ut > these shmem counters are named "file" which is going to make things confu= sing. > So hoping to solve that before commit 66f44583f9b6 ("mm: shmem: add mTHP > counters for anonymous shmem") goes upstream (it is currently in mm-stabl= e). > > Admittedly, this change means the mTHP stat names are not the same as the= legacy > PMD-size THP names, but I think that's a smaller issue than having "file_= " mTHP > stats that only count shmem, then having to introduce "file2_" or "pgcach= e_" > stats for the regular file memory, which is even more inconsistent IMHO. = I guess > the alternative is to count both shmem and file in these mTHP stats (that= 's how > they were documented anyway) but I think it's better to be able to consid= er them > separately like we do for all the other counters. > > Thanks, > Ryan > > Documentation/admin-guide/mm/transhuge.rst | 12 ++++++------ > include/linux/huge_mm.h | 6 +++--- > mm/huge_memory.c | 12 ++++++------ > mm/shmem.c | 8 ++++---- > 4 files changed, 19 insertions(+), 19 deletions(-) > > diff --git a/Documentation/admin-guide/mm/transhuge.rst b/Documentation/a= dmin-guide/mm/transhuge.rst > index 747c811ee8f1..8b891689fc13 100644 > --- a/Documentation/admin-guide/mm/transhuge.rst > +++ b/Documentation/admin-guide/mm/transhuge.rst > @@ -496,16 +496,16 @@ swpout_fallback > Usually because failed to allocate some continuous swap space > for the huge page. > > -file_alloc > - is incremented every time a file huge page is successfully > +shmem_alloc > + is incremented every time a shmem huge page is successfully > allocated. > > -file_fallback > - is incremented if a file huge page is attempted to be allocated > +shmem_fallback > + is incremented if a shmem huge page is attempted to be allocated > but fails and instead falls back to using small pages. > > -file_fallback_charge > - is incremented if a file huge page cannot be charged and instead > +shmem_fallback_charge > + is incremented if a shmem huge page cannot be charged and instead > falls back to using small pages even though the allocation was > successful. > > diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h > index acb6ac24a07e..cff002be83eb 100644 > --- a/include/linux/huge_mm.h > +++ b/include/linux/huge_mm.h > @@ -269,9 +269,9 @@ enum mthp_stat_item { > MTHP_STAT_ANON_FAULT_FALLBACK_CHARGE, > MTHP_STAT_SWPOUT, > MTHP_STAT_SWPOUT_FALLBACK, > - MTHP_STAT_FILE_ALLOC, > - MTHP_STAT_FILE_FALLBACK, > - MTHP_STAT_FILE_FALLBACK_CHARGE, > + MTHP_STAT_SHMEM_ALLOC, > + MTHP_STAT_SHMEM_FALLBACK, > + MTHP_STAT_SHMEM_FALLBACK_CHARGE, > MTHP_STAT_SPLIT, > MTHP_STAT_SPLIT_FAILED, > MTHP_STAT_SPLIT_DEFERRED, > diff --git a/mm/huge_memory.c b/mm/huge_memory.c > index 9ec64aa2be94..f9696c94e211 100644 > --- a/mm/huge_memory.c > +++ b/mm/huge_memory.c > @@ -568,9 +568,9 @@ DEFINE_MTHP_STAT_ATTR(anon_fault_fallback, MTHP_STAT_= ANON_FAULT_FALLBACK); > DEFINE_MTHP_STAT_ATTR(anon_fault_fallback_charge, MTHP_STAT_ANON_FAULT_F= ALLBACK_CHARGE); > DEFINE_MTHP_STAT_ATTR(swpout, MTHP_STAT_SWPOUT); > DEFINE_MTHP_STAT_ATTR(swpout_fallback, MTHP_STAT_SWPOUT_FALLBACK); > -DEFINE_MTHP_STAT_ATTR(file_alloc, MTHP_STAT_FILE_ALLOC); > -DEFINE_MTHP_STAT_ATTR(file_fallback, MTHP_STAT_FILE_FALLBACK); > -DEFINE_MTHP_STAT_ATTR(file_fallback_charge, MTHP_STAT_FILE_FALLBACK_CHAR= GE); > +DEFINE_MTHP_STAT_ATTR(shmem_alloc, MTHP_STAT_SHMEM_ALLOC); > +DEFINE_MTHP_STAT_ATTR(shmem_fallback, MTHP_STAT_SHMEM_FALLBACK); > +DEFINE_MTHP_STAT_ATTR(shmem_fallback_charge, MTHP_STAT_SHMEM_FALLBACK_CH= ARGE); > DEFINE_MTHP_STAT_ATTR(split, MTHP_STAT_SPLIT); > DEFINE_MTHP_STAT_ATTR(split_failed, MTHP_STAT_SPLIT_FAILED); > DEFINE_MTHP_STAT_ATTR(split_deferred, MTHP_STAT_SPLIT_DEFERRED); > @@ -581,9 +581,9 @@ static struct attribute *stats_attrs[] =3D { > &anon_fault_fallback_charge_attr.attr, > &swpout_attr.attr, > &swpout_fallback_attr.attr, > - &file_alloc_attr.attr, > - &file_fallback_attr.attr, > - &file_fallback_charge_attr.attr, > + &shmem_alloc_attr.attr, > + &shmem_fallback_attr.attr, > + &shmem_fallback_charge_attr.attr, > &split_attr.attr, > &split_failed_attr.attr, > &split_deferred_attr.attr, > diff --git a/mm/shmem.c b/mm/shmem.c > index 921d59c3d669..f24dfbd387ba 100644 > --- a/mm/shmem.c > +++ b/mm/shmem.c > @@ -1777,7 +1777,7 @@ static struct folio *shmem_alloc_and_add_folio(stru= ct vm_fault *vmf, > if (pages =3D=3D HPAGE_PMD_NR) > count_vm_event(THP_FILE_FALLBACK); > #ifdef CONFIG_TRANSPARENT_HUGEPAGE > - count_mthp_stat(order, MTHP_STAT_FILE_FALLBACK); > + count_mthp_stat(order, MTHP_STAT_SHMEM_FALLBACK); > #endif > order =3D next_order(&suitable_orders, order); > } > @@ -1804,8 +1804,8 @@ static struct folio *shmem_alloc_and_add_folio(stru= ct vm_fault *vmf, > count_vm_event(THP_FILE_FALLBACK_CHARGE); > } > #ifdef CONFIG_TRANSPARENT_HUGEPAGE > - count_mthp_stat(folio_order(folio), MTHP_STAT_FIL= E_FALLBACK); > - count_mthp_stat(folio_order(folio), MTHP_STAT_FIL= E_FALLBACK_CHARGE); > + count_mthp_stat(folio_order(folio), MTHP_STAT_SHM= EM_FALLBACK); > + count_mthp_stat(folio_order(folio), MTHP_STAT_SHM= EM_FALLBACK_CHARGE); > #endif > } > goto unlock; > @@ -2181,7 +2181,7 @@ static int shmem_get_folio_gfp(struct inode *inode,= pgoff_t index, > if (folio_test_pmd_mappable(folio)) > count_vm_event(THP_FILE_ALLOC); > #ifdef CONFIG_TRANSPARENT_HUGEPAGE > - count_mthp_stat(folio_order(folio), MTHP_STAT_FIL= E_ALLOC); > + count_mthp_stat(folio_order(folio), MTHP_STAT_SHM= EM_ALLOC); > #endif > goto alloced; > } > -- > 2.43.0 > Thanks Barry