On Mon, Apr 06, 2020 at 12:04:37PM -0400, Brian Foster wrote:
> 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...
Ok, I'll add the following:
[Note: XFS_MOUNT_WSYNC is set when the admin mounts the filesystem
with either the 'wsync' or 'sync' mount options, which effectively means
that we're classifying reflink/dedupe as IO operations and making them
synchronous when required.]
Thanks for reviewing. :)
--D
> 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
> > > >
> > >
> >
>
