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

[David Hildenbrand <david@redhat.com>]

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.

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.

... Willy, what would be your take?

-- 
Cheers,

David / dhildenb