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

On Wed 01-04-26 18:41:56, OGAWA Hirofumi wrote:
> Jan Kara <[email protected]> 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

Yes, but for this to be possible you would have to:
1) stop using dir-A between dir create & file creates
2) create enough memory pressure to cycle the dentry of dir-A through the
LRU and reclaim it
3) continue memory pressure to cycle the inode for dir-A through the LRU
and reclaim it

So the amount of work that already has to happen to trigger flushing of
a single block is so large that IMHO that flush will be lost in the noise.
 
> >> 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?

Yes, to separate metadata buffer head tracking into a separate structure we
must remove the handling of buffer head list from generic inode reclaim (as
the filesystem has no way to provide the separate tracking structure there).
Of course we could add a filesystem hook to inode reclaim to allow for
handling of metadata bhs but:

a) I'd rather do that in a way that is usable also for other issues
filesystems have with inode reclaim as I mentioned in this thread before

b) I don't think it's warranted for FAT etc. at this point as I don't think
the possible overhead of metadata bh flushing on inode reclaim will be a
problem in practice.

But of course we can reevaluate if my gut feeling is wrong and someone
comes with a workload which significantly regresses due to these changes.

								Honza
-- 
Jan Kara <[email protected]>
SUSE Labs, CR

--
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>