Re: [PATCH v2] mm: Optimized hugepage zeroing & copying from user

[Vlastimil Babka <vbabka@suse.cz>]

On 4/21/20 12:09 PM, Will Deacon wrote:
> On Tue, Apr 21, 2020 at 03:06:21PM +0530, Prathu Baronia wrote:
>> With below v2 patch we observe a significantly(~65%) improved zeroing time for
>> hugepages.
> 
> What patch? I assume you mean:
> 
> https://lore.kernel.org/linux-mm/20200414153829.GA15230@oneplus.com/
> 
> but you've trimmed all the details!
> 
>> We profiled the clear_huge_page() using ftrace on Qualcomm's SM8150 platform
>> under controlled conditions(i.e. only CPU0 and 6 turned on and set to max
>> frequency, and DDR set to performance governor).
>> 
>> The existing method uses a reverse traversal of a section of a hugepage which
>> based on our series of experiments proves slower than a oneshot(v2) approach on
>> ARM64.(more details in mail thread)
>> 
>> We didn't see any benefit on x86 so v2 probably won't find any place in the main
>> memory.c code.
> 
> Do you know why you don't see any benefit on x86? It seems unusual that
> something like this would vary so wildly between two modern architectures.
> I'd like to understand what's going on.

It was suspected that current Intel can prefetch forward and backwards, and the
tested ARM64 microarchitecture only backwards, can it be true? The current code
does clearing backwards.

>> We are currently thinking of making this optimization ARM64 specific for better
>> performance by placing this in arch/arm64/mm/memory.c(to be created) file. We
>> would really appreciate if you can share your opinion on this.
> 
> There's no need for arch-specific optimisation. Please do it in core code,
> and allow architectures to opt-out if necessary. That means you probably
> need to respond to:
> 
> https://lore.kernel.org/linux-mm/20200417074851.GE26326@shao2-debian/

Note that this can be also viewed differently. It was commit c79b57e462b5 ("mm:
hugetlb: clear target sub-page last when clearing huge page") that introduced
the existing implementation, based on x86 numbers and probably the same test
that generated the regression report. It's likely that said commit thus
regressed arm64.

In that case the generic implementation should be just reverted to be simple and
not assume any (micro)architectural details. If any architecture wants an
optimized version they could add it opt-in, and justifify it by using real
workloads, not microbenchmarks.

> because that doesn't look as rosy as the numbers you're seeing.
> 
> Thanks,
> 
> Will
>