Re: [PATCH] mm: avoid zeroing user movable page twice with init_on_alloc=1

[Zi Yan <ziy@nvidia.com>]

On 4 Dec 2024, at 10:41, Vlastimil Babka wrote:

> On 12/4/24 16:24, Zi Yan wrote:
>> On 4 Dec 2024, at 5:41, Geert Uytterhoeven wrote:
>>
>> The provided config does not have THP on, so the changes to mm/huge_memory.c
>> and mm/memory.c do not apply.
>>
>> Can you try the patch below and see if the machine boots? Thanks.
>
> Hmm looks like mips has some involved clear_user_page()
> in arch/mips/include/asm/page.h
>
> So maybe the clearing done as part of page allocator isn't enough here.
>
Basically, mips needs to flush data cache if kmap address is aliased to
userspace address. This means when mips has THP on, the patch below
is not enough to fix the issue.

In post_alloc_hook(), it does not make sense to pass userspace address
in to determine whether to flush dcache or not.

One way to fix it is to add something like arch_userpage_post_alloc()
to flush dcache if kmap address is aliased to userspace address.
But my questions are that
1) if kmap address will always be the same for two separate kmap_local() calls,
2) how much overheads the additional kmap_local() and kunmap_local() have.

>>
>> diff --git a/include/linux/highmem.h b/include/linux/highmem.h
>> index 6e452bd8e7e3..bec9bd715acf 100644
>> --- a/include/linux/highmem.h
>> +++ b/include/linux/highmem.h
>> @@ -224,7 +224,13 @@ static inline
>>  struct folio *vma_alloc_zeroed_movable_folio(struct vm_area_struct *vma,
>>  				   unsigned long vaddr)
>>  {
>> -	return vma_alloc_folio(GFP_HIGHUSER_MOVABLE | __GFP_ZERO, 0, vma, vaddr);
>> +	struct folio *folio;
>> +
>> +	folio = vma_alloc_folio(GFP_HIGHUSER_MOVABLE, 0, vma, vaddr);
>> +	if (folio)
>> +		clear_user_highpage(&folio->page, vaddr);
>> +
>> +	return folio;
>>  }
>>  #endif
>>
>>
>> Best Regards,
>> Yan, Zi
>>


Best Regards,
Yan, Zi