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

On Tue 31-03-26 19:40:01, OGAWA Hirofumi wrote:
> Jan Kara <[email protected]> writes:
> >> 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.
> >
> > I expect in practice you'll hardly be able to observe the difference as
> > inodes usually get quite a while to be reclaimed at which point the dirty
> > buffers would be already flushed by background writeback. I don't see how
> > this change would lead specifically to "write amplification" - that would
> > mean frequent redirtying of the same metadata buffer of an inode
> > interleaved with frequent reclaims of the inode and I don't see how that
> > would happen in a realistic setting.
> >
> > If someone comes with a realistic workload which would suffer significant
> > regression from this change, then of course we should address it. I have
> > plans for adding an interface for filesystems to expose the information
> > that inode has some pending dirty metadata and a way to flush them from
> > flush worker because that is a common need a lot of filesystems has and
> > doing the flushing from .evict isn't always doable due to locking
> > constraints.
> 
> 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.

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

> > I'm still thinking about details but this has to be a properly
> > abstracted interface all filesystems can use and not a special hack for a
> > handful of old filesystems.
> 
> Sounds great. How about we delay this behavior change until this
> interface?

I would prefer not since that would delay fixing the data consistency
issues and I expect that reclaim discussion not to be very fast to
conclude.

								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>