| Message ID | 20221024074232.151383-4-liushixin2@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Refactor __kmem_cache_create() and fix memory leak | expand |
Hi Liu,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on vbabka-slab/for-next]
[also build test ERROR on linus/master v6.1-rc2 next-20221024]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Liu-Shixin/Refactor-__kmem_cache_create-and-fix-memory-leak/20221024-145607
base: git://git.kernel.org/pub/scm/linux/kernel/git/vbabka/slab.git for-next
patch link: https://lore.kernel.org/r/20221024074232.151383-4-liushixin2%40huawei.com
patch subject: [PATCH 3/4] mm/slab_common: Separate sysfs_slab_add() and debugfs_slab_add() from __kmem_cache_create()
config: i386-tinyconfig
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/b431907fc9bd145502e0bdb34fcb4b1b67770f2a
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Liu-Shixin/Refactor-__kmem_cache_create-and-fix-memory-leak/20221024-145607
git checkout b431907fc9bd145502e0bdb34fcb4b1b67770f2a
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
mm/slab_common.c: In function 'create_cache':
>> mm/slab_common.c:239:23: error: implicit declaration of function 'sysfs_slab_add' [-Werror=implicit-function-declaration]
239 | err = sysfs_slab_add(s);
| ^~~~~~~~~~~~~~
>> mm/slab_common.c:244:17: error: implicit declaration of function 'debugfs_slab_add'; did you mean 'debugfs_slab_release'? [-Werror=implicit-function-declaration]
244 | debugfs_slab_add(s);
| ^~~~~~~~~~~~~~~~
| debugfs_slab_release
cc1: some warnings being treated as errors
vim +/sysfs_slab_add +239 mm/slab_common.c
204
205 static struct kmem_cache *create_cache(const char *name,
206 unsigned int object_size, unsigned int align,
207 slab_flags_t flags, unsigned int useroffset,
208 unsigned int usersize, void (*ctor)(void *),
209 struct kmem_cache *root_cache)
210 {
211 struct kmem_cache *s;
212 const char *cache_name;
213 int err = -ENOMEM;
214
215 if (WARN_ON(useroffset + usersize > object_size))
216 useroffset = usersize = 0;
217
218 s = kmem_cache_zalloc(kmem_cache, GFP_KERNEL);
219 if (!s)
220 return ERR_PTR(err);
221
222 cache_name = kstrdup_const(name, GFP_KERNEL);
223 if (!cache_name)
224 goto out_free_cache;
225
226 s->name = cache_name;
227 s->size = s->object_size = object_size;
228 s->align = align;
229 s->ctor = ctor;
230 s->useroffset = useroffset;
231 s->usersize = usersize;
232
233 err = __kmem_cache_create(s, flags);
234 if (err)
235 goto out_free_name;
236
237 /* Mutex is not taken during early boot */
238 if (slab_state >= FULL) {
> 239 err = sysfs_slab_add(s);
240 if (err) {
241 slab_kmem_cache_release(s);
242 return ERR_PTR(err);
243 }
> 244 debugfs_slab_add(s);
245 }
246
247 s->refcount = 1;
248 list_add(&s->list, &slab_caches);
249 return s;
250
251 out_free_name:
252 kfree_const(s->name);
253 out_free_cache:
254 kmem_cache_free(kmem_cache, s);
255 return ERR_PTR(err);
256 }
257
Hi Liu,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on vbabka-slab/for-next]
[also build test ERROR on linus/master v6.1-rc2 next-20221024]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Liu-Shixin/Refactor-__kmem_cache_create-and-fix-memory-leak/20221024-145607
base: git://git.kernel.org/pub/scm/linux/kernel/git/vbabka/slab.git for-next
patch link: https://lore.kernel.org/r/20221024074232.151383-4-liushixin2%40huawei.com
patch subject: [PATCH 3/4] mm/slab_common: Separate sysfs_slab_add() and debugfs_slab_add() from __kmem_cache_create()
config: mips-buildonly-randconfig-r001-20221023
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 791a7ae1ba3efd6bca96338e10ffde557ba83920)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install mips cross compiling tool for clang build
# apt-get install binutils-mipsel-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/b431907fc9bd145502e0bdb34fcb4b1b67770f2a
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Liu-Shixin/Refactor-__kmem_cache_create-and-fix-memory-leak/20221024-145607
git checkout b431907fc9bd145502e0bdb34fcb4b1b67770f2a
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=mips SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> mm/slab_common.c:239:9: error: call to undeclared function 'sysfs_slab_add'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
err = sysfs_slab_add(s);
^
>> mm/slab_common.c:244:3: error: call to undeclared function 'debugfs_slab_add'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
debugfs_slab_add(s);
^
2 errors generated.
vim +/sysfs_slab_add +239 mm/slab_common.c
204
205 static struct kmem_cache *create_cache(const char *name,
206 unsigned int object_size, unsigned int align,
207 slab_flags_t flags, unsigned int useroffset,
208 unsigned int usersize, void (*ctor)(void *),
209 struct kmem_cache *root_cache)
210 {
211 struct kmem_cache *s;
212 const char *cache_name;
213 int err = -ENOMEM;
214
215 if (WARN_ON(useroffset + usersize > object_size))
216 useroffset = usersize = 0;
217
218 s = kmem_cache_zalloc(kmem_cache, GFP_KERNEL);
219 if (!s)
220 return ERR_PTR(err);
221
222 cache_name = kstrdup_const(name, GFP_KERNEL);
223 if (!cache_name)
224 goto out_free_cache;
225
226 s->name = cache_name;
227 s->size = s->object_size = object_size;
228 s->align = align;
229 s->ctor = ctor;
230 s->useroffset = useroffset;
231 s->usersize = usersize;
232
233 err = __kmem_cache_create(s, flags);
234 if (err)
235 goto out_free_name;
236
237 /* Mutex is not taken during early boot */
238 if (slab_state >= FULL) {
> 239 err = sysfs_slab_add(s);
240 if (err) {
241 slab_kmem_cache_release(s);
242 return ERR_PTR(err);
243 }
> 244 debugfs_slab_add(s);
245 }
246
247 s->refcount = 1;
248 list_add(&s->list, &slab_caches);
249 return s;
250
251 out_free_name:
252 kfree_const(s->name);
253 out_free_cache:
254 kmem_cache_free(kmem_cache, s);
255 return ERR_PTR(err);
256 }
257
Hi Liu,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on vbabka-slab/for-next]
[also build test ERROR on linus/master v6.1-rc2 next-20221024]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Liu-Shixin/Refactor-__kmem_cache_create-and-fix-memory-leak/20221024-145607
base: git://git.kernel.org/pub/scm/linux/kernel/git/vbabka/slab.git for-next
patch link: https://lore.kernel.org/r/20221024074232.151383-4-liushixin2%40huawei.com
patch subject: [PATCH 3/4] mm/slab_common: Separate sysfs_slab_add() and debugfs_slab_add() from __kmem_cache_create()
config: x86_64-randconfig-a005-20221024
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/b431907fc9bd145502e0bdb34fcb4b1b67770f2a
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Liu-Shixin/Refactor-__kmem_cache_create-and-fix-memory-leak/20221024-145607
git checkout b431907fc9bd145502e0bdb34fcb4b1b67770f2a
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> mm/slab_common.c:239:9: error: implicit declaration of function 'sysfs_slab_add' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
err = sysfs_slab_add(s);
^
>> mm/slab_common.c:244:3: error: implicit declaration of function 'debugfs_slab_add' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
debugfs_slab_add(s);
^
mm/slab_common.c:244:3: note: did you mean 'sysfs_slab_add'?
mm/slab_common.c:239:9: note: 'sysfs_slab_add' declared here
err = sysfs_slab_add(s);
^
2 errors generated.
vim +/sysfs_slab_add +239 mm/slab_common.c
204
205 static struct kmem_cache *create_cache(const char *name,
206 unsigned int object_size, unsigned int align,
207 slab_flags_t flags, unsigned int useroffset,
208 unsigned int usersize, void (*ctor)(void *),
209 struct kmem_cache *root_cache)
210 {
211 struct kmem_cache *s;
212 const char *cache_name;
213 int err = -ENOMEM;
214
215 if (WARN_ON(useroffset + usersize > object_size))
216 useroffset = usersize = 0;
217
218 s = kmem_cache_zalloc(kmem_cache, GFP_KERNEL);
219 if (!s)
220 return ERR_PTR(err);
221
222 cache_name = kstrdup_const(name, GFP_KERNEL);
223 if (!cache_name)
224 goto out_free_cache;
225
226 s->name = cache_name;
227 s->size = s->object_size = object_size;
228 s->align = align;
229 s->ctor = ctor;
230 s->useroffset = useroffset;
231 s->usersize = usersize;
232
233 err = __kmem_cache_create(s, flags);
234 if (err)
235 goto out_free_name;
236
237 /* Mutex is not taken during early boot */
238 if (slab_state >= FULL) {
> 239 err = sysfs_slab_add(s);
240 if (err) {
241 slab_kmem_cache_release(s);
242 return ERR_PTR(err);
243 }
> 244 debugfs_slab_add(s);
245 }
246
247 s->refcount = 1;
248 list_add(&s->list, &slab_caches);
249 return s;
250
251 out_free_name:
252 kfree_const(s->name);
253 out_free_cache:
254 kmem_cache_free(kmem_cache, s);
255 return ERR_PTR(err);
256 }
257
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..f146dea3f9de 100644 --- a/mm/slab_common.c +++ b/mm/slab_common.c @@ -234,6 +234,16 @@ static struct kmem_cache *create_cache(const char *name, if (err) goto out_free_name; + /* 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); + } + 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;
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 | 10 +++++++++ mm/slub.c | 44 +++++++--------------------------------- 3 files changed, 28 insertions(+), 37 deletions(-)