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

From: Chengguang Xu <cgxu519@mykernel.net>
To: "Miklos Szeredi" <miklos@szeredi.hu>
Cc: "Jan Kara" <jack@suse.cz>, "Amir Goldstein" <amir73il@gmail.com>,
	"overlayfs" <linux-unionfs@vger.kernel.org>,
	"linux-fsdevel" <linux-fsdevel@vger.kernel.org>
Subject: Re: [RFC PATCH v4 4/9] ovl: mark overlayfs' inode dirty on modification
Date: Mon, 12 Apr 2021 19:58:27 +0800	[thread overview]
Message-ID: <178c5f281d9.dde5cb9920853.3545294445882801731@mykernel.net> (raw)
In-Reply-To: <CAJfpegtyUXcyiUG=YH1Hi06qwuYdtDL_kArQxN9mJUj7JJWZ0w@mail.gmail.com>

---- 在 星期五, 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
 > >
 > >
 > 