It's been observed that kzalloc() on lookup_all_xattrs() are called millions of times on Android, quickly becoming the top abuser of slub memory allocator. Use a dedicated kmem cache pool for xattr lookups to mitigate this. Signed-off-by: Park Ju Hyung <qkrwngud825@gmail.com> --- fs/f2fs/f2fs.h | 6 ++++++ fs/f2fs/super.c | 8 +++++++- fs/f2fs/xattr.c | 33 ++++++++++++++++++++++++--------- 3 files changed, 37 insertions(+), 10 deletions(-) diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index 9c6388253c9d2..3046ca2ebd121 100644 --- a/fs/f2fs/f2fs.h +++ b/fs/f2fs/f2fs.h @@ -3510,6 +3510,12 @@ void f2fs_exit_sysfs(void); int f2fs_register_sysfs(struct f2fs_sb_info *sbi); void f2fs_unregister_sysfs(struct f2fs_sb_info *sbi); +/* + * xattr.c + */ +int __init f2fs_init_xattr_caches(void); +void f2fs_destroy_xattr_caches(void); + /* * crypto support */ diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c index 6d262d13251cf..abb59d9e25848 100644 --- a/fs/f2fs/super.c +++ b/fs/f2fs/super.c @@ -3614,9 +3614,12 @@ static int __init init_f2fs_fs(void) err = init_inodecache(); if (err) goto fail; - err = f2fs_create_node_manager_caches(); + err = f2fs_init_xattr_caches(); if (err) goto free_inodecache; + err = f2fs_create_node_manager_caches(); + if (err) + goto fail_xattr_caches; err = f2fs_create_segment_manager_caches(); if (err) goto free_node_manager_caches; @@ -3656,6 +3659,8 @@ static int __init init_f2fs_fs(void) f2fs_destroy_segment_manager_caches(); free_node_manager_caches: f2fs_destroy_node_manager_caches(); +fail_xattr_caches: + f2fs_destroy_xattr_caches(); free_inodecache: destroy_inodecache(); fail: @@ -3673,6 +3678,7 @@ static void __exit exit_f2fs_fs(void) f2fs_destroy_checkpoint_caches(); f2fs_destroy_segment_manager_caches(); f2fs_destroy_node_manager_caches(); + f2fs_destroy_xattr_caches(); destroy_inodecache(); f2fs_destroy_trace_ios(); } diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c index e791741d193b8..635b50ea3e5e8 100644 --- a/fs/f2fs/xattr.c +++ b/fs/f2fs/xattr.c @@ -22,6 +22,23 @@ #include "f2fs.h" #include "xattr.h" +static struct kmem_cache *f2fs_xattr_cachep; + +int __init f2fs_init_xattr_caches(void) +{ + f2fs_xattr_cachep = f2fs_kmem_cache_create("xattr_entry", + VALID_XATTR_BLOCK_SIZE + XATTR_PADDING_SIZE); + if (!f2fs_xattr_cachep) + return -ENOMEM; + + return 0; +} + +void f2fs_destroy_xattr_caches(void) +{ + kmem_cache_destroy(f2fs_xattr_cachep); +} + static int f2fs_xattr_generic_get(const struct xattr_handler *handler, struct dentry *unused, struct inode *inode, const char *name, void *buffer, size_t size) @@ -312,7 +329,7 @@ static int lookup_all_xattrs(struct inode *inode, struct page *ipage, return -ENODATA; *base_size = XATTR_SIZE(xnid, inode) + XATTR_PADDING_SIZE; - txattr_addr = f2fs_kzalloc(F2FS_I_SB(inode), *base_size, GFP_NOFS); + txattr_addr = kmem_cache_zalloc(f2fs_xattr_cachep, GFP_NOFS); if (!txattr_addr) return -ENOMEM; @@ -358,7 +375,7 @@ static int lookup_all_xattrs(struct inode *inode, struct page *ipage, *base_addr = txattr_addr; return 0; out: - kvfree(txattr_addr); + kmem_cache_free(f2fs_xattr_cachep, txattr_addr); return err; } @@ -367,13 +384,11 @@ static int read_all_xattrs(struct inode *inode, struct page *ipage, { struct f2fs_xattr_header *header; nid_t xnid = F2FS_I(inode)->i_xattr_nid; - unsigned int size = VALID_XATTR_BLOCK_SIZE; unsigned int inline_size = inline_xattr_size(inode); void *txattr_addr; int err; - txattr_addr = f2fs_kzalloc(F2FS_I_SB(inode), - inline_size + size + XATTR_PADDING_SIZE, GFP_NOFS); + txattr_addr = kmem_cache_zalloc(f2fs_xattr_cachep, GFP_NOFS); if (!txattr_addr) return -ENOMEM; @@ -401,7 +416,7 @@ static int read_all_xattrs(struct inode *inode, struct page *ipage, *base_addr = txattr_addr; return 0; fail: - kvfree(txattr_addr); + kmem_cache_free(f2fs_xattr_cachep, txattr_addr); return err; } @@ -528,7 +543,7 @@ int f2fs_getxattr(struct inode *inode, int index, const char *name, } error = size; out: - kvfree(base_addr); + kmem_cache_free(f2fs_xattr_cachep, base_addr); return error; } @@ -574,7 +589,7 @@ ssize_t f2fs_listxattr(struct dentry *dentry, char *buffer, size_t buffer_size) } error = buffer_size - rest; cleanup: - kvfree(base_addr); + kmem_cache_free(f2fs_xattr_cachep, base_addr); return error; } @@ -712,7 +727,7 @@ static int __f2fs_setxattr(struct inode *inode, int index, if (!error && S_ISDIR(inode->i_mode)) set_sbi_flag(F2FS_I_SB(inode), SBI_NEED_CP); exit: - kvfree(base_addr); + kmem_cache_free(f2fs_xattr_cachep, base_addr); return error; } -- 2.21.0 _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Hi everyone. This is a RFC patch. This patch introduces an even bigger problem, which is forcing all xattr lookup memory allocations to be made in 4076B, when in reality, 4076B allocations are only made during initial mounts and the rests are made in 204B, unnecessarily wasting memory. In my testing, 4076B allocations are only done 4 times during mount and the rests(millions) are in 204B. I'd like to ask the maintainers to suggest some bright ideas on how to tackle this correctly. (e.g. Use kmem pool only for 204B allocations and fallback to regular kzalloc() if (*base_size != 204)?) Thanks. On Fri, Jul 12, 2019 at 12:06 AM Park Ju Hyung <qkrwngud825@gmail.com> wrote: > > It's been observed that kzalloc() on lookup_all_xattrs() are called millions > of times on Android, quickly becoming the top abuser of slub memory allocator. > > Use a dedicated kmem cache pool for xattr lookups to mitigate this. > > Signed-off-by: Park Ju Hyung <qkrwngud825@gmail.com> > --- > fs/f2fs/f2fs.h | 6 ++++++ > fs/f2fs/super.c | 8 +++++++- > fs/f2fs/xattr.c | 33 ++++++++++++++++++++++++--------- > 3 files changed, 37 insertions(+), 10 deletions(-) > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > index 9c6388253c9d2..3046ca2ebd121 100644 > --- a/fs/f2fs/f2fs.h > +++ b/fs/f2fs/f2fs.h > @@ -3510,6 +3510,12 @@ void f2fs_exit_sysfs(void); > int f2fs_register_sysfs(struct f2fs_sb_info *sbi); > void f2fs_unregister_sysfs(struct f2fs_sb_info *sbi); > > +/* > + * xattr.c > + */ > +int __init f2fs_init_xattr_caches(void); > +void f2fs_destroy_xattr_caches(void); > + > /* > * crypto support > */ > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c > index 6d262d13251cf..abb59d9e25848 100644 > --- a/fs/f2fs/super.c > +++ b/fs/f2fs/super.c > @@ -3614,9 +3614,12 @@ static int __init init_f2fs_fs(void) > err = init_inodecache(); > if (err) > goto fail; > - err = f2fs_create_node_manager_caches(); > + err = f2fs_init_xattr_caches(); > if (err) > goto free_inodecache; > + err = f2fs_create_node_manager_caches(); > + if (err) > + goto fail_xattr_caches; > err = f2fs_create_segment_manager_caches(); > if (err) > goto free_node_manager_caches; > @@ -3656,6 +3659,8 @@ static int __init init_f2fs_fs(void) > f2fs_destroy_segment_manager_caches(); > free_node_manager_caches: > f2fs_destroy_node_manager_caches(); > +fail_xattr_caches: > + f2fs_destroy_xattr_caches(); > free_inodecache: > destroy_inodecache(); > fail: > @@ -3673,6 +3678,7 @@ static void __exit exit_f2fs_fs(void) > f2fs_destroy_checkpoint_caches(); > f2fs_destroy_segment_manager_caches(); > f2fs_destroy_node_manager_caches(); > + f2fs_destroy_xattr_caches(); > destroy_inodecache(); > f2fs_destroy_trace_ios(); > } > diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c > index e791741d193b8..635b50ea3e5e8 100644 > --- a/fs/f2fs/xattr.c > +++ b/fs/f2fs/xattr.c > @@ -22,6 +22,23 @@ > #include "f2fs.h" > #include "xattr.h" > > +static struct kmem_cache *f2fs_xattr_cachep; > + > +int __init f2fs_init_xattr_caches(void) > +{ > + f2fs_xattr_cachep = f2fs_kmem_cache_create("xattr_entry", > + VALID_XATTR_BLOCK_SIZE + XATTR_PADDING_SIZE); > + if (!f2fs_xattr_cachep) > + return -ENOMEM; > + > + return 0; > +} > + > +void f2fs_destroy_xattr_caches(void) > +{ > + kmem_cache_destroy(f2fs_xattr_cachep); > +} > + > static int f2fs_xattr_generic_get(const struct xattr_handler *handler, > struct dentry *unused, struct inode *inode, > const char *name, void *buffer, size_t size) > @@ -312,7 +329,7 @@ static int lookup_all_xattrs(struct inode *inode, struct page *ipage, > return -ENODATA; > > *base_size = XATTR_SIZE(xnid, inode) + XATTR_PADDING_SIZE; > - txattr_addr = f2fs_kzalloc(F2FS_I_SB(inode), *base_size, GFP_NOFS); > + txattr_addr = kmem_cache_zalloc(f2fs_xattr_cachep, GFP_NOFS); > if (!txattr_addr) > return -ENOMEM; > > @@ -358,7 +375,7 @@ static int lookup_all_xattrs(struct inode *inode, struct page *ipage, > *base_addr = txattr_addr; > return 0; > out: > - kvfree(txattr_addr); > + kmem_cache_free(f2fs_xattr_cachep, txattr_addr); > return err; > } > > @@ -367,13 +384,11 @@ static int read_all_xattrs(struct inode *inode, struct page *ipage, > { > struct f2fs_xattr_header *header; > nid_t xnid = F2FS_I(inode)->i_xattr_nid; > - unsigned int size = VALID_XATTR_BLOCK_SIZE; > unsigned int inline_size = inline_xattr_size(inode); > void *txattr_addr; > int err; > > - txattr_addr = f2fs_kzalloc(F2FS_I_SB(inode), > - inline_size + size + XATTR_PADDING_SIZE, GFP_NOFS); > + txattr_addr = kmem_cache_zalloc(f2fs_xattr_cachep, GFP_NOFS); > if (!txattr_addr) > return -ENOMEM; > > @@ -401,7 +416,7 @@ static int read_all_xattrs(struct inode *inode, struct page *ipage, > *base_addr = txattr_addr; > return 0; > fail: > - kvfree(txattr_addr); > + kmem_cache_free(f2fs_xattr_cachep, txattr_addr); > return err; > } > > @@ -528,7 +543,7 @@ int f2fs_getxattr(struct inode *inode, int index, const char *name, > } > error = size; > out: > - kvfree(base_addr); > + kmem_cache_free(f2fs_xattr_cachep, base_addr); > return error; > } > > @@ -574,7 +589,7 @@ ssize_t f2fs_listxattr(struct dentry *dentry, char *buffer, size_t buffer_size) > } > error = buffer_size - rest; > cleanup: > - kvfree(base_addr); > + kmem_cache_free(f2fs_xattr_cachep, base_addr); > return error; > } > > @@ -712,7 +727,7 @@ static int __f2fs_setxattr(struct inode *inode, int index, > if (!error && S_ISDIR(inode->i_mode)) > set_sbi_flag(F2FS_I_SB(inode), SBI_NEED_CP); > exit: > - kvfree(base_addr); > + kmem_cache_free(f2fs_xattr_cachep, base_addr); > return error; > } > > -- > 2.21.0 > _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
On 07/12, Ju Hyung Park wrote: > Hi everyone. > > This is a RFC patch. > > This patch introduces an even bigger problem, which is forcing all > xattr lookup memory allocations to be made in 4076B, when in reality, > 4076B allocations are only made during initial mounts and the rests > are made in 204B, unnecessarily wasting memory. > > In my testing, 4076B allocations are only done 4 times during mount > and the rests(millions) are in 204B. > > I'd like to ask the maintainers to suggest some bright ideas on how to > tackle this correctly. > (e.g. Use kmem pool only for 204B allocations and fallback to regular > kzalloc() if (*base_size != 204)?) How about adding two paths? One is kzalloc() for normal case, and the other is slab alloc for inline_xattr case? > > Thanks. > > On Fri, Jul 12, 2019 at 12:06 AM Park Ju Hyung <qkrwngud825@gmail.com> wrote: > > > > It's been observed that kzalloc() on lookup_all_xattrs() are called millions > > of times on Android, quickly becoming the top abuser of slub memory allocator. > > > > Use a dedicated kmem cache pool for xattr lookups to mitigate this. > > > > Signed-off-by: Park Ju Hyung <qkrwngud825@gmail.com> > > --- > > fs/f2fs/f2fs.h | 6 ++++++ > > fs/f2fs/super.c | 8 +++++++- > > fs/f2fs/xattr.c | 33 ++++++++++++++++++++++++--------- > > 3 files changed, 37 insertions(+), 10 deletions(-) > > > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > > index 9c6388253c9d2..3046ca2ebd121 100644 > > --- a/fs/f2fs/f2fs.h > > +++ b/fs/f2fs/f2fs.h > > @@ -3510,6 +3510,12 @@ void f2fs_exit_sysfs(void); > > int f2fs_register_sysfs(struct f2fs_sb_info *sbi); > > void f2fs_unregister_sysfs(struct f2fs_sb_info *sbi); > > > > +/* > > + * xattr.c > > + */ > > +int __init f2fs_init_xattr_caches(void); > > +void f2fs_destroy_xattr_caches(void); > > + > > /* > > * crypto support > > */ > > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c > > index 6d262d13251cf..abb59d9e25848 100644 > > --- a/fs/f2fs/super.c > > +++ b/fs/f2fs/super.c > > @@ -3614,9 +3614,12 @@ static int __init init_f2fs_fs(void) > > err = init_inodecache(); > > if (err) > > goto fail; > > - err = f2fs_create_node_manager_caches(); > > + err = f2fs_init_xattr_caches(); > > if (err) > > goto free_inodecache; > > + err = f2fs_create_node_manager_caches(); > > + if (err) > > + goto fail_xattr_caches; > > err = f2fs_create_segment_manager_caches(); > > if (err) > > goto free_node_manager_caches; > > @@ -3656,6 +3659,8 @@ static int __init init_f2fs_fs(void) > > f2fs_destroy_segment_manager_caches(); > > free_node_manager_caches: > > f2fs_destroy_node_manager_caches(); > > +fail_xattr_caches: > > + f2fs_destroy_xattr_caches(); > > free_inodecache: > > destroy_inodecache(); > > fail: > > @@ -3673,6 +3678,7 @@ static void __exit exit_f2fs_fs(void) > > f2fs_destroy_checkpoint_caches(); > > f2fs_destroy_segment_manager_caches(); > > f2fs_destroy_node_manager_caches(); > > + f2fs_destroy_xattr_caches(); > > destroy_inodecache(); > > f2fs_destroy_trace_ios(); > > } > > diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c > > index e791741d193b8..635b50ea3e5e8 100644 > > --- a/fs/f2fs/xattr.c > > +++ b/fs/f2fs/xattr.c > > @@ -22,6 +22,23 @@ > > #include "f2fs.h" > > #include "xattr.h" > > > > +static struct kmem_cache *f2fs_xattr_cachep; > > + > > +int __init f2fs_init_xattr_caches(void) > > +{ > > + f2fs_xattr_cachep = f2fs_kmem_cache_create("xattr_entry", > > + VALID_XATTR_BLOCK_SIZE + XATTR_PADDING_SIZE); > > + if (!f2fs_xattr_cachep) > > + return -ENOMEM; > > + > > + return 0; > > +} > > + > > +void f2fs_destroy_xattr_caches(void) > > +{ > > + kmem_cache_destroy(f2fs_xattr_cachep); > > +} > > + > > static int f2fs_xattr_generic_get(const struct xattr_handler *handler, > > struct dentry *unused, struct inode *inode, > > const char *name, void *buffer, size_t size) > > @@ -312,7 +329,7 @@ static int lookup_all_xattrs(struct inode *inode, struct page *ipage, > > return -ENODATA; > > > > *base_size = XATTR_SIZE(xnid, inode) + XATTR_PADDING_SIZE; > > - txattr_addr = f2fs_kzalloc(F2FS_I_SB(inode), *base_size, GFP_NOFS); > > + txattr_addr = kmem_cache_zalloc(f2fs_xattr_cachep, GFP_NOFS); > > if (!txattr_addr) > > return -ENOMEM; > > > > @@ -358,7 +375,7 @@ static int lookup_all_xattrs(struct inode *inode, struct page *ipage, > > *base_addr = txattr_addr; > > return 0; > > out: > > - kvfree(txattr_addr); > > + kmem_cache_free(f2fs_xattr_cachep, txattr_addr); > > return err; > > } > > > > @@ -367,13 +384,11 @@ static int read_all_xattrs(struct inode *inode, struct page *ipage, > > { > > struct f2fs_xattr_header *header; > > nid_t xnid = F2FS_I(inode)->i_xattr_nid; > > - unsigned int size = VALID_XATTR_BLOCK_SIZE; > > unsigned int inline_size = inline_xattr_size(inode); > > void *txattr_addr; > > int err; > > > > - txattr_addr = f2fs_kzalloc(F2FS_I_SB(inode), > > - inline_size + size + XATTR_PADDING_SIZE, GFP_NOFS); > > + txattr_addr = kmem_cache_zalloc(f2fs_xattr_cachep, GFP_NOFS); > > if (!txattr_addr) > > return -ENOMEM; > > > > @@ -401,7 +416,7 @@ static int read_all_xattrs(struct inode *inode, struct page *ipage, > > *base_addr = txattr_addr; > > return 0; > > fail: > > - kvfree(txattr_addr); > > + kmem_cache_free(f2fs_xattr_cachep, txattr_addr); > > return err; > > } > > > > @@ -528,7 +543,7 @@ int f2fs_getxattr(struct inode *inode, int index, const char *name, > > } > > error = size; > > out: > > - kvfree(base_addr); > > + kmem_cache_free(f2fs_xattr_cachep, base_addr); > > return error; > > } > > > > @@ -574,7 +589,7 @@ ssize_t f2fs_listxattr(struct dentry *dentry, char *buffer, size_t buffer_size) > > } > > error = buffer_size - rest; > > cleanup: > > - kvfree(base_addr); > > + kmem_cache_free(f2fs_xattr_cachep, base_addr); > > return error; > > } > > > > @@ -712,7 +727,7 @@ static int __f2fs_setxattr(struct inode *inode, int index, > > if (!error && S_ISDIR(inode->i_mode)) > > set_sbi_flag(F2FS_I_SB(inode), SBI_NEED_CP); > > exit: > > - kvfree(base_addr); > > + kmem_cache_free(f2fs_xattr_cachep, base_addr); > > return error; > > } > > > > -- > > 2.21.0 > > > > > _______________________________________________ > Linux-f2fs-devel mailing list > Linux-f2fs-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
On 2019/7/12 1:06, Jaegeuk Kim wrote: > On 07/12, Ju Hyung Park wrote: >> Hi everyone. >> >> This is a RFC patch. >> >> This patch introduces an even bigger problem, which is forcing all >> xattr lookup memory allocations to be made in 4076B, when in reality, >> 4076B allocations are only made during initial mounts and the rests >> are made in 204B, unnecessarily wasting memory. >> >> In my testing, 4076B allocations are only done 4 times during mount >> and the rests(millions) are in 204B. >> >> I'd like to ask the maintainers to suggest some bright ideas on how to >> tackle this correctly. >> (e.g. Use kmem pool only for 204B allocations and fallback to regular >> kzalloc() if (*base_size != 204)?) > > How about adding two paths? One is kzalloc() for normal case, and the other > is slab alloc for inline_xattr case? Agreed, Something like this: if (!xnid) addr = f2fs_kmem_cache_alloc(); else addr = f2fs_kzalloc(); .... if (!xnid) kmem_cache_free(, addr); else kvfree(addr); Thanks, > >> >> Thanks. >> >> On Fri, Jul 12, 2019 at 12:06 AM Park Ju Hyung <qkrwngud825@gmail.com> wrote: >>> >>> It's been observed that kzalloc() on lookup_all_xattrs() are called millions >>> of times on Android, quickly becoming the top abuser of slub memory allocator. >>> >>> Use a dedicated kmem cache pool for xattr lookups to mitigate this. >>> >>> Signed-off-by: Park Ju Hyung <qkrwngud825@gmail.com> >>> --- >>> fs/f2fs/f2fs.h | 6 ++++++ >>> fs/f2fs/super.c | 8 +++++++- >>> fs/f2fs/xattr.c | 33 ++++++++++++++++++++++++--------- >>> 3 files changed, 37 insertions(+), 10 deletions(-) >>> >>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h >>> index 9c6388253c9d2..3046ca2ebd121 100644 >>> --- a/fs/f2fs/f2fs.h >>> +++ b/fs/f2fs/f2fs.h >>> @@ -3510,6 +3510,12 @@ void f2fs_exit_sysfs(void); >>> int f2fs_register_sysfs(struct f2fs_sb_info *sbi); >>> void f2fs_unregister_sysfs(struct f2fs_sb_info *sbi); >>> >>> +/* >>> + * xattr.c >>> + */ >>> +int __init f2fs_init_xattr_caches(void); >>> +void f2fs_destroy_xattr_caches(void); >>> + >>> /* >>> * crypto support >>> */ >>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c >>> index 6d262d13251cf..abb59d9e25848 100644 >>> --- a/fs/f2fs/super.c >>> +++ b/fs/f2fs/super.c >>> @@ -3614,9 +3614,12 @@ static int __init init_f2fs_fs(void) >>> err = init_inodecache(); >>> if (err) >>> goto fail; >>> - err = f2fs_create_node_manager_caches(); >>> + err = f2fs_init_xattr_caches(); >>> if (err) >>> goto free_inodecache; >>> + err = f2fs_create_node_manager_caches(); >>> + if (err) >>> + goto fail_xattr_caches; >>> err = f2fs_create_segment_manager_caches(); >>> if (err) >>> goto free_node_manager_caches; >>> @@ -3656,6 +3659,8 @@ static int __init init_f2fs_fs(void) >>> f2fs_destroy_segment_manager_caches(); >>> free_node_manager_caches: >>> f2fs_destroy_node_manager_caches(); >>> +fail_xattr_caches: >>> + f2fs_destroy_xattr_caches(); >>> free_inodecache: >>> destroy_inodecache(); >>> fail: >>> @@ -3673,6 +3678,7 @@ static void __exit exit_f2fs_fs(void) >>> f2fs_destroy_checkpoint_caches(); >>> f2fs_destroy_segment_manager_caches(); >>> f2fs_destroy_node_manager_caches(); >>> + f2fs_destroy_xattr_caches(); >>> destroy_inodecache(); >>> f2fs_destroy_trace_ios(); >>> } >>> diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c >>> index e791741d193b8..635b50ea3e5e8 100644 >>> --- a/fs/f2fs/xattr.c >>> +++ b/fs/f2fs/xattr.c >>> @@ -22,6 +22,23 @@ >>> #include "f2fs.h" >>> #include "xattr.h" >>> >>> +static struct kmem_cache *f2fs_xattr_cachep; >>> + >>> +int __init f2fs_init_xattr_caches(void) >>> +{ >>> + f2fs_xattr_cachep = f2fs_kmem_cache_create("xattr_entry", >>> + VALID_XATTR_BLOCK_SIZE + XATTR_PADDING_SIZE); >>> + if (!f2fs_xattr_cachep) >>> + return -ENOMEM; >>> + >>> + return 0; >>> +} >>> + >>> +void f2fs_destroy_xattr_caches(void) >>> +{ >>> + kmem_cache_destroy(f2fs_xattr_cachep); >>> +} >>> + >>> static int f2fs_xattr_generic_get(const struct xattr_handler *handler, >>> struct dentry *unused, struct inode *inode, >>> const char *name, void *buffer, size_t size) >>> @@ -312,7 +329,7 @@ static int lookup_all_xattrs(struct inode *inode, struct page *ipage, >>> return -ENODATA; >>> >>> *base_size = XATTR_SIZE(xnid, inode) + XATTR_PADDING_SIZE; >>> - txattr_addr = f2fs_kzalloc(F2FS_I_SB(inode), *base_size, GFP_NOFS); >>> + txattr_addr = kmem_cache_zalloc(f2fs_xattr_cachep, GFP_NOFS); >>> if (!txattr_addr) >>> return -ENOMEM; >>> >>> @@ -358,7 +375,7 @@ static int lookup_all_xattrs(struct inode *inode, struct page *ipage, >>> *base_addr = txattr_addr; >>> return 0; >>> out: >>> - kvfree(txattr_addr); >>> + kmem_cache_free(f2fs_xattr_cachep, txattr_addr); >>> return err; >>> } >>> >>> @@ -367,13 +384,11 @@ static int read_all_xattrs(struct inode *inode, struct page *ipage, >>> { >>> struct f2fs_xattr_header *header; >>> nid_t xnid = F2FS_I(inode)->i_xattr_nid; >>> - unsigned int size = VALID_XATTR_BLOCK_SIZE; >>> unsigned int inline_size = inline_xattr_size(inode); >>> void *txattr_addr; >>> int err; >>> >>> - txattr_addr = f2fs_kzalloc(F2FS_I_SB(inode), >>> - inline_size + size + XATTR_PADDING_SIZE, GFP_NOFS); >>> + txattr_addr = kmem_cache_zalloc(f2fs_xattr_cachep, GFP_NOFS); >>> if (!txattr_addr) >>> return -ENOMEM; >>> >>> @@ -401,7 +416,7 @@ static int read_all_xattrs(struct inode *inode, struct page *ipage, >>> *base_addr = txattr_addr; >>> return 0; >>> fail: >>> - kvfree(txattr_addr); >>> + kmem_cache_free(f2fs_xattr_cachep, txattr_addr); >>> return err; >>> } >>> >>> @@ -528,7 +543,7 @@ int f2fs_getxattr(struct inode *inode, int index, const char *name, >>> } >>> error = size; >>> out: >>> - kvfree(base_addr); >>> + kmem_cache_free(f2fs_xattr_cachep, base_addr); >>> return error; >>> } >>> >>> @@ -574,7 +589,7 @@ ssize_t f2fs_listxattr(struct dentry *dentry, char *buffer, size_t buffer_size) >>> } >>> error = buffer_size - rest; >>> cleanup: >>> - kvfree(base_addr); >>> + kmem_cache_free(f2fs_xattr_cachep, base_addr); >>> return error; >>> } >>> >>> @@ -712,7 +727,7 @@ static int __f2fs_setxattr(struct inode *inode, int index, >>> if (!error && S_ISDIR(inode->i_mode)) >>> set_sbi_flag(F2FS_I_SB(inode), SBI_NEED_CP); >>> exit: >>> - kvfree(base_addr); >>> + kmem_cache_free(f2fs_xattr_cachep, base_addr); >>> return error; >>> } >>> >>> -- >>> 2.21.0 >>> >> >> >> _______________________________________________ >> Linux-f2fs-devel mailing list >> Linux-f2fs-devel@lists.sourceforge.net >> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel > > > _______________________________________________ > Linux-f2fs-devel mailing list > Linux-f2fs-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel > . > _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Hi Jaegeuk, Ju Hyung, Yaroslav, I can see "f2fs: xattr: reserve cache for xattr allocations" has been merged in dev-test branch, however, it doesn't exist in f2fs mailing list, so I can not comment on it.... Can anyone send it to the list? Thanks, _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Hi Chao and Jaegeuk, I have no idea how that patch got merged. We(me and Yaro) were supposed to work on doing some finishing touches to the patch before sending it to upstream. I'll personally check with Yaro. Jaegeuk, please remove the patch. That patch has numerous issues, biggest one being hardcoded size(SZ_256). Also, I need to figure out how to allocate kmem cache per mounts. Thanks. On Mon, Jul 29, 2019 at 4:28 PM Chao Yu <yuchao0@huawei.com> wrote: > > Hi Jaegeuk, Ju Hyung, Yaroslav, > > I can see "f2fs: xattr: reserve cache for xattr allocations" has been merged in > dev-test branch, however, it doesn't exist in f2fs mailing list, so I can not > comment on it.... Can anyone send it to the list? > > Thanks, _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Hi Ju Hyung, On 2019/7/29 15:48, Ju Hyung Park wrote: > Hi Chao and Jaegeuk, > > I have no idea how that patch got merged. > > We(me and Yaro) were supposed to work on doing some finishing touches > to the patch before sending it to upstream. > > I'll personally check with Yaro. > > Jaegeuk, please remove the patch. > That patch has numerous issues, biggest one being hardcoded size(SZ_256). > > Also, I need to figure out how to allocate kmem cache per mounts. That would need a per sbi kmem cache, I guess it's reasonable and worth when we mount multiple f2fs partitions with different inline xattr size. However, IIRC, inline_xattr_size can be changed after remount, so how can we handle different inline xattr size in one mount... that's also a problem. BTW, in old patch I found kvfree() should not be replaced as f2fs_kzalloc() can use vmalloc to allocate memroy. - kvfree(txattr_addr); + if (!inline_size) + kfree(txattr_addr); Thanks, > > Thanks. > > On Mon, Jul 29, 2019 at 4:28 PM Chao Yu <yuchao0@huawei.com> wrote: >> >> Hi Jaegeuk, Ju Hyung, Yaroslav, >> >> I can see "f2fs: xattr: reserve cache for xattr allocations" has been merged in >> dev-test branch, however, it doesn't exist in f2fs mailing list, so I can not >> comment on it.... Can anyone send it to the list? >> >> Thanks, > . > _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
On 07/29, Ju Hyung Park wrote: > Hi Chao and Jaegeuk, > > I have no idea how that patch got merged. > > We(me and Yaro) were supposed to work on doing some finishing touches > to the patch before sending it to upstream. > > I'll personally check with Yaro. > > Jaegeuk, please remove the patch. > That patch has numerous issues, biggest one being hardcoded size(SZ_256). That branch is for testing purpose and kinda TODO list for me. :) Anyway, I'll remove it, so please post it later with better shape. Thanks, > > Also, I need to figure out how to allocate kmem cache per mounts. > > Thanks. > > On Mon, Jul 29, 2019 at 4:28 PM Chao Yu <yuchao0@huawei.com> wrote: > > > > Hi Jaegeuk, Ju Hyung, Yaroslav, > > > > I can see "f2fs: xattr: reserve cache for xattr allocations" has been merged in > > dev-test branch, however, it doesn't exist in f2fs mailing list, so I can not > > comment on it.... Can anyone send it to the list? > > > > Thanks, _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel