Re: [PATCH v6 6/9] mm: thp: Add "recommend" option for anon_orders
From: David Hildenbrand
Date: Mon Oct 09 2023 - 10:44:19 EST
On 09.10.23 13:45, Ryan Roberts wrote:
On 06/10/2023 23:28, Yu Zhao wrote:
On Fri, Oct 6, 2023 at 2:08 PM David Hildenbrand <david@xxxxxxxxxx> wrote:
On 29.09.23 13:44, Ryan Roberts wrote:
In addition to passing a bitfield of folio orders to enable for THP,
allow the string "recommend" to be written, which has the effect of
causing the system to enable the orders preferred by the architecture
and by the mm. The user can see what these orders are by subsequently
reading back the file.
Note that these recommended orders are expected to be static for a given
boot of the system, and so the keyword "auto" was deliberately not used,
as I want to reserve it for a possible future use where the "best" order
is chosen more dynamically at runtime.
Recommended orders are determined as follows:
- PMD_ORDER: The traditional THP size
- arch_wants_pte_order() if implemented by the arch
- PAGE_ALLOC_COSTLY_ORDER: The largest order kept on per-cpu free list
arch_wants_pte_order() can be overridden by the architecture if desired.
Some architectures (e.g. arm64) can coalsece TLB entries if a contiguous
set of ptes map physically contigious, naturally aligned memory, so this
mechanism allows the architecture to optimize as required.
Here we add the default implementation of arch_wants_pte_order(), used
when the architecture does not define it, which returns -1, implying
that the HW has no preference.
Signed-off-by: Ryan Roberts <ryan.roberts@xxxxxxx>
---
Documentation/admin-guide/mm/transhuge.rst | 4 ++++
include/linux/pgtable.h | 13 +++++++++++++
mm/huge_memory.c | 14 +++++++++++---
3 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/Documentation/admin-guide/mm/transhuge.rst b/Documentation/admin-guide/mm/transhuge.rst
index 732c3b2f4ba8..d6363d4efa3a 100644
--- a/Documentation/admin-guide/mm/transhuge.rst
+++ b/Documentation/admin-guide/mm/transhuge.rst
@@ -187,6 +187,10 @@ pages (=16K if the page size is 4K). The example above enables order-9
By enabling multiple orders, allocation of each order will be
attempted, highest to lowest, until a successful allocation is made.
If the PMD-order is unset, then no PMD-sized THPs will be allocated.
+It is also possible to enable the recommended set of orders, which
+will be optimized for the architecture and mm::
+
+ echo recommend >/sys/kernel/mm/transparent_hugepage/anon_orders
The kernel will ignore any orders that it does not support so read the
file back to determine which orders are enabled::
diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
index af7639c3b0a3..0e110ce57cc3 100644
--- a/include/linux/pgtable.h
+++ b/include/linux/pgtable.h
@@ -393,6 +393,19 @@ static inline void arch_check_zapped_pmd(struct vm_area_struct *vma,
}
#endif
+#ifndef arch_wants_pte_order
+/*
+ * Returns preferred folio order for pte-mapped memory. Must be in range [0,
+ * PMD_ORDER) and must not be order-1 since THP requires large folios to be at
+ * least order-2. Negative value implies that the HW has no preference and mm
+ * will choose it's own default order.
+ */
+static inline int arch_wants_pte_order(void)
+{
+ return -1;
+}
+#endif
+
#ifndef __HAVE_ARCH_PTEP_GET_AND_CLEAR
static inline pte_t ptep_get_and_clear(struct mm_struct *mm,
unsigned long address,
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index bcecce769017..e2e2d3906a21 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -464,10 +464,18 @@ static ssize_t anon_orders_store(struct kobject *kobj,
int err;
int ret = count;
unsigned int orders;
+ int arch;
- err = kstrtouint(buf, 0, &orders);
- if (err)
- ret = -EINVAL;
+ if (sysfs_streq(buf, "recommend")) {
+ arch = max(arch_wants_pte_order(), PAGE_ALLOC_COSTLY_ORDER);
+ orders = BIT(arch);
+ orders |= BIT(PAGE_ALLOC_COSTLY_ORDER);
+ orders |= BIT(PMD_ORDER);
+ } else {
+ err = kstrtouint(buf, 0, &orders);
+ if (err)
+ ret = -EINVAL;
+ }
if (ret > 0) {
orders &= THP_ORDERS_ALL_ANON;
:/ don't really like that. Regarding my proposal, one could have
something like that in an "auto" setting for the "enabled" value, or a
"recommended" setting [not sure].
Me either.
Again this is something I call random -- we only discussed "auto",
and yes, the commit message above explained why "recommended" here but
it has never surfaced in previous discussions, has it?
The context in which we discussed "auto" was for a future aspiration to
automatically determine the order that should be used for a given allocation to
balance perf vs internal fragmentation.
The case we are talking about here is completely different; I had a pre-existing
feature from previous versions of the series, which would allow the arch to
specify its preferred order (originally proposed by Yu, IIRC). In moving the
allocation size decision to user space, I felt that we still needed a mechanism
whereby the arch could express its preference. And "recommend" is what I came up
with.
All of the friction we are currently having is around this feature, I think?
Certainly all the links you provided in the other thread all point to
conversations skirting around it. How about I just drop it for this initial
patch set? Just let user space decide what sizes it wants (per David's interface
proposal)? I can see I'm trying to get a square peg into a round hole.
Dropping it for the initial patch set sounds like a very good idea.
Telling people what to enable initially when they want to play with it
will work out just fine.
[Ideally, we plan ahead to have such "auto" settings in the future, as I
expressed.]
--
Cheers,
David / dhildenb