Re: [PATCH 1/9] xfs: fix reflink quota reservation accounting error

[Brian Foster <bfoster@redhat.com>]

On Wed, Jun 24, 2020 at 06:17:45PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Quota reservations are supposed to account for the blocks that might be
> allocated due to a bmap btree split.  Reflink doesn't do this, so fix
> this to make the quota accounting more accurate before we start
> rearranging things.
> 
> Fixes: 862bb360ef56 ("xfs: reflink extents from one file to another")
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/xfs_reflink.c |   21 ++++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)
> 
> 
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index 107bf2a2f344..d89201d40891 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -1003,6 +1003,7 @@ xfs_reflink_remap_extent(
>  	xfs_filblks_t		rlen;
>  	xfs_filblks_t		unmap_len;
>  	xfs_off_t		newlen;
> +	int64_t			qres;
>  	int			error;
>  
>  	unmap_len = irec->br_startoff + irec->br_blockcount - destoff;
> @@ -1025,13 +1026,19 @@ xfs_reflink_remap_extent(
>  	xfs_ilock(ip, XFS_ILOCK_EXCL);
>  	xfs_trans_ijoin(tp, ip, 0);
>  
> -	/* If we're not just clearing space, then do we have enough quota? */
> -	if (real_extent) {
> -		error = xfs_trans_reserve_quota_nblks(tp, ip,
> -				irec->br_blockcount, 0, XFS_QMOPT_RES_REGBLKS);
> -		if (error)
> -			goto out_cancel;
> -	}
> +	/*
> +	 * Reserve quota for this operation.  We don't know if the first unmap
> +	 * in the dest file will cause a bmap btree split, so we always reserve
> +	 * at least enough blocks for that split.  If the extent being mapped
> +	 * in is written, we need to reserve quota for that too.
> +	 */
> +	qres = XFS_EXTENTADD_SPACE_RES(mp, XFS_DATA_FORK);
> +	if (real_extent)
> +		qres += irec->br_blockcount;
> +	error = xfs_trans_reserve_quota_nblks(tp, ip, qres, 0,
> +			XFS_QMOPT_RES_REGBLKS);
> +	if (error)
> +		goto out_cancel;
>  
>  	trace_xfs_reflink_remap(ip, irec->br_startoff,
>  				irec->br_blockcount, irec->br_startblock);
>