On Mon, Apr 06, 2020 at 08:31:54AM -0700, Darrick J. Wong wrote:
> On Mon, Apr 06, 2020 at 08:14:37AM -0400, Brian Foster wrote:
> > On Fri, Apr 03, 2020 at 02:55:22PM +0200, Christoph Hellwig wrote:
> > > Reflink should force the log out to disk if the filesystem was mounted
> > > with wsync, the same as most other operations in xfs.
> > >
> >
> > Isn't WSYNC for namespace operations? Why is this needed for reflink?
>
> The manpage says that 'wsync' (the mount option) is for making namespace
> operations synchronous.
>
> However, xfs_init_fs_context sets XFS_MOUNT_WSYNC if the admin set
> the 'sync' mount option, which makes all IO synchronous.
>
Ok.. so we're considering reflink a form of I/O.. I suppose that makes
sense, though it would be nice to explain that in the commit log...
Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>
> > > Fixes: 3fc9f5e409319 ("xfs: remove xfs_reflink_remap_range")
> >
> > At a glance this looks like a refactoring patch. What does this fix?
>
> It probably ought to be 862bb360ef569f ("xfs: reflink extents from one
> file to another") but so much of that was refactored for 5.0 that
> backporting this fix will require changing a totally different function
> (xfs_reflink_remap_range) in a totally different file (xfs_reflink.c).
>
> --D
>
> > Brian
> >
> > > Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> > > ---
> > > fs/xfs/xfs_file.c | 4 ++++
> > > 1 file changed, 4 insertions(+)
> > >
> > > diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> > > index 68e1cbb3cfcc..4b8bdecc3863 100644
> > > --- a/fs/xfs/xfs_file.c
> > > +++ b/fs/xfs/xfs_file.c
> > > @@ -1059,7 +1059,11 @@ xfs_file_remap_range(
> > >
> > > ret = xfs_reflink_update_dest(dest, pos_out + len, cowextsize,
> > > remap_flags);
> > > + if (ret)
> > > + goto out_unlock;
> > >
> > > + if (mp->m_flags & XFS_MOUNT_WSYNC)
> > > + xfs_log_force_inode(dest);
> > > out_unlock:
> > > xfs_reflink_remap_unlock(file_in, file_out);
> > > if (ret)
> > > --
> > > 2.25.1
> > >
> >
>
