Re: [PATCH] mm: drop the 'anon_' prefix for swap-out mTHP counters

[Baolin Wang]

On 2024/5/23 09:14, Huang, Ying wrote:
> Barry Song <21cnbao@xxxxxxxxx> writes:
>
> > On Wed, May 22, 2024 at 9:38 PM Baolin Wang
> > <baolin.wang@xxxxxxxxxxxxxxxxx> 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 swap
> > > > > counter
> > > > > names.
> > > > >
> > > > > Suggested-by: "Huang, Ying" <ying.huang@xxxxxxxxx>
> > > > > Signed-off-by: Baolin Wang <baolin.wang@xxxxxxxxxxxxxxxxx>
> > > > > ---
> > > >
> > > > 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 that.
> > >
> > > Do you mean the shmem mTHP allocation counters in previous
> > > discussion[1]? But for 'anon_swpout' and 'anon_swpout_fallback', I can
> > > not find previous discussions that provided a reason for adding the
> > > ‘anon_’ 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@xxxxxxxxxx/
> >
> > 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 though 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.

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