On Thu, Apr 02, 2020 at 07:53:44AM -0700, Darrick J. Wong wrote: > > > Looks reasonable. That being said I really hate the way we handle > > > this - I've been wanting to rework the wsync/dirsync code to just mark > > > as transaction as dirsync or wsync and then let xfs_trans_commit handle > > > checking if the file system is mounted with the option to clean this > > > mess up. Let me see if I could resurrect that quickly. > > > > Resurrected and under testing now. While forward porting your patch > > I noticed it could be much simpler even without the refactor by just > > using xfs_trans_set_sync. The downside of that is that the log force > > is under the inode locks, but so are the log forces for all other wysnc > > induced log forces. So I think you should just submit this in the > > simplified version matching the rest of the wsync users as a fix. If > > we want to optimize it later on that should be done as a separate patch > > and for all wsync/dirsync users. > > Can you please send in a Reviewed-by so I can get this moving? :) In case the above wasn't clear: I don't think this is the right way to go. Just to fix the reflink vs wsync bug I think we want a one-liner like this: diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c index b0ce04ffd3cd..e2cc7b84ca6c 100644 --- a/fs/xfs/xfs_reflink.c +++ b/fs/xfs/xfs_reflink.c @@ -948,6 +948,7 @@ xfs_reflink_update_dest( xfs_trans_log_inode(tp, dest, XFS_ILOG_CORE); + xfs_trans_set_sync(tp); error = xfs_trans_commit(tp); if (error) goto out_error;
