Received-SPF: pass (google.com: domain of linux-kernel+bounces-186903-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Precedence: bulk
MIME-Version: 1.0
References: <0e2a6f232e7579a2e4407ecf075531980d97f286.1716367360.git.baolin.wang@linux.alibaba.com>
 <22ac01a3-ddbb-4114-88cd-ad1a31982dad@redhat.com> <51ba1fc1-fd77-4601-8d27-459162fd008c@linux.alibaba.com>
 <CAGsJ_4zSuOTPi+zkS_kvS5T0MsdMBR+2gpXukJt0aMPrEnCDZg@mail.gmail.com>
 <875xv5ba8t.fsf@yhuang6-desk2.ccr.corp.intel.com> <18aa865a-6d4a-4dcf-99ce-bcfbc0c92f19@linux.alibaba.com>
In-Reply-To: <18aa865a-6d4a-4dcf-99ce-bcfbc0c92f19@linux.alibaba.com>
From: Barry Song <21cnbao@gmail.com>
Date: Thu, 23 May 2024 14:12:50 +1200
Message-ID: <CAGsJ_4xeuLS9L=ayUSor4kXs8B1c2bY01cGZYrR7QjdwQWu7Lg@mail.gmail.com>
Subject: Re: [PATCH] mm: drop the 'anon_' prefix for swap-out mTHP counters
To: Baolin Wang <baolin.wang@linux.alibaba.com>
Cc: "Huang, Ying" <ying.huang@intel.com>, David Hildenbrand <david@redhat.com>, akpm@linux-foundation.org, 
	willy@infradead.org, ryan.roberts@arm.com, ziy@nvidia.com, linux-mm@kvack.org, 
	linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Thu, May 23, 2024 at 1:38=E2=80=AFPM Baolin Wang
<baolin.wang@linux.alibaba.com> wrote:
>
>
>
> On 2024/5/23 09:14, Huang, Ying wrote:
> > Barry Song <21cnbao@gmail.com> writes:
> >
> >> On Wed, May 22, 2024 at 9:38=E2=80=AFPM Baolin Wang
> >> <baolin.wang@linux.alibaba.com> wrote:
> >>>
> >>>
> >>>
> >>> On 2024/5/22 16:58, David Hildenbrand wrote:
> >>>> On 22.05.24 10:51, Baolin Wang wrote:
> >>>>> The mTHP swap related counters: 'anon_swpout' and
> >>>>> 'anon_swpout_fallback' are
> >>>>> confusing with an 'anon_' prefix, since the shmem can swap out
> >>>>> non-anonymous
> >>>>> pages. So drop the 'anon_' prefix to keep consistent with the old s=
wap
> >>>>> counter
> >>>>> names.
> >>>>>
> >>>>> Suggested-by: "Huang, Ying" <ying.huang@intel.com>
> >>>>> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
> >>>>> ---
> >>>>
> >>>> Am I daydreaming or did we add the anon_ for a reason and discussed =
the
> >>>> interaction with shmem? At least I remember some discussion around t=
hat.
> >>>
> >>> Do you mean the shmem mTHP allocation counters in previous
> >>> discussion[1]? But for 'anon_swpout' and 'anon_swpout_fallback', I ca=
n
> >>> not find previous discussions that provided a reason for adding the
> >>> =E2=80=98anon_=E2=80=99 prefix. Barry, any comments? Thanks.
> >>
> >> HI Baolin,
> >> We had tons of emails discussing about namin and I found this email,
> >>
> >> https://lore.kernel.org/all/bca6d142-15fd-4af5-9f71-821f891e8305@redha=
t.com/
> >>
> >> David had this comment,
> >> "I'm wondering if these should be ANON specific for now. We might want=
 to
> >> add others (shmem, file) in the future."
> >>
> >> This is likely how the 'anon_' prefix started being added, although it
> >> wasn't specifically
> >> targeting swapout.
> >>
> >> I sense your patch slightly alters the behavior of thp_swpout_fallback
> >> in /proc/vmstat.
> >> Previously, we didn't classify them as THP_SWPOUT_FALLBACK, even thoug=
h we
> >> always split them.
> >
> > IIUC, "fallback" means you try to do something, but fail, so try
> > something else as fallback.  If so, then we don't need to count
> > splitting shmem large folio as fallback.
>
> Agree. In additon, IIUC we have never counted splitting shmem large
> folio as THP_SWPOUT_FALLBACK before or after this patch.

Hi Baolin,

My point is that THP_SWPOUT* has been dedicated to anonymous memory for yea=
rs
because we have not had the capability to perform THP_SWPOUT for shared mem=
ory
before. This is the historical context of thp_swpout* in /proc/vmstat,
even though it is
not ideal. Therefore, placing shmem sysfs entries in
/sys/kernel/mm/transparent_hugepage/hugepages-2048kB/stats
allows us to monitor SWPOUT and SWPOUT FALLBACK for shmem without altering
the tradition of /proc/vmstat.

But I am not firm on this because I don't see the necessity to
differentiate shmem's
swpout from anon's swpout. They basically seem the same while anon mTHP
faults might be significantly different from file mTHP faults, in which cas=
e we
must distinguish them. So please send version 2 with the updated documentat=
ion.
I believe it should target v6.10-rc rather than v6.11 to avoid ABI
conflicts if it is
accepted.

>
> > For example, before commit 5ed890ce5147 ("mm: vmscan: avoid split durin=
g
> > shrink_folio_list()"), if folio_entire_mapcount() =3D=3D 0, we will spl=
it
> > the THP.  But we will not count it as "fallback" because we haven't
> > tried to swap it out as a whole.

Thanks
Barry