> -----Original Message----- > From: Vlastimil Babka <vbabka@xxxxxxx> > Sent: Thursday, December 7, 2023 12:15 AM > To: sxwjean@xxxxxx; 42.hyeyoo@xxxxxxxxx; cl@xxxxxxxxx; linux- > mm@xxxxxxxxx > Cc: penberg@xxxxxxxxxx; rientjes@xxxxxxxxxx; iamjoonsoo.kim@xxxxxxx; > roman.gushchin@xxxxxxxxx; corbet@xxxxxxx; keescook@xxxxxxxxxxxx; > arnd@xxxxxxxx; akpm@xxxxxxxxxxxxxxxxxxxx; gregkh@xxxxxxxxxxxxxxxxxxx; > linux-doc@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Song, Xiongwei > <Xiongwei.Song@xxxxxxxxxxxxx> > Subject: Re: [RFC PATCH v2 2/3] mm/slub: unify all sl[au]b parameters with > "slab_$param" > > On 12/3/23 01:15, sxwjean@xxxxxx wrote: > > From: Xiongwei Song <xiongwei.song@xxxxxxxxxxxxx> > > > > Since the SLAB allocator has been removed, so we need to clean up the > > "we can clean up", as we don't really "need" Ok, make sense. > > > sl[au]b_$params. However, the "slab/SLAB" terms should be keep for > > long-term rather than "slub/SLUB". Hence, we should use "slab_$param" > > I'd phrase it: With only one slab allocator left, it's better to use the > generic "slab" term instead of "slub" which is an implementation detail. > Hence ... Ok. > > as the primary prefix, which is pointed out by Vlastimil Babka. For more > > information please see [1]. > > > > This patch is changing the following slab parameters > > - slub_max_order > > - slub_min_order > > - slub_min_objects > > - slub_debug > > to > > - slab_max_order > > - slab_min_order > > - slab_min_objects > > - slab_debug > > as the primary slab parameters in > > Documentation/admin-guide/kernel-parameters.txt and source, and rename > all > > setup functions of them too. Meanwhile, "slub_$params" can also be > passed > > Not sure about renaming the code at this point, I would just rename the > user-visible parameters and their documentation and any comment that refers > to the parameters. Functions and variables can come later as part of wider > slub/slab change if we decide to do so? Ok. Will create a separate patch as you mentioned in the comment of patch 3. > > > by command line, which is to keep backward compatibility. Also mark all > > "slub_$params" as legacy. > > > > The function > > static int __init setup_slub_debug(char *str); > > , which is to setup debug flags inside a slab during kernel init, is > > changed to setup_slab_debug_flags(), which is to prevent the name > > conflict. Because there is another function > > void setup_slab_debug(struct kmem_cache *s, struct slab *slab, > > void *addr); > > , which is to poison slab space, would have name conflict with the prior > > one. > > Another reason to defer code naming changes. Ok. > > > For parameter "slub_debug", beside replacing it with "slab_debug", there > > are several global variables, local variables and functions which are > > related with the parameter, let's rename them all. > > > > Remove the separate descriptions for slub_[no]merge, append legacy tip > > for them at the end of descriptions of slab_[no]merge. > > > > I didn't change the parameters in Documentation/mm/slub.rst because the > > file name is still "slub.rst", and slub_$params still can be used in > > kernel command line to keep backward compatibility. > > > > [1] https://lore.kernel.org/linux-mm/7512b350-4317-21a0-fab3- > 4101bc4d8f7a@xxxxxxx/ > > > > Signed-off-by: Xiongwei Song <xiongwei.song@xxxxxxxxxxxxx> > > --- > > .../admin-guide/kernel-parameters.txt | 44 +++--- > > drivers/misc/lkdtm/heap.c | 2 +- > > mm/Kconfig.debug | 6 +- > > mm/slab.h | 16 +- > > mm/slab_common.c | 8 +- > > mm/slub.c | 142 +++++++++--------- > > 6 files changed, 109 insertions(+), 109 deletions(-) > > > > diff --git a/Documentation/admin-guide/kernel-parameters.txt > b/Documentation/admin-guide/kernel-parameters.txt > > index 9f94baeb2f82..d01c12e2a247 100644 > > --- a/Documentation/admin-guide/kernel-parameters.txt > > +++ b/Documentation/admin-guide/kernel-parameters.txt > > @@ -5869,6 +5869,8 @@ > > slab_merge [MM] > > Enable merging of slabs with similar size when the > > kernel is built without CONFIG_SLAB_MERGE_DEFAULT. > > + (slub_merge is accepted too, but it's supported for > > + legacy) > > How about a shorter note (and always start on new line) > > (slub_merge legacy name also accepted for now) Ok. > > > > > slab_nomerge [MM] > > Disable merging of slabs with similar size. May be > > @@ -5882,47 +5884,41 @@ > > unchanged). Debug options disable merging on their > > own. > > For more information see Documentation/mm/slub.rst. > > + (slub_nomerge is accepted too, but it's supported for > > + legacy) > > > > - 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 > > + slab_debug[=options[,slabs][;[options[,slabs]]...] [MM] > > I think we should re-sort alphabetically after the slub_ -> slab_ change. Ok. > > > + Enabling slab_debug allows one to determine the > > culprit if slab objects become corrupted. Enabling > > - slub_debug can create guard zones around objects and > > + slab_debug can create guard zones around objects and > > may poison objects when not in use. Also tracks the > > last alloc / free. For more information see > > - Documentation/mm/slub.rst. > > + Documentation/mm/slub.rst. (slub_debug is accepted > > + too, but it's supported for legacy) > > > > - slub_max_order= [MM, SLUB] > > + slab_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. > > + Documentation/mm/slub.rst. (slub_max_order is > > + accepted too, but it's supported for legacy) > > > > - slub_min_objects= [MM, SLUB] > > + slab_min_objects= [MM] > > The minimum number of objects per slab. SLUB will > > - increase the slab order up to slub_max_order to > > + increase the slab order up to slab_max_order to > > generate a sufficiently large slab able to contain > > the number of objects indicated. The higher the number > > of objects the smaller the overhead of tracking slabs > > and the less frequently locks need to be acquired. > > For more information see Documentation/mm/slub.rst. > > + (slub_min_objects is accepted too, but it's supported > > + for legacy) > > > > - slub_min_order= [MM, SLUB] > > + 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. > > + lower than slab_max_order. For more information see > > "lower or equal to" (more precise, while at it) Yes, I agree. Thanks for all the detailed comments. Will update. Regards, Xiongwei > > > + Documentation/mm/slub.rst. (slub_min_order is accepted > > + too, but it's supported for legacy) > > > > smart2= [HW] > > Format: <io1>[,<io2>[,...,<io8>]] > > Thanks!
