On 11/01/2024 15:41, Ryan Roberts wrote:
> Change the readahead config so that if it is being requested for an
> executable mapping, do a synchronous read of an arch-specified size in a
> naturally aligned manner.
>
> On arm64 if memory is physically contiguous and naturally aligned to the
> "contpte" size, we can use contpte mappings, which improves utilization
> of the TLB. When paired with the "multi-size THP" changes, this works
> well to reduce dTLB pressure. However iTLB pressure is still high due to
> executable mappings having a low liklihood of being in the required
> folio size and mapping alignment, even when the filesystem supports
> readahead into large folios (e.g. XFS).
>
> The reason for the low liklihood is that the current readahead algorithm
> starts with an order-2 folio and increases the folio order by 2 every
> time the readahead mark is hit. But most executable memory is faulted in
> fairly randomly and so the readahead mark is rarely hit and most
> executable folios remain order-2. This is observed impirically and
> confirmed from discussion with a gnu linker expert; in general, the
> linker does nothing to group temporally accessed text together
> spacially. Additionally, with the current read-around approach there are
> no alignment guarrantees between the file and folio. This is
> insufficient for arm64's contpte mapping requirement (order-4 for 4K
> base pages).
>
> So it seems reasonable to special-case the read(ahead) logic for
> executable mappings. The trade-off is performance improvement (due to
> more efficient storage of the translations in iTLB) vs potential read
> amplification (due to reading too much data around the fault which won't
> be used), and the latter is independent of base page size. I've chosen
> 64K folio size for arm64 which benefits both the 4K and 16K base page
> size configs and shouldn't lead to any further read-amplification since
> the old read-around path was (usually) reading blocks of 128K (with the
> last 32K being async).
>
> Performance Benchmarking
> ------------------------
>
> The below shows kernel compilation and speedometer javascript benchmarks
> on Ampere Altra arm64 system. (The contpte patch series is applied in
> the baseline).
>
> First, confirmation that this patch causes more memory to be contained
> in 64K folios (this is for all file-backed memory so includes
> non-executable too):
>
> | File-backed folios | Speedometer | Kernel Compile |
> | by size as percentage |-----------------|-----------------|
> | of all mapped file mem | before | after | before | after |
> |=========================|========|========|========|========|
> |file-thp-aligned-16kB | 45% | 9% | 46% | 7% |
> |file-thp-aligned-32kB | 2% | 0% | 3% | 1% |
> |file-thp-aligned-64kB | 3% | 63% | 5% | 80% |
> |file-thp-aligned-128kB | 11% | 11% | 0% | 0% |
> |file-thp-unaligned-16kB | 1% | 0% | 3% | 1% |
> |file-thp-unaligned-128kB | 1% | 0% | 0% | 0% |
> |file-thp-partial | 0% | 0% | 0% | 0% |
> |-------------------------|--------|--------|--------|--------|
> |file-cont-aligned-64kB | 16% | 75% | 5% | 80% |
>
> The above shows that for both use cases, the amount of file memory
> backed by 16K folios reduces and the amount backed by 64K folios
> increases significantly. And the amount of memory that is contpte-mapped
> significantly increases (last line).
>
> And this is reflected in performance improvement:
>
> Kernel Compilation (smaller is faster):
> | kernel | real-time | kern-time | user-time | peak memory |
> |----------|-------------|-------------|-------------|---------------|
> | before | 0.0% | 0.0% | 0.0% | 0.0% |
> | after | -1.6% | -2.1% | -1.7% | 0.0% |
>
> Speedometer (bigger is faster):
> | kernel | runs_per_min | peak memory |
> |----------|----------------|---------------|
> | before | 0.0% | 0.0% |
> | after | 1.3% | 1.0% |
>
> Both benchmarks show a ~1.5% improvement once the patch is applied.
>
> Alternatives
> ------------
>
> I considered (and rejected for now - but I anticipate this patch will
> stimulate discussion around what the best approach is) alternative
> approaches:
>
> - Expose a global user-controlled knob to set the preferred folio
> size; this would move policy to user space and allow (e.g.) setting
> it to PMD-size for even better iTLB utilizaiton. But this would add
> ABI, and I prefer to start with the simplest approach first. It also
> has the downside that a change wouldn't apply to memory already in
> the page cache that is in active use (e.g. libc) so we don't get the
> same level of utilization as for something that is fixed from boot.
>
> - Add a per-vma attribute to allow user space to specify preferred
> folio size for memory faulted from the range. (we've talked about
> such a control in the context of mTHP). The dynamic loader would
> then be responsible for adding the annotations. Again this feels
> like something that could be added later if value was demonstrated.
>
> - Enhance MADV_COLLAPSE to collapse to THP sizes less than PMD-size.
> This would still require dynamic linker involvement, but would
> additionally neccessitate a copy and all memory in the range would
> be synchronously faulted in, adding to application load time. It
> would work for filesystems that don't support large folios though.
>
> Signed-off-by: Ryan Roberts <ryan.roberts@xxxxxxx>
> ---
>
> Hi all,
>
> I originally concocted something similar to this, with Matthew's help, as a
> quick proof of concept hack. Since then I've tried a few different approaches
> but always came back to this as the simplest solution. I expect this will raise
> a few eyebrows but given it is providing a real performance win, I hope we can
> converge to something that can be upstreamed.
>
> This depends on my contpte series to actually set the contiguous bit in the page
> table.
>
> Thanks,
> Ryan
>
>
> arch/arm64/include/asm/pgtable.h | 12 ++++++++++++
> include/linux/pgtable.h | 12 ++++++++++++
> mm/filemap.c | 19 +++++++++++++++++++
> 3 files changed, 43 insertions(+)
>
> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> index f5bf059291c3..8f8f3f7eb8d8 100644
> --- a/arch/arm64/include/asm/pgtable.h
> +++ b/arch/arm64/include/asm/pgtable.h
> @@ -1143,6 +1143,18 @@ static inline void update_mmu_cache_range(struct vm_fault *vmf,
> */
> #define arch_wants_old_prefaulted_pte cpu_has_hw_af
>
> +/*
> + * Request exec memory is read into pagecache in at least 64K folios. The
> + * trade-off here is performance improvement due to storing translations more
> + * effciently in the iTLB vs the potential for read amplification due to reading
> + * data from disk that won't be used. The latter is independent of base page
> + * size, so we set a page-size independent block size of 64K. This size can be
> + * contpte-mapped when 4K base pages are in use (16 pages into 1 iTLB entry),
> + * and HPA can coalesce it (4 pages into 1 TLB entry) when 16K base pages are in
> + * use.
> + */
> +#define arch_wants_exec_folio_order(void) ilog2(SZ_64K >> PAGE_SHIFT)
Just noticed this errant "void". I originally had this as an inline function,
but changed it to a macro to align with the other "arch_wants" macros above. So
I missed it when refactoring. Anyway, it should be benign but I'll fix it in the
next version, if we keep going in this direction.
> +
> static inline bool pud_sect_supported(void)
> {
> return PAGE_SIZE == SZ_4K;
> diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
> index 170925379534..57090616d09c 100644
> --- a/include/linux/pgtable.h
> +++ b/include/linux/pgtable.h
> @@ -428,6 +428,18 @@ static inline bool arch_has_hw_pte_young(void)
> }
> #endif
>
> +#ifndef arch_wants_exec_folio_order
> +/*
> + * Returns preferred minimum folio order for executable file-backed memory. Must
> + * be in range [0, PMD_ORDER]. Negative value implies that the HW has no
> + * preference and mm will not special-case executable memory in the pagecache.
> + */
> +static inline int arch_wants_exec_folio_order(void)
> +{
> + return -1;
> +}
> +#endif
> +
> #ifndef arch_check_zapped_pte
> static inline void arch_check_zapped_pte(struct vm_area_struct *vma,
> pte_t pte)
> diff --git a/mm/filemap.c b/mm/filemap.c
> index 67ba56ecdd32..80a76d755534 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -3115,6 +3115,25 @@ static struct file *do_sync_mmap_readahead(struct vm_fault *vmf)
> }
> #endif
>
> + /*
> + * Allow arch to request a preferred minimum folio order for executable
> + * memory. This can often be beneficial to performance if (e.g.) arm64
> + * can contpte-map the folio. Executable memory rarely benefits from
> + * read-ahead anyway, due to its random access nature.
> + */
> + if (vm_flags & VM_EXEC) {
> + int order = arch_wants_exec_folio_order();
> +
> + if (order >= 0) {
> + fpin = maybe_unlock_mmap_for_io(vmf, fpin);
> + ra->size = 1UL << order;
> + ra->async_size = 0;
> + ractl._index &= ~((unsigned long)ra->size - 1);
> + page_cache_ra_order(&ractl, ra, order);
> + return fpin;
> + }
> + }
> +
> /* If we don't want any read-ahead, don't bother */
> if (vm_flags & VM_RAND_READ)
> return fpin;
> --
> 2.25.1
>
