* [PATCH] f2fs: use kmem_cache pool during inline xattr lookups
@ 2020-02-25 1:42 Chao Yu
2020-02-25 6:37 ` Ju Hyung Park
2020-02-25 8:21 ` kbuild test robot
0 siblings, 2 replies; 4+ messages in thread
From: Chao Yu @ 2020-02-25 1:42 UTC (permalink / raw)
To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, chao, Chao Yu, Park Ju Hyung
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>
Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
Hi, Ju Hyung,
Let me know if you have any objection on this patch. :)
fs/f2fs/f2fs.h | 3 +++
fs/f2fs/super.c | 10 ++++++++-
fs/f2fs/xattr.c | 54 ++++++++++++++++++++++++++++++++++++++++++++-----
fs/f2fs/xattr.h | 4 ++++
4 files changed, 65 insertions(+), 6 deletions(-)
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 12a197e89a3e..23b93a116c73 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1486,6 +1486,9 @@ struct f2fs_sb_info {
__u32 s_chksum_seed;
struct workqueue_struct *post_read_wq; /* post read workqueue */
+
+ struct kmem_cache *inline_xattr_slab; /* inline xattr entry */
+ unsigned int inline_xattr_slab_size; /* default inline xattr slab size */
};
struct f2fs_private_dio {
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index d241e07c0bfa..0b16204d3b7d 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -1201,6 +1201,7 @@ static void f2fs_put_super(struct super_block *sb)
kvfree(sbi->raw_super);
destroy_device_list(sbi);
+ f2fs_destroy_xattr_caches(sbi);
mempool_destroy(sbi->write_io_dummy);
#ifdef CONFIG_QUOTA
for (i = 0; i < MAXQUOTAS; i++)
@@ -3457,12 +3458,17 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
}
}
+ /* init per sbi slab cache */
+ err = f2fs_init_xattr_caches(sbi);
+ if (err)
+ goto free_io_dummy;
+
/* get an inode for meta space */
sbi->meta_inode = f2fs_iget(sb, F2FS_META_INO(sbi));
if (IS_ERR(sbi->meta_inode)) {
f2fs_err(sbi, "Failed to read F2FS meta data inode");
err = PTR_ERR(sbi->meta_inode);
- goto free_io_dummy;
+ goto free_xattr_cache;
}
err = f2fs_get_valid_checkpoint(sbi);
@@ -3735,6 +3741,8 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
make_bad_inode(sbi->meta_inode);
iput(sbi->meta_inode);
sbi->meta_inode = NULL;
+free_xattr_cache:
+ f2fs_destroy_xattr_caches(sbi);
free_io_dummy:
mempool_destroy(sbi->write_io_dummy);
free_percpu:
diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
index a3360a97e624..e46a10eb0e42 100644
--- a/fs/f2fs/xattr.c
+++ b/fs/f2fs/xattr.c
@@ -23,6 +23,25 @@
#include "xattr.h"
#include "segment.h"
+static void *xattr_alloc(struct f2fs_sb_info *sbi, int size, bool *is_inline)
+{
+ *is_inline = (size == sbi->inline_xattr_slab_size);
+
+ if (*is_inline)
+ return kmem_cache_zalloc(sbi->inline_xattr_slab, GFP_NOFS);
+
+ return f2fs_kzalloc(sbi, size, GFP_NOFS);
+}
+
+static void xattr_free(struct f2fs_sb_info *sbi, void *xattr_addr,
+ bool is_inline)
+{
+ if (is_inline)
+ kmem_cache_free(sbi->inline_xattr_slab, xattr_addr);
+ else
+ kvfree(xattr_addr);
+}
+
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)
@@ -301,7 +320,8 @@ static int read_xattr_block(struct inode *inode, void *txattr_addr)
static int lookup_all_xattrs(struct inode *inode, struct page *ipage,
unsigned int index, unsigned int len,
const char *name, struct f2fs_xattr_entry **xe,
- void **base_addr, int *base_size)
+ void **base_addr, int *base_size,
+ bool *is_inline)
{
void *cur_addr, *txattr_addr, *last_txattr_addr;
void *last_addr = NULL;
@@ -313,7 +333,7 @@ static int lookup_all_xattrs(struct inode *inode, struct page *ipage,
return -ENODATA;
*base_size = XATTR_SIZE(inode) + XATTR_PADDING_SIZE;
- txattr_addr = f2fs_kzalloc(F2FS_I_SB(inode), *base_size, GFP_NOFS);
+ txattr_addr = xattr_alloc(F2FS_I_SB(inode), *base_size, is_inline);
if (!txattr_addr)
return -ENOMEM;
@@ -362,7 +382,7 @@ static int lookup_all_xattrs(struct inode *inode, struct page *ipage,
*base_addr = txattr_addr;
return 0;
out:
- kvfree(txattr_addr);
+ xattr_free(F2FS_I_SB(inode), txattr_addr, *is_inline);
return err;
}
@@ -499,6 +519,7 @@ int f2fs_getxattr(struct inode *inode, int index, const char *name,
unsigned int size, len;
void *base_addr = NULL;
int base_size;
+ bool is_inline;
if (name == NULL)
return -EINVAL;
@@ -509,7 +530,7 @@ int f2fs_getxattr(struct inode *inode, int index, const char *name,
down_read(&F2FS_I(inode)->i_xattr_sem);
error = lookup_all_xattrs(inode, ipage, index, len, name,
- &entry, &base_addr, &base_size);
+ &entry, &base_addr, &base_size, &is_inline);
up_read(&F2FS_I(inode)->i_xattr_sem);
if (error)
return error;
@@ -532,7 +553,7 @@ int f2fs_getxattr(struct inode *inode, int index, const char *name,
}
error = size;
out:
- kvfree(base_addr);
+ xattr_free(F2FS_I_SB(inode), base_addr, is_inline);
return error;
}
@@ -767,3 +788,26 @@ int f2fs_setxattr(struct inode *inode, int index, const char *name,
f2fs_update_time(sbi, REQ_TIME);
return err;
}
+
+int f2fs_init_xattr_caches(struct f2fs_sb_info *sbi)
+{
+ dev_t dev = sbi->sb->s_bdev->bd_dev;
+ char slab_name[32];
+
+ sprintf(slab_name, "f2fs_xattr_entry-%u:%u", MAJOR(dev), MINOR(dev));
+
+ sbi->inline_xattr_slab_size = F2FS_OPTION(sbi).inline_xattr_size *
+ sizeof(__le32) + XATTR_PADDING_SIZE;
+
+ sbi->inline_xattr_slab = f2fs_kmem_cache_create(slab_name,
+ sbi->inline_xattr_slab_size);
+ if (!sbi->inline_xattr_slab)
+ return -ENOMEM;
+
+ return 0;
+}
+
+void f2fs_destroy_xattr_caches(struct f2fs_sb_info *sbi)
+{
+ kmem_cache_destroy(sbi->inline_xattr_slab);
+}
diff --git a/fs/f2fs/xattr.h b/fs/f2fs/xattr.h
index 574beea46494..7345dfa7b7a9 100644
--- a/fs/f2fs/xattr.h
+++ b/fs/f2fs/xattr.h
@@ -131,6 +131,8 @@ extern int f2fs_setxattr(struct inode *, int, const char *,
extern int f2fs_getxattr(struct inode *, int, const char *, void *,
size_t, struct page *);
extern ssize_t f2fs_listxattr(struct dentry *, char *, size_t);
+extern int f2fs_init_xattr_caches(struct f2fs_sb_info *);
+extern void f2fs_destroy_xattr_caches(struct f2fs_sb_info *);
#else
#define f2fs_xattr_handlers NULL
@@ -151,6 +153,8 @@ static inline ssize_t f2fs_listxattr(struct dentry *dentry, char *buffer,
{
return -EOPNOTSUPP;
}
+static int f2fs_init_xattr_caches(struct f2fs_sb_info *sbi) { return 0; }
+static void f2fs_destroy_xattr_caches(struct f2fs_sb_info *sbi) { return 0; }
#endif
#ifdef CONFIG_F2FS_FS_SECURITY
--
2.18.0.rc1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] f2fs: use kmem_cache pool during inline xattr lookups
2020-02-25 1:42 [PATCH] f2fs: use kmem_cache pool during inline xattr lookups Chao Yu
@ 2020-02-25 6:37 ` Ju Hyung Park
2020-02-25 8:44 ` Chao Yu
2020-02-25 8:21 ` kbuild test robot
1 sibling, 1 reply; 4+ messages in thread
From: Ju Hyung Park @ 2020-02-25 6:37 UTC (permalink / raw)
To: Chao Yu; +Cc: Jaegeuk Kim, linux-f2fs-devel, linux-kernel, Chao Yu
Hi Chao.
Sorry for the late reply, I've been busy with my uni stuffs.
It's nice to see this issue getting some attentions back, but
unfortunately, I can't test it at the moment.
Here's the patch we used to see the alloc statistics:
https://pastebin.com/bhBh2dgs by Sultan Alsawaf.
Hope it helps.
On Tue, Feb 25, 2020 at 10:43 AM Chao Yu <yuchao0@huawei.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>
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
>
> Hi, Ju Hyung,
>
> Let me know if you have any objection on this patch. :)
>
> fs/f2fs/f2fs.h | 3 +++
> fs/f2fs/super.c | 10 ++++++++-
> fs/f2fs/xattr.c | 54 ++++++++++++++++++++++++++++++++++++++++++++-----
> fs/f2fs/xattr.h | 4 ++++
> 4 files changed, 65 insertions(+), 6 deletions(-)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 12a197e89a3e..23b93a116c73 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -1486,6 +1486,9 @@ struct f2fs_sb_info {
> __u32 s_chksum_seed;
>
> struct workqueue_struct *post_read_wq; /* post read workqueue */
> +
> + struct kmem_cache *inline_xattr_slab; /* inline xattr entry */
> + unsigned int inline_xattr_slab_size; /* default inline xattr slab size */
> };
>
> struct f2fs_private_dio {
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index d241e07c0bfa..0b16204d3b7d 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -1201,6 +1201,7 @@ static void f2fs_put_super(struct super_block *sb)
> kvfree(sbi->raw_super);
>
> destroy_device_list(sbi);
> + f2fs_destroy_xattr_caches(sbi);
> mempool_destroy(sbi->write_io_dummy);
> #ifdef CONFIG_QUOTA
> for (i = 0; i < MAXQUOTAS; i++)
> @@ -3457,12 +3458,17 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
> }
> }
>
> + /* init per sbi slab cache */
> + err = f2fs_init_xattr_caches(sbi);
> + if (err)
> + goto free_io_dummy;
> +
> /* get an inode for meta space */
> sbi->meta_inode = f2fs_iget(sb, F2FS_META_INO(sbi));
> if (IS_ERR(sbi->meta_inode)) {
> f2fs_err(sbi, "Failed to read F2FS meta data inode");
> err = PTR_ERR(sbi->meta_inode);
> - goto free_io_dummy;
> + goto free_xattr_cache;
> }
>
> err = f2fs_get_valid_checkpoint(sbi);
> @@ -3735,6 +3741,8 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
> make_bad_inode(sbi->meta_inode);
> iput(sbi->meta_inode);
> sbi->meta_inode = NULL;
> +free_xattr_cache:
> + f2fs_destroy_xattr_caches(sbi);
> free_io_dummy:
> mempool_destroy(sbi->write_io_dummy);
> free_percpu:
> diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
> index a3360a97e624..e46a10eb0e42 100644
> --- a/fs/f2fs/xattr.c
> +++ b/fs/f2fs/xattr.c
> @@ -23,6 +23,25 @@
> #include "xattr.h"
> #include "segment.h"
>
> +static void *xattr_alloc(struct f2fs_sb_info *sbi, int size, bool *is_inline)
> +{
> + *is_inline = (size == sbi->inline_xattr_slab_size);
> +
> + if (*is_inline)
> + return kmem_cache_zalloc(sbi->inline_xattr_slab, GFP_NOFS);
> +
> + return f2fs_kzalloc(sbi, size, GFP_NOFS);
> +}
> +
> +static void xattr_free(struct f2fs_sb_info *sbi, void *xattr_addr,
> + bool is_inline)
> +{
> + if (is_inline)
> + kmem_cache_free(sbi->inline_xattr_slab, xattr_addr);
> + else
> + kvfree(xattr_addr);
> +}
> +
> 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)
> @@ -301,7 +320,8 @@ static int read_xattr_block(struct inode *inode, void *txattr_addr)
> static int lookup_all_xattrs(struct inode *inode, struct page *ipage,
> unsigned int index, unsigned int len,
> const char *name, struct f2fs_xattr_entry **xe,
> - void **base_addr, int *base_size)
> + void **base_addr, int *base_size,
> + bool *is_inline)
> {
> void *cur_addr, *txattr_addr, *last_txattr_addr;
> void *last_addr = NULL;
> @@ -313,7 +333,7 @@ static int lookup_all_xattrs(struct inode *inode, struct page *ipage,
> return -ENODATA;
>
> *base_size = XATTR_SIZE(inode) + XATTR_PADDING_SIZE;
> - txattr_addr = f2fs_kzalloc(F2FS_I_SB(inode), *base_size, GFP_NOFS);
> + txattr_addr = xattr_alloc(F2FS_I_SB(inode), *base_size, is_inline);
> if (!txattr_addr)
> return -ENOMEM;
>
> @@ -362,7 +382,7 @@ static int lookup_all_xattrs(struct inode *inode, struct page *ipage,
> *base_addr = txattr_addr;
> return 0;
> out:
> - kvfree(txattr_addr);
> + xattr_free(F2FS_I_SB(inode), txattr_addr, *is_inline);
> return err;
> }
>
> @@ -499,6 +519,7 @@ int f2fs_getxattr(struct inode *inode, int index, const char *name,
> unsigned int size, len;
> void *base_addr = NULL;
> int base_size;
> + bool is_inline;
>
> if (name == NULL)
> return -EINVAL;
> @@ -509,7 +530,7 @@ int f2fs_getxattr(struct inode *inode, int index, const char *name,
>
> down_read(&F2FS_I(inode)->i_xattr_sem);
> error = lookup_all_xattrs(inode, ipage, index, len, name,
> - &entry, &base_addr, &base_size);
> + &entry, &base_addr, &base_size, &is_inline);
> up_read(&F2FS_I(inode)->i_xattr_sem);
> if (error)
> return error;
> @@ -532,7 +553,7 @@ int f2fs_getxattr(struct inode *inode, int index, const char *name,
> }
> error = size;
> out:
> - kvfree(base_addr);
> + xattr_free(F2FS_I_SB(inode), base_addr, is_inline);
> return error;
> }
>
> @@ -767,3 +788,26 @@ int f2fs_setxattr(struct inode *inode, int index, const char *name,
> f2fs_update_time(sbi, REQ_TIME);
> return err;
> }
> +
> +int f2fs_init_xattr_caches(struct f2fs_sb_info *sbi)
> +{
> + dev_t dev = sbi->sb->s_bdev->bd_dev;
> + char slab_name[32];
> +
> + sprintf(slab_name, "f2fs_xattr_entry-%u:%u", MAJOR(dev), MINOR(dev));
> +
> + sbi->inline_xattr_slab_size = F2FS_OPTION(sbi).inline_xattr_size *
> + sizeof(__le32) + XATTR_PADDING_SIZE;
> +
> + sbi->inline_xattr_slab = f2fs_kmem_cache_create(slab_name,
> + sbi->inline_xattr_slab_size);
> + if (!sbi->inline_xattr_slab)
> + return -ENOMEM;
> +
> + return 0;
> +}
> +
> +void f2fs_destroy_xattr_caches(struct f2fs_sb_info *sbi)
> +{
> + kmem_cache_destroy(sbi->inline_xattr_slab);
> +}
> diff --git a/fs/f2fs/xattr.h b/fs/f2fs/xattr.h
> index 574beea46494..7345dfa7b7a9 100644
> --- a/fs/f2fs/xattr.h
> +++ b/fs/f2fs/xattr.h
> @@ -131,6 +131,8 @@ extern int f2fs_setxattr(struct inode *, int, const char *,
> extern int f2fs_getxattr(struct inode *, int, const char *, void *,
> size_t, struct page *);
> extern ssize_t f2fs_listxattr(struct dentry *, char *, size_t);
> +extern int f2fs_init_xattr_caches(struct f2fs_sb_info *);
> +extern void f2fs_destroy_xattr_caches(struct f2fs_sb_info *);
> #else
>
> #define f2fs_xattr_handlers NULL
> @@ -151,6 +153,8 @@ static inline ssize_t f2fs_listxattr(struct dentry *dentry, char *buffer,
> {
> return -EOPNOTSUPP;
> }
> +static int f2fs_init_xattr_caches(struct f2fs_sb_info *sbi) { return 0; }
> +static void f2fs_destroy_xattr_caches(struct f2fs_sb_info *sbi) { return 0; }
> #endif
>
> #ifdef CONFIG_F2FS_FS_SECURITY
> --
> 2.18.0.rc1
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] f2fs: use kmem_cache pool during inline xattr lookups
2020-02-25 1:42 [PATCH] f2fs: use kmem_cache pool during inline xattr lookups Chao Yu
2020-02-25 6:37 ` Ju Hyung Park
@ 2020-02-25 8:21 ` kbuild test robot
1 sibling, 0 replies; 4+ messages in thread
From: kbuild test robot @ 2020-02-25 8:21 UTC (permalink / raw)
To: Chao Yu, Chao Yu
Cc: kbuild-all, jaegeuk, linux-f2fs-devel, linux-kernel, chao,
Chao Yu, Park Ju Hyung
[-- Attachment #1: Type: text/plain, Size: 3279 bytes --]
Hi Chao,
I love your patch! Perhaps something to improve:
[auto build test WARNING on f2fs/dev-test]
[cannot apply to v5.6-rc3 next-20200224]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Chao-Yu/f2fs-use-kmem_cache-pool-during-inline-xattr-lookups/20200225-094422
base: https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git dev-test
config: x86_64-randconfig-s2-20200225 (attached as .config)
compiler: gcc-7 (Debian 7.5.0-5) 7.5.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
In file included from fs/f2fs/dir.c:15:0:
fs/f2fs/xattr.h: In function 'f2fs_destroy_xattr_caches':
>> fs/f2fs/xattr.h:157:74: warning: 'return' with a value, in function returning void
static void f2fs_destroy_xattr_caches(struct f2fs_sb_info *sbi) { return 0; }
^
fs/f2fs/xattr.h:157:13: note: declared here
static void f2fs_destroy_xattr_caches(struct f2fs_sb_info *sbi) { return 0; }
^~~~~~~~~~~~~~~~~~~~~~~~~
At top level:
fs/f2fs/xattr.h:157:13: warning: 'f2fs_destroy_xattr_caches' defined but not used [-Wunused-function]
fs/f2fs/xattr.h:156:12: warning: 'f2fs_init_xattr_caches' defined but not used [-Wunused-function]
static int f2fs_init_xattr_caches(struct f2fs_sb_info *sbi) { return 0; }
^~~~~~~~~~~~~~~~~~~~~~
--
In file included from fs/f2fs/super.c:31:0:
fs/f2fs/xattr.h: In function 'f2fs_destroy_xattr_caches':
>> fs/f2fs/xattr.h:157:74: warning: 'return' with a value, in function returning void
static void f2fs_destroy_xattr_caches(struct f2fs_sb_info *sbi) { return 0; }
^
fs/f2fs/xattr.h:157:13: note: declared here
static void f2fs_destroy_xattr_caches(struct f2fs_sb_info *sbi) { return 0; }
^~~~~~~~~~~~~~~~~~~~~~~~~
vim +/return +157 fs/f2fs/xattr.h
137
138 #define f2fs_xattr_handlers NULL
139 static inline int f2fs_setxattr(struct inode *inode, int index,
140 const char *name, const void *value, size_t size,
141 struct page *page, int flags)
142 {
143 return -EOPNOTSUPP;
144 }
145 static inline int f2fs_getxattr(struct inode *inode, int index,
146 const char *name, void *buffer,
147 size_t buffer_size, struct page *dpage)
148 {
149 return -EOPNOTSUPP;
150 }
151 static inline ssize_t f2fs_listxattr(struct dentry *dentry, char *buffer,
152 size_t buffer_size)
153 {
154 return -EOPNOTSUPP;
155 }
156 static int f2fs_init_xattr_caches(struct f2fs_sb_info *sbi) { return 0; }
> 157 static void f2fs_destroy_xattr_caches(struct f2fs_sb_info *sbi) { return 0; }
158 #endif
159
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 38836 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] f2fs: use kmem_cache pool during inline xattr lookups
2020-02-25 6:37 ` Ju Hyung Park
@ 2020-02-25 8:44 ` Chao Yu
0 siblings, 0 replies; 4+ messages in thread
From: Chao Yu @ 2020-02-25 8:44 UTC (permalink / raw)
To: Ju Hyung Park; +Cc: Jaegeuk Kim, linux-f2fs-devel, linux-kernel, Chao Yu
Hi Ju Hyung,
Thanks for the reply.
On 2020/2/25 14:37, Ju Hyung Park wrote:
> Hi Chao.
>
> Sorry for the late reply, I've been busy with my uni stuffs.
>
> It's nice to see this issue getting some attentions back, but
> unfortunately, I can't test it at the moment.
>
> Here's the patch we used to see the alloc statistics:
> https://pastebin.com/bhBh2dgs by Sultan Alsawaf.
Thanks for providing statistic patch, let me check this latter.
Thanks,
>
> Hope it helps.
>
> On Tue, Feb 25, 2020 at 10:43 AM Chao Yu <yuchao0@huawei.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>
>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>> ---
>>
>> Hi, Ju Hyung,
>>
>> Let me know if you have any objection on this patch. :)
>>
>> fs/f2fs/f2fs.h | 3 +++
>> fs/f2fs/super.c | 10 ++++++++-
>> fs/f2fs/xattr.c | 54 ++++++++++++++++++++++++++++++++++++++++++++-----
>> fs/f2fs/xattr.h | 4 ++++
>> 4 files changed, 65 insertions(+), 6 deletions(-)
>>
>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>> index 12a197e89a3e..23b93a116c73 100644
>> --- a/fs/f2fs/f2fs.h
>> +++ b/fs/f2fs/f2fs.h
>> @@ -1486,6 +1486,9 @@ struct f2fs_sb_info {
>> __u32 s_chksum_seed;
>>
>> struct workqueue_struct *post_read_wq; /* post read workqueue */
>> +
>> + struct kmem_cache *inline_xattr_slab; /* inline xattr entry */
>> + unsigned int inline_xattr_slab_size; /* default inline xattr slab size */
>> };
>>
>> struct f2fs_private_dio {
>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>> index d241e07c0bfa..0b16204d3b7d 100644
>> --- a/fs/f2fs/super.c
>> +++ b/fs/f2fs/super.c
>> @@ -1201,6 +1201,7 @@ static void f2fs_put_super(struct super_block *sb)
>> kvfree(sbi->raw_super);
>>
>> destroy_device_list(sbi);
>> + f2fs_destroy_xattr_caches(sbi);
>> mempool_destroy(sbi->write_io_dummy);
>> #ifdef CONFIG_QUOTA
>> for (i = 0; i < MAXQUOTAS; i++)
>> @@ -3457,12 +3458,17 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
>> }
>> }
>>
>> + /* init per sbi slab cache */
>> + err = f2fs_init_xattr_caches(sbi);
>> + if (err)
>> + goto free_io_dummy;
>> +
>> /* get an inode for meta space */
>> sbi->meta_inode = f2fs_iget(sb, F2FS_META_INO(sbi));
>> if (IS_ERR(sbi->meta_inode)) {
>> f2fs_err(sbi, "Failed to read F2FS meta data inode");
>> err = PTR_ERR(sbi->meta_inode);
>> - goto free_io_dummy;
>> + goto free_xattr_cache;
>> }
>>
>> err = f2fs_get_valid_checkpoint(sbi);
>> @@ -3735,6 +3741,8 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
>> make_bad_inode(sbi->meta_inode);
>> iput(sbi->meta_inode);
>> sbi->meta_inode = NULL;
>> +free_xattr_cache:
>> + f2fs_destroy_xattr_caches(sbi);
>> free_io_dummy:
>> mempool_destroy(sbi->write_io_dummy);
>> free_percpu:
>> diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
>> index a3360a97e624..e46a10eb0e42 100644
>> --- a/fs/f2fs/xattr.c
>> +++ b/fs/f2fs/xattr.c
>> @@ -23,6 +23,25 @@
>> #include "xattr.h"
>> #include "segment.h"
>>
>> +static void *xattr_alloc(struct f2fs_sb_info *sbi, int size, bool *is_inline)
>> +{
>> + *is_inline = (size == sbi->inline_xattr_slab_size);
>> +
>> + if (*is_inline)
>> + return kmem_cache_zalloc(sbi->inline_xattr_slab, GFP_NOFS);
>> +
>> + return f2fs_kzalloc(sbi, size, GFP_NOFS);
>> +}
>> +
>> +static void xattr_free(struct f2fs_sb_info *sbi, void *xattr_addr,
>> + bool is_inline)
>> +{
>> + if (is_inline)
>> + kmem_cache_free(sbi->inline_xattr_slab, xattr_addr);
>> + else
>> + kvfree(xattr_addr);
>> +}
>> +
>> 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)
>> @@ -301,7 +320,8 @@ static int read_xattr_block(struct inode *inode, void *txattr_addr)
>> static int lookup_all_xattrs(struct inode *inode, struct page *ipage,
>> unsigned int index, unsigned int len,
>> const char *name, struct f2fs_xattr_entry **xe,
>> - void **base_addr, int *base_size)
>> + void **base_addr, int *base_size,
>> + bool *is_inline)
>> {
>> void *cur_addr, *txattr_addr, *last_txattr_addr;
>> void *last_addr = NULL;
>> @@ -313,7 +333,7 @@ static int lookup_all_xattrs(struct inode *inode, struct page *ipage,
>> return -ENODATA;
>>
>> *base_size = XATTR_SIZE(inode) + XATTR_PADDING_SIZE;
>> - txattr_addr = f2fs_kzalloc(F2FS_I_SB(inode), *base_size, GFP_NOFS);
>> + txattr_addr = xattr_alloc(F2FS_I_SB(inode), *base_size, is_inline);
>> if (!txattr_addr)
>> return -ENOMEM;
>>
>> @@ -362,7 +382,7 @@ static int lookup_all_xattrs(struct inode *inode, struct page *ipage,
>> *base_addr = txattr_addr;
>> return 0;
>> out:
>> - kvfree(txattr_addr);
>> + xattr_free(F2FS_I_SB(inode), txattr_addr, *is_inline);
>> return err;
>> }
>>
>> @@ -499,6 +519,7 @@ int f2fs_getxattr(struct inode *inode, int index, const char *name,
>> unsigned int size, len;
>> void *base_addr = NULL;
>> int base_size;
>> + bool is_inline;
>>
>> if (name == NULL)
>> return -EINVAL;
>> @@ -509,7 +530,7 @@ int f2fs_getxattr(struct inode *inode, int index, const char *name,
>>
>> down_read(&F2FS_I(inode)->i_xattr_sem);
>> error = lookup_all_xattrs(inode, ipage, index, len, name,
>> - &entry, &base_addr, &base_size);
>> + &entry, &base_addr, &base_size, &is_inline);
>> up_read(&F2FS_I(inode)->i_xattr_sem);
>> if (error)
>> return error;
>> @@ -532,7 +553,7 @@ int f2fs_getxattr(struct inode *inode, int index, const char *name,
>> }
>> error = size;
>> out:
>> - kvfree(base_addr);
>> + xattr_free(F2FS_I_SB(inode), base_addr, is_inline);
>> return error;
>> }
>>
>> @@ -767,3 +788,26 @@ int f2fs_setxattr(struct inode *inode, int index, const char *name,
>> f2fs_update_time(sbi, REQ_TIME);
>> return err;
>> }
>> +
>> +int f2fs_init_xattr_caches(struct f2fs_sb_info *sbi)
>> +{
>> + dev_t dev = sbi->sb->s_bdev->bd_dev;
>> + char slab_name[32];
>> +
>> + sprintf(slab_name, "f2fs_xattr_entry-%u:%u", MAJOR(dev), MINOR(dev));
>> +
>> + sbi->inline_xattr_slab_size = F2FS_OPTION(sbi).inline_xattr_size *
>> + sizeof(__le32) + XATTR_PADDING_SIZE;
>> +
>> + sbi->inline_xattr_slab = f2fs_kmem_cache_create(slab_name,
>> + sbi->inline_xattr_slab_size);
>> + if (!sbi->inline_xattr_slab)
>> + return -ENOMEM;
>> +
>> + return 0;
>> +}
>> +
>> +void f2fs_destroy_xattr_caches(struct f2fs_sb_info *sbi)
>> +{
>> + kmem_cache_destroy(sbi->inline_xattr_slab);
>> +}
>> diff --git a/fs/f2fs/xattr.h b/fs/f2fs/xattr.h
>> index 574beea46494..7345dfa7b7a9 100644
>> --- a/fs/f2fs/xattr.h
>> +++ b/fs/f2fs/xattr.h
>> @@ -131,6 +131,8 @@ extern int f2fs_setxattr(struct inode *, int, const char *,
>> extern int f2fs_getxattr(struct inode *, int, const char *, void *,
>> size_t, struct page *);
>> extern ssize_t f2fs_listxattr(struct dentry *, char *, size_t);
>> +extern int f2fs_init_xattr_caches(struct f2fs_sb_info *);
>> +extern void f2fs_destroy_xattr_caches(struct f2fs_sb_info *);
>> #else
>>
>> #define f2fs_xattr_handlers NULL
>> @@ -151,6 +153,8 @@ static inline ssize_t f2fs_listxattr(struct dentry *dentry, char *buffer,
>> {
>> return -EOPNOTSUPP;
>> }
>> +static int f2fs_init_xattr_caches(struct f2fs_sb_info *sbi) { return 0; }
>> +static void f2fs_destroy_xattr_caches(struct f2fs_sb_info *sbi) { return 0; }
>> #endif
>>
>> #ifdef CONFIG_F2FS_FS_SECURITY
>> --
>> 2.18.0.rc1
>>
> .
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-02-25 8:45 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-25 1:42 [PATCH] f2fs: use kmem_cache pool during inline xattr lookups Chao Yu
2020-02-25 6:37 ` Ju Hyung Park
2020-02-25 8:44 ` Chao Yu
2020-02-25 8:21 ` kbuild test robot
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).