On Mon, May 30, 2022 at 7:07 PM Anshuman Khandual
[off-list ref] wrote:

Hello Barry,

Hi Anshuman,
thanks!

On 5/27/22 15:36, Barry Song wrote:

quoted

From: Barry Song <redacted>

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 ?

quoted

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 :-)

quoted

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.

quoted

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.

quoted

Cc: "Huang, Ying" <redacted>
Cc: Minchan Kim <minchan@kernel.org>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Hugh Dickins <hughd@google.com>
Cc: Andrea Arcangeli <redacted>
Cc: Anshuman Khandual <redacted>
Cc: Steven Price <steven.price@arm.com>
Cc: Yang Shi <redacted>
Signed-off-by: Barry Song <redacted>
---
 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?

quoted

+
 /*
  * 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.

quoted

+ * false

Similarly a 'full stop' here will be better as well.

yep.

quoted

+ */
+#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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel