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

[Jan Kara <jack@suse.cz>]

On Tue 10-11-20 23:12:14, Chengguang Xu wrote:
>  ---- 在 星期二, 2020-11-10 21:45:51 Jan Kara <jack@suse.cz> 撰写 ----
>  > 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).
> 
> Hi Jan,
> 
> I not only checked upper inode state but also checked upper inode
> mmap(shared) state using  mapping_writably_mapped(upper->i_mapping).
> Maybe it's better to move i_state check after mmap check but isn't above
> checks enough for mmapped file? 

Ah, sorry, I'm blind! I missed the mapping_writably_mapped() check. Thanks
for explanation.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR