On Mon, Oct 17, 2016 at 02:05:19PM +0200, Christoph Hellwig wrote:
> We need the iolock protection to stabilizie the IS_SWAPFILE and IS_IMMUTABLE
> values, as well as preventing new buffered writers re-dirtying the file data
> that we just wrote out.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
Reviewed-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> ---
> fs/xfs/xfs_file.c | 62 ++++++++++++++++++++++++++++++++++------------------
> fs/xfs/xfs_reflink.c | 15 -------------
> 2 files changed, 41 insertions(+), 36 deletions(-)
>
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index 012a960..0960264 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -996,38 +996,54 @@ xfs_file_share_range(
> inode_out = file_inode(file_out);
> bs = inode_out->i_sb->s_blocksize;
>
> + /* Lock both files against IO */
> + same_inode = (inode_in == inode_out);
> + if (same_inode) {
> + xfs_ilock(XFS_I(inode_in), XFS_IOLOCK_EXCL);
> + xfs_ilock(XFS_I(inode_in), XFS_MMAPLOCK_EXCL);
> + } else {
> + xfs_lock_two_inodes(XFS_I(inode_in), XFS_I(inode_out),
> + XFS_IOLOCK_EXCL);
> + xfs_lock_two_inodes(XFS_I(inode_in), XFS_I(inode_out),
> + XFS_MMAPLOCK_EXCL);
> + }
> +
> /* Don't touch certain kinds of inodes */
> + ret = -EPERM;
> if (IS_IMMUTABLE(inode_out))
> - return -EPERM;
> - if (IS_SWAPFILE(inode_in) ||
> - IS_SWAPFILE(inode_out))
> - return -ETXTBSY;
> -
> - same_inode = (inode_in == inode_out);
> + goto out_unlock;
> + ret = -ETXTBSY;
> + if (IS_SWAPFILE(inode_in) || IS_SWAPFILE(inode_out))
> + goto out_unlock;
>
> /* Don't reflink dirs, pipes, sockets... */
> + ret = -EISDIR;
> if (S_ISDIR(inode_in->i_mode) || S_ISDIR(inode_out->i_mode))
> - return -EISDIR;
> + goto out_unlock;
> + ret = -EINVAL;
> if (S_ISFIFO(inode_in->i_mode) || S_ISFIFO(inode_out->i_mode))
> - return -EINVAL;
> + goto out_unlock;
> if (!S_ISREG(inode_in->i_mode) || !S_ISREG(inode_out->i_mode))
> - return -EINVAL;
> + goto out_unlock;
>
> /* Don't share DAX file data for now. */
> if (IS_DAX(inode_in) || IS_DAX(inode_out))
> - return -EINVAL;
> + goto out_unlock;
>
> /* Are we going all the way to the end? */
> isize = i_size_read(inode_in);
> - if (isize == 0)
> - return 0;
> + if (isize == 0) {
> + ret = 0;
> + goto out_unlock;
> + }
> +
> if (len == 0)
> len = isize - pos_in;
>
> /* Ensure offsets don't wrap and the input is inside i_size */
> if (pos_in + len < pos_in || pos_out + len < pos_out ||
> pos_in + len > isize)
> - return -EINVAL;
> + goto out_unlock;
>
> /* Don't allow dedupe past EOF in the dest file */
> if (is_dedupe) {
> @@ -1035,7 +1051,7 @@ xfs_file_share_range(
>
> disize = i_size_read(inode_out);
> if (pos_out >= disize || pos_out + len > disize)
> - return -EINVAL;
> + goto out_unlock;
> }
>
> /* If we're linking to EOF, continue to the block boundary. */
> @@ -1047,28 +1063,32 @@ xfs_file_share_range(
> /* Only reflink if we're aligned to block boundaries */
> if (!IS_ALIGNED(pos_in, bs) || !IS_ALIGNED(pos_in + blen, bs) ||
> !IS_ALIGNED(pos_out, bs) || !IS_ALIGNED(pos_out + blen, bs))
> - return -EINVAL;
> + goto out_unlock;
>
> /* Don't allow overlapped reflink within the same file */
> if (same_inode && pos_out + blen > pos_in && pos_out < pos_in + blen)
> - return -EINVAL;
> + goto out_unlock;
>
> /* Wait for the completion of any pending IOs on srcfile */
> ret = xfs_file_wait_for_io(inode_in, pos_in, len);
> if (ret)
> - goto out;
> + goto out_unlock;
> ret = xfs_file_wait_for_io(inode_out, pos_out, len);
> if (ret)
> - goto out;
> + goto out_unlock;
>
> if (is_dedupe)
> flags |= XFS_REFLINK_DEDUPE;
> ret = xfs_reflink_remap_range(XFS_I(inode_in), pos_in, XFS_I(inode_out),
> pos_out, len, flags);
> - if (ret < 0)
> - goto out;
>
> -out:
> +out_unlock:
> + xfs_iunlock(XFS_I(inode_in), XFS_MMAPLOCK_EXCL);
> + xfs_iunlock(XFS_I(inode_in), XFS_IOLOCK_EXCL);
> + if (!same_inode) {
> + xfs_iunlock(XFS_I(inode_out), XFS_MMAPLOCK_EXCL);
> + xfs_iunlock(XFS_I(inode_out), XFS_IOLOCK_EXCL);
> + }
> return ret;
> }
>
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index 5965e94..d012746 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -1341,15 +1341,6 @@ xfs_reflink_remap_range(
>
> trace_xfs_reflink_remap_range(src, srcoff, len, dest, destoff);
>
> - /* Lock both files against IO */
> - if (src->i_ino == dest->i_ino) {
> - xfs_ilock(src, XFS_IOLOCK_EXCL);
> - xfs_ilock(src, XFS_MMAPLOCK_EXCL);
> - } else {
> - xfs_lock_two_inodes(src, dest, XFS_IOLOCK_EXCL);
> - xfs_lock_two_inodes(src, dest, XFS_MMAPLOCK_EXCL);
> - }
> -
> /*
> * Check that the extents are the same.
> */
> @@ -1401,12 +1392,6 @@ xfs_reflink_remap_range(
> goto out_error;
>
> out_error:
> - xfs_iunlock(src, XFS_MMAPLOCK_EXCL);
> - xfs_iunlock(src, XFS_IOLOCK_EXCL);
> - if (src->i_ino != dest->i_ino) {
> - xfs_iunlock(dest, XFS_MMAPLOCK_EXCL);
> - xfs_iunlock(dest, XFS_IOLOCK_EXCL);
> - }
> if (error)
> trace_xfs_reflink_remap_range_error(dest, error, _RET_IP_);
> return error;
> --
> 2.1.4
>
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
