Implements the optimization noted in f75fdf22b0a8 ("fuse: don't use
->d_time"), as the additional memory can be significant. (In particular,
on SLAB configurations this 8-byte alloc becomes 32 bytes). Per-dentry,
this can consume significant memory.
Reviewed-by: Shakeel Butt <[email protected]>
Signed-off-by: Khazhismel Kumykov <[email protected]>
---
v3:
reapplied on fuse/for-next, droping the fuse_request_alloc refactor
it was already done :) (and account new per-file alloc)
fs/fuse/dir.c | 36 ++++++++++++++++++++++++++++++------
1 file changed, 30 insertions(+), 6 deletions(-)
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index ba0a175d7578..58557d4817e9 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -24,11 +24,34 @@ static void fuse_advise_use_readdirplus(struct inode *dir)
set_bit(FUSE_I_ADVISE_RDPLUS, &fi->state);
}
+#if BITS_PER_LONG >= 64
+static inline void __fuse_dentry_settime(struct dentry *entry, u64 time)
+{
+ entry->d_fsdata = (void *) time;
+}
+
+static inline u64 fuse_dentry_time(const struct dentry *entry)
+{
+ return (u64)entry->d_fsdata;
+}
+
+#else
union fuse_dentry {
u64 time;
struct rcu_head rcu;
};
+static inline void __fuse_dentry_settime(struct dentry *dentry, u64 time)
+{
+ ((union fuse_dentry *) dentry->d_fsdata)->time = time;
+}
+
+static inline u64 fuse_dentry_time(const struct dentry *entry)
+{
+ return ((union fuse_dentry *) entry->d_fsdata)->time;
+}
+#endif
+
static void fuse_dentry_settime(struct dentry *dentry, u64 time)
{
struct fuse_conn *fc = get_fuse_conn_super(dentry->d_sb);
@@ -47,12 +70,7 @@ static void fuse_dentry_settime(struct dentry *dentry, u64 time)
spin_unlock(&dentry->d_lock);
}
- ((union fuse_dentry *) dentry->d_fsdata)->time = time;
-}
-
-static inline u64 fuse_dentry_time(const struct dentry *entry)
-{
- return ((union fuse_dentry *) entry->d_fsdata)->time;
+ __fuse_dentry_settime(dentry, time);
}
/*
@@ -258,6 +276,7 @@ static int fuse_dentry_revalidate(struct dentry *entry, unsigned int flags)
goto out;
}
+#if BITS_PER_LONG < 64
static int fuse_dentry_init(struct dentry *dentry)
{
dentry->d_fsdata = kzalloc(sizeof(union fuse_dentry), GFP_KERNEL);
@@ -270,6 +289,7 @@ static void fuse_dentry_release(struct dentry *dentry)
kfree_rcu(fd, rcu);
}
+#endif
static int fuse_dentry_delete(const struct dentry *dentry)
{
@@ -279,13 +299,17 @@ static int fuse_dentry_delete(const struct dentry *dentry)
const struct dentry_operations fuse_dentry_operations = {
.d_revalidate = fuse_dentry_revalidate,
.d_delete = fuse_dentry_delete,
+#if BITS_PER_LONG < 64
.d_init = fuse_dentry_init,
.d_release = fuse_dentry_release,
+#endif
};
const struct dentry_operations fuse_root_dentry_operations = {
+#if BITS_PER_LONG < 64
.d_init = fuse_dentry_init,
.d_release = fuse_dentry_release,
+#endif
};
int fuse_valid_type(int m)
--
2.23.0.237.gc6a4ce50a0-goog
account per-file, dentry, and inode data
blockdev/superblock and temporary per-request data was left alone, as
this usually isn't accounted
Signed-off-by: Khazhismel Kumykov <[email protected]>
Reviewed-by: Shakeel Butt <[email protected]>
---
fs/fuse/dir.c | 3 ++-
fs/fuse/file.c | 5 +++--
fs/fuse/inode.c | 3 ++-
3 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 58557d4817e9..d572c900bb0f 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -279,7 +279,8 @@ static int fuse_dentry_revalidate(struct dentry *entry, unsigned int flags)
#if BITS_PER_LONG < 64
static int fuse_dentry_init(struct dentry *dentry)
{
- dentry->d_fsdata = kzalloc(sizeof(union fuse_dentry), GFP_KERNEL);
+ dentry->d_fsdata = kzalloc(sizeof(union fuse_dentry),
+ GFP_KERNEL_ACCOUNT | __GFP_RECLAIMABLE);
return dentry->d_fsdata ? 0 : -ENOMEM;
}
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index a2ea347c4d2c..862aff3665b5 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -63,12 +63,13 @@ struct fuse_file *fuse_file_alloc(struct fuse_conn *fc)
{
struct fuse_file *ff;
- ff = kzalloc(sizeof(struct fuse_file), GFP_KERNEL);
+ ff = kzalloc(sizeof(struct fuse_file), GFP_KERNEL_ACCOUNT);
if (unlikely(!ff))
return NULL;
ff->fc = fc;
- ff->release_args = kzalloc(sizeof(*ff->release_args), GFP_KERNEL);
+ ff->release_args = kzalloc(sizeof(*ff->release_args),
+ GFP_KERNEL_ACCOUNT);
if (!ff->release_args) {
kfree(ff);
return NULL;
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 3d598a5bb5b5..6cb445bed89d 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -66,7 +66,8 @@ static struct file_system_type fuseblk_fs_type;
struct fuse_forget_link *fuse_alloc_forget(void)
{
- return kzalloc(sizeof(struct fuse_forget_link), GFP_KERNEL);
+ return kzalloc(sizeof(struct fuse_forget_link),
+ GFP_KERNEL_ACCOUNT | __GFP_RECLAIMABLE);
}
static struct inode *fuse_alloc_inode(struct super_block *sb)
--
2.23.0.237.gc6a4ce50a0-goog
On Tue, Sep 17, 2019 at 1:56 AM Khazhismel Kumykov <[email protected]> wrote:
>
> Implements the optimization noted in f75fdf22b0a8 ("fuse: don't use
> ->d_time"), as the additional memory can be significant. (In particular,
> on SLAB configurations this 8-byte alloc becomes 32 bytes). Per-dentry,
> this can consume significant memory.
Applied, thanks.
Miklos
On Tue, Sep 17, 2019 at 1:56 AM Khazhismel Kumykov <[email protected]> wrote:
>
> account per-file, dentry, and inode data
>
> blockdev/superblock and temporary per-request data was left alone, as
> this usually isn't accounted
>
> Signed-off-by: Khazhismel Kumykov <[email protected]>
> Reviewed-by: Shakeel Butt <[email protected]>
> ---
> fs/fuse/dir.c | 3 ++-
> fs/fuse/file.c | 5 +++--
> fs/fuse/inode.c | 3 ++-
> 3 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> index 58557d4817e9..d572c900bb0f 100644
> --- a/fs/fuse/dir.c
> +++ b/fs/fuse/dir.c
> @@ -279,7 +279,8 @@ static int fuse_dentry_revalidate(struct dentry *entry, unsigned int flags)
> #if BITS_PER_LONG < 64
> static int fuse_dentry_init(struct dentry *dentry)
> {
> - dentry->d_fsdata = kzalloc(sizeof(union fuse_dentry), GFP_KERNEL);
> + dentry->d_fsdata = kzalloc(sizeof(union fuse_dentry),
> + GFP_KERNEL_ACCOUNT | __GFP_RECLAIMABLE);
>
> return dentry->d_fsdata ? 0 : -ENOMEM;
> }
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index a2ea347c4d2c..862aff3665b5 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -63,12 +63,13 @@ struct fuse_file *fuse_file_alloc(struct fuse_conn *fc)
> {
> struct fuse_file *ff;
>
> - ff = kzalloc(sizeof(struct fuse_file), GFP_KERNEL);
> + ff = kzalloc(sizeof(struct fuse_file), GFP_KERNEL_ACCOUNT);
> if (unlikely(!ff))
> return NULL;
>
> ff->fc = fc;
> - ff->release_args = kzalloc(sizeof(*ff->release_args), GFP_KERNEL);
> + ff->release_args = kzalloc(sizeof(*ff->release_args),
> + GFP_KERNEL_ACCOUNT);
> if (!ff->release_args) {
> kfree(ff);
> return NULL;
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index 3d598a5bb5b5..6cb445bed89d 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -66,7 +66,8 @@ static struct file_system_type fuseblk_fs_type;
>
> struct fuse_forget_link *fuse_alloc_forget(void)
> {
> - return kzalloc(sizeof(struct fuse_forget_link), GFP_KERNEL);
> + return kzalloc(sizeof(struct fuse_forget_link),
> + GFP_KERNEL_ACCOUNT | __GFP_RECLAIMABLE);
What does __GFP_RECLAIMBALE signify in slab allocs?
You understand that the forget_link is not reclaimable in the sense,
that it requires action (reading requests from the fuse device) from
the userspace filesystem daemon?
Thanks,
Miklos
On Tue, Sep 17, 2019 at 12:52 AM Miklos Szeredi <[email protected]> wrote:
>
> On Tue, Sep 17, 2019 at 1:56 AM Khazhismel Kumykov <[email protected]> wrote:
> > struct fuse_forget_link *fuse_alloc_forget(void)
> > {
> > - return kzalloc(sizeof(struct fuse_forget_link), GFP_KERNEL);
> > + return kzalloc(sizeof(struct fuse_forget_link),
> > + GFP_KERNEL_ACCOUNT | __GFP_RECLAIMABLE);
>
> What does __GFP_RECLAIMBALE signify in slab allocs?
>
Marking these allocations reclaimable hints to mm how much we can
reclaim overall by shrinking, so if it is reclaimable we should mark
it as such.
For d_fsdata, the lifetime is linked to the dentry, which is
reclaimable, so it makes sense here.
> You understand that the forget_link is not reclaimable in the sense,
> that it requires action (reading requests from the fuse device) from
> the userspace filesystem daemon?
>
Ah, I see, whenever we evict the fuse_inode, we queue a forget
command, which usually waits for userspace. So it's not actually
linked to the inode itself, and yeah, if we need userspace to
intervene we shouldn't treat forget_link as reclaimable. I had figured
since fuse_inode is reclaimable, this should be too, but missed that
disconnect, thanks.
account per-file, dentry, and inode data
blockdev/superblock and temporary per-request data was left alone, as
this usually isn't accounted
Reviewed-by: Shakeel Butt <[email protected]>
Signed-off-by: Khazhismel Kumykov <[email protected]>
---
fs/fuse/dir.c | 3 ++-
fs/fuse/file.c | 5 +++--
fs/fuse/inode.c | 2 +-
3 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 58557d4817e9..d572c900bb0f 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -279,7 +279,8 @@ static int fuse_dentry_revalidate(struct dentry *entry, unsigned int flags)
#if BITS_PER_LONG < 64
static int fuse_dentry_init(struct dentry *dentry)
{
- dentry->d_fsdata = kzalloc(sizeof(union fuse_dentry), GFP_KERNEL);
+ dentry->d_fsdata = kzalloc(sizeof(union fuse_dentry),
+ GFP_KERNEL_ACCOUNT | __GFP_RECLAIMABLE);
return dentry->d_fsdata ? 0 : -ENOMEM;
}
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 8c7578b95d2c..0f0225686aee 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -63,12 +63,13 @@ struct fuse_file *fuse_file_alloc(struct fuse_conn *fc)
{
struct fuse_file *ff;
- ff = kzalloc(sizeof(struct fuse_file), GFP_KERNEL);
+ ff = kzalloc(sizeof(struct fuse_file), GFP_KERNEL_ACCOUNT);
if (unlikely(!ff))
return NULL;
ff->fc = fc;
- ff->release_args = kzalloc(sizeof(*ff->release_args), GFP_KERNEL);
+ ff->release_args = kzalloc(sizeof(*ff->release_args),
+ GFP_KERNEL_ACCOUNT);
if (!ff->release_args) {
kfree(ff);
return NULL;
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 3d598a5bb5b5..e040e2a2b621 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -66,7 +66,7 @@ static struct file_system_type fuseblk_fs_type;
struct fuse_forget_link *fuse_alloc_forget(void)
{
- return kzalloc(sizeof(struct fuse_forget_link), GFP_KERNEL);
+ return kzalloc(sizeof(struct fuse_forget_link), GFP_KERNEL_ACCOUNT);
}
static struct inode *fuse_alloc_inode(struct super_block *sb)
--
2.23.0.237.gc6a4ce50a0-goog