diff mbox series

[RFC,v4,4/9] ovl: mark overlayfs' inode dirty on modification

Message ID 20201113065555.147276-5-cgxu519@mykernel.net (mailing list archive)
State New, archived
Headers show
Series implement containerized syncfs for overlayfs | expand

Commit Message

Chengguang Xu Nov. 13, 2020, 6:55 a.m. UTC
Mark overlayfs' inode dirty on modification so that
we can recognize target inodes during syncfs.

Signed-off-by: Chengguang Xu <cgxu519@mykernel.net>
---
 fs/overlayfs/inode.c     |  1 +
 fs/overlayfs/overlayfs.h |  4 ++++
 fs/overlayfs/util.c      | 14 ++++++++++++++
 3 files changed, 19 insertions(+)

Comments

Miklos Szeredi April 9, 2021, 1:45 p.m. UTC | #1
On Fri, Nov 13, 2020 at 7:57 AM Chengguang Xu <cgxu519@mykernel.net> wrote:
>
> Mark overlayfs' inode dirty on modification so that
> we can recognize target inodes during syncfs.
>
> Signed-off-by: Chengguang Xu <cgxu519@mykernel.net>
> ---
>  fs/overlayfs/inode.c     |  1 +
>  fs/overlayfs/overlayfs.h |  4 ++++
>  fs/overlayfs/util.c      | 14 ++++++++++++++
>  3 files changed, 19 insertions(+)
>
> diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
> index 8cfa75e86f56..342693657ab0 100644
> --- a/fs/overlayfs/inode.c
> +++ b/fs/overlayfs/inode.c
> @@ -468,6 +468,7 @@ int ovl_update_time(struct inode *inode, struct timespec64 *ts, int flags)
>                 if (upperpath.dentry) {
>                         touch_atime(&upperpath);
>                         inode->i_atime = d_inode(upperpath.dentry)->i_atime;
> +                       ovl_mark_inode_dirty(inode);
>                 }
>         }
>         return 0;
> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> index f8880aa2ba0e..eaf1d5b05d8e 100644
> --- a/fs/overlayfs/overlayfs.h
> +++ b/fs/overlayfs/overlayfs.h
> @@ -247,6 +247,7 @@ static inline bool ovl_open_flags_need_copy_up(int flags)
>  }
>
>  /* util.c */
> +void ovl_mark_inode_dirty(struct inode *inode);
>  int ovl_want_write(struct dentry *dentry);
>  void ovl_drop_write(struct dentry *dentry);
>  struct dentry *ovl_workdir(struct dentry *dentry);
> @@ -472,6 +473,9 @@ static inline void ovl_copyattr(struct inode *from, struct inode *to)
>         to->i_mtime = from->i_mtime;
>         to->i_ctime = from->i_ctime;
>         i_size_write(to, i_size_read(from));
> +
> +       if (ovl_inode_upper(to) && from->i_state & I_DIRTY_ALL)
> +               ovl_mark_inode_dirty(to);
>  }

Okay, ovl_copyattr() certainly seems a good place to copy dirtyness as well.

What I'm fearing is that it does not cover all the places where
underlying inode can be dirtied.  This really needs an audit of all
filesystem modifying operations.

>
>  static inline void ovl_copyflags(struct inode *from, struct inode *to)
> diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
> index 23f475627d07..a6f59df744ae 100644
> --- a/fs/overlayfs/util.c
> +++ b/fs/overlayfs/util.c
> @@ -950,3 +950,17 @@ char *ovl_get_redirect_xattr(struct ovl_fs *ofs, struct dentry *dentry,
>         kfree(buf);
>         return ERR_PTR(res);
>  }
> +
> +/*
> + * We intentionally add I_DIRTY_SYNC flag regardless dirty flag
> + * of upper inode so that we have chance to invoke ->write_inode
> + * to re-dirty overlayfs' inode during writeback process.
> + */
> +void ovl_mark_inode_dirty(struct inode *inode)
> +{
> +       struct inode *upper = ovl_inode_upper(inode);
> +       unsigned long iflag = I_DIRTY_SYNC;
> +
> +       iflag |= upper->i_state & I_DIRTY_ALL;
> +       __mark_inode_dirty(inode, iflag);
> +}
> --
> 2.26.2
>
>
Chengguang Xu April 12, 2021, 11:58 a.m. UTC | #2
---- 在 星期五, 2021-04-09 21:45:28 Miklos Szeredi <miklos@szeredi.hu> 撰写 ----
 > On Fri, Nov 13, 2020 at 7:57 AM Chengguang Xu <cgxu519@mykernel.net> wrote:
 > >
 > > Mark overlayfs' inode dirty on modification so that
 > > we can recognize target inodes during syncfs.
 > >
 > > Signed-off-by: Chengguang Xu <cgxu519@mykernel.net>
 > > ---
 > >  fs/overlayfs/inode.c     |  1 +
 > >  fs/overlayfs/overlayfs.h |  4 ++++
 > >  fs/overlayfs/util.c      | 14 ++++++++++++++
 > >  3 files changed, 19 insertions(+)
 > >
 > > diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
 > > index 8cfa75e86f56..342693657ab0 100644
 > > --- a/fs/overlayfs/inode.c
 > > +++ b/fs/overlayfs/inode.c
 > > @@ -468,6 +468,7 @@ int ovl_update_time(struct inode *inode, struct timespec64 *ts, int flags)
 > >                 if (upperpath.dentry) {
 > >                         touch_atime(&upperpath);
 > >                         inode->i_atime = d_inode(upperpath.dentry)->i_atime;
 > > +                       ovl_mark_inode_dirty(inode);
 > >                 }
 > >         }
 > >         return 0;
 > > diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
 > > index f8880aa2ba0e..eaf1d5b05d8e 100644
 > > --- a/fs/overlayfs/overlayfs.h
 > > +++ b/fs/overlayfs/overlayfs.h
 > > @@ -247,6 +247,7 @@ static inline bool ovl_open_flags_need_copy_up(int flags)
 > >  }
 > >
 > >  /* util.c */
 > > +void ovl_mark_inode_dirty(struct inode *inode);
 > >  int ovl_want_write(struct dentry *dentry);
 > >  void ovl_drop_write(struct dentry *dentry);
 > >  struct dentry *ovl_workdir(struct dentry *dentry);
 > > @@ -472,6 +473,9 @@ static inline void ovl_copyattr(struct inode *from, struct inode *to)
 > >         to->i_mtime = from->i_mtime;
 > >         to->i_ctime = from->i_ctime;
 > >         i_size_write(to, i_size_read(from));
 > > +
 > > +       if (ovl_inode_upper(to) && from->i_state & I_DIRTY_ALL)
 > > +               ovl_mark_inode_dirty(to);
 > >  }
 > 
 > Okay, ovl_copyattr() certainly seems a good place to copy dirtyness as well.
 > 
 > What I'm fearing is that it does not cover all the places where
 > underlying inode can be dirtied.  This really needs an audit of all
 > filesystem modifying operations.
 
You are right. Let me fix it in next version.

Thanks,
Chengguang


 > >
 > >  static inline void ovl_copyflags(struct inode *from, struct inode *to)
 > > diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
 > > index 23f475627d07..a6f59df744ae 100644
 > > --- a/fs/overlayfs/util.c
 > > +++ b/fs/overlayfs/util.c
 > > @@ -950,3 +950,17 @@ char *ovl_get_redirect_xattr(struct ovl_fs *ofs, struct dentry *dentry,
 > >         kfree(buf);
 > >         return ERR_PTR(res);
 > >  }
 > > +
 > > +/*
 > > + * We intentionally add I_DIRTY_SYNC flag regardless dirty flag
 > > + * of upper inode so that we have chance to invoke ->write_inode
 > > + * to re-dirty overlayfs' inode during writeback process.
 > > + */
 > > +void ovl_mark_inode_dirty(struct inode *inode)
 > > +{
 > > +       struct inode *upper = ovl_inode_upper(inode);
 > > +       unsigned long iflag = I_DIRTY_SYNC;
 > > +
 > > +       iflag |= upper->i_state & I_DIRTY_ALL;
 > > +       __mark_inode_dirty(inode, iflag);
 > > +}
 > > --
 > > 2.26.2
 > >
 > >
 >
diff mbox series

Patch

diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index 8cfa75e86f56..342693657ab0 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -468,6 +468,7 @@  int ovl_update_time(struct inode *inode, struct timespec64 *ts, int flags)
 		if (upperpath.dentry) {
 			touch_atime(&upperpath);
 			inode->i_atime = d_inode(upperpath.dentry)->i_atime;
+			ovl_mark_inode_dirty(inode);
 		}
 	}
 	return 0;
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index f8880aa2ba0e..eaf1d5b05d8e 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -247,6 +247,7 @@  static inline bool ovl_open_flags_need_copy_up(int flags)
 }
 
 /* util.c */
+void ovl_mark_inode_dirty(struct inode *inode);
 int ovl_want_write(struct dentry *dentry);
 void ovl_drop_write(struct dentry *dentry);
 struct dentry *ovl_workdir(struct dentry *dentry);
@@ -472,6 +473,9 @@  static inline void ovl_copyattr(struct inode *from, struct inode *to)
 	to->i_mtime = from->i_mtime;
 	to->i_ctime = from->i_ctime;
 	i_size_write(to, i_size_read(from));
+
+	if (ovl_inode_upper(to) && from->i_state & I_DIRTY_ALL)
+		ovl_mark_inode_dirty(to);
 }
 
 static inline void ovl_copyflags(struct inode *from, struct inode *to)
diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index 23f475627d07..a6f59df744ae 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -950,3 +950,17 @@  char *ovl_get_redirect_xattr(struct ovl_fs *ofs, struct dentry *dentry,
 	kfree(buf);
 	return ERR_PTR(res);
 }
+
+/*
+ * We intentionally add I_DIRTY_SYNC flag regardless dirty flag
+ * of upper inode so that we have chance to invoke ->write_inode
+ * to re-dirty overlayfs' inode during writeback process.
+ */
+void ovl_mark_inode_dirty(struct inode *inode)
+{
+	struct inode *upper = ovl_inode_upper(inode);
+	unsigned long iflag = I_DIRTY_SYNC;
+
+	iflag |= upper->i_state & I_DIRTY_ALL;
+	__mark_inode_dirty(inode, iflag);
+}