Received-SPF: pass (google.com: domain of linux-kernel+bounces-186916-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <2ee94722-456f-4db0-9ed9-3f1c72239a75@linux.alibaba.com>
Date: Thu, 23 May 2024 10:31:53 +0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] mm: drop the 'anon_' prefix for swap-out mTHP counters
To: Barry Song <21cnbao@gmail.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
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>
 <CAGsJ_4xeuLS9L=ayUSor4kXs8B1c2bY01cGZYrR7QjdwQWu7Lg@mail.gmail.com>
From: Baolin Wang <baolin.wang@linux.alibaba.com>
In-Reply-To: <CAGsJ_4xeuLS9L=ayUSor4kXs8B1c2bY01cGZYrR7QjdwQWu7Lg@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit


On 2024/5/23 10:12, Barry Song wrote:
> On Thu, May 23, 2024 at 1:38 PM 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 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.
> 
> Hi Baolin,
> 
> My point is that THP_SWPOUT* has been dedicated to anonymous memory for years
> because we have not had the capability to perform THP_SWPOUT for shared memory
> 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.

OK, I see your point. IMO this patch will not change the behaviors of 
thp_swpout* in /proc/vmstat until adding support for large folio 
swap-out for shmem[1]. Anyway we can talk about this in that thread.

[1] 
https://lore.kernel.org/all/cover.1716285099.git.baolin.wang@linux.alibaba.com/

> 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 case we
> must distinguish them. So please send version 2 with the updated documentation.
> I believe it should target v6.10-rc rather than v6.11 to avoid ABI
> conflicts if it is
> accepted.

Sure. Will do. Thanks.