All of lore.kernel.org
 help / color / mirror / Atom feed
From: Liu Shixin <liushixin2@huawei.com>
To: 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>,
	Vlastimil Babka <vbabka@suse.cz>,
	"Roman Gushchin" <roman.gushchin@linux.dev>,
	Hyeonggon Yoo <42.hyeyoo@gmail.com>
Cc: <linux-mm@kvack.org>, <linux-kernel@vger.kernel.org>,
	Liu Shixin <liushixin2@huawei.com>
Subject: [PATCH v3 2/3] mm/slub: Refactor __kmem_cache_create()
Date: Sat, 12 Nov 2022 19:20:54 +0800	[thread overview]
Message-ID: <20221112112055.1111078-3-liushixin2@huawei.com> (raw)
In-Reply-To: <20221112112055.1111078-1-liushixin2@huawei.com>

Separate sysfs_slab_add() and debugfs_slab_add() from __kmem_cache_create()
can help to fix a memory leak about kobject. After this patch, we can fix
the memory leak naturally by calling kobject_put() to free kobject and
associated kmem_cache when sysfs_slab_add() failed.

Besides, after that, we can easy to provide sysfs and debugfs support for
other allocators too.

Signed-off-by: Liu Shixin <liushixin2@huawei.com>
---
 include/linux/slub_def.h | 11 ++++++++++
 mm/slab_common.c         | 12 +++++++++++
 mm/slub.c                | 44 +++++++---------------------------------
 3 files changed, 30 insertions(+), 37 deletions(-)

diff --git a/include/linux/slub_def.h b/include/linux/slub_def.h
index f9c68a9dac04..26d56c4c74d1 100644
--- a/include/linux/slub_def.h
+++ b/include/linux/slub_def.h
@@ -144,9 +144,14 @@ struct kmem_cache {
 
 #ifdef CONFIG_SYSFS
 #define SLAB_SUPPORTS_SYSFS
+int sysfs_slab_add(struct kmem_cache *);
 void sysfs_slab_unlink(struct kmem_cache *);
 void sysfs_slab_release(struct kmem_cache *);
 #else
+static inline int sysfs_slab_add(struct kmem_cache *s)
+{
+	return 0;
+}
 static inline void sysfs_slab_unlink(struct kmem_cache *s)
 {
 }
@@ -155,6 +160,12 @@ static inline void sysfs_slab_release(struct kmem_cache *s)
 }
 #endif
 
+#if defined(CONFIG_DEBUG_FS) && defined(CONFIG_SLUB_DEBUG)
+void debugfs_slab_add(struct kmem_cache *);
+#else
+static inline void debugfs_slab_add(struct kmem_cache *s) { }
+#endif
+
 void *fixup_red_left(struct kmem_cache *s, void *p);
 
 static inline void *nearest_obj(struct kmem_cache *cache, const struct slab *slab,
diff --git a/mm/slab_common.c b/mm/slab_common.c
index e5f430a17d95..55e2cf064dfe 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -234,6 +234,18 @@ static struct kmem_cache *create_cache(const char *name,
 	if (err)
 		goto out_free_name;
 
+#ifdef SLAB_SUPPORTS_SYSFS
+	/* Mutex is not taken during early boot */
+	if (slab_state >= FULL) {
+		err = sysfs_slab_add(s);
+		if (err) {
+			slab_kmem_cache_release(s);
+			return ERR_PTR(err);
+		}
+		debugfs_slab_add(s);
+	}
+#endif
+
 	s->refcount = 1;
 	list_add(&s->list, &slab_caches);
 	return s;
diff --git a/mm/slub.c b/mm/slub.c
index ba94eb6fda78..a1ad759753ce 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -299,20 +299,12 @@ struct track {
 enum track_item { TRACK_ALLOC, TRACK_FREE };
 
 #ifdef CONFIG_SYSFS
-static int sysfs_slab_add(struct kmem_cache *);
 static int sysfs_slab_alias(struct kmem_cache *, const char *);
 #else
-static inline int sysfs_slab_add(struct kmem_cache *s) { return 0; }
 static inline int sysfs_slab_alias(struct kmem_cache *s, const char *p)
 							{ return 0; }
 #endif
 
-#if defined(CONFIG_DEBUG_FS) && defined(CONFIG_SLUB_DEBUG)
-static void debugfs_slab_add(struct kmem_cache *);
-#else
-static inline void debugfs_slab_add(struct kmem_cache *s) { }
-#endif
-
 static inline void stat(const struct kmem_cache *s, enum stat_item si)
 {
 #ifdef CONFIG_SLUB_STATS
@@ -4297,7 +4289,7 @@ static int calculate_sizes(struct kmem_cache *s)
 	return !!oo_objects(s->oo);
 }
 
-static int kmem_cache_open(struct kmem_cache *s, slab_flags_t flags)
+int __kmem_cache_create(struct kmem_cache *s, slab_flags_t flags)
 {
 	s->flags = kmem_cache_flags(s->size, flags, s->name);
 #ifdef CONFIG_SLAB_FREELIST_HARDENED
@@ -4900,30 +4892,6 @@ __kmem_cache_alias(const char *name, unsigned int size, unsigned int align,
 	return s;
 }
 
-int __kmem_cache_create(struct kmem_cache *s, slab_flags_t flags)
-{
-	int err;
-
-	err = kmem_cache_open(s, flags);
-	if (err)
-		return err;
-
-	/* Mutex is not taken during early boot */
-	if (slab_state <= UP)
-		return 0;
-
-	err = sysfs_slab_add(s);
-	if (err) {
-		__kmem_cache_release(s);
-		return err;
-	}
-
-	if (s->flags & SLAB_STORE_USER)
-		debugfs_slab_add(s);
-
-	return 0;
-}
-
 #ifdef CONFIG_SYSFS
 static int count_inuse(struct slab *slab)
 {
@@ -5913,7 +5881,7 @@ static char *create_unique_id(struct kmem_cache *s)
 	return name;
 }
 
-static int sysfs_slab_add(struct kmem_cache *s)
+int sysfs_slab_add(struct kmem_cache *s)
 {
 	int err;
 	const char *name;
@@ -6236,10 +6204,13 @@ static const struct file_operations slab_debugfs_fops = {
 	.release = slab_debug_trace_release,
 };
 
-static void debugfs_slab_add(struct kmem_cache *s)
+void debugfs_slab_add(struct kmem_cache *s)
 {
 	struct dentry *slab_cache_dir;
 
+	if (!(s->flags & SLAB_STORE_USER))
+		return;
+
 	if (unlikely(!slab_debugfs_root))
 		return;
 
@@ -6264,8 +6235,7 @@ static int __init slab_debugfs_init(void)
 	slab_debugfs_root = debugfs_create_dir("slab", NULL);
 
 	list_for_each_entry(s, &slab_caches, list)
-		if (s->flags & SLAB_STORE_USER)
-			debugfs_slab_add(s);
+		debugfs_slab_add(s);
 
 	return 0;
 
-- 
2.25.1


  parent reply	other threads:[~2022-11-12 10:33 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-12 11:20 [PATCH v3 0/3] Refactor __kmem_cache_create() and fix memory leak Liu Shixin
2022-11-12 10:47 ` Liu Shixin
2022-11-12 11:20 ` [PATCH v3 1/3] mm/slab_common: Move cache_name to create_cache() Liu Shixin
2022-11-12 11:20 ` Liu Shixin [this message]
2022-11-12 11:20 ` [PATCH v3 3/3] mm/slub: Fix memory leak of kobj->name in sysfs_slab_add() Liu Shixin

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=20221112112055.1111078-3-liushixin2@huawei.com \
    --to=liushixin2@huawei.com \
    --cc=42.hyeyoo@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=cl@linux.com \
    --cc=iamjoonsoo.kim@lge.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=penberg@kernel.org \
    --cc=rientjes@google.com \
    --cc=roman.gushchin@linux.dev \
    --cc=vbabka@suse.cz \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.