2021-11-22 03:16:52

by Chengguang Xu

[permalink] [raw]
Subject: [RFC PATCH V6 3/7] ovl: implement overlayfs' own ->write_inode operation

From: Chengguang Xu <[email protected]>

Sync dirty data and meta of upper inode in overlayfs' ->write_inode()
and redirty overlayfs' inode.

Signed-off-by: Chengguang Xu <[email protected]>
---
fs/overlayfs/super.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)

diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 18a12088a37b..12acf0ec7395 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -15,6 +15,7 @@
#include <linux/seq_file.h>
#include <linux/posix_acl_xattr.h>
#include <linux/exportfs.h>
+#include <linux/writeback.h>
#include "overlayfs.h"

MODULE_AUTHOR("Miklos Szeredi <[email protected]>");
@@ -406,12 +407,32 @@ static int ovl_remount(struct super_block *sb, int *flags, char *data)
return ret;
}

+static int ovl_write_inode(struct inode *inode,
+ struct writeback_control *wbc)
+{
+ struct ovl_fs *ofs = inode->i_sb->s_fs_info;
+ struct inode *upper_inode = ovl_inode_upper(inode);
+ int ret = 0;
+
+ if (!upper_inode)
+ return 0;
+
+ if (!ovl_should_sync(ofs))
+ return 0;
+
+ ret = write_inode_now(upper_inode, wbc->sync_mode == WB_SYNC_ALL);
+ mark_inode_dirty(inode);
+
+ return ret;
+}
+
static const struct super_operations ovl_super_operations = {
.alloc_inode = ovl_alloc_inode,
.free_inode = ovl_free_inode,
.destroy_inode = ovl_destroy_inode,
.drop_inode = generic_delete_inode,
.put_super = ovl_put_super,
+ .write_inode = ovl_write_inode,
.sync_fs = ovl_sync_fs,
.statfs = ovl_statfs,
.show_options = ovl_show_options,
--
2.27.0




2021-11-26 09:16:34

by Jan Kara

[permalink] [raw]
Subject: Re: [RFC PATCH V6 3/7] ovl: implement overlayfs' own ->write_inode operation

On Mon 22-11-21 11:00:34, Chengguang Xu wrote:
> From: Chengguang Xu <[email protected]>
>
> Sync dirty data and meta of upper inode in overlayfs' ->write_inode()
> and redirty overlayfs' inode.
>
> Signed-off-by: Chengguang Xu <[email protected]>

Looks good. I'm still not 100% convinced keeping inodes dirty all the time
will not fire back in excessive writeback activity (e.g. flush worker will
traverse the list of all dirty inodes every 30 seconds and try to write
them) but I guess we can start with this and if people complain, dirtiness
handling can be improved. So feel free to add:

Reviewed-by: Jan Kara <[email protected]>

Honza

> ---
> fs/overlayfs/super.c | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
>
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index 18a12088a37b..12acf0ec7395 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -15,6 +15,7 @@
> #include <linux/seq_file.h>
> #include <linux/posix_acl_xattr.h>
> #include <linux/exportfs.h>
> +#include <linux/writeback.h>
> #include "overlayfs.h"
>
> MODULE_AUTHOR("Miklos Szeredi <[email protected]>");
> @@ -406,12 +407,32 @@ static int ovl_remount(struct super_block *sb, int *flags, char *data)
> return ret;
> }
>
> +static int ovl_write_inode(struct inode *inode,
> + struct writeback_control *wbc)
> +{
> + struct ovl_fs *ofs = inode->i_sb->s_fs_info;
> + struct inode *upper_inode = ovl_inode_upper(inode);
> + int ret = 0;
> +
> + if (!upper_inode)
> + return 0;
> +
> + if (!ovl_should_sync(ofs))
> + return 0;
> +
> + ret = write_inode_now(upper_inode, wbc->sync_mode == WB_SYNC_ALL);
> + mark_inode_dirty(inode);
> +
> + return ret;
> +}
> +
> static const struct super_operations ovl_super_operations = {
> .alloc_inode = ovl_alloc_inode,
> .free_inode = ovl_free_inode,
> .destroy_inode = ovl_destroy_inode,
> .drop_inode = generic_delete_inode,
> .put_super = ovl_put_super,
> + .write_inode = ovl_write_inode,
> .sync_fs = ovl_sync_fs,
> .statfs = ovl_statfs,
> .show_options = ovl_show_options,
> --
> 2.27.0
>
>
--
Jan Kara <[email protected]>
SUSE Labs, CR

2021-11-26 13:33:41

by Chengguang Xu

[permalink] [raw]
Subject: Re: [RFC PATCH V6 3/7] ovl: implement overlayfs' own ->write_inode operation

---- 在 星期五, 2021-11-26 17:14:30 Jan Kara <[email protected]> 撰写 ----
> On Mon 22-11-21 11:00:34, Chengguang Xu wrote:
> > From: Chengguang Xu <[email protected]>
> >
> > Sync dirty data and meta of upper inode in overlayfs' ->write_inode()
> > and redirty overlayfs' inode.
> >
> > Signed-off-by: Chengguang Xu <[email protected]>
>
> Looks good. I'm still not 100% convinced keeping inodes dirty all the time
> will not fire back in excessive writeback activity (e.g. flush worker will
> traverse the list of all dirty inodes every 30 seconds and try to write
> them) but I guess we can start with this and if people complain, dirtiness
> handling can be improved.

Hi Jan,

Thanks for the review and suggestion.


Thanks,
Chengguang



> So feel free to add:
>
> Reviewed-by: Jan Kara <[email protected]>
>
> Honza
>
> > ---
> > fs/overlayfs/super.c | 21 +++++++++++++++++++++
> > 1 file changed, 21 insertions(+)
> >
> > diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> > index 18a12088a37b..12acf0ec7395 100644
> > --- a/fs/overlayfs/super.c
> > +++ b/fs/overlayfs/super.c
> > @@ -15,6 +15,7 @@
> > #include <linux/seq_file.h>
> > #include <linux/posix_acl_xattr.h>
> > #include <linux/exportfs.h>
> > +#include <linux/writeback.h>
> > #include "overlayfs.h"
> >
> > MODULE_AUTHOR("Miklos Szeredi <[email protected]>");
> > @@ -406,12 +407,32 @@ static int ovl_remount(struct super_block *sb, int *flags, char *data)
> > return ret;
> > }
> >
> > +static int ovl_write_inode(struct inode *inode,
> > + struct writeback_control *wbc)
> > +{
> > + struct ovl_fs *ofs = inode->i_sb->s_fs_info;
> > + struct inode *upper_inode = ovl_inode_upper(inode);
> > + int ret = 0;
> > +
> > + if (!upper_inode)
> > + return 0;
> > +
> > + if (!ovl_should_sync(ofs))
> > + return 0;
> > +
> > + ret = write_inode_now(upper_inode, wbc->sync_mode == WB_SYNC_ALL);
> > + mark_inode_dirty(inode);
> > +
> > + return ret;
> > +}
> > +
> > static const struct super_operations ovl_super_operations = {
> > .alloc_inode = ovl_alloc_inode,
> > .free_inode = ovl_free_inode,
> > .destroy_inode = ovl_destroy_inode,
> > .drop_inode = generic_delete_inode,
> > .put_super = ovl_put_super,
> > + .write_inode = ovl_write_inode,
> > .sync_fs = ovl_sync_fs,
> > .statfs = ovl_statfs,
> > .show_options = ovl_show_options,
> > --
> > 2.27.0
> >
> >
> --
> Jan Kara <[email protected]>
> SUSE Labs, CR
>