On Thu, Sep 03, 2020 at 08:11:00PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
>
> Commit 5833112df7e9 tried to make it so that a remap operation would
> force the log out to disk if the filesystem is mounted with mandatory
> synchronous writes. Unfortunately, that commit failed to handle the
> case where the inode or the file descriptor require mandatory
> synchronous writes.
>
> Refactor the check into into a helper that will look for all three
> conditions, and now we can treat reflink just like any other synchronous
> write.
>
> Fixes: 5833112df7e9 ("xfs: reflink should force the log out if mounted with wsync")
More of a process thought than an issue with this particular patch, but
I feel like the Fixes tag thing gets more watered down as we attempt to
apply it to more patches. Is it really necessary here? If so, what's the
reasoning? I thought it was more of a "this previous patch has a bug,"
but that link seems a bit tenuous here given the original patch refers
specifically to wsync. Sure, a stable kernel probably wants both
patches, but is that really the primary purpose of "Fixes?"
> Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> ---
Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>
> fs/xfs/xfs_file.c | 17 ++++++++++++++++-
> 1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index c31cd3be9fb2..ee43f137830c 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -1008,6 +1008,21 @@ xfs_file_fadvise(
> return ret;
> }
>
> +/* Does this file, inode, or mount want synchronous writes? */
> +static inline bool xfs_file_sync_writes(struct file *filp)
> +{
> + struct xfs_inode *ip = XFS_I(file_inode(filp));
> +
> + if (ip->i_mount->m_flags & XFS_MOUNT_WSYNC)
> + return true;
> + if (filp->f_flags & (__O_SYNC | O_DSYNC))
> + return true;
> + if (IS_SYNC(file_inode(filp)))
> + return true;
> +
> + return false;
> +}
> +
> STATIC loff_t
> xfs_file_remap_range(
> struct file *file_in,
> @@ -1065,7 +1080,7 @@ xfs_file_remap_range(
> if (ret)
> goto out_unlock;
>
> - if (mp->m_flags & XFS_MOUNT_WSYNC)
> + if (xfs_file_sync_writes(file_in) || xfs_file_sync_writes(file_out))
> xfs_log_force_inode(dest);
> out_unlock:
> xfs_iunlock2_io_mmap(src, dest);
>
