Re: [RFC PATCH v3 07/10] ovl: implement overlayfs' ->write_inode operation

[Amir Goldstein <amir73il@gmail.com>]

On Tue, Nov 10, 2020 at 3:45 PM Jan Kara <jack@suse.cz> wrote:
>
> On Sun 08-11-20 22:03:04, Chengguang Xu wrote:
> > +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 = ovl_inode_upper(inode);
> > +     unsigned long iflag = 0;
> > +     int ret = 0;
> > +
> > +     if (!upper)
> > +             return 0;
> > +
> > +     if (!ovl_should_sync(ofs))
> > +             return 0;
> > +
> > +     if (upper->i_sb->s_op->write_inode)
> > +             ret = upper->i_sb->s_op->write_inode(inode, wbc);
> > +
> > +     iflag |= upper->i_state & I_DIRTY_ALL;
> > +
> > +     if (mapping_writably_mapped(upper->i_mapping) ||
> > +         mapping_tagged(upper->i_mapping, PAGECACHE_TAG_WRITEBACK))
> > +             iflag |= I_DIRTY_PAGES;
> > +
> > +     if (iflag)
> > +             ovl_mark_inode_dirty(inode);
>
> I think you didn't incorporate feedback we were speaking about in the last
> version of the series. May comment in [1] still applies - you can miss
> inodes dirtied through mmap when you decide to clean the inode here. So
> IMHO you need something like:
>
>         if (inode_is_open_for_write(inode))
>                 ovl_mark_inode_dirty(inode);
>
> here to keep inode dirty while it is open for write (and not based on upper
> inode state which is unreliable).
>

Just to be clear, as long as the ovl inode is open for write, the upper inode
is also open for write via the realfile reference, but not the other
way around -
after open(); mmap(); close() of the overlay file, the ovl inode is not
open for write, but the upper inode is, because ovl_mmap() maps the
realfile and upper inode without taking any reference on the ovl file/inode.

Hence the check for mapping_writably_mapped(upper->i_mapping)
above.

Thanks,
Amir.