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

Jan Kara <[email protected]> writes:

> On Sun 29-03-26 22:55:09, OGAWA Hirofumi wrote:
>> Jan Kara <[email protected]> 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 <[email protected]>
>> > ---
>> >  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.

Hm, it looks like not new issue, isn't it? Why we have changed now in
this series?

It is including trade off write amplification vs reliability (i.e. may
not call fsync()), for example. So I think we should not add it easily.

Thanks.
-- 
OGAWA Hirofumi <[email protected]>

--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to [email protected].  For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"[email protected]">[email protected]</a>