On Mon, May 30, 2022 at 7:07 PM Anshuman Khandual <anshuman.khandual@xxxxxxx> wrote: > > Hello Barry, Hi Anshuman, thanks! > > On 5/27/22 15:36, Barry Song wrote: > > From: Barry Song <v-songbaohua@xxxxxxxx> > > > > THP_SWAP has been proved to improve the swap throughput significantly > > on x86_64 according to commit bd4c82c22c367e ("mm, THP, swap: delay > > splitting THP after swapped out"). > It will be useful to run similar experiments on arm64 platform to > demonstrate tangible benefit, else we might be just enabling this > feature just because x86 has it. Do you have some data points ? > > > As long as arm64 uses 4K page size, it is quite similar with x86_64 > > by having 2MB PMD THP. So we are going to get similar improvement. > > This is an assumption without any data points (until now). Please > do provide some results. Fair enough though I believe THP_SWP is arch-independent. Our testing will post data. Plus, we do need it for real use cases with some possible out-of-tree code for this moment. so this patch does not originate only because x86 has it :-) > > > For other page sizes such as 16KB and 64KB, PMD might be too large. > > Negative side effects such as IO latency might be a problem. Thus, > > we can only safely enable the counterpart of X86_64. > > Incorrect reasoning. Although sometimes it might be okay to enable > a feature on platforms with possible assumptions about its benefits, > but to claim 'similar improvement, safely, .. etc' while comparing > against x86 4K page config without data points, is not very helpful. > > > A corner case is that MTE has an assumption that only base pages > > can be swapped. We won't enable THP_SWP for ARM64 hardware with > > MTE support until MTE is re-arched. > > re-arched ?? Did you imply that MTE is reworked to support THP ? I think at least MTE should be able to coexist with THP_SWP though I am not quite sure if MTE can be re-worked to fully support THP. > > > > > Cc: "Huang, Ying" <ying.huang@xxxxxxxxx> > > Cc: Minchan Kim <minchan@xxxxxxxxxx> > > Cc: Johannes Weiner <hannes@xxxxxxxxxxx> > > Cc: Hugh Dickins <hughd@xxxxxxxxxx> > > Cc: Andrea Arcangeli <aarcange@xxxxxxxxxx> > > Cc: Anshuman Khandual <anshuman.khandual@xxxxxxx> > > Cc: Steven Price <steven.price@xxxxxxx> > > Cc: Yang Shi <shy828301@xxxxxxxxx> > > Signed-off-by: Barry Song <v-songbaohua@xxxxxxxx> > > --- > > arch/arm64/Kconfig | 1 + > > arch/arm64/include/asm/pgtable.h | 2 ++ > > include/linux/huge_mm.h | 12 ++++++++++++ > > mm/swap_slots.c | 2 +- > > 4 files changed, 16 insertions(+), 1 deletion(-) > > > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > > index a4968845e67f..5306009df2dc 100644 > > --- a/arch/arm64/Kconfig > > +++ b/arch/arm64/Kconfig > > @@ -101,6 +101,7 @@ config ARM64 > > select ARCH_WANT_HUGETLB_PAGE_OPTIMIZE_VMEMMAP > > select ARCH_WANT_LD_ORPHAN_WARN > > select ARCH_WANTS_NO_INSTR > > + select ARCH_WANTS_THP_SWAP if ARM64_4K_PAGES > > select ARCH_HAS_UBSAN_SANITIZE_ALL > > select ARM_AMBA > > select ARM_ARCH_TIMER > > diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h > > index 0b6632f18364..06076139c72c 100644 > > --- a/arch/arm64/include/asm/pgtable.h > > +++ b/arch/arm64/include/asm/pgtable.h > > @@ -45,6 +45,8 @@ > > __flush_tlb_range(vma, addr, end, PUD_SIZE, false, 1) > > #endif /* CONFIG_TRANSPARENT_HUGEPAGE */ > > > > +#define arch_thp_swp_supported !system_supports_mte > > Does it check for 'system_supports_mte' as a symbol or call system_supports_mte() > to ascertain runtime MTE support ? It might well be correct, but just does not > look much intuitive. yep. looks a bit weird. but considering we only need this for arm64 and arch_thp_swp_supported is a macro, I can't find a better way to make code modification smaller than this in mm core, arm64 and x86. and probably we will totally remove it once we make MTE co-exist with THP_SWP. Do you have any suggestions for a better solution? > > > + > > /* > > * Outside of a few very special situations (e.g. hibernation), we always > > * use broadcast TLB invalidation instructions, therefore a spurious page > > diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h > > index de29821231c9..4ddaf6ad73ef 100644 > > --- a/include/linux/huge_mm.h > > +++ b/include/linux/huge_mm.h > > @@ -461,4 +461,16 @@ static inline int split_folio_to_list(struct folio *folio, > > return split_huge_page_to_list(&folio->page, list); > > } > > > > +/* > > + * archs that select ARCH_WANTS_THP_SWAP but don't support THP_SWP due to > > + * limitations in the implementation like arm64 MTE can override this to > > A small nit. > > A 'comma' here will be better. s/arm64 MTE can/arm64 MTE, can/ yep. > > > + * false > > Similarly a 'full stop' here will be better as well. > yep. > > + */ > > +#ifndef arch_thp_swp_supported > > +static inline bool arch_thp_swp_supported(void) > > +{ > > + return true; > > +} > > +#endif > > + > > #endif /* _LINUX_HUGE_MM_H */ > > diff --git a/mm/swap_slots.c b/mm/swap_slots.c > > index 2a65a89b5b4d..10b94d64cc25 100644 > > --- a/mm/swap_slots.c > > +++ b/mm/swap_slots.c > > @@ -307,7 +307,7 @@ swp_entry_t folio_alloc_swap(struct folio *folio) > > entry.val = 0; > > > > if (folio_test_large(folio)) { > > - if (IS_ENABLED(CONFIG_THP_SWAP)) > > + if (IS_ENABLED(CONFIG_THP_SWAP) && arch_thp_swp_supported()) > > get_swap_pages(1, &entry, folio_nr_pages(folio)); > > goto out; > > } > > - Anshuman Thanks Barry