Re: [PATCH 41/41] fs: Unify generic_file_fsync() with mmb methods

On Mon 23-03-26 22:56:30, Christoph Hellwig wrote:
> On Fri, Mar 20, 2026 at 02:41:36PM +0100, Jan Kara wrote:
> > Taking inode lock when writing out the inode seems pointless in
> > particular because there are lots of places (most notably sync(2) path)
> > that don't do that so hardly anything can depend on it.
> 
> This is really something that needs to stand out clearly for bisecting
> and documentation.  I.e. make this a patch on its own and preferably
> before all the other refactoring that already is affected by moving
> between the implementations at the beginning of the series.
>
> > So let's remove __generic_file_fsync() and use
> > generic_mmb_fsync_noflush() instead to reduce code duplication. Arguably
> > this leaks a bit of buffer_head knowledge into fs/libfs.c which is not
> > great but avoiding the duplication seems worth it.
> 
> You could just pass a callback to the generic version.  The cost of an
> indirect call should not matter compared to the rest of the fsync code.
> That would also be a nice thing before all the renaming, as that means
> we could add the version with the callback first to unify the
> implementations and then the file systems are switched away from
> the buffers fsync variant to explicitly pass a callback, or to not
> pass a callback when they currently get the default one.

OK, makes sense. I can put the patch removing inode_lock from
__generic_file_fsync() at the place in the series where we start
dealing with fsync handlers. Then I'd introduce fsync variant with the
callback and then convert filesystems. As I was thinking about it, it would
be natural for the callback to be called sync_metadata and handle
writeout of the metadata including the inode. That would actually simplify
life in the following series I wanted to write which will make sure that
fsync properly writes out & waits for the buffer head containing the inode
(currently if background flush work happens to write inode first, buffer
head is not written to the backing device during fsync). And if the
callback isn't provided, we'd just write out the inode. That sounds
reasonable to me.

								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>