Re: [PATCH 15/42] fat: Sync and invalidate metadata buffers from fat_evict_inode()

[Jan Kara <jack@suse.cz>]

On Sun 29-03-26 22:55:09, OGAWA Hirofumi wrote:
> Jan Kara <jack@suse.cz> writes:
> > There are only very few filesystems using generic metadata buffer head
> > tracking and everybody is paying the overhead. When we remove this
> > tracking for inode reclaim code .evict will start to see inodes with
> > metadata buffers attached so write them out and prune them.
> >
> > Signed-off-by: Jan Kara <jack@suse.cz>
> > ---
> >  fs/fat/inode.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/fat/inode.c b/fs/fat/inode.c
> > index 3cc5fb01afa1..ce88602b0d57 100644
> > --- a/fs/fat/inode.c
> > +++ b/fs/fat/inode.c
> > @@ -657,8 +657,10 @@ static void fat_evict_inode(struct inode *inode)
> >  	if (!inode->i_nlink) {
> >  		inode->i_size = 0;
> >  		fat_truncate_blocks(inode, 0);
> > -	} else
> > +	} else {
> > +		sync_mapping_buffers(inode->i_mapping);
> 
> Hm, why do we have to add this here? For FAT, if buffers are still
> dirty, buffers will be flushed via bdev flush?

The reason why I've put sync_mapping_buffers() here is the following
sequence:
  fd = open("file")
  write(fd)
  close(fd)
    - now data gets written out, dentry & inode can get evicted from memory
  fd = open("file")
  fsync(fd)
    - this should flush all dirty metadata associated with "file" but if we
      didn't call sync_mapping_buffers() during inode eviction we wouldn't
      have a way to do that.

So in general I think sync_mapping_buffers() call is indeed needed.

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