Re: [PATCH 6/9] xfs: reflink can skip remap existing mappings

[Brian Foster <bfoster@redhat.com>]

On Wed, Jun 24, 2020 at 06:18:18PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> If the source and destination map are identical, we can skip the remap
> step to save some time.
> 
> 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 72de7179399d..f1156f121b7d 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -1031,6 +1031,23 @@ xfs_reflink_remap_extent(
>  
>  	trace_xfs_reflink_remap_extent_dest(ip, &smap);
>  
> +	/*
> +	 * Two extents mapped to the same physical block must not have
> +	 * different states; that's filesystem corruption.  Move on to the next
> +	 * extent if they're both holes or both the same physical extent.
> +	 */
> +	if (dmap->br_startblock == smap.br_startblock) {
> +		ASSERT(dmap->br_startblock == smap.br_startblock);

That assert duplicates the logic in the if statement. Was this intended
to be the length check I asked for? If so it looks like that was added
previously so perhaps this can just drop off. With that fixed up:

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

> +		if (dmap->br_state != smap.br_state)
> +			error = -EFSCORRUPTED;
> +		goto out_cancel;
> +	}
> +
> +	/* If both extents are unwritten, leave them alone. */
> +	if (dmap->br_state == XFS_EXT_UNWRITTEN &&
> +	    smap.br_state == XFS_EXT_UNWRITTEN)
> +		goto out_cancel;
> +
>  	/* No reflinking if the AG of the dest mapping is low on space. */
>  	if (dmap_written) {
>  		error = xfs_reflink_ag_has_free_space(mp,
>