| Message ID | 20231203001501.126339-4-sxwjean@me.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | supplement of slab allocator removal | expand |
On Sun, Dec 3, 2023 at 9:16 AM <sxwjean@me.com> wrote: > > From: Xiongwei Song <xiongwei.song@windriver.com> > > There is no a value assigned to slub_min_objects by default, it always > is 0 that is initialized by compiler if no assigned value by command line. > min_objects is calculated based on processor numbers in calculate_order(). > For more details, see commit 9b2cd506e5f2 ("slub: Calculate min_objects > based on number of processors.") > > Signed-off-by: Xiongwei Song <xiongwei.song@windriver.com> While slub_min_objects equals zero by default, 'min_objects' overrides it to 4 * (fls(nr_cpus) + 1) when not set. so when slub_min_objects is not set, it would be equal to or higher than 4. I'm not sure this level of implementation detail is worth documenting. Also, I think patch 2 should update Documentation/mm/slub.rst too. (slub_$param -> slab_param) > --- > Documentation/mm/slub.rst | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Documentation/mm/slub.rst b/Documentation/mm/slub.rst > index be75971532f5..1f4399581449 100644 > --- a/Documentation/mm/slub.rst > +++ b/Documentation/mm/slub.rst > @@ -150,7 +150,7 @@ list_lock once in a while to deal with partial slabs. That overhead is > governed by the order of the allocation for each slab. The allocations > can be influenced by kernel parameters: > > -.. slub_min_objects=x (default 4) > +.. slub_min_objects=x (default 0) > .. slub_min_order=x (default 0) > .. slub_max_order=x (default 3 (PAGE_ALLOC_COSTLY_ORDER)) > > -- > 2.34.1 >
Hi Hyeonggon, > -----Original Message----- > From: Hyeonggon Yoo <42.hyeyoo@gmail.com> > Sent: Tuesday, December 5, 2023 8:54 AM > To: sxwjean@me.com > Cc: vbabka@suse.cz; cl@linux.com; linux-mm@kvack.org; penberg@kernel.org; > rientjes@google.com; iamjoonsoo.kim@lge.com; roman.gushchin@linux.dev; > corbet@lwn.net; keescook@chromium.org; arnd@arndb.de; akpm@linux-foundation.org; > gregkh@linuxfoundation.org; linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org; Song, > Xiongwei <Xiongwei.Song@windriver.com> > Subject: Re: [PATCH v2 3/3] mm/slub: correct the default value of slub_min_objects in doc > > On Sun, Dec 3, 2023 at 9:16 AM <sxwjean@me.com> wrote: > > > > From: Xiongwei Song <xiongwei.song@windriver.com> > > > > There is no a value assigned to slub_min_objects by default, it always > > is 0 that is initialized by compiler if no assigned value by command line. > > min_objects is calculated based on processor numbers in calculate_order(). > > For more details, see commit 9b2cd506e5f2 ("slub: Calculate min_objects > > based on number of processors.") > > > > Signed-off-by: Xiongwei Song <xiongwei.song@windriver.com> > > While slub_min_objects equals zero by default, 'min_objects' overrides it to > 4 * (fls(nr_cpus) + 1) when not set. so when slub_min_objects is not > set, it would be > equal to or higher than 4. I'm not sure this level of implementation > detail is worth documenting. commit 9b2cd506e5f2 ("slub: Calculate min_objects based on number of processors.") has already given "processors min_objects" pair, do we really need to document the specific detail? > > Also, I think patch 2 should update Documentation/mm/slub.rst too. > (slub_$param -> slab_param) I think people can know slub_$params are still supported by Documentation/mm/slub.rst, so we don't need to say the info again in this file. Is it better to do so just before removing slub_$params completely? Regards, Xiongwei > > > --- > > Documentation/mm/slub.rst | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/Documentation/mm/slub.rst b/Documentation/mm/slub.rst > > index be75971532f5..1f4399581449 100644 > > --- a/Documentation/mm/slub.rst > > +++ b/Documentation/mm/slub.rst > > @@ -150,7 +150,7 @@ list_lock once in a while to deal with partial slabs. That overhead is > > governed by the order of the allocation for each slab. The allocations > > can be influenced by kernel parameters: > > > > -.. slub_min_objects=x (default 4) > > +.. slub_min_objects=x (default 0) > > .. slub_min_order=x (default 0) > > .. slub_max_order=x (default 3 (PAGE_ALLOC_COSTLY_ORDER)) > > > > -- > > 2.34.1 > >
On Tue, Dec 5, 2023 at 11:11 PM Song, Xiongwei <Xiongwei.Song@windriver.com> wrote: > > Hi Hyeonggon, > > > -----Original Message----- > > From: Hyeonggon Yoo <42.hyeyoo@gmail.com> > > Sent: Tuesday, December 5, 2023 8:54 AM > > To: sxwjean@me.com > > Cc: vbabka@suse.cz; cl@linux.com; linux-mm@kvack.org; penberg@kernel.org; > > rientjes@google.com; iamjoonsoo.kim@lge.com; roman.gushchin@linux.dev; > > corbet@lwn.net; keescook@chromium.org; arnd@arndb.de; akpm@linux-foundation.org; > > gregkh@linuxfoundation.org; linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org; Song, > > Xiongwei <Xiongwei.Song@windriver.com> > > Subject: Re: [PATCH v2 3/3] mm/slub: correct the default value of slub_min_objects in doc > > > > On Sun, Dec 3, 2023 at 9:16 AM <sxwjean@me.com> wrote: > > > > > > From: Xiongwei Song <xiongwei.song@windriver.com> > > > > > > There is no a value assigned to slub_min_objects by default, it always > > > is 0 that is initialized by compiler if no assigned value by command line. > > > min_objects is calculated based on processor numbers in calculate_order(). > > > For more details, see commit 9b2cd506e5f2 ("slub: Calculate min_objects > > > based on number of processors.") > > > > > > Signed-off-by: Xiongwei Song <xiongwei.song@windriver.com> > > > > While slub_min_objects equals zero by default, 'min_objects' overrides it to > > 4 * (fls(nr_cpus) + 1) when not set. so when slub_min_objects is not > > set, it would be > > equal to or higher than 4. I'm not sure this level of implementation > > detail is worth documenting. > > commit 9b2cd506e5f2 ("slub: Calculate min_objects based on number of processors.") > has already given "processors min_objects" pair, do we really need to document > the specific detail? No, I don't think it needs to be documented, but neither do I think "slub_min_objects is 0 by default" is correctly documented... > > Also, I think patch 2 should update Documentation/mm/slub.rst too. > > (slub_$param -> slab_param) > I think people can know slub_$params are still supported by > Documentation/mm/slub.rst, so we don't need to say the info again in > this file. Is it better to do so just before removing slub_$params > completely? If we're deprecating and planning to drop slub_$params in the future, IMHO it'd be less confusing if we change it now, rather than when removing slub_$params completely (probably 10 years later)? Thanks, Hyeonggon > > > --- > > > Documentation/mm/slub.rst | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/Documentation/mm/slub.rst b/Documentation/mm/slub.rst > > > index be75971532f5..1f4399581449 100644 > > > --- a/Documentation/mm/slub.rst > > > +++ b/Documentation/mm/slub.rst > > > @@ -150,7 +150,7 @@ list_lock once in a while to deal with partial slabs. That overhead is > > > governed by the order of the allocation for each slab. The allocations > > > can be influenced by kernel parameters: > > > > > > -.. slub_min_objects=x (default 4) > > > +.. slub_min_objects=x (default 0) > > > .. slub_min_order=x (default 0) > > > .. slub_max_order=x (default 3 (PAGE_ALLOC_COSTLY_ORDER)) > > > > > > -- > > > 2.34.1 > > >
> -----Original Message----- > From: Hyeonggon Yoo <42.hyeyoo@gmail.com> > Sent: Wednesday, December 6, 2023 8:23 AM > To: Song, Xiongwei <Xiongwei.Song@windriver.com> > Cc: sxwjean@me.com; vbabka@suse.cz; cl@linux.com; linux-mm@kvack.org; > penberg@kernel.org; rientjes@google.com; iamjoonsoo.kim@lge.com; > roman.gushchin@linux.dev; corbet@lwn.net; keescook@chromium.org; arnd@arndb.de; > akpm@linux-foundation.org; gregkh@linuxfoundation.org; linux-doc@vger.kernel.org; linux- > kernel@vger.kernel.org > Subject: Re: [PATCH v2 3/3] mm/slub: correct the default value of slub_min_objects in doc > > CAUTION: This email comes from a non Wind River email account! > Do not click links or open attachments unless you recognize the sender and know the content > is safe. > > On Tue, Dec 5, 2023 at 11:11 PM Song, Xiongwei > <Xiongwei.Song@windriver.com> wrote: > > > > Hi Hyeonggon, > > > > > -----Original Message----- > > > From: Hyeonggon Yoo <42.hyeyoo@gmail.com> > > > Sent: Tuesday, December 5, 2023 8:54 AM > > > To: sxwjean@me.com > > > Cc: vbabka@suse.cz; cl@linux.com; linux-mm@kvack.org; penberg@kernel.org; > > > rientjes@google.com; iamjoonsoo.kim@lge.com; roman.gushchin@linux.dev; > > > corbet@lwn.net; keescook@chromium.org; arnd@arndb.de; akpm@linux- > foundation.org; > > > gregkh@linuxfoundation.org; linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org; > Song, > > > Xiongwei <Xiongwei.Song@windriver.com> > > > Subject: Re: [PATCH v2 3/3] mm/slub: correct the default value of slub_min_objects in > doc > > > > > > On Sun, Dec 3, 2023 at 9:16 AM <sxwjean@me.com> wrote: > > > > > > > > From: Xiongwei Song <xiongwei.song@windriver.com> > > > > > > > > There is no a value assigned to slub_min_objects by default, it always > > > > is 0 that is initialized by compiler if no assigned value by command line. > > > > min_objects is calculated based on processor numbers in calculate_order(). > > > > For more details, see commit 9b2cd506e5f2 ("slub: Calculate min_objects > > > > based on number of processors.") > > > > > > > > Signed-off-by: Xiongwei Song <xiongwei.song@windriver.com> > > > > > > While slub_min_objects equals zero by default, 'min_objects' overrides it to > > > 4 * (fls(nr_cpus) + 1) when not set. so when slub_min_objects is not > > > set, it would be > > > equal to or higher than 4. I'm not sure this level of implementation > > > detail is worth documenting. > > > > commit 9b2cd506e5f2 ("slub: Calculate min_objects based on number of processors.") > > has already given "processors min_objects" pair, do we really need to document > > the specific detail? > > No, I don't think it needs to be documented, but neither do I think > "slub_min_objects is > 0 by default" is correctly documented... > > > > Also, I think patch 2 should update Documentation/mm/slub.rst too. > > > (slub_$param -> slab_param) > > I think people can know slub_$params are still supported by > > Documentation/mm/slub.rst, so we don't need to say the info again in > > this file. Is it better to do so just before removing slub_$params > > completely? > > If we're deprecating and planning to drop slub_$params in the future, > IMHO it'd be less confusing if we change it now, rather than > when removing slub_$params completely (probably 10 years later)? Ok, sure. Will update in next version. Regards, Xiongwei > > Thanks, > Hyeonggon > > > > > --- > > > > Documentation/mm/slub.rst | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/Documentation/mm/slub.rst b/Documentation/mm/slub.rst > > > > index be75971532f5..1f4399581449 100644 > > > > --- a/Documentation/mm/slub.rst > > > > +++ b/Documentation/mm/slub.rst > > > > @@ -150,7 +150,7 @@ list_lock once in a while to deal with partial slabs. That overhead > is > > > > governed by the order of the allocation for each slab. The allocations > > > > can be influenced by kernel parameters: > > > > > > > > -.. slub_min_objects=x (default 4) > > > > +.. slub_min_objects=x (default 0) > > > > .. slub_min_order=x (default 0) > > > > .. slub_max_order=x (default 3 (PAGE_ALLOC_COSTLY_ORDER)) > > > > > > > > -- > > > > 2.34.1 > > > >
On 12/5/23 01:53, Hyeonggon Yoo wrote: > On Sun, Dec 3, 2023 at 9:16 AM <sxwjean@me.com> wrote: >> >> From: Xiongwei Song <xiongwei.song@windriver.com> >> >> There is no a value assigned to slub_min_objects by default, it always >> is 0 that is initialized by compiler if no assigned value by command line. >> min_objects is calculated based on processor numbers in calculate_order(). >> For more details, see commit 9b2cd506e5f2 ("slub: Calculate min_objects >> based on number of processors.") >> >> Signed-off-by: Xiongwei Song <xiongwei.song@windriver.com> > > While slub_min_objects equals zero by default, 'min_objects' overrides it to > 4 * (fls(nr_cpus) + 1) when not set. so when slub_min_objects is not > set, it would be > equal to or higher than 4. I'm not sure this level of implementation > detail is worth documenting. We could say e.g. "(default: automaticaly scaled by number of cpus)" > Also, I think patch 2 should update Documentation/mm/slub.rst too. > (slub_$param -> slab_param) I'd do it as separate patch, not part of patch 2. >> --- >> Documentation/mm/slub.rst | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/Documentation/mm/slub.rst b/Documentation/mm/slub.rst >> index be75971532f5..1f4399581449 100644 >> --- a/Documentation/mm/slub.rst >> +++ b/Documentation/mm/slub.rst >> @@ -150,7 +150,7 @@ list_lock once in a while to deal with partial slabs. That overhead is >> governed by the order of the allocation for each slab. The allocations >> can be influenced by kernel parameters: >> >> -.. slub_min_objects=x (default 4) >> +.. slub_min_objects=x (default 0) >> .. slub_min_order=x (default 0) >> .. slub_max_order=x (default 3 (PAGE_ALLOC_COSTLY_ORDER)) >> >> -- >> 2.34.1 >>
> -----Original Message----- > From: Vlastimil Babka <vbabka@suse.cz> > Sent: Thursday, December 7, 2023 12:34 AM > To: Hyeonggon Yoo <42.hyeyoo@gmail.com>; sxwjean@me.com > Cc: cl@linux.com; linux-mm@kvack.org; penberg@kernel.org; rientjes@google.com; > iamjoonsoo.kim@lge.com; roman.gushchin@linux.dev; corbet@lwn.net; > keescook@chromium.org; arnd@arndb.de; akpm@linux-foundation.org; > gregkh@linuxfoundation.org; linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org; Song, > Xiongwei <Xiongwei.Song@windriver.com> > Subject: Re: [PATCH v2 3/3] mm/slub: correct the default value of slub_min_objects in doc > > On 12/5/23 01:53, Hyeonggon Yoo wrote: > > On Sun, Dec 3, 2023 at 9:16 AM <sxwjean@me.com> wrote: > >> > >> From: Xiongwei Song <xiongwei.song@windriver.com> > >> > >> There is no a value assigned to slub_min_objects by default, it always > >> is 0 that is initialized by compiler if no assigned value by command line. > >> min_objects is calculated based on processor numbers in calculate_order(). > >> For more details, see commit 9b2cd506e5f2 ("slub: Calculate min_objects > >> based on number of processors.") > >> > >> Signed-off-by: Xiongwei Song <xiongwei.song@windriver.com> > > > > While slub_min_objects equals zero by default, 'min_objects' overrides it to > > 4 * (fls(nr_cpus) + 1) when not set. so when slub_min_objects is not > > set, it would be > > equal to or higher than 4. I'm not sure this level of implementation > > detail is worth documenting. > > We could say e.g. "(default: automaticaly scaled by number of cpus)" Ah, looks better. > > > Also, I think patch 2 should update Documentation/mm/slub.rst too. > > (slub_$param -> slab_param) > > I'd do it as separate patch, not part of patch 2. Will do. Thanks > > >> --- > >> Documentation/mm/slub.rst | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/Documentation/mm/slub.rst b/Documentation/mm/slub.rst > >> index be75971532f5..1f4399581449 100644 > >> --- a/Documentation/mm/slub.rst > >> +++ b/Documentation/mm/slub.rst > >> @@ -150,7 +150,7 @@ list_lock once in a while to deal with partial slabs. That overhead > is > >> governed by the order of the allocation for each slab. The allocations > >> can be influenced by kernel parameters: > >> > >> -.. slub_min_objects=x (default 4) > >> +.. slub_min_objects=x (default 0) > >> .. slub_min_order=x (default 0) > >> .. slub_max_order=x (default 3 (PAGE_ALLOC_COSTLY_ORDER)) > >> > >> -- > >> 2.34.1 > >>
diff --git a/Documentation/mm/slub.rst b/Documentation/mm/slub.rst index be75971532f5..1f4399581449 100644 --- a/Documentation/mm/slub.rst +++ b/Documentation/mm/slub.rst @@ -150,7 +150,7 @@ list_lock once in a while to deal with partial slabs. That overhead is governed by the order of the allocation for each slab. The allocations can be influenced by kernel parameters: -.. slub_min_objects=x (default 4) +.. slub_min_objects=x (default 0) .. slub_min_order=x (default 0) .. slub_max_order=x (default 3 (PAGE_ALLOC_COSTLY_ORDER))