On Fri, Nov 1, 2024 at 3:20 AM Maíra Canal <mcanal@xxxxxxxxxx> wrote: > > On 31/10/24 10:33, David Hildenbrand wrote: > > On 31.10.24 14:24, Maíra Canal wrote: > >> Hi David, > >> > >> On 31/10/24 09:57, David Hildenbrand wrote: > >>> On 31.10.24 13:51, Maíra Canal wrote: > >>>> Hi David, > >>>> > >>>> On 31/10/24 09:37, David Hildenbrand wrote: > >>>>> On 30.10.24 13:58, Maíra Canal wrote: > >>>>>> Add the ``thp_shmem=`` kernel command line to allow specifying the > >>>>>> default policy of each supported shmem hugepage size. The kernel > >>>>>> parameter > >>>>>> accepts the following format: > >>>>>> > >>>>>> thp_shmem=<size>[KMG],<size>[KMG]:<policy>;<size>[KMG]- > >>>>>> <size>[KMG]:<policy> > >>>>>> > >>>>>> For example, > >>>>>> > >>>>>> thp_shmem=16K-64K:always;128K,512K:inherit;256K:advise;1M-2M:never;4M-8M:within_size > >>>>>> > >>>>>> By configuring the default policy of several shmem hugepages, the > >>>>>> user > >>>>>> can take advantage of mTHP before it's been configured through sysfs. > >>>>>> > >>>>>> Signed-off-by: Maíra Canal <mcanal@xxxxxxxxxx> > >>>>>> --- > >>>>>> .../admin-guide/kernel-parameters.txt | 10 ++ > >>>>>> Documentation/admin-guide/mm/transhuge.rst | 17 +++ > >>>>>> mm/shmem.c | 109 +++++++++++++ > >>>>>> ++++- > >>>>>> 3 files changed, 135 insertions(+), 1 deletion(-) > >>>>>> > >>>> > >>>> [...] > >>>> > >>>>>> diff --git a/mm/shmem.c b/mm/shmem.c > >>>>>> index dfcc88ec6e34..c2299fa0b345 100644 > >>>>>> --- a/mm/shmem.c > >>>>>> +++ b/mm/shmem.c > >>>>>> @@ -136,6 +136,7 @@ static unsigned long huge_shmem_orders_always > >>>>>> __read_mostly; > >>>>>> static unsigned long huge_shmem_orders_madvise __read_mostly; > >>>>>> static unsigned long huge_shmem_orders_inherit __read_mostly; > >>>>>> static unsigned long huge_shmem_orders_within_size __read_mostly; > >>>>>> +static bool shmem_orders_configured __initdata; > >>>>>> #endif > >>>>>> #ifdef CONFIG_TMPFS > >>>>>> @@ -5027,7 +5028,8 @@ void __init shmem_init(void) > >>>>>> * Default to setting PMD-sized THP to inherit the global > >>>>>> setting and > >>>>>> * disable all other multi-size THPs. > >>>>>> */ > >>>>>> - huge_shmem_orders_inherit = BIT(HPAGE_PMD_ORDER); > >>>>>> + if (!shmem_orders_configured) > >>>>>> + huge_shmem_orders_inherit = BIT(HPAGE_PMD_ORDER); > >>>>>> #endif > >>>>>> return; > >>>>>> @@ -5180,6 +5182,26 @@ struct kobj_attribute > >>>>>> thpsize_shmem_enabled_attr = > >>>>>> #if defined(CONFIG_TRANSPARENT_HUGEPAGE) > >>>>>> +static inline int get_order_from_str(const char *size_str) > >>>>>> +{ > >>>>>> + unsigned long size; > >>>>>> + char *endptr; > >>>>>> + int order; > >>>>>> + > >>>>>> + size = memparse(size_str, &endptr); > >>>>>> + > >>>>>> + if (!is_power_of_2(size)) > >>>>>> + goto err; > >>>>>> + order = get_order(size); > >>>>>> + if (BIT(order) & ~THP_ORDERS_ALL_FILE_DEFAULT) > >>>>>> + goto err; > >>>>>> + > >>>>>> + return order; > >>>>>> +err: > >>>>>> + pr_err("invalid size %s in thp_shmem boot parameter\n", > >>>>>> size_str); > >>>>>> + return -EINVAL; > >>>>>> +} > >>>>> > >>>>> Hm, mostly copy and paste. You could reuse existing > >>>>> get_order_from_str() > >>>>> simply by passing in the supported orders and moving error > >>>>> reporting to > >>>>> the caller. > >>>>> > >>>> > >>>> Can I use functions from mm/huge_memory.c here? > >>> > >>> Yes, that's the idea. > >>> > >> > >> Unfortunately, it isn't possible without adding the function to a > >> header. > > > > Well ... sure, what's the problem with that? > > David & Barry, how do you feel about adding `get_order_from_str()` to > lib/cmdline.c? I'd vote to leave it as is. If, at some point, the controls for shared memory and anonymous memory are moved to a file, that would be the right time to call the same get_order_from_str() for both. This is too trivial to warrant being an exposed API in huge_memory.h or cmdline. > > Best Regards, > - Maíra > Thanks Barry