[PATCH v6 6/9] mm: thp: Add "recommend" option for anon_orders
David Hildenbrand
david at redhat.com
Mon Oct 9 07:43:29 PDT 2023
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 at redhat.com> 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 at arm.com>
>>>> ---
>>>> 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
More information about the linux-arm-kernel
mailing list