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

["Huang\, Ying" <ying.huang@intel.com>]

Prathu Baronia <prathu.baronia@oneplus.com> writes:

> In !HIGHMEM cases, specially in 64-bit architectures, we don't need temp mapping
> of pages. Hence, k(map|unmap)_atomic() acts as nothing more than multiple
> barrier() calls, for example for a 2MB hugepage in clear_huge_page() these are
> called 512 times i.e. to map and unmap each subpage that means in total 2048
> barrier calls. This called for optimization. Simply getting VADDR from page does
> the job for us. This also applies to the copy_user_huge_page() function.
>
> With kmap_atomic() out of the picture we can use memset and memcpy for sizes
> larger than 4K. Instead of a left-right approach to access the target subpage,
> getting the VADDR from the page and using memset directly in a simple experiment
> we observed a 64% improvement in time over the current approach.
>
> With this(v2) patch we observe 65.85%(under controlled conditions) improvement
> over the current approach. 

Can you describe your test?

> Currently process_huge_page iterates over subpages in a left-right manner
> targeting the subpage that was accessed to be processed at last to keep the
> cache hot around the faulting address. This caused a latency issue because as we
> observed in the case of ARM64 the reverse access is much slower than forward
> access and much much slower than oneshot access because of the pre-fetcher
> behaviour. The following simple userspace experiment to allocate
> 100MB(total_size) of pages and writing to it using memset(oneshot), forward
> order loop and a reverse order loop gave us a good insight:-
>
> --------------------------------------------------------------------------------
> Test code snippet:
> --------------------------------------------------------------------------------
>   /* One shot memset */
>   memset (r, 0xd, total_size);
>
>   /* traverse in forward order */
>   for (j = 0; j < total_pages; j++)
>     {
>       memset (q + (j * SZ_4K), 0xc, SZ_4K);
>     }
>
>   /* traverse in reverse order */
>   for (i = 0; i < total_pages; i++)
>     {
>       memset (p + total_size - (i + 1) * SZ_4K, 0xb, SZ_4K);
>     }

You have tested the chunk sizes 4KB and 2MB, can you test some values in
between?  For example 32KB or 64KB?  Maybe there's a sweet point with
some smaller granularity and good performance.

> ----------------------------------------------------------------------
> Results:
> ----------------------------------------------------------------------
> Results for ARM64 target (SM8150 , CPU0 & 6 are online, running at max
> frequency)
> All numbers are mean of 100 iterations. Variation is ignorable.
> ----------------------------------------------------------------------
> - Oneshot : 3389.26 us
> - Forward : 8876.16 us
> - Reverse : 18157.6 us
> ----------------------------------------------------------------------
>
> ----------------------------------------------------------------------
> Results for x86-64 (Intel(R) Core(TM) i7-8700 CPU @ 3.20GHz, only CPU 0 in max
> frequency, DDR also running at max frequency.)
> All numbers are mean of 100 iterations. Variation is ignorable.
> ----------------------------------------------------------------------
> - Oneshot : 3203.49 us
> - Forward : 5766.46 us
> - Reverse : 5187.86 us
> ----------------------------------------------------------------------
>
> Hence refactor the function process_huge_page() to process the hugepage
> in oneshot manner using oneshot version of routines clear_huge_page() and
> copy_user_huge_page() for !HIGHMEM cases.
>
> These oneshot routines do zeroing using memset and copying using memcpy since we
> observed after extensive testing on ARM64 and some local testing on x86 memset
> and memcpy routines are highly optimized and with the above data points in hand
> it made sense to utilize them directly instead of looping over all subpages.
> These oneshot routines do zero and copy with a small offset(default kept as 32KB for
> now) to keep the cache hot around the faulting address. This offset is dependent
> on the cache size and hence can be kept as a tunable configuration option.
>
> The below profiles are for ARM64(SM8150, CPU0 & 6 are online, running at max
> frequency, DDR also running at max frequency.)
>
> ----------------------------------------------------------------------
> Ftrace Results(clear_huge_page_profile()):
> ----------------------------------------------------------------------
> All timing values are in microseconds(us)
> ----------------------------------------------------------------------
> Base:
>         - CPU0:
>                 - Samples: 95
>                 - Mean: 242.099 us
>                 - Std dev: 45.0096 us
>         - CPU6:
>                 - Samples: 61
>                 - Mean: 258.372 us
>                 - Std dev: 22.0754 us
> ----------------------------------------------------------------------
> v2:
>         - CPU0:
>                 - Samples: 63
>                 - Mean: 112.297 us
>                 - Std dev: 0.310989 us
>         - CPU6:
>                 - Samples: 99
>                 - Mean: 67.359 us
>                 - Std dev: 1.15997 us
> ----------------------------------------------------------------------

In addition to clearing the huge page itself, we need to consider how it
impact the application which accesses the huge page.  For example, the
huge page may be accessed twice, once in kernel (zeroing) and once in
the user space (initializing).  So please find a way to test it.  As
pointed out by Alexander, please consider the cache contention among
the logical CPUs too.

Best Regards,
Huang, Ying