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

[OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>]

Jan Kara <jack@suse.cz> writes:

>> I think it would happen with normal operation, for example, copy many
>> files more than total memory. I think this would be much common than
>> write=>close=>open=>fsync in your example.
>
> When you copy a lot of files which are large in total, I agree the flushing
> can be triggered. But I don't think it will trigger any excessive IO
> because the metadata blocks being flushed aren't redirtied after the inode
> is evicted. So blocks may be written out earlier but I don't think they
> will be written out more times. For FAT for example you track only
> directory blocks in these lists so when directory inode will be getting
> evicted, you may see earlier writeout of dirty directory blocks but that's
> all.

Hm, metadata block is shared by several inodes. So earlier flush
makes fewer chance to combining multiple dirties.

For example,
  create dir-A
  reclaimed and flushed dir-A
  add new entries to dir-A
  lost chance to combining re-dirty of dir-A

>> Anyway, with it, reclaimed
>> inode metadata will be flushed forcibly and frequently (yeah, may not be
>> significant though. but I can't see the benefit for users from this
>> change.), and lost to chance combining multiple time of dirty while copy
>> many files.
>
> The benefit for users is 24 bytes saved for the majority of inodes that are
> there in the system - all the virtual inodes on sysfs / proc filesystem,
> all tmpfs inodes, all XFS inodes, all ext4 inodes when using journal (once I
> optimize ext4 code a bit), etc. So actually quite a bit of kernel memory
> saved in common configurations.
>
> Another win is that with metadata buffer head tracking now separated, I can
> modify that code (which will require growing the tracking structure) to
> properly track buffer head containing the inode and flush it on fsync(2).
> Currently there's a race that if flush worker writes out inode before
> fsync(2), then fsync(2) does not writeout the buffer containing the inode
> at all and thus data is not really persistent. This is actually my initial
> motivation for this refactoring since growing inode for everybody to fix
> data consistency issues of FAT/ext2/udf isn't popular these days...

Agree, it is good.  I'm only saying about the flushing earlier. To
implement it, is the flush earlier really necessary?

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>