diff mbox series

[3/3] xfs: introduce xfs_bremapi_from_cowfork()

Message ID 20220209073655.22162-4-hsiangkao@linux.alibaba.com (mailing list archive)
State Superseded
Headers show
Series xfs: some end COW remapping optimization | expand

Commit Message

Gao Xiang Feb. 9, 2022, 7:36 a.m. UTC
Previously, xfs_reflink_end_cow_extent() will unconditionally unmap
the corresponding old extent and remap an extent from COW fork.
However, it seems somewhat ineffective since the old bmbt records can
be directly updated for many cases instead.

This patch uses introduced xfs_bmap_update_extent_real() in the
previous patch for most extent inclusive cases or it will fall back
to the old way if such replacement is not possible.

Actually, we're planing to use a modified alway-cow like atomic write
approach internally, therefore it'd be nice to do some optimization
to reduce some metadata overhead.

Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
---
 fs/xfs/libxfs/xfs_bmap.c | 117 ++++++++++++++++++++++++++++++++++++---
 fs/xfs/libxfs/xfs_bmap.h |   3 +
 fs/xfs/xfs_reflink.c     |  19 +------
 3 files changed, 112 insertions(+), 27 deletions(-)

Comments

Darrick J. Wong Feb. 16, 2022, 1:24 a.m. UTC | #1
On Wed, Feb 09, 2022 at 03:36:55PM +0800, Gao Xiang wrote:
> Previously, xfs_reflink_end_cow_extent() will unconditionally unmap
> the corresponding old extent and remap an extent from COW fork.
> However, it seems somewhat ineffective since the old bmbt records can
> be directly updated for many cases instead.
> 
> This patch uses introduced xfs_bmap_update_extent_real() in the
> previous patch for most extent inclusive cases or it will fall back
> to the old way if such replacement is not possible.
> 
> Actually, we're planing to use a modified alway-cow like atomic write
> approach internally, therefore it'd be nice to do some optimization
> to reduce some metadata overhead.
> 
> Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
> ---
>  fs/xfs/libxfs/xfs_bmap.c | 117 ++++++++++++++++++++++++++++++++++++---
>  fs/xfs/libxfs/xfs_bmap.h |   3 +
>  fs/xfs/xfs_reflink.c     |  19 +------
>  3 files changed, 112 insertions(+), 27 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index a10476dee701..0e132f811f7a 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -5880,6 +5880,114 @@ xfs_bmap_collapse_extents(
>  	return error;
>  }
>  
> +/* Deferred mapping is only for real extents in the data fork. */
> +static bool
> +xfs_bmap_is_update_needed(
> +	struct xfs_bmbt_irec	*bmap)
> +{
> +	return  bmap->br_startblock != HOLESTARTBLOCK &&
> +		bmap->br_startblock != DELAYSTARTBLOCK;
> +}
> +
> +/* del is an extent from COW fork */
> +int
> +xfs_bremapi_from_cowfork(
> +	struct xfs_trans	*tp,
> +	struct xfs_inode	*ip,
> +	struct xfs_bmbt_irec	*icow)
> +{
> +	int			error;
> +	xfs_filblks_t		rlen;
> +
> +	/* Use the old (unmap-remap) way for real-time inodes instead */
> +	if (!XFS_IS_REALTIME_INODE(ip) && xfs_bmap_is_update_needed(icow)) {

When would be be remapping a realtime file with a COW fork?

> +		xfs_fileoff_t		start, end, max_len;
> +		struct xfs_bmbt_irec	got;
> +		struct xfs_iext_cursor	icur;
> +		struct xfs_btree_cur	*cur = NULL;
> +		struct xfs_ifork	*ifp = XFS_IFORK_PTR(ip, XFS_DATA_FORK);
> +		int			logflags = 0;
> +
> +		error = xfs_iread_extents(tp, ip, XFS_DATA_FORK);
> +		if (error)
> +			return error;
> +
> +		max_len = xfs_refcount_max_unmap(tp->t_log_res);
> +		if (max_len < icow->br_blockcount) {
> +			icow->br_startoff += icow->br_blockcount - max_len;
> +			icow->br_startblock += icow->br_blockcount - max_len;
> +			icow->br_blockcount = max_len;
> +		}
> +
> +		end = icow->br_startoff + icow->br_blockcount;
> +		if (!xfs_iext_count(ifp) || !xfs_iext_lookup_extent_before(ip,
> +				ifp, &end, &icur, &got) ||
> +		    isnullstartblock(got.br_startblock) ||
> +		    icow->br_startoff + icow->br_blockcount > got.br_startoff +
> +				got.br_blockcount) {
> +			error = -EAGAIN;
> +		} else {
> +			end = icow->br_startoff + icow->br_blockcount;
> +			start = XFS_FILEOFF_MAX(icow->br_startoff,
> +						got.br_startoff);
> +			ASSERT(start < end);
> +
> +			/* Trim the extent to what we need */
> +			xfs_trim_extent(icow, start, end - start);
> +			xfs_trim_extent(&got, start, end - start);
> +
> +			if (ifp->if_format == XFS_DINODE_FMT_BTREE) {
> +				cur = xfs_bmbt_init_cursor(tp->t_mountp, tp, ip,
> +							   XFS_DATA_FORK);
> +				cur->bc_ino.flags = 0;
> +			}
> +
> +			/*
> +			 * Free the CoW orphan record (it should be done here
> +			 * before updating extent due to rmapbt update)
> +			 */
> +			xfs_refcount_free_cow_extent(tp, icow->br_startblock,
> +						     icow->br_blockcount);
> +
> +			xfs_bmap_update_extent_real(tp, ip, XFS_DATA_FORK,
> +					&icur, &cur, icow, &logflags, false);

Hmm... are you directly updating the data fork mapping record from the
COW fork mapping record?  Is the performance advantage you mentioned
earlier a result of this code no longer logging a bmap map intent item
and reducing the transaction roll count by one?

--D

> +
> +			/* Free previous referenced space */
> +			xfs_refcount_decrease_extent(tp, &got);
> +
> +			trace_xfs_reflink_cow_remap(ip, icow);
> +			error = 0;
> +		}
> +		if (cur)
> +			xfs_btree_del_cursor(cur, 0);
> +		if (logflags)
> +			xfs_trans_log_inode(tp, ip, logflags);
> +		if (!error)
> +			return 0;
> +	}
> +
> +	rlen = icow->br_blockcount;
> +	error = __xfs_bunmapi(tp, ip, icow->br_startoff, &rlen, 0, 1);
> +	if (error)
> +		return error;
> +
> +	/* Trim the extent to whatever got unmapped. */
> +	xfs_trim_extent(icow, icow->br_startoff + rlen,
> +			icow->br_blockcount - rlen);
> +	/* Free the CoW orphan record. */
> +	xfs_refcount_free_cow_extent(tp, icow->br_startblock,
> +				     icow->br_blockcount);
> +
> +	/* Map the new blocks into the data fork. */
> +	xfs_bmap_map_extent(tp, ip, icow);
> +
> +	/* Charge this new data fork mapping to the on-disk quota. */
> +	xfs_trans_mod_dquot_byino(tp, ip, XFS_TRANS_DQ_DELBCOUNT,
> +			(long)icow->br_blockcount);
> +	trace_xfs_reflink_cow_remap(ip, icow);
> +	return 0;
> +}
> +
>  /* Make sure we won't be right-shifting an extent past the maximum bound. */
>  int
>  xfs_bmap_can_insert_extents(
> @@ -6123,15 +6231,6 @@ xfs_bmap_split_extent(
>  	return error;
>  }
>  
> -/* Deferred mapping is only for real extents in the data fork. */
> -static bool
> -xfs_bmap_is_update_needed(
> -	struct xfs_bmbt_irec	*bmap)
> -{
> -	return  bmap->br_startblock != HOLESTARTBLOCK &&
> -		bmap->br_startblock != DELAYSTARTBLOCK;
> -}
> -
>  /* Record a bmap intent. */
>  static int
>  __xfs_bmap_add(
> diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
> index c52ff94786e2..9da1cff41c1c 100644
> --- a/fs/xfs/libxfs/xfs_bmap.h
> +++ b/fs/xfs/libxfs/xfs_bmap.h
> @@ -220,6 +220,9 @@ int	xfs_bmap_update_extent_real(struct xfs_trans *tp,
>  		struct xfs_inode *ip, int whichfork,
>  		struct xfs_iext_cursor *icur, struct xfs_btree_cur **curp,
>  		struct xfs_bmbt_irec *new, int *logflagsp, bool convert);
> +int
> +xfs_bremapi_from_cowfork(struct xfs_trans *tp, struct xfs_inode *ip,
> +		struct xfs_bmbt_irec *icow);
>  
>  enum xfs_bmap_intent_type {
>  	XFS_BMAP_MAP = 1,
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index 276387a6a85d..75bd2e03cd5b 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -590,7 +590,6 @@ xfs_reflink_end_cow_extent(
>  	struct xfs_mount	*mp = ip->i_mount;
>  	struct xfs_trans	*tp;
>  	struct xfs_ifork	*ifp = XFS_IFORK_PTR(ip, XFS_COW_FORK);
> -	xfs_filblks_t		rlen;
>  	unsigned int		resblks;
>  	int			error;
>  
> @@ -651,26 +650,10 @@ xfs_reflink_end_cow_extent(
>  		goto out_cancel;
>  	}
>  
> -	/* Unmap the old blocks in the data fork. */
> -	rlen = del.br_blockcount;
> -	error = __xfs_bunmapi(tp, ip, del.br_startoff, &rlen, 0, 1);
> +	error = xfs_bremapi_from_cowfork(tp, ip, &del);
>  	if (error)
>  		goto out_cancel;
>  
> -	/* Trim the extent to whatever got unmapped. */
> -	xfs_trim_extent(&del, del.br_startoff + rlen, del.br_blockcount - rlen);
> -	trace_xfs_reflink_cow_remap(ip, &del);
> -
> -	/* Free the CoW orphan record. */
> -	xfs_refcount_free_cow_extent(tp, del.br_startblock, del.br_blockcount);
> -
> -	/* Map the new blocks into the data fork. */
> -	xfs_bmap_map_extent(tp, ip, &del);
> -
> -	/* Charge this new data fork mapping to the on-disk quota. */
> -	xfs_trans_mod_dquot_byino(tp, ip, XFS_TRANS_DQ_DELBCOUNT,
> -			(long)del.br_blockcount);
> -
>  	/* Remove the mapping from the CoW fork. */
>  	xfs_bmap_del_extent_cow(ip, &icur, &got, &del);
>  
> -- 
> 2.24.4
>
Gao Xiang Feb. 16, 2022, 2:03 a.m. UTC | #2
Hi Darrick,

On Tue, Feb 15, 2022 at 05:24:33PM -0800, Darrick J. Wong wrote:
> On Wed, Feb 09, 2022 at 03:36:55PM +0800, Gao Xiang wrote:
> > Previously, xfs_reflink_end_cow_extent() will unconditionally unmap
> > the corresponding old extent and remap an extent from COW fork.
> > However, it seems somewhat ineffective since the old bmbt records can
> > be directly updated for many cases instead.
> > 
> > This patch uses introduced xfs_bmap_update_extent_real() in the
> > previous patch for most extent inclusive cases or it will fall back
> > to the old way if such replacement is not possible.
> > 
> > Actually, we're planing to use a modified alway-cow like atomic write
> > approach internally, therefore it'd be nice to do some optimization
> > to reduce some metadata overhead.
> > 
> > Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
> > ---
> >  fs/xfs/libxfs/xfs_bmap.c | 117 ++++++++++++++++++++++++++++++++++++---
> >  fs/xfs/libxfs/xfs_bmap.h |   3 +
> >  fs/xfs/xfs_reflink.c     |  19 +------
> >  3 files changed, 112 insertions(+), 27 deletions(-)
> > 
> > diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> > index a10476dee701..0e132f811f7a 100644
> > --- a/fs/xfs/libxfs/xfs_bmap.c
> > +++ b/fs/xfs/libxfs/xfs_bmap.c
> > @@ -5880,6 +5880,114 @@ xfs_bmap_collapse_extents(
> >  	return error;
> >  }
> >  
> > +/* Deferred mapping is only for real extents in the data fork. */
> > +static bool
> > +xfs_bmap_is_update_needed(
> > +	struct xfs_bmbt_irec	*bmap)
> > +{
> > +	return  bmap->br_startblock != HOLESTARTBLOCK &&
> > +		bmap->br_startblock != DELAYSTARTBLOCK;
> > +}
> > +
> > +/* del is an extent from COW fork */
> > +int
> > +xfs_bremapi_from_cowfork(
> > +	struct xfs_trans	*tp,
> > +	struct xfs_inode	*ip,
> > +	struct xfs_bmbt_irec	*icow)
> > +{
> > +	int			error;
> > +	xfs_filblks_t		rlen;
> > +
> > +	/* Use the old (unmap-remap) way for real-time inodes instead */
> > +	if (!XFS_IS_REALTIME_INODE(ip) && xfs_bmap_is_update_needed(icow)) {
> 
> When would be be remapping a realtime file with a COW fork?

Sorry, I missed this part, this is a necessary check.
Thanks for pointing out this.

> 
> > +		xfs_fileoff_t		start, end, max_len;
> > +		struct xfs_bmbt_irec	got;
> > +		struct xfs_iext_cursor	icur;
> > +		struct xfs_btree_cur	*cur = NULL;
> > +		struct xfs_ifork	*ifp = XFS_IFORK_PTR(ip, XFS_DATA_FORK);
> > +		int			logflags = 0;
> > +
> > +		error = xfs_iread_extents(tp, ip, XFS_DATA_FORK);
> > +		if (error)
> > +			return error;
> > +
> > +		max_len = xfs_refcount_max_unmap(tp->t_log_res);
> > +		if (max_len < icow->br_blockcount) {
> > +			icow->br_startoff += icow->br_blockcount - max_len;
> > +			icow->br_startblock += icow->br_blockcount - max_len;
> > +			icow->br_blockcount = max_len;
> > +		}
> > +
> > +		end = icow->br_startoff + icow->br_blockcount;
> > +		if (!xfs_iext_count(ifp) || !xfs_iext_lookup_extent_before(ip,
> > +				ifp, &end, &icur, &got) ||
> > +		    isnullstartblock(got.br_startblock) ||
> > +		    icow->br_startoff + icow->br_blockcount > got.br_startoff +
> > +				got.br_blockcount) {
> > +			error = -EAGAIN;
> > +		} else {
> > +			end = icow->br_startoff + icow->br_blockcount;
> > +			start = XFS_FILEOFF_MAX(icow->br_startoff,
> > +						got.br_startoff);
> > +			ASSERT(start < end);
> > +
> > +			/* Trim the extent to what we need */
> > +			xfs_trim_extent(icow, start, end - start);
> > +			xfs_trim_extent(&got, start, end - start);
> > +
> > +			if (ifp->if_format == XFS_DINODE_FMT_BTREE) {
> > +				cur = xfs_bmbt_init_cursor(tp->t_mountp, tp, ip,
> > +							   XFS_DATA_FORK);
> > +				cur->bc_ino.flags = 0;
> > +			}
> > +
> > +			/*
> > +			 * Free the CoW orphan record (it should be done here
> > +			 * before updating extent due to rmapbt update)
> > +			 */
> > +			xfs_refcount_free_cow_extent(tp, icow->br_startblock,
> > +						     icow->br_blockcount);
> > +
> > +			xfs_bmap_update_extent_real(tp, ip, XFS_DATA_FORK,
> > +					&icur, &cur, icow, &logflags, false);
> 
> Hmm... are you directly updating the data fork mapping record from the
> COW fork mapping record?  Is the performance advantage you mentioned
> earlier a result of this code no longer logging a bmap map intent item
> and reducing the transaction roll count by one?

Yes, roughly I think it reduces bmap again overhead.

Thanks,
Gao Xiang

> 
> --D
> 
> > +
> > +			/* Free previous referenced space */
> > +			xfs_refcount_decrease_extent(tp, &got);
> > +
> > +			trace_xfs_reflink_cow_remap(ip, icow);
> > +			error = 0;
> > +		}
> > +		if (cur)
> > +			xfs_btree_del_cursor(cur, 0);
> > +		if (logflags)
> > +			xfs_trans_log_inode(tp, ip, logflags);
> > +		if (!error)
> > +			return 0;
> > +	}
> > +
> > +	rlen = icow->br_blockcount;
> > +	error = __xfs_bunmapi(tp, ip, icow->br_startoff, &rlen, 0, 1);
> > +	if (error)
> > +		return error;
> > +
> > +	/* Trim the extent to whatever got unmapped. */
> > +	xfs_trim_extent(icow, icow->br_startoff + rlen,
> > +			icow->br_blockcount - rlen);
> > +	/* Free the CoW orphan record. */
> > +	xfs_refcount_free_cow_extent(tp, icow->br_startblock,
> > +				     icow->br_blockcount);
> > +
> > +	/* Map the new blocks into the data fork. */
> > +	xfs_bmap_map_extent(tp, ip, icow);
> > +
> > +	/* Charge this new data fork mapping to the on-disk quota. */
> > +	xfs_trans_mod_dquot_byino(tp, ip, XFS_TRANS_DQ_DELBCOUNT,
> > +			(long)icow->br_blockcount);
> > +	trace_xfs_reflink_cow_remap(ip, icow);
> > +	return 0;
> > +}
> > +
> >  /* Make sure we won't be right-shifting an extent past the maximum bound. */
> >  int
> >  xfs_bmap_can_insert_extents(
> > @@ -6123,15 +6231,6 @@ xfs_bmap_split_extent(
> >  	return error;
> >  }
> >  
> > -/* Deferred mapping is only for real extents in the data fork. */
> > -static bool
> > -xfs_bmap_is_update_needed(
> > -	struct xfs_bmbt_irec	*bmap)
> > -{
> > -	return  bmap->br_startblock != HOLESTARTBLOCK &&
> > -		bmap->br_startblock != DELAYSTARTBLOCK;
> > -}
> > -
> >  /* Record a bmap intent. */
> >  static int
> >  __xfs_bmap_add(
> > diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
> > index c52ff94786e2..9da1cff41c1c 100644
> > --- a/fs/xfs/libxfs/xfs_bmap.h
> > +++ b/fs/xfs/libxfs/xfs_bmap.h
> > @@ -220,6 +220,9 @@ int	xfs_bmap_update_extent_real(struct xfs_trans *tp,
> >  		struct xfs_inode *ip, int whichfork,
> >  		struct xfs_iext_cursor *icur, struct xfs_btree_cur **curp,
> >  		struct xfs_bmbt_irec *new, int *logflagsp, bool convert);
> > +int
> > +xfs_bremapi_from_cowfork(struct xfs_trans *tp, struct xfs_inode *ip,
> > +		struct xfs_bmbt_irec *icow);
> >  
> >  enum xfs_bmap_intent_type {
> >  	XFS_BMAP_MAP = 1,
> > diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> > index 276387a6a85d..75bd2e03cd5b 100644
> > --- a/fs/xfs/xfs_reflink.c
> > +++ b/fs/xfs/xfs_reflink.c
> > @@ -590,7 +590,6 @@ xfs_reflink_end_cow_extent(
> >  	struct xfs_mount	*mp = ip->i_mount;
> >  	struct xfs_trans	*tp;
> >  	struct xfs_ifork	*ifp = XFS_IFORK_PTR(ip, XFS_COW_FORK);
> > -	xfs_filblks_t		rlen;
> >  	unsigned int		resblks;
> >  	int			error;
> >  
> > @@ -651,26 +650,10 @@ xfs_reflink_end_cow_extent(
> >  		goto out_cancel;
> >  	}
> >  
> > -	/* Unmap the old blocks in the data fork. */
> > -	rlen = del.br_blockcount;
> > -	error = __xfs_bunmapi(tp, ip, del.br_startoff, &rlen, 0, 1);
> > +	error = xfs_bremapi_from_cowfork(tp, ip, &del);
> >  	if (error)
> >  		goto out_cancel;
> >  
> > -	/* Trim the extent to whatever got unmapped. */
> > -	xfs_trim_extent(&del, del.br_startoff + rlen, del.br_blockcount - rlen);
> > -	trace_xfs_reflink_cow_remap(ip, &del);
> > -
> > -	/* Free the CoW orphan record. */
> > -	xfs_refcount_free_cow_extent(tp, del.br_startblock, del.br_blockcount);
> > -
> > -	/* Map the new blocks into the data fork. */
> > -	xfs_bmap_map_extent(tp, ip, &del);
> > -
> > -	/* Charge this new data fork mapping to the on-disk quota. */
> > -	xfs_trans_mod_dquot_byino(tp, ip, XFS_TRANS_DQ_DELBCOUNT,
> > -			(long)del.br_blockcount);
> > -
> >  	/* Remove the mapping from the CoW fork. */
> >  	xfs_bmap_del_extent_cow(ip, &icur, &got, &del);
> >  
> > -- 
> > 2.24.4
> >
diff mbox series

Patch

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index a10476dee701..0e132f811f7a 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -5880,6 +5880,114 @@  xfs_bmap_collapse_extents(
 	return error;
 }
 
+/* Deferred mapping is only for real extents in the data fork. */
+static bool
+xfs_bmap_is_update_needed(
+	struct xfs_bmbt_irec	*bmap)
+{
+	return  bmap->br_startblock != HOLESTARTBLOCK &&
+		bmap->br_startblock != DELAYSTARTBLOCK;
+}
+
+/* del is an extent from COW fork */
+int
+xfs_bremapi_from_cowfork(
+	struct xfs_trans	*tp,
+	struct xfs_inode	*ip,
+	struct xfs_bmbt_irec	*icow)
+{
+	int			error;
+	xfs_filblks_t		rlen;
+
+	/* Use the old (unmap-remap) way for real-time inodes instead */
+	if (!XFS_IS_REALTIME_INODE(ip) && xfs_bmap_is_update_needed(icow)) {
+		xfs_fileoff_t		start, end, max_len;
+		struct xfs_bmbt_irec	got;
+		struct xfs_iext_cursor	icur;
+		struct xfs_btree_cur	*cur = NULL;
+		struct xfs_ifork	*ifp = XFS_IFORK_PTR(ip, XFS_DATA_FORK);
+		int			logflags = 0;
+
+		error = xfs_iread_extents(tp, ip, XFS_DATA_FORK);
+		if (error)
+			return error;
+
+		max_len = xfs_refcount_max_unmap(tp->t_log_res);
+		if (max_len < icow->br_blockcount) {
+			icow->br_startoff += icow->br_blockcount - max_len;
+			icow->br_startblock += icow->br_blockcount - max_len;
+			icow->br_blockcount = max_len;
+		}
+
+		end = icow->br_startoff + icow->br_blockcount;
+		if (!xfs_iext_count(ifp) || !xfs_iext_lookup_extent_before(ip,
+				ifp, &end, &icur, &got) ||
+		    isnullstartblock(got.br_startblock) ||
+		    icow->br_startoff + icow->br_blockcount > got.br_startoff +
+				got.br_blockcount) {
+			error = -EAGAIN;
+		} else {
+			end = icow->br_startoff + icow->br_blockcount;
+			start = XFS_FILEOFF_MAX(icow->br_startoff,
+						got.br_startoff);
+			ASSERT(start < end);
+
+			/* Trim the extent to what we need */
+			xfs_trim_extent(icow, start, end - start);
+			xfs_trim_extent(&got, start, end - start);
+
+			if (ifp->if_format == XFS_DINODE_FMT_BTREE) {
+				cur = xfs_bmbt_init_cursor(tp->t_mountp, tp, ip,
+							   XFS_DATA_FORK);
+				cur->bc_ino.flags = 0;
+			}
+
+			/*
+			 * Free the CoW orphan record (it should be done here
+			 * before updating extent due to rmapbt update)
+			 */
+			xfs_refcount_free_cow_extent(tp, icow->br_startblock,
+						     icow->br_blockcount);
+
+			xfs_bmap_update_extent_real(tp, ip, XFS_DATA_FORK,
+					&icur, &cur, icow, &logflags, false);
+
+			/* Free previous referenced space */
+			xfs_refcount_decrease_extent(tp, &got);
+
+			trace_xfs_reflink_cow_remap(ip, icow);
+			error = 0;
+		}
+		if (cur)
+			xfs_btree_del_cursor(cur, 0);
+		if (logflags)
+			xfs_trans_log_inode(tp, ip, logflags);
+		if (!error)
+			return 0;
+	}
+
+	rlen = icow->br_blockcount;
+	error = __xfs_bunmapi(tp, ip, icow->br_startoff, &rlen, 0, 1);
+	if (error)
+		return error;
+
+	/* Trim the extent to whatever got unmapped. */
+	xfs_trim_extent(icow, icow->br_startoff + rlen,
+			icow->br_blockcount - rlen);
+	/* Free the CoW orphan record. */
+	xfs_refcount_free_cow_extent(tp, icow->br_startblock,
+				     icow->br_blockcount);
+
+	/* Map the new blocks into the data fork. */
+	xfs_bmap_map_extent(tp, ip, icow);
+
+	/* Charge this new data fork mapping to the on-disk quota. */
+	xfs_trans_mod_dquot_byino(tp, ip, XFS_TRANS_DQ_DELBCOUNT,
+			(long)icow->br_blockcount);
+	trace_xfs_reflink_cow_remap(ip, icow);
+	return 0;
+}
+
 /* Make sure we won't be right-shifting an extent past the maximum bound. */
 int
 xfs_bmap_can_insert_extents(
@@ -6123,15 +6231,6 @@  xfs_bmap_split_extent(
 	return error;
 }
 
-/* Deferred mapping is only for real extents in the data fork. */
-static bool
-xfs_bmap_is_update_needed(
-	struct xfs_bmbt_irec	*bmap)
-{
-	return  bmap->br_startblock != HOLESTARTBLOCK &&
-		bmap->br_startblock != DELAYSTARTBLOCK;
-}
-
 /* Record a bmap intent. */
 static int
 __xfs_bmap_add(
diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
index c52ff94786e2..9da1cff41c1c 100644
--- a/fs/xfs/libxfs/xfs_bmap.h
+++ b/fs/xfs/libxfs/xfs_bmap.h
@@ -220,6 +220,9 @@  int	xfs_bmap_update_extent_real(struct xfs_trans *tp,
 		struct xfs_inode *ip, int whichfork,
 		struct xfs_iext_cursor *icur, struct xfs_btree_cur **curp,
 		struct xfs_bmbt_irec *new, int *logflagsp, bool convert);
+int
+xfs_bremapi_from_cowfork(struct xfs_trans *tp, struct xfs_inode *ip,
+		struct xfs_bmbt_irec *icow);
 
 enum xfs_bmap_intent_type {
 	XFS_BMAP_MAP = 1,
diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index 276387a6a85d..75bd2e03cd5b 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -590,7 +590,6 @@  xfs_reflink_end_cow_extent(
 	struct xfs_mount	*mp = ip->i_mount;
 	struct xfs_trans	*tp;
 	struct xfs_ifork	*ifp = XFS_IFORK_PTR(ip, XFS_COW_FORK);
-	xfs_filblks_t		rlen;
 	unsigned int		resblks;
 	int			error;
 
@@ -651,26 +650,10 @@  xfs_reflink_end_cow_extent(
 		goto out_cancel;
 	}
 
-	/* Unmap the old blocks in the data fork. */
-	rlen = del.br_blockcount;
-	error = __xfs_bunmapi(tp, ip, del.br_startoff, &rlen, 0, 1);
+	error = xfs_bremapi_from_cowfork(tp, ip, &del);
 	if (error)
 		goto out_cancel;
 
-	/* Trim the extent to whatever got unmapped. */
-	xfs_trim_extent(&del, del.br_startoff + rlen, del.br_blockcount - rlen);
-	trace_xfs_reflink_cow_remap(ip, &del);
-
-	/* Free the CoW orphan record. */
-	xfs_refcount_free_cow_extent(tp, del.br_startblock, del.br_blockcount);
-
-	/* Map the new blocks into the data fork. */
-	xfs_bmap_map_extent(tp, ip, &del);
-
-	/* Charge this new data fork mapping to the on-disk quota. */
-	xfs_trans_mod_dquot_byino(tp, ip, XFS_TRANS_DQ_DELBCOUNT,
-			(long)del.br_blockcount);
-
 	/* Remove the mapping from the CoW fork. */
 	xfs_bmap_del_extent_cow(ip, &icur, &got, &del);