Re: [PATCH 12/41] fs: Drop sync_mapping_buffers() from __generic_file_fsync()

On Tue 24-03-26 06:17:16, Christoph Hellwig wrote:
> On Tue, Mar 24, 2026 at 01:34:57PM +0100, Jan Kara wrote:
> > I'm fine with simple_fsync() name for the helper with the trivial behavior
> > of writing out the mapping and the inode. Code wise this will look somewhat
> > different given what you've suggested for the last patch.
> 
> Yeah, the pitfalls of going sequentially through the series :)
> 
> But sketching this out I'm not even sure all this makes sense any more.
> Maybe instad of the allback we should just have a helper for checking the
> inode state like:
> 
> static inline bool inode_need_fsync(struct inode *inode, bool datasync)
> {
> 	enum inode_state_flags_enum state = inode_state_read_once(inode);
> 
> 	if (!(state & I_DIRTY_ALL))
>                 return false;
>         if (datasync && !(state & I_DIRTY_DATASYNC))
>                 retun false;
> 	return true;
> }
> 
> and otherwise just open code the calls int the two implementations
> without any callbacks, as it feels cleaner to avoid the entanglement.

Leaving the two implementations separate certainly works for me as well
(that's why I've put that patch to the end because I've expected some
discussions around it :)). Just the amount of common trivial calls you need
to do (fdatawrite(), sync_inode_metadata(),
file_check_and_advance_wb_err(), blkdev_issue_flush()) looked high enough
to me to be worth merging the implementations. But I don't feel strongly
either way.

								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>