Re: [PATCH v1] mm: shmem: Rename mTHP shmem counters

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

On 2024/7/8 20:29, Ryan Roberts wrote:
> On 08/07/2024 12:36, Barry Song wrote:
>> On Mon, Jul 8, 2024 at 11:24 PM Ryan Roberts <ryan.roberts@arm.com> 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?
> 
> Embarrassingly, I don't recall that converstation at all :-| but at least what I
> said then is consistent with what I've done in this patch.
> 
> I think David's conclusion from that thread was to call them FILE_, and add both
> shmem and pagecache counts to those counters, meaning we can keep the same name
> as legacy THP counters. But those legacy THP counters only count shmem, and I
> don't think we would get away with adding pagecache counts to those at this
> point? (argument: they have been around for long time and there is a risk that
> user space relies on them and if they were to dramatically increase due to
> pagecache addition now that could break things). In that case, there is still
> inconsistency, but its worse; the names are consistent but the semantics are
> inconsistent.
> 
> So my vote is to change to SHMEM_ as per this patch :)
> 
>>
>> [1] https://lore.kernel.org/linux-mm/05d0096e4ec3e572d1d52d33a31a661321ac1551.1713755580.git.baolin.wang@linux.alibaba.com/

My original preference was also for SHMEM-specific counters :)
So feel free to add:
Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>