Skip to content

Commit 1f27ef4

Browse files
chaseyuJaegeuk Kim
authored andcommitted
f2fs: use global inline_xattr_slab instead of per-sb slab cache
As Hong Yun reported in mailing list: loop7: detected capacity change from 0 to 131072 ------------[ cut here ]------------ kmem_cache of name 'f2fs_xattr_entry-7:7' already exists WARNING: CPU: 0 PID: 24426 at mm/slab_common.c:110 kmem_cache_sanity_check mm/slab_common.c:109 [inline] WARNING: CPU: 0 PID: 24426 at mm/slab_common.c:110 __kmem_cache_create_args+0xa6/0x320 mm/slab_common.c:307 CPU: 0 UID: 0 PID: 24426 Comm: syz.7.1370 Not tainted 6.17.0-rc4 #1 PREEMPT(full) Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014 RIP: 0010:kmem_cache_sanity_check mm/slab_common.c:109 [inline] RIP: 0010:__kmem_cache_create_args+0xa6/0x320 mm/slab_common.c:307 Call Trace:  __kmem_cache_create include/linux/slab.h:353 [inline]  f2fs_kmem_cache_create fs/f2fs/f2fs.h:2943 [inline]  f2fs_init_xattr_caches+0xa5/0xe0 fs/f2fs/xattr.c:843  f2fs_fill_super+0x1645/0x2620 fs/f2fs/super.c:4918  get_tree_bdev_flags+0x1fb/0x260 fs/super.c:1692  vfs_get_tree+0x43/0x140 fs/super.c:1815  do_new_mount+0x201/0x550 fs/namespace.c:3808  do_mount fs/namespace.c:4136 [inline]  __do_sys_mount fs/namespace.c:4347 [inline]  __se_sys_mount+0x298/0x2f0 fs/namespace.c:4324  do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]  do_syscall_64+0x8e/0x3a0 arch/x86/entry/syscall_64.c:94  entry_SYSCALL_64_after_hwframe+0x76/0x7e The bug can be reproduced w/ below scripts: - mount /dev/vdb /mnt1 - mount /dev/vdc /mnt2 - umount /mnt1 - mounnt /dev/vdb /mnt1 The reason is if we created two slab caches, named f2fs_xattr_entry-7:3 and f2fs_xattr_entry-7:7, and they have the same slab size. Actually, slab system will only create one slab cache core structure which has slab name of "f2fs_xattr_entry-7:3", and two slab caches share the same structure and cache address. So, if we destroy f2fs_xattr_entry-7:3 cache w/ cache address, it will decrease reference count of slab cache, rather than release slab cache entirely, since there is one more user has referenced the cache. Then, if we try to create slab cache w/ name "f2fs_xattr_entry-7:3" again, slab system will find that there is existed cache which has the same name and trigger the warning. Let's changes to use global inline_xattr_slab instead of per-sb slab cache for fixing. Fixes: a999150 ("f2fs: use kmem_cache pool during inline xattr lookups") Cc: stable@kernel.org Reported-by: Hong Yun <yhong@link.cuhk.edu.hk> Tested-by: Hong Yun <yhong@link.cuhk.edu.hk> Signed-off-by: Chao Yu <chao@kernel.org> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
1 parent 10b591e commit 1f27ef4

File tree

4 files changed

+25
-37
lines changed

4 files changed

+25
-37
lines changed

fs/f2fs/f2fs.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1885,9 +1885,6 @@ struct f2fs_sb_info {
18851885
spinlock_t error_lock; /* protect errors/stop_reason array */
18861886
bool error_dirty; /* errors of sb is dirty */
18871887

1888-
struct kmem_cache *inline_xattr_slab; /* inline xattr entry */
1889-
unsigned int inline_xattr_slab_size; /* default inline xattr slab size */
1890-
18911888
/* For reclaimed segs statistics per each GC mode */
18921889
unsigned int gc_segment_mode; /* GC state for reclaimed segments */
18931890
unsigned int gc_reclaimed_segs[MAX_GC_MODE]; /* Reclaimed segs for each mode */

fs/f2fs/super.c

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2027,7 +2027,6 @@ static void f2fs_put_super(struct super_block *sb)
20272027
kfree(sbi->raw_super);
20282028

20292029
f2fs_destroy_page_array_cache(sbi);
2030-
f2fs_destroy_xattr_caches(sbi);
20312030
#ifdef CONFIG_QUOTA
20322031
for (i = 0; i < MAXQUOTAS; i++)
20332032
kfree(F2FS_OPTION(sbi).s_qf_names[i]);
@@ -4975,13 +4974,9 @@ static int f2fs_fill_super(struct super_block *sb, struct fs_context *fc)
49754974
if (err)
49764975
goto free_iostat;
49774976

4978-
/* init per sbi slab cache */
4979-
err = f2fs_init_xattr_caches(sbi);
4980-
if (err)
4981-
goto free_percpu;
49824977
err = f2fs_init_page_array_cache(sbi);
49834978
if (err)
4984-
goto free_xattr_cache;
4979+
goto free_percpu;
49854980

49864981
/* get an inode for meta space */
49874982
sbi->meta_inode = f2fs_iget(sb, F2FS_META_INO(sbi));
@@ -5310,8 +5305,6 @@ static int f2fs_fill_super(struct super_block *sb, struct fs_context *fc)
53105305
sbi->meta_inode = NULL;
53115306
free_page_array_cache:
53125307
f2fs_destroy_page_array_cache(sbi);
5313-
free_xattr_cache:
5314-
f2fs_destroy_xattr_caches(sbi);
53155308
free_percpu:
53165309
destroy_percpu_info(sbi);
53175310
free_iostat:
@@ -5514,10 +5507,15 @@ static int __init init_f2fs_fs(void)
55145507
err = f2fs_create_casefold_cache();
55155508
if (err)
55165509
goto free_compress_cache;
5517-
err = register_filesystem(&f2fs_fs_type);
5510+
err = f2fs_init_xattr_cache();
55185511
if (err)
55195512
goto free_casefold_cache;
5513+
err = register_filesystem(&f2fs_fs_type);
5514+
if (err)
5515+
goto free_xattr_cache;
55205516
return 0;
5517+
free_xattr_cache:
5518+
f2fs_destroy_xattr_cache();
55215519
free_casefold_cache:
55225520
f2fs_destroy_casefold_cache();
55235521
free_compress_cache:
@@ -5558,6 +5556,7 @@ static int __init init_f2fs_fs(void)
55585556
static void __exit exit_f2fs_fs(void)
55595557
{
55605558
unregister_filesystem(&f2fs_fs_type);
5559+
f2fs_destroy_xattr_cache();
55615560
f2fs_destroy_casefold_cache();
55625561
f2fs_destroy_compress_cache();
55635562
f2fs_destroy_compress_mempool();

fs/f2fs/xattr.c

Lines changed: 11 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,12 @@
2323
#include "xattr.h"
2424
#include "segment.h"
2525

26+
static struct kmem_cache *inline_xattr_slab;
2627
static void *xattr_alloc(struct f2fs_sb_info *sbi, int size, bool *is_inline)
2728
{
28-
if (likely(size == sbi->inline_xattr_slab_size)) {
29+
if (likely(size == DEFAULT_XATTR_SLAB_SIZE)) {
2930
*is_inline = true;
30-
return f2fs_kmem_cache_alloc(sbi->inline_xattr_slab,
31+
return f2fs_kmem_cache_alloc(inline_xattr_slab,
3132
GFP_F2FS_ZERO, false, sbi);
3233
}
3334
*is_inline = false;
@@ -38,7 +39,7 @@ static void xattr_free(struct f2fs_sb_info *sbi, void *xattr_addr,
3839
bool is_inline)
3940
{
4041
if (is_inline)
41-
kmem_cache_free(sbi->inline_xattr_slab, xattr_addr);
42+
kmem_cache_free(inline_xattr_slab, xattr_addr);
4243
else
4344
kfree(xattr_addr);
4445
}
@@ -830,25 +831,14 @@ int f2fs_setxattr(struct inode *inode, int index, const char *name,
830831
return err;
831832
}
832833

833-
int f2fs_init_xattr_caches(struct f2fs_sb_info *sbi)
834+
int __init f2fs_init_xattr_cache(void)
834835
{
835-
dev_t dev = sbi->sb->s_bdev->bd_dev;
836-
char slab_name[32];
837-
838-
sprintf(slab_name, "f2fs_xattr_entry-%u:%u", MAJOR(dev), MINOR(dev));
839-
840-
sbi->inline_xattr_slab_size = F2FS_OPTION(sbi).inline_xattr_size *
841-
sizeof(__le32) + XATTR_PADDING_SIZE;
842-
843-
sbi->inline_xattr_slab = f2fs_kmem_cache_create(slab_name,
844-
sbi->inline_xattr_slab_size);
845-
if (!sbi->inline_xattr_slab)
846-
return -ENOMEM;
847-
848-
return 0;
836+
inline_xattr_slab = f2fs_kmem_cache_create("f2fs_xattr_entry",
837+
DEFAULT_XATTR_SLAB_SIZE);
838+
return inline_xattr_slab ? 0 : -ENOMEM;
849839
}
850840

851-
void f2fs_destroy_xattr_caches(struct f2fs_sb_info *sbi)
841+
void f2fs_destroy_xattr_cache(void)
852842
{
853-
kmem_cache_destroy(sbi->inline_xattr_slab);
854-
}
843+
kmem_cache_destroy(inline_xattr_slab);
844+
}

fs/f2fs/xattr.h

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,8 @@ struct f2fs_xattr_entry {
8989
F2FS_TOTAL_EXTRA_ATTR_SIZE / sizeof(__le32) - \
9090
DEF_INLINE_RESERVED_SIZE - \
9191
MIN_INLINE_DENTRY_SIZE / sizeof(__le32))
92+
#define DEFAULT_XATTR_SLAB_SIZE (DEFAULT_INLINE_XATTR_ADDRS * \
93+
sizeof(__le32) + XATTR_PADDING_SIZE)
9294

9395
/*
9496
* On-disk structure of f2fs_xattr
@@ -132,8 +134,8 @@ int f2fs_setxattr(struct inode *, int, const char *, const void *,
132134
int f2fs_getxattr(struct inode *, int, const char *, void *,
133135
size_t, struct folio *);
134136
ssize_t f2fs_listxattr(struct dentry *, char *, size_t);
135-
int f2fs_init_xattr_caches(struct f2fs_sb_info *);
136-
void f2fs_destroy_xattr_caches(struct f2fs_sb_info *);
137+
int __init f2fs_init_xattr_cache(void);
138+
void f2fs_destroy_xattr_cache(void);
137139
#else
138140

139141
#define f2fs_xattr_handlers NULL
@@ -150,8 +152,8 @@ static inline int f2fs_getxattr(struct inode *inode, int index,
150152
{
151153
return -EOPNOTSUPP;
152154
}
153-
static inline int f2fs_init_xattr_caches(struct f2fs_sb_info *sbi) { return 0; }
154-
static inline void f2fs_destroy_xattr_caches(struct f2fs_sb_info *sbi) { }
155+
static inline int __init f2fs_init_xattr_cache(void) { return 0; }
156+
static inline void f2fs_destroy_xattr_cache(void) { }
155157
#endif
156158

157159
#ifdef CONFIG_F2FS_FS_SECURITY

0 commit comments

Comments
 (0)