Re: [PATCH v2] Documentation: kernel-parameters: remove slab_max_order and noaliencache

[Vlastimil Babka <vbabka@suse.cz>]

On 11/26/23 08:25, Song, Xiongwei wrote:
> Hi Vlastimil,
> 
>> -----Original Message-----
>> From: Vlastimil Babka <vbabka@suse.cz>
>> Sent: Friday, November 24, 2023 7:24 PM
>> To: sxwjean@me.com; 42.hyeyoo@gmail.com; linux-mm@kvack.org
>> Cc: cl@linux.com; penberg@kernel.org; rientjes@google.com; iamjoonsoo.kim@lge.com;
>> akpm@linux-foundation.org; roman.gushchin@linux.dev; corbet@lwn.net; linux-
>> doc@vger.kernel.org; linux-kernel@vger.kernel.org; Song, Xiongwei
>> <Xiongwei.Song@windriver.com>
>> Subject: Re: [PATCH v2] Documentation: kernel-parameters: remove slab_max_order and
>> noaliencache
>> 
>> 
>> On 11/22/23 15:36, sxwjean@me.com wrote:
>> > From: Xiongwei Song <xiongwei.song@windriver.com>
>> >
>> > Since slab allocator has already been removed. There is no users about
>> > slab_max_order and noaliencache, so let's remove them.
>> >
>> > Signed-off-by: Xiongwei Song <xiongwei.song@windriver.com>
>> > ---
>> > v2: Hyeonggon Yoo <42.hyeyoo@gmail.com> suggested that noaliencache should be
>> > removed too. Here adding this change. The patch is based on [1].
>> >
>> > [1] https://git.kernel.org/pub/scm/linux/kernel/git/vbabka/linux.git/log/?h=slab-remove-
>> slab-v2r1
>> >
>> > v1: https://lore.kernel.org/linux-mm/20231120091214.150502-1-
>> sxwjean@me.com/T/#m55ebb45851bc86d650baf65dfe8296d33c5b1126
>> > ---
>> >  Documentation/admin-guide/kernel-parameters.txt | 10 ----------
>> >  1 file changed, 10 deletions(-)
>> >
>> > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-
>> guide/kernel-parameters.txt
>> > index 65731b060e3f..d56a5beefe24 100644
>> > --- a/Documentation/admin-guide/kernel-parameters.txt
>> > +++ b/Documentation/admin-guide/kernel-parameters.txt
>> > @@ -3740,10 +3740,6 @@
>> >       no5lvl          [X86-64,RISCV] Disable 5-level paging mode. Forces
>> >                       kernel to use 4-level paging instead.
>> >
>> > -     noaliencache    [MM, NUMA, SLAB] Disables the allocation of alien
>> > -                     caches in the slab allocator.  Saves per-node memory,
>> > -                     but will impact performance.
>> 
>> No question about this one, can be deleted.
>> 
>> > -
>> >       noalign         [KNL,ARM]
>> >
>> >       noaltinstr      [S390] Disables alternative instructions patching
>> > @@ -5887,12 +5883,6 @@
>> >                       own.
>> >                       For more information see Documentation/mm/slub.rst.
>> >
>> > -     slab_max_order= [MM, SLAB]
>> > -                     Determines the maximum allowed order for slabs.
>> > -                     A high setting may cause OOMs due to memory
>> > -                     fragmentation.  Defaults to 1 for systems with
>> > -                     more than 32MB of RAM, 0 otherwise.
>> 
>> I think here we should consider the long-term plan first. It's a bit
>> unfortunate (in hindsight) SLUB brought its own prefix of parameters, even
>> if some became interchangeable aliases later (slab/slub_nomerge), some not.
>> I think it would be best to unify them, and consider the string "slub" an
>> implementation detail of the general "slab allocator" term going forward.
>> 
>> So what I'd propose is that we change all parameters to accept a
>> "slab_$param" as a primary and documented name (and the description can
>> contain just [MM] tag, no [SLAB] or [SLUB] needed), with "slub_$param" is
>> also accepted as an alias where it exists today, and there's just a note
>> that the slub_$param name is also accepted in the description of the
>> canonical parameter, not in a separate description. Then maybe in a few
>> years we can mark the old names as deprecated and start issuing low-key
>> warnings (while still accepting them), and in 10 years maybe remove them
>> completely. Thoughts?
> 
> Sorry, I didn't know the SLUB history, thanks for the comments and proposal. 
> 
> Did you mean the rough diff below?
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 65731b060e3f..db6d2ebe7c7d 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -5871,10 +5871,12 @@
>          slram=          [HW,MTD]
> 
>          slab_merge      [MM]
> +       slub_merge      [MM]
>                          Enable merging of slabs with similar size when the
>                          kernel is built without CONFIG_SLAB_MERGE_DEFAULT.

I'd hope the result look more like this, so the duplicate names are not
so prominent.

        slab_merge      [MM]
                        Enable merging of slabs with similar size when the
                        kernel is built without CONFIG_SLAB_MERGE_DEFAULT.
                        (slub_merge also accepted as an alias)

Note that it's not just a Documentation change anymore, as many of the
parameters don't have the slab_ variants yet wired up.

> 
>          slab_nomerge    [MM]
> +       slub_nomerge    [MM]
>                          Disable merging of slabs with similar size. May be
>                          necessary if there is some reason to distinguish
>                          allocs to different slabs, especially in hardened
> @@ -5887,12 +5889,6 @@
>                          own.
>                          For more information see Documentation/mm/slub.rst.
> 
> -       slab_max_order= [MM, SLAB]
> -                       Determines the maximum allowed order for slabs.
> -                       A high setting may cause OOMs due to memory
> -                       fragmentation.  Defaults to 1 for systems with
> -                       more than 32MB of RAM, 0 otherwise.
> -
>          slub_debug[=options[,slabs][;[options[,slabs]]...]      [MM, SLUB]
>                          Enabling slub_debug allows one to determine the
>                          culprit if slab objects become corrupted. Enabling
> @@ -5901,13 +5897,15 @@
>                          last alloc / free. For more information see
>                          Documentation/mm/slub.rst.
> 
> -       slub_max_order= [MM, SLUB]
> +       slab_max_order= [MM]
> +       slub_max_order= [MM]
>                          Determines the maximum allowed order for slabs.
>                          A high setting may cause OOMs due to memory
>                          fragmentation. For more information see
>                          Documentation/mm/slub.rst.
> 
> -       slub_min_objects=       [MM, SLUB]
> +       slab_min_objects=       [MM]
> +       slub_min_objects=       [MM]
>                          The minimum number of objects per slab. SLUB will
>                          increase the slab order up to slub_max_order to
>                          generate a sufficiently large slab able to contain
> @@ -5916,18 +5914,12 @@
>                          and the less frequently locks need to be acquired.
>                          For more information see Documentation/mm/slub.rst.
> 
> -       slub_min_order= [MM, SLUB]
> +       slub_min_order= [MM]
> +       slab_min_order= [MM]
>                          Determines the minimum page order for slabs.
> Must be
>                          lower than slub_max_order.
>                          For more information see Documentation/mm/slub.rst.
> 
> -       slub_merge      [MM, SLUB]
> -                       Same with slab_merge.
> -
> -       slub_nomerge    [MM, SLUB]
> -                       Same with slab_nomerge. This is supported for
> legacy.
> -                       See slab_nomerge for more information.
> -
>          smart2=         [HW]
>                          Format: <io1>[,<io2>[,...,<io8>]]
> 
> If so I think we should use slab_&param in mm/slub.c. When hitting "slub_$param"
> we need to assign the value to "slab_&param" like "slab_nomerge", right?
> 
> Regards,
> Xiongwei