Re: [PATCH 3/6] xfs: call xfs_qm_dqattach before performing reflink operations

[Brian Foster <bfoster@redhat.com>]

On Sat, Jan 20, 2018 at 09:34:10PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Ensure that we've attached all the necessary dquots before performing
> reflink operations so that quota accounting is accurate.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  fs/xfs/xfs_reflink.c |   17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> 
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index 5d1ff5a..947d0637 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -54,6 +54,7 @@
>  #include "xfs_rmap_btree.h"
>  #include "xfs_sb.h"
>  #include "xfs_ag_resv.h"
> +#include "xfs_qm.h"
>  
>  /*
>   * Copy on Write of Shared Blocks
> @@ -282,6 +283,10 @@ xfs_reflink_reserve_cow(
>  	 * tree.
>  	 */
>  
> +	error = xfs_qm_dqattach_locked(ip, 0);
> +	if (error)
> +		return error;
> +

The same call exists further down in the function. Was the intent to
move it? I suspect we don't need it twice, at least.

>  	if (!xfs_iext_lookup_extent(ip, ifp, imap->br_startoff, &icur, &got))
>  		eof = true;
>  	if (!eof && got.br_startoff <= imap->br_startoff) {
> @@ -396,6 +401,10 @@ xfs_reflink_allocate_cow(
>  	ASSERT(xfs_is_reflink_inode(ip));
>  	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL | XFS_ILOCK_SHARED));
>  
> +	error = xfs_qm_dqattach_locked(ip, 0);
> +	if (error)
> +		return error;
> +

Similar pattern here, but for this one the assert above suggests we
could have the shared lock. xfs_qm_dqattach_locked() looks like it
expects the exclusive lock (and that's what it looks like the second
call deals with). Hm?

Brian

>  	/*
>  	 * Even if the extent is not shared we might have a preallocation for
>  	 * it in the COW fork.  If so use it.
> @@ -1356,6 +1365,14 @@ xfs_reflink_remap_range(
>  	if (IS_DAX(inode_in) || IS_DAX(inode_out))
>  		goto out_unlock;
>  
> +	/* Attach dquots to both inodes */
> +	ret = xfs_qm_dqattach(src, 0);
> +	if (ret)
> +		goto out_unlock;
> +	ret = xfs_qm_dqattach(dest, 0);
> +	if (ret)
> +		goto out_unlock;
> +
>  	ret = vfs_clone_file_prep_inodes(inode_in, pos_in, inode_out, pos_out,
>  			&len, is_dedupe);
>  	if (ret <= 0)
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html