Re: [PATCH] fs/proc: fix KPF_KSM reported for all anonymous pages

["David Hildenbrand (Arm)" <david@kernel.org>]

On 6/23/26 03:37, Jinjiang Tu wrote:
> 
> 在 2026/6/22 19:45, David Hildenbrand (Arm) 写道:
>> On 6/22/26 11:15, Jinjiang Tu wrote:
>>> Reading /proc/kpageflags for any anonymous page returns KPF_KSM set, even
>>> when KSM is not in use. As a result, tools misclassify all anonymous pages
>>> as KSM merged.
>>>
>>> In stable_page_flags(), if the page is anonymous, then use (mapping &
>>> FOLIO_MAPPING_KSM) check to identify if the anonymous page is KSM page.
>>> However, FOLIO_MAPPING_KSM is FOLIO_MAPPING_ANON | FOLIO_MAPPING_ANON_KSM,
>>> (mapping & FOLIO_MAPPING_KSM) check returns true for all nonymous pages.
>>>
>>> To fix it, use FOLIO_MAPPING_ANON_KSM instead.
>>>
>>> Fixes: dee3d0bef2b0 ("proc: rewrite stable_page_flags()")
>> Right,
>>
>> #define PAGE_MAPPING_KSM       (PAGE_MAPPING_ANON | PAGE_MAPPING_ANON_KSM)
>>
>> Which we later renamed to FOLIO_MAPPING_KSM.
>>
>>
>> Before switching to manual flag checks, PageKsm() translated to folio_test_ksm()
>> that checked whether the values actually matched:
>>
>> ((unsigned long)folio->mapping & PAGE_MAPPING_FLAGS) == PAGE_MAPPING_KSM;
>>
>>
>> This only affects /proc/kpageflags (well, and hwpoison inject on a weird testing
>> interface), so it's not really that relevant for real workloads (debugging and
>> testing).
>>
>> So not sure whether we should CC:stable. Likely not.
> 
> /proc/kpageflags is generally used only for analysis and is unlikely to be
> used in production environments. I found this issue due to I was analyzing
> pfns allocated by which stacks are KSM-merged. So I think it's unnecessary
> to CC:stable.
> 
>>> Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com>
>>> ---
>>>   fs/proc/page.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/fs/proc/page.c b/fs/proc/page.c
>>> index f9b2c2c906cd..cef8ded97610 100644
>>> --- a/fs/proc/page.c
>>> +++ b/fs/proc/page.c
>>> @@ -173,7 +173,7 @@ u64 stable_page_flags(const struct page *page)
>>>           u |= 1 << KPF_MMAP;
>>>       if (is_anon) {
>>>           u |= 1 << KPF_ANON;
>>> -        if (mapping & FOLIO_MAPPING_KSM)
>>> +        if (mapping & FOLIO_MAPPING_ANON_KSM)
>>
>> Wonder whether we should just do
>>
>>         if ((mapping & FOLIO_MAPPING_FLAGS) == FOLIO_MAPPING_KSM)
>>
>> To match what we have in folio_test_ksm.
>>
>> (although I doubt we would reuse this flag for other purposes, likely
>> it's more future proof to check it like that)
> 
> Both are ok. The following check has checked FOLIO_MAPPING_ANON,
> 
>     if (is_anon) {
>         if (mapping & FOLIO_MAPPING_ANON_KSM)
>     }
> 
> So it's equivalent to do
>     if ((mapping & FOLIO_MAPPING_FLAGS) == FOLIO_MAPPING_KSM)
> or
>     if (mapping & FOLIO_MAPPING_ANON_KSM)

As I said, matching precisely what we have in folio_test_ksm() is clearer. We
don't have any users of FOLIO_MAPPING_ANON_KSM outside of page-flags.h for a
reason :)

-- 
Cheers,

David