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@xxxxxxxxxx>> ---
>> 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@xxxxxxxxxx>
> 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?
>
