Re: [PATCH] mm/huge_memory: ensure huge_zero_folio won't have large_rmappable flag set

[Miaohe Lin <linmiaohe@huawei.com>]

On 2024/9/17 19:59, David Hildenbrand wrote:
> On 14.09.24 03:53, Miaohe Lin wrote:
>> Ensure huge_zero_folio won't have large_rmappable flag set. So it can be
>> reported as thp,zero correctly through stable_page_flags().
>>
>> Fixes: 5691753d73a2 ("mm: convert huge_zero_page to huge_zero_folio")
>> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>> ---
>>   mm/huge_memory.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>> index 2a73efea02d7..4e34b7f89daf 100644
>> --- a/mm/huge_memory.c
>> +++ b/mm/huge_memory.c
>> @@ -218,6 +218,8 @@ static bool get_huge_zero_page(void)
>>           count_vm_event(THP_ZERO_PAGE_ALLOC_FAILED);
>>           return false;
>>       }
>> +    /* Ensure zero folio won't have large_rmappable flag set. */
>> +    folio_clear_large_rmappable(zero_folio);
>>       preempt_disable();
>>       if (cmpxchg(&huge_zero_folio, NULL, zero_folio)) {
>>           preempt_enable();
> 
> Doesn't that rather fix
> 
> commit 4c8763e84aae4d04d94b35aca9f7db6a8930ad77
> Author: Ran Xiaokai <ran.xiaokai@zte.com.cn>
> Date:   Fri Jul 5 10:43:43 2024 +0000
> 
>     kpageflags: detect isolated KPF_THP folios
> 
> 
> ?
> 
> We could fix it simply by changing the order of checks in there.

Sure, we can fix it like above but huge zero folio shouldn't have large_rmappable flag set anyway.

> 
> It makes sense, though. The huge zeropage is not tracked via the rmap ... ever. Mapcounts etc are unused. But clearing that flag is just ugly.
> 
> I wonder if the real problem lies in using folio_alloc() here, and that we should be never setting the flag in the first place .... Yes, we want a folio, but not really an rmappable one.

There was a similar problem. Hugetlb folio has large_rmappable flag set through using folio_alloc. And that cause kernel panic when migrating hugetlb folio.

Please see commit 1390a3334a48 ("mm/hugetlb: fix kernel NULL pointer dereference when migrating hugetlb folio").

Thanks.
.

> 
> ... Willy, what would be your take?
>