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

[Baolin Wang <baolin.wang@linux.alibaba.com>]

On 2024/5/23 09:14, Huang, Ying wrote:
> Barry Song <21cnbao@gmail.com> writes:
> 
>> On Wed, May 22, 2024 at 9:38 PM 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 swap
>>>>> 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 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@redhat.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 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.