From: Muchun Song <songmuchun@bytedance.com>
To: Joonsoo Kim <js1304@gmail.com>
Cc: Christoph Lameter <cl@linux.com>,
Pekka Enberg <penberg@kernel.org>,
David Rientjes <rientjes@google.com>,
Joonsoo Kim <iamjoonsoo.kim@lge.com>,
Andrew Morton <akpm@linux-foundation.org>,
Linux Memory Management List <linux-mm@kvack.org>,
LKML <linux-kernel@vger.kernel.org>
Subject: Re: [External] Re: [PATCH 3/3] mm/slub: Fix release all resources used by a slab cache
Date: Mon, 15 Jun 2020 14:41:19 +0800 [thread overview]
Message-ID: <CAMZfGtUQ+0w=n9YSQjQc1uwJFJrvtfj=rcTnNYnewCPiHmDLEw@mail.gmail.com> (raw)
In-Reply-To: <CAAmzW4N1DRBxsMAX2a_jxUW10vOwW1VsGj8ztuvvWT4NB3Ocnw@mail.gmail.com>
On Mon, Jun 15, 2020 at 2:23 PM Joonsoo Kim <js1304@gmail.com> wrote:
>
> 2020년 6월 14일 (일) 오후 9:39, Muchun Song <songmuchun@bytedance.com>님이 작성:
> >
> > The function of __kmem_cache_shutdown() is that release all resources
> > used by the slab cache, while currently it stop release resources when
> > the preceding node is not empty.
> >
> > Signed-off-by: Muchun Song <songmuchun@bytedance.com>
> > ---
> > mm/slub.c | 7 ++++---
> > 1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/mm/slub.c b/mm/slub.c
> > index b73505df3de2..4e477ef0f2b9 100644
> > --- a/mm/slub.c
> > +++ b/mm/slub.c
> > @@ -3839,6 +3839,7 @@ bool __kmem_cache_empty(struct kmem_cache *s)
> > */
> > int __kmem_cache_shutdown(struct kmem_cache *s)
> > {
> > + int ret = 0;
> > int node;
> > struct kmem_cache_node *n;
> >
> > @@ -3846,11 +3847,11 @@ int __kmem_cache_shutdown(struct kmem_cache *s)
> > /* Attempt to free all objects */
> > for_each_kmem_cache_node(s, node, n) {
> > free_partial(s, n);
> > - if (node_nr_slabs(n))
> > - return 1;
> > + if (!ret && node_nr_slabs(n))
> > + ret = 1;
> > }
>
> I don't think that this is an improvement.
>
> If the shutdown condition isn't met, we don't need to process further.
> Just 'return 1' looks okay to me.
>
> And, with this change, sysfs_slab_remove() is called even if the
> shutdown is failed.
> It's better not to have side effects when failing.
If someone calls __kmem_cache_shutdown, he may want to release
resources used by the slab cache as much as possible. If we continue,
we may release more pages. From this point, is it an improvement?
--
Yours,
Muchun
next prev parent reply other threads:[~2020-06-15 6:41 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-14 12:39 [PATCH 0/3] mm/slub: Fix slabs_node return value Muchun Song
2020-06-14 12:39 ` [PATCH 1/3] mm/slub: Fix slabs_node return value when CONFIG_SLUB_DEBUG disabled Muchun Song
2020-06-15 6:11 ` Joonsoo Kim
2020-06-15 6:26 ` [External] " Muchun Song
2020-06-15 16:46 ` Vlastimil Babka
2020-06-14 12:39 ` [PATCH 2/3] mm/slub: Use node_nr_slabs() instead of slabs_node() Muchun Song
2020-06-15 6:15 ` Joonsoo Kim
2020-06-15 6:20 ` [External] " Muchun Song
2020-06-14 12:39 ` [PATCH 3/3] mm/slub: Fix release all resources used by a slab cache Muchun Song
2020-06-15 6:23 ` Joonsoo Kim
2020-06-15 6:41 ` Muchun Song [this message]
2020-06-15 7:25 ` [External] " Joonsoo Kim
2020-06-15 7:50 ` Muchun Song
2020-06-17 16:19 ` [PATCH 0/3] mm/slub: Fix slabs_node return value Christopher Lameter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAMZfGtUQ+0w=n9YSQjQc1uwJFJrvtfj=rcTnNYnewCPiHmDLEw@mail.gmail.com' \
--to=songmuchun@bytedance.com \
--cc=akpm@linux-foundation.org \
--cc=cl@linux.com \
--cc=iamjoonsoo.kim@lge.com \
--cc=js1304@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=penberg@kernel.org \
--cc=rientjes@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).