optimize the COW I/O path V2

optimize the COW I/O path V2

[Christoph Hellwig]

This series contains a couple optimizations for the COW I/O path:

 - avoid any overhead for reads from COW files
 - streamline the COW handling in write_begin
 - reduce the number of extent lookups during COW I/O completion

It does not touch the direct I/O path (yet), I still have a few
unsolved problems there.

Changes since V1:
 - incorporated misc feedback from Brian

[PATCH 1/9] iomap: add IOMAP_REPORT

[Christoph Hellwig]

This allows the file system to tell a FIEMAP from a read operation, and thus
avoids the need to report flags that aren't actually used in the read path.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/iomap.c            |  2 +-
 include/linux/iomap.h | 17 +++++++++++------
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/fs/iomap.c b/fs/iomap.c
index 013d1d3..a922040 100644
--- a/fs/iomap.c
+++ b/fs/iomap.c
@@ -561,7 +561,7 @@ int iomap_fiemap(struct inode *inode, struct fiemap_extent_info *fi,
 	}
 
 	while (len > 0) {
-		ret = iomap_apply(inode, start, len, 0, ops, &ctx,
+		ret = iomap_apply(inode, start, len, IOMAP_REPORT, ops, &ctx,
 				iomap_fiemap_actor);
 		/* inode with no (attribute) mapping will give ENOENT */
 		if (ret == -ENOENT)
diff --git a/include/linux/iomap.h b/include/linux/iomap.h
index e63e288..7892f55 100644
--- a/include/linux/iomap.h
+++ b/include/linux/iomap.h
@@ -19,11 +19,15 @@ struct vm_fault;
 #define IOMAP_UNWRITTEN	0x04	/* blocks allocated @blkno in unwritten state */
 
 /*
- * Flags for iomap mappings:
+ * Flags for all iomap mappings:
  */
-#define IOMAP_F_MERGED	0x01	/* contains multiple blocks/extents */
-#define IOMAP_F_SHARED	0x02	/* block shared with another file */
-#define IOMAP_F_NEW	0x04	/* blocks have been newly allocated */
+#define IOMAP_F_NEW	0x01	/* blocks have been newly allocated */
+
+/*
+ * Flags that only need to be reported for IOMAP_REPORT requests:
+ */
+#define IOMAP_F_MERGED	0x10	/* contains multiple blocks/extents */
+#define IOMAP_F_SHARED	0x20	/* block shared with another file */
 
 /*
  * Magic value for blkno:
@@ -42,8 +46,9 @@ struct iomap {
 /*
  * Flags for iomap_begin / iomap_end.  No flag implies a read.
  */
-#define IOMAP_WRITE		(1 << 0)
-#define IOMAP_ZERO		(1 << 1)
+#define IOMAP_WRITE		(1 << 0) /* writing, must allocate blocks */
+#define IOMAP_ZERO		(1 << 1) /* zeroing operation, may skip holes */
+#define IOMAP_REPORT		(1 << 2) /* report extent status, e.g. FIEMAP */
 
 struct iomap_ops {
 	/*
-- 
2.1.4

Re: [PATCH 1/9] iomap: add IOMAP_REPORT

[Brian Foster]

On Sat, Oct 15, 2016 at 10:52:26AM +0200, Christoph Hellwig wrote:
> This allows the file system to tell a FIEMAP from a read operation, and thus
> avoids the need to report flags that aren't actually used in the read path.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---

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

>  fs/iomap.c            |  2 +-
>  include/linux/iomap.h | 17 +++++++++++------
>  2 files changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/iomap.c b/fs/iomap.c
> index 013d1d3..a922040 100644
> --- a/fs/iomap.c
> +++ b/fs/iomap.c
> @@ -561,7 +561,7 @@ int iomap_fiemap(struct inode *inode, struct fiemap_extent_info *fi,
>  	}
>  
>  	while (len > 0) {
> -		ret = iomap_apply(inode, start, len, 0, ops, &ctx,
> +		ret = iomap_apply(inode, start, len, IOMAP_REPORT, ops, &ctx,
>  				iomap_fiemap_actor);
>  		/* inode with no (attribute) mapping will give ENOENT */
>  		if (ret == -ENOENT)
> diff --git a/include/linux/iomap.h b/include/linux/iomap.h
> index e63e288..7892f55 100644
> --- a/include/linux/iomap.h
> +++ b/include/linux/iomap.h
> @@ -19,11 +19,15 @@ struct vm_fault;
>  #define IOMAP_UNWRITTEN	0x04	/* blocks allocated @blkno in unwritten state */
>  
>  /*
> - * Flags for iomap mappings:
> + * Flags for all iomap mappings:
>   */
> -#define IOMAP_F_MERGED	0x01	/* contains multiple blocks/extents */
> -#define IOMAP_F_SHARED	0x02	/* block shared with another file */
> -#define IOMAP_F_NEW	0x04	/* blocks have been newly allocated */
> +#define IOMAP_F_NEW	0x01	/* blocks have been newly allocated */
> +
> +/*
> + * Flags that only need to be reported for IOMAP_REPORT requests:
> + */
> +#define IOMAP_F_MERGED	0x10	/* contains multiple blocks/extents */
> +#define IOMAP_F_SHARED	0x20	/* block shared with another file */
>  
>  /*
>   * Magic value for blkno:
> @@ -42,8 +46,9 @@ struct iomap {
>  /*
>   * Flags for iomap_begin / iomap_end.  No flag implies a read.
>   */
> -#define IOMAP_WRITE		(1 << 0)
> -#define IOMAP_ZERO		(1 << 1)
> +#define IOMAP_WRITE		(1 << 0) /* writing, must allocate blocks */
> +#define IOMAP_ZERO		(1 << 1) /* zeroing operation, may skip holes */
> +#define IOMAP_REPORT		(1 << 2) /* report extent status, e.g. FIEMAP */
>  
>  struct iomap_ops {
>  	/*
> -- 
> 2.1.4
> 
> --
> 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

Re: [PATCH 1/9] iomap: add IOMAP_REPORT

[Darrick J. Wong]

On Sat, Oct 15, 2016 at 10:52:26AM +0200, Christoph Hellwig wrote:
> This allows the file system to tell a FIEMAP from a read operation, and thus
> avoids the need to report flags that aren't actually used in the read path.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

> ---
>  fs/iomap.c            |  2 +-
>  include/linux/iomap.h | 17 +++++++++++------
>  2 files changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/iomap.c b/fs/iomap.c
> index 013d1d3..a922040 100644
> --- a/fs/iomap.c
> +++ b/fs/iomap.c
> @@ -561,7 +561,7 @@ int iomap_fiemap(struct inode *inode, struct fiemap_extent_info *fi,
>  	}
>  
>  	while (len > 0) {
> -		ret = iomap_apply(inode, start, len, 0, ops, &ctx,
> +		ret = iomap_apply(inode, start, len, IOMAP_REPORT, ops, &ctx,
>  				iomap_fiemap_actor);
>  		/* inode with no (attribute) mapping will give ENOENT */
>  		if (ret == -ENOENT)
> diff --git a/include/linux/iomap.h b/include/linux/iomap.h
> index e63e288..7892f55 100644
> --- a/include/linux/iomap.h
> +++ b/include/linux/iomap.h
> @@ -19,11 +19,15 @@ struct vm_fault;
>  #define IOMAP_UNWRITTEN	0x04	/* blocks allocated @blkno in unwritten state */
>  
>  /*
> - * Flags for iomap mappings:
> + * Flags for all iomap mappings:
>   */
> -#define IOMAP_F_MERGED	0x01	/* contains multiple blocks/extents */
> -#define IOMAP_F_SHARED	0x02	/* block shared with another file */
> -#define IOMAP_F_NEW	0x04	/* blocks have been newly allocated */
> +#define IOMAP_F_NEW	0x01	/* blocks have been newly allocated */
> +
> +/*
> + * Flags that only need to be reported for IOMAP_REPORT requests:
> + */
> +#define IOMAP_F_MERGED	0x10	/* contains multiple blocks/extents */
> +#define IOMAP_F_SHARED	0x20	/* block shared with another file */
>  
>  /*
>   * Magic value for blkno:
> @@ -42,8 +46,9 @@ struct iomap {
>  /*
>   * Flags for iomap_begin / iomap_end.  No flag implies a read.
>   */
> -#define IOMAP_WRITE		(1 << 0)
> -#define IOMAP_ZERO		(1 << 1)
> +#define IOMAP_WRITE		(1 << 0) /* writing, must allocate blocks */
> +#define IOMAP_ZERO		(1 << 1) /* zeroing operation, may skip holes */
> +#define IOMAP_REPORT		(1 << 2) /* report extent status, e.g. FIEMAP */
>  
>  struct iomap_ops {
>  	/*
> -- 
> 2.1.4
> 

[PATCH 2/9] xfs: add xfs_trim_extent

[Christoph Hellwig]

From: "Darrick J. Wong" <darrick.wong@oracle.com>

This helpers allows to trim an extent to a subset of it's original range
while making sure the block numbers in it remain valid,

In the future xfs_trim_extent and xfs_bmapi_trim_map should probably be
merged in some form.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
[hch: split from a previous patch from Darrick, moved around and added
 support for "raw" delayed extents"]
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_bmap.c | 33 +++++++++++++++++++++++++++++++++
 fs/xfs/libxfs/xfs_bmap.h |  2 ++
 2 files changed, 35 insertions(+)

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index c27344c..016dacc 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -3999,6 +3999,39 @@ xfs_bmap_alloc(
 	return xfs_bmap_btalloc(ap);
 }
 
+/* Trim extent to fit a logical block range. */
+void
+xfs_trim_extent(
+	struct xfs_bmbt_irec	*irec,
+	xfs_fileoff_t		bno,
+	xfs_filblks_t		len)
+{
+	xfs_fileoff_t		distance;
+	xfs_fileoff_t		end = bno + len;
+
+	if (irec->br_startoff + irec->br_blockcount <= bno ||
+	    irec->br_startoff >= end) {
+		irec->br_blockcount = 0;
+		return;
+	}
+
+	if (irec->br_startoff < bno) {
+		distance = bno - irec->br_startoff;
+		if (isnullstartblock(irec->br_startblock))
+			irec->br_startblock = DELAYSTARTBLOCK;
+		if (irec->br_startblock != DELAYSTARTBLOCK &&
+		    irec->br_startblock != HOLESTARTBLOCK)
+			irec->br_startblock += distance;
+		irec->br_startoff += distance;
+		irec->br_blockcount -= distance;
+	}
+
+	if (end < irec->br_startoff + irec->br_blockcount) {
+		distance = irec->br_startoff + irec->br_blockcount - end;
+		irec->br_blockcount -= distance;
+	}
+}
+
 /*
  * Trim the returned map to the required bounds
  */
diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
index f97db71..eb86af0 100644
--- a/fs/xfs/libxfs/xfs_bmap.h
+++ b/fs/xfs/libxfs/xfs_bmap.h
@@ -190,6 +190,8 @@ void	xfs_bmap_trace_exlist(struct xfs_inode *ip, xfs_extnum_t cnt,
 #define	XFS_BMAP_TRACE_EXLIST(ip,c,w)
 #endif
 
+void	xfs_trim_extent(struct xfs_bmbt_irec *irec, xfs_fileoff_t bno,
+		xfs_filblks_t len);
 int	xfs_bmap_add_attrfork(struct xfs_inode *ip, int size, int rsvd);
 void	xfs_bmap_local_to_extents_empty(struct xfs_inode *ip, int whichfork);
 void	xfs_bmap_add_free(struct xfs_mount *mp, struct xfs_defer_ops *dfops,
-- 
2.1.4

Re: [PATCH 2/9] xfs: add xfs_trim_extent

[Brian Foster]

On Sat, Oct 15, 2016 at 10:52:27AM +0200, Christoph Hellwig wrote:
> From: "Darrick J. Wong" <darrick.wong@oracle.com>
> 
> This helpers allows to trim an extent to a subset of it's original range
> while making sure the block numbers in it remain valid,
> 
> In the future xfs_trim_extent and xfs_bmapi_trim_map should probably be
> merged in some form.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> [hch: split from a previous patch from Darrick, moved around and added
>  support for "raw" delayed extents"]
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---

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

>  fs/xfs/libxfs/xfs_bmap.c | 33 +++++++++++++++++++++++++++++++++
>  fs/xfs/libxfs/xfs_bmap.h |  2 ++
>  2 files changed, 35 insertions(+)
> 
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index c27344c..016dacc 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -3999,6 +3999,39 @@ xfs_bmap_alloc(
>  	return xfs_bmap_btalloc(ap);
>  }
>  
> +/* Trim extent to fit a logical block range. */
> +void
> +xfs_trim_extent(
> +	struct xfs_bmbt_irec	*irec,
> +	xfs_fileoff_t		bno,
> +	xfs_filblks_t		len)
> +{
> +	xfs_fileoff_t		distance;
> +	xfs_fileoff_t		end = bno + len;
> +
> +	if (irec->br_startoff + irec->br_blockcount <= bno ||
> +	    irec->br_startoff >= end) {
> +		irec->br_blockcount = 0;
> +		return;
> +	}
> +
> +	if (irec->br_startoff < bno) {
> +		distance = bno - irec->br_startoff;
> +		if (isnullstartblock(irec->br_startblock))
> +			irec->br_startblock = DELAYSTARTBLOCK;
> +		if (irec->br_startblock != DELAYSTARTBLOCK &&
> +		    irec->br_startblock != HOLESTARTBLOCK)
> +			irec->br_startblock += distance;
> +		irec->br_startoff += distance;
> +		irec->br_blockcount -= distance;
> +	}
> +
> +	if (end < irec->br_startoff + irec->br_blockcount) {
> +		distance = irec->br_startoff + irec->br_blockcount - end;
> +		irec->br_blockcount -= distance;
> +	}
> +}
> +
>  /*
>   * Trim the returned map to the required bounds
>   */
> diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
> index f97db71..eb86af0 100644
> --- a/fs/xfs/libxfs/xfs_bmap.h
> +++ b/fs/xfs/libxfs/xfs_bmap.h
> @@ -190,6 +190,8 @@ void	xfs_bmap_trace_exlist(struct xfs_inode *ip, xfs_extnum_t cnt,
>  #define	XFS_BMAP_TRACE_EXLIST(ip,c,w)
>  #endif
>  
> +void	xfs_trim_extent(struct xfs_bmbt_irec *irec, xfs_fileoff_t bno,
> +		xfs_filblks_t len);
>  int	xfs_bmap_add_attrfork(struct xfs_inode *ip, int size, int rsvd);
>  void	xfs_bmap_local_to_extents_empty(struct xfs_inode *ip, int whichfork);
>  void	xfs_bmap_add_free(struct xfs_mount *mp, struct xfs_defer_ops *dfops,
> -- 
> 2.1.4
> 
> --
> 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

Re: [PATCH 2/9] xfs: add xfs_trim_extent

[Darrick J. Wong]

On Sat, Oct 15, 2016 at 10:52:27AM +0200, Christoph Hellwig wrote:
> From: "Darrick J. Wong" <darrick.wong@oracle.com>
> 
> This helpers allows to trim an extent to a subset of it's original range
> while making sure the block numbers in it remain valid,
> 
> In the future xfs_trim_extent and xfs_bmapi_trim_map should probably be
> merged in some form.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> [hch: split from a previous patch from Darrick, moved around and added
>  support for "raw" delayed extents"]
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

> ---
>  fs/xfs/libxfs/xfs_bmap.c | 33 +++++++++++++++++++++++++++++++++
>  fs/xfs/libxfs/xfs_bmap.h |  2 ++
>  2 files changed, 35 insertions(+)
> 
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index c27344c..016dacc 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -3999,6 +3999,39 @@ xfs_bmap_alloc(
>  	return xfs_bmap_btalloc(ap);
>  }
>  
> +/* Trim extent to fit a logical block range. */
> +void
> +xfs_trim_extent(
> +	struct xfs_bmbt_irec	*irec,
> +	xfs_fileoff_t		bno,
> +	xfs_filblks_t		len)
> +{
> +	xfs_fileoff_t		distance;
> +	xfs_fileoff_t		end = bno + len;
> +
> +	if (irec->br_startoff + irec->br_blockcount <= bno ||
> +	    irec->br_startoff >= end) {
> +		irec->br_blockcount = 0;
> +		return;
> +	}
> +
> +	if (irec->br_startoff < bno) {
> +		distance = bno - irec->br_startoff;
> +		if (isnullstartblock(irec->br_startblock))
> +			irec->br_startblock = DELAYSTARTBLOCK;
> +		if (irec->br_startblock != DELAYSTARTBLOCK &&
> +		    irec->br_startblock != HOLESTARTBLOCK)
> +			irec->br_startblock += distance;
> +		irec->br_startoff += distance;
> +		irec->br_blockcount -= distance;
> +	}
> +
> +	if (end < irec->br_startoff + irec->br_blockcount) {
> +		distance = irec->br_startoff + irec->br_blockcount - end;
> +		irec->br_blockcount -= distance;
> +	}
> +}
> +
>  /*
>   * Trim the returned map to the required bounds
>   */
> diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
> index f97db71..eb86af0 100644
> --- a/fs/xfs/libxfs/xfs_bmap.h
> +++ b/fs/xfs/libxfs/xfs_bmap.h
> @@ -190,6 +190,8 @@ void	xfs_bmap_trace_exlist(struct xfs_inode *ip, xfs_extnum_t cnt,
>  #define	XFS_BMAP_TRACE_EXLIST(ip,c,w)
>  #endif
>  
> +void	xfs_trim_extent(struct xfs_bmbt_irec *irec, xfs_fileoff_t bno,
> +		xfs_filblks_t len);
>  int	xfs_bmap_add_attrfork(struct xfs_inode *ip, int size, int rsvd);
>  void	xfs_bmap_local_to_extents_empty(struct xfs_inode *ip, int whichfork);
>  void	xfs_bmap_add_free(struct xfs_mount *mp, struct xfs_defer_ops *dfops,
> -- 
> 2.1.4
> 

[PATCH 3/9] xfs: handle "raw" delayed extents xfs_reflink_trim_around_shared

[Christoph Hellwig]

Delalloc extents in the extent list contain the number of reserved
indirect blocks in their startblock value and don't use the magic
DELAYSTARTBLOCK constant.  Ensure that xfs_reflink_trim_around_shared
handles them properly by checking for isnullstartblock().

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_reflink.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index 5965e94..46c824c 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -182,7 +182,8 @@ xfs_reflink_trim_around_shared(
 	if (!xfs_is_reflink_inode(ip) ||
 	    ISUNWRITTEN(irec) ||
 	    irec->br_startblock == HOLESTARTBLOCK ||
-	    irec->br_startblock == DELAYSTARTBLOCK) {
+	    irec->br_startblock == DELAYSTARTBLOCK ||
+	    isnullstartblock(irec->br_startblock)) {
 		*shared = false;
 		return 0;
 	}
-- 
2.1.4

Re: [PATCH 3/9] xfs: handle "raw" delayed extents xfs_reflink_trim_around_shared

[Brian Foster]

On Sat, Oct 15, 2016 at 10:52:28AM +0200, Christoph Hellwig wrote:
> Delalloc extents in the extent list contain the number of reserved
> indirect blocks in their startblock value and don't use the magic
> DELAYSTARTBLOCK constant.  Ensure that xfs_reflink_trim_around_shared
> handles them properly by checking for isnullstartblock().
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---

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

>  fs/xfs/xfs_reflink.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index 5965e94..46c824c 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -182,7 +182,8 @@ xfs_reflink_trim_around_shared(
>  	if (!xfs_is_reflink_inode(ip) ||
>  	    ISUNWRITTEN(irec) ||
>  	    irec->br_startblock == HOLESTARTBLOCK ||
> -	    irec->br_startblock == DELAYSTARTBLOCK) {
> +	    irec->br_startblock == DELAYSTARTBLOCK ||
> +	    isnullstartblock(irec->br_startblock)) {
>  		*shared = false;
>  		return 0;
>  	}
> -- 
> 2.1.4
> 
> --
> 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

Re: [PATCH 3/9] xfs: handle "raw" delayed extents xfs_reflink_trim_around_shared

[Darrick J. Wong]

On Sat, Oct 15, 2016 at 10:52:28AM +0200, Christoph Hellwig wrote:
> Delalloc extents in the extent list contain the number of reserved
> indirect blocks in their startblock value and don't use the magic
> DELAYSTARTBLOCK constant.  Ensure that xfs_reflink_trim_around_shared
> handles them properly by checking for isnullstartblock().
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

> ---
>  fs/xfs/xfs_reflink.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index 5965e94..46c824c 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -182,7 +182,8 @@ xfs_reflink_trim_around_shared(
>  	if (!xfs_is_reflink_inode(ip) ||
>  	    ISUNWRITTEN(irec) ||
>  	    irec->br_startblock == HOLESTARTBLOCK ||
> -	    irec->br_startblock == DELAYSTARTBLOCK) {
> +	    irec->br_startblock == DELAYSTARTBLOCK ||
> +	    isnullstartblock(irec->br_startblock)) {
>  		*shared = false;
>  		return 0;
>  	}
> -- 
> 2.1.4
> 

[PATCH 4/9] xfs: don't bother looking at the refcount tree for reads

[Christoph Hellwig]

There is no need to trim an extent into a shared or non-shared one, or
report any flags for plain old reads.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/xfs_iomap.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index d907eb9..1dabf2e 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -996,11 +996,14 @@ xfs_file_iomap_begin(
 		return error;
 	}
 
-	/* Trim the mapping to the nearest shared extent boundary. */
-	error = xfs_reflink_trim_around_shared(ip, &imap, &shared, &trimmed);
-	if (error) {
-		xfs_iunlock(ip, lockmode);
-		return error;
+	if (flags & (IOMAP_WRITE | IOMAP_ZERO | IOMAP_REPORT)) {
+		/* Trim the mapping to the nearest shared extent boundary. */
+		error = xfs_reflink_trim_around_shared(ip, &imap, &shared,
+				&trimmed);
+		if (error) {
+			xfs_iunlock(ip, lockmode);
+			return error;
+		}
 	}
 
 	if ((flags & IOMAP_WRITE) && imap_needs_alloc(inode, &imap, nimaps)) {
-- 
2.1.4

Re: [PATCH 4/9] xfs: don't bother looking at the refcount tree for reads

[Brian Foster]

On Sat, Oct 15, 2016 at 10:52:29AM +0200, Christoph Hellwig wrote:
> There is no need to trim an extent into a shared or non-shared one, or
> report any flags for plain old reads.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Brian Foster <bfoster@redhat.com>
> ---

I think the tag above was meant for the subsequent patch, but anyways..
:P

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

>  fs/xfs/xfs_iomap.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> index d907eb9..1dabf2e 100644
> --- a/fs/xfs/xfs_iomap.c
> +++ b/fs/xfs/xfs_iomap.c
> @@ -996,11 +996,14 @@ xfs_file_iomap_begin(
>  		return error;
>  	}
>  
> -	/* Trim the mapping to the nearest shared extent boundary. */
> -	error = xfs_reflink_trim_around_shared(ip, &imap, &shared, &trimmed);
> -	if (error) {
> -		xfs_iunlock(ip, lockmode);
> -		return error;
> +	if (flags & (IOMAP_WRITE | IOMAP_ZERO | IOMAP_REPORT)) {
> +		/* Trim the mapping to the nearest shared extent boundary. */
> +		error = xfs_reflink_trim_around_shared(ip, &imap, &shared,
> +				&trimmed);
> +		if (error) {
> +			xfs_iunlock(ip, lockmode);
> +			return error;
> +		}
>  	}
>  
>  	if ((flags & IOMAP_WRITE) && imap_needs_alloc(inode, &imap, nimaps)) {
> -- 
> 2.1.4
> 
> --
> 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

Re: [PATCH 4/9] xfs: don't bother looking at the refcount tree for reads

[Darrick J. Wong]

On Sat, Oct 15, 2016 at 10:52:29AM +0200, Christoph Hellwig wrote:
> There is no need to trim an extent into a shared or non-shared one, or
> report any flags for plain old reads.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Brian Foster <bfoster@redhat.com>

Looks ok to me,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

Will give all these patches (this series and the other cleanup series) a
thorough run through a test cycle and report back.

--D

> ---
>  fs/xfs/xfs_iomap.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> index d907eb9..1dabf2e 100644
> --- a/fs/xfs/xfs_iomap.c
> +++ b/fs/xfs/xfs_iomap.c
> @@ -996,11 +996,14 @@ xfs_file_iomap_begin(
>  		return error;
>  	}
>  
> -	/* Trim the mapping to the nearest shared extent boundary. */
> -	error = xfs_reflink_trim_around_shared(ip, &imap, &shared, &trimmed);
> -	if (error) {
> -		xfs_iunlock(ip, lockmode);
> -		return error;
> +	if (flags & (IOMAP_WRITE | IOMAP_ZERO | IOMAP_REPORT)) {
> +		/* Trim the mapping to the nearest shared extent boundary. */
> +		error = xfs_reflink_trim_around_shared(ip, &imap, &shared,
> +				&trimmed);
> +		if (error) {
> +			xfs_iunlock(ip, lockmode);
> +			return error;
> +		}
>  	}
>  
>  	if ((flags & IOMAP_WRITE) && imap_needs_alloc(inode, &imap, nimaps)) {
> -- 
> 2.1.4
> 

[PATCH 5/9] xfs: optimize writes to reflink files

[Christoph Hellwig]

Instead of reserving space as the first thing in write_begin move it past
reading the extent in the data fork.  That way we only have to read from
the data fork once and can reuse that information for trimming the extent
to the shared/unshared boundary.  Additionally this allows to easily
limit the actual write size to said boundary, and avoid a roundtrip on the
ilock.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_iomap.c   |  56 +++++++++++++-------
 fs/xfs/xfs_reflink.c | 141 +++++++++++++++++++++------------------------------
 fs/xfs/xfs_reflink.h |   4 +-
 fs/xfs/xfs_trace.h   |   3 +-
 4 files changed, 100 insertions(+), 104 deletions(-)

diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 1dabf2e..436e109 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -566,6 +566,17 @@ xfs_file_iomap_begin_delay(
 	xfs_bmap_search_extents(ip, offset_fsb, XFS_DATA_FORK, &eof, &idx,
 			&got, &prev);
 	if (!eof && got.br_startoff <= offset_fsb) {
+		if (xfs_is_reflink_inode(ip)) {
+			bool		shared;
+
+			end_fsb = min(XFS_B_TO_FSB(mp, offset + count),
+					maxbytes_fsb);
+			xfs_trim_extent(&got, offset_fsb, end_fsb - offset_fsb);
+			error = xfs_reflink_reserve_cow(ip, &got, &shared);
+			if (error)
+				goto out_unlock;
+		}
+
 		trace_xfs_iomap_found(ip, offset, count, 0, &got);
 		goto done;
 	}
@@ -961,19 +972,13 @@ xfs_file_iomap_begin(
 	struct xfs_mount	*mp = ip->i_mount;
 	struct xfs_bmbt_irec	imap;
 	xfs_fileoff_t		offset_fsb, end_fsb;
-	bool			shared, trimmed;
 	int			nimaps = 1, error = 0;
+	bool			shared = false, trimmed = false;
 	unsigned		lockmode;
 
 	if (XFS_FORCED_SHUTDOWN(mp))
 		return -EIO;
 
-	if ((flags & (IOMAP_WRITE | IOMAP_ZERO)) && xfs_is_reflink_inode(ip)) {
-		error = xfs_reflink_reserve_cow_range(ip, offset, length);
-		if (error < 0)
-			return error;
-	}
-
 	if ((flags & IOMAP_WRITE) && !IS_DAX(inode) &&
 		   !xfs_get_extsz_hint(ip)) {
 		/* Reserve delalloc blocks for regular writeback. */
@@ -981,7 +986,16 @@ xfs_file_iomap_begin(
 				iomap);
 	}
 
-	lockmode = xfs_ilock_data_map_shared(ip);
+	/*
+	 * COW writes will allocate delalloc space, so we need to make sure
+	 * to take the lock exclusively here.
+	 */
+	if ((flags & (IOMAP_WRITE | IOMAP_ZERO)) && xfs_is_reflink_inode(ip)) {
+		lockmode = XFS_ILOCK_EXCL;
+		xfs_ilock(ip, XFS_ILOCK_EXCL);
+	} else {
+		lockmode = xfs_ilock_data_map_shared(ip);
+	}
 
 	ASSERT(offset <= mp->m_super->s_maxbytes);
 	if ((xfs_fsize_t)offset + length > mp->m_super->s_maxbytes)
@@ -991,19 +1005,24 @@ xfs_file_iomap_begin(
 
 	error = xfs_bmapi_read(ip, offset_fsb, end_fsb - offset_fsb, &imap,
 			       &nimaps, 0);
-	if (error) {
-		xfs_iunlock(ip, lockmode);
-		return error;
-	}
+	if (error)
+		goto out_unlock;
 
-	if (flags & (IOMAP_WRITE | IOMAP_ZERO | IOMAP_REPORT)) {
+	if (flags & IOMAP_REPORT) {
 		/* Trim the mapping to the nearest shared extent boundary. */
 		error = xfs_reflink_trim_around_shared(ip, &imap, &shared,
 				&trimmed);
-		if (error) {
-			xfs_iunlock(ip, lockmode);
-			return error;
-		}
+		if (error)
+			goto out_unlock;
+	}
+
+	if ((flags & (IOMAP_WRITE | IOMAP_ZERO)) && xfs_is_reflink_inode(ip)) {
+		error = xfs_reflink_reserve_cow(ip, &imap, &shared);
+		if (error)
+			goto out_unlock;
+
+		end_fsb = imap.br_startoff + imap.br_blockcount;
+		length = XFS_FSB_TO_B(mp, end_fsb) - offset;
 	}
 
 	if ((flags & IOMAP_WRITE) && imap_needs_alloc(inode, &imap, nimaps)) {
@@ -1042,6 +1061,9 @@ xfs_file_iomap_begin(
 	if (shared)
 		iomap->flags |= IOMAP_F_SHARED;
 	return 0;
+out_unlock:
+	xfs_iunlock(ip, lockmode);
+	return error;
 }
 
 static int
diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index 46c824c..5d230ea 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -228,50 +228,54 @@ xfs_reflink_trim_around_shared(
 	}
 }
 
-/* Create a CoW reservation for a range of blocks within a file. */
-static int
-__xfs_reflink_reserve_cow(
+/*
+ * Trim the passed in imap to the next shared/unshared extent boundary, and
+ * if imap->br_startoff points to a shared extent reserve space for it in the
+ * COW fork.  In this case *shared is set to true, else to false.
+ *
+ * Note that imap will always contain the block numbers for the existing blocks
+ * in the data fork, as the upper layers need them for read-modify-write
+ * operations.
+ */
+int
+xfs_reflink_reserve_cow(
 	struct xfs_inode	*ip,
-	xfs_fileoff_t		*offset_fsb,
-	xfs_fileoff_t		end_fsb,
-	bool			*skipped)
+	struct xfs_bmbt_irec	*imap,
+	bool			*shared)
 {
-	struct xfs_bmbt_irec	got, prev, imap;
-	xfs_fileoff_t		orig_end_fsb;
-	int			nimaps, eof = 0, error = 0;
-	bool			shared = false, trimmed = false;
+	struct xfs_bmbt_irec	got, prev;
+	xfs_fileoff_t		end_fsb, orig_end_fsb;
+	int			eof = 0, error = 0;
+	bool			trimmed;
 	xfs_extnum_t		idx;
 	xfs_extlen_t		align;
 
-	/* Already reserved?  Skip the refcount btree access. */
-	xfs_bmap_search_extents(ip, *offset_fsb, XFS_COW_FORK, &eof, &idx,
+	/*
+	 * Search the COW fork extent list first.  This serves two purposes:
+	 * first this implement the speculative preallocation using cowextisze,
+	 * so that we also unshared block adjacent to shared blocks instead
+	 * of just the shared blocks themselves.  Second the lookup in the
+	 * extent list is generally faster than going out to the shared extent
+	 * tree.
+	 */
+	xfs_bmap_search_extents(ip, imap->br_startoff, XFS_COW_FORK, &eof, &idx,
 			&got, &prev);
-	if (!eof && got.br_startoff <= *offset_fsb) {
-		end_fsb = orig_end_fsb = got.br_startoff + got.br_blockcount;
-		trace_xfs_reflink_cow_found(ip, &got);
-		goto done;
-	}
+	if (!eof && got.br_startoff <= imap->br_startoff) {
+		trace_xfs_reflink_cow_found(ip, imap);
+		xfs_trim_extent(imap, got.br_startoff, got.br_blockcount);
 
-	/* Read extent from the source file. */
-	nimaps = 1;
-	error = xfs_bmapi_read(ip, *offset_fsb, end_fsb - *offset_fsb,
-			&imap, &nimaps, 0);
-	if (error)
-		goto out_unlock;
-	ASSERT(nimaps == 1);
+		*shared = true;
+		return 0;
+	}
 
 	/* Trim the mapping to the nearest shared extent boundary. */
-	error = xfs_reflink_trim_around_shared(ip, &imap, &shared, &trimmed);
+	error = xfs_reflink_trim_around_shared(ip, imap, shared, &trimmed);
 	if (error)
-		goto out_unlock;
-
-	end_fsb = orig_end_fsb = imap.br_startoff + imap.br_blockcount;
+		return error;
 
 	/* Not shared?  Just report the (potentially capped) extent. */
-	if (!shared) {
-		*skipped = true;
-		goto done;
-	}
+	if (!*shared)
+		return 0;
 
 	/*
 	 * Fork all the shared blocks from our write offset until the end of
@@ -279,73 +283,40 @@ __xfs_reflink_reserve_cow(
 	 */
 	error = xfs_qm_dqattach_locked(ip, 0);
 	if (error)
-		goto out_unlock;
+		return error;
+
+	end_fsb = orig_end_fsb = imap->br_startoff + imap->br_blockcount;
 
 	align = xfs_eof_alignment(ip, xfs_get_cowextsz_hint(ip));
 	if (align)
 		end_fsb = roundup_64(end_fsb, align);
 
 retry:
-	error = xfs_bmapi_reserve_delalloc(ip, XFS_COW_FORK, *offset_fsb,
-			end_fsb - *offset_fsb, &got,
-			&prev, &idx, eof);
+	error = xfs_bmapi_reserve_delalloc(ip, XFS_COW_FORK, imap->br_startoff,
+			end_fsb - imap->br_startoff, &got, &prev, &idx, eof);
 	switch (error) {
 	case 0:
 		break;
 	case -ENOSPC:
 	case -EDQUOT:
 		/* retry without any preallocation */
-		trace_xfs_reflink_cow_enospc(ip, &imap);
+		trace_xfs_reflink_cow_enospc(ip, imap);
 		if (end_fsb != orig_end_fsb) {
 			end_fsb = orig_end_fsb;
 			goto retry;
 		}
 		/*FALLTHRU*/
 	default:
-		goto out_unlock;
+		return error;
 	}
 
 	if (end_fsb != orig_end_fsb)
 		xfs_inode_set_cowblocks_tag(ip);
 
 	trace_xfs_reflink_cow_alloc(ip, &got);
-done:
-	*offset_fsb = end_fsb;
-out_unlock:
-	return error;
+	return 0;
 }
 
-/* Create a CoW reservation for part of a file. */
-int
-xfs_reflink_reserve_cow_range(
-	struct xfs_inode	*ip,
-	xfs_off_t		offset,
-	xfs_off_t		count)
-{
-	struct xfs_mount	*mp = ip->i_mount;
-	xfs_fileoff_t		offset_fsb, end_fsb;
-	bool			skipped = false;
-	int			error;
-
-	trace_xfs_reflink_reserve_cow_range(ip, offset, count);
-
-	offset_fsb = XFS_B_TO_FSBT(mp, offset);
-	end_fsb = XFS_B_TO_FSB(mp, offset + count);
-
-	xfs_ilock(ip, XFS_ILOCK_EXCL);
-	while (offset_fsb < end_fsb) {
-		error = __xfs_reflink_reserve_cow(ip, &offset_fsb, end_fsb,
-				&skipped);
-		if (error) {
-			trace_xfs_reflink_reserve_cow_range_error(ip, error,
-				_RET_IP_);
-			break;
-		}
-	}
-	xfs_iunlock(ip, XFS_ILOCK_EXCL);
-
-	return error;
-}
 
 /* Allocate all CoW reservations covering a range of blocks in a file. */
 static int
@@ -359,9 +330,8 @@ __xfs_reflink_allocate_cow(
 	struct xfs_defer_ops	dfops;
 	struct xfs_trans	*tp;
 	xfs_fsblock_t		first_block;
-	xfs_fileoff_t		next_fsb;
 	int			nimaps = 1, error;
-	bool			skipped = false;
+	bool			shared;
 
 	xfs_defer_init(&dfops, &first_block);
 
@@ -372,33 +342,38 @@ __xfs_reflink_allocate_cow(
 
 	xfs_ilock(ip, XFS_ILOCK_EXCL);
 
-	next_fsb = *offset_fsb;
-	error = __xfs_reflink_reserve_cow(ip, &next_fsb, end_fsb, &skipped);
+	/* Read extent from the source file. */
+	nimaps = 1;
+	error = xfs_bmapi_read(ip, *offset_fsb, end_fsb - *offset_fsb,
+			&imap, &nimaps, 0);
+	if (error)
+		goto out_unlock;
+	ASSERT(nimaps == 1);
+
+	error = xfs_reflink_reserve_cow(ip, &imap, &shared);
 	if (error)
 		goto out_trans_cancel;
 
-	if (skipped) {
-		*offset_fsb = next_fsb;
+	if (!shared) {
+		*offset_fsb = imap.br_startoff + imap.br_blockcount;
 		goto out_trans_cancel;
 	}
 
 	xfs_trans_ijoin(tp, ip, 0);
-	error = xfs_bmapi_write(tp, ip, *offset_fsb, next_fsb - *offset_fsb,
+	error = xfs_bmapi_write(tp, ip, imap.br_startoff, imap.br_blockcount,
 			XFS_BMAPI_COWFORK, &first_block,
 			XFS_EXTENTADD_SPACE_RES(mp, XFS_DATA_FORK),
 			&imap, &nimaps, &dfops);
 	if (error)
 		goto out_trans_cancel;
 
-	/* We might not have been able to map the whole delalloc extent */
-	*offset_fsb = min(*offset_fsb + imap.br_blockcount, next_fsb);
-
 	error = xfs_defer_finish(&tp, &dfops, NULL);
 	if (error)
 		goto out_trans_cancel;
 
 	error = xfs_trans_commit(tp);
 
+	*offset_fsb = imap.br_startoff + imap.br_blockcount;
 out_unlock:
 	xfs_iunlock(ip, XFS_ILOCK_EXCL);
 	return error;
diff --git a/fs/xfs/xfs_reflink.h b/fs/xfs/xfs_reflink.h
index 5dc3c8a..9f76924 100644
--- a/fs/xfs/xfs_reflink.h
+++ b/fs/xfs/xfs_reflink.h
@@ -26,8 +26,8 @@ extern int xfs_reflink_find_shared(struct xfs_mount *mp, xfs_agnumber_t agno,
 extern int xfs_reflink_trim_around_shared(struct xfs_inode *ip,
 		struct xfs_bmbt_irec *irec, bool *shared, bool *trimmed);
 
-extern int xfs_reflink_reserve_cow_range(struct xfs_inode *ip,
-		xfs_off_t offset, xfs_off_t count);
+extern int xfs_reflink_reserve_cow(struct xfs_inode *ip,
+		struct xfs_bmbt_irec *imap, bool *shared);
 extern int xfs_reflink_allocate_cow_range(struct xfs_inode *ip,
 		xfs_off_t offset, xfs_off_t count);
 extern bool xfs_reflink_find_cow_mapping(struct xfs_inode *ip, xfs_off_t offset,
diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index ad188d3..72f9f6b 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -3346,7 +3346,7 @@ DEFINE_INODE_IREC_EVENT(xfs_reflink_cow_alloc);
 DEFINE_INODE_IREC_EVENT(xfs_reflink_cow_found);
 DEFINE_INODE_IREC_EVENT(xfs_reflink_cow_enospc);
 
-DEFINE_RW_EVENT(xfs_reflink_reserve_cow_range);
+DEFINE_RW_EVENT(xfs_reflink_reserve_cow);
 DEFINE_RW_EVENT(xfs_reflink_allocate_cow_range);
 
 DEFINE_INODE_IREC_EVENT(xfs_reflink_bounce_dio_write);
@@ -3358,7 +3358,6 @@ DEFINE_SIMPLE_IO_EVENT(xfs_reflink_end_cow);
 DEFINE_INODE_IREC_EVENT(xfs_reflink_cow_remap);
 DEFINE_INODE_IREC_EVENT(xfs_reflink_cow_remap_piece);
 
-DEFINE_INODE_ERROR_EVENT(xfs_reflink_reserve_cow_range_error);
 DEFINE_INODE_ERROR_EVENT(xfs_reflink_allocate_cow_range_error);
 DEFINE_INODE_ERROR_EVENT(xfs_reflink_cancel_cow_range_error);
 DEFINE_INODE_ERROR_EVENT(xfs_reflink_end_cow_error);
-- 
2.1.4

Re: [PATCH 5/9] xfs: optimize writes to reflink files

[Brian Foster]

On Sat, Oct 15, 2016 at 10:52:30AM +0200, Christoph Hellwig wrote:
> Instead of reserving space as the first thing in write_begin move it past
> reading the extent in the data fork.  That way we only have to read from
> the data fork once and can reuse that information for trimming the extent
> to the shared/unshared boundary.  Additionally this allows to easily
> limit the actual write size to said boundary, and avoid a roundtrip on the
> ilock.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---

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

>  fs/xfs/xfs_iomap.c   |  56 +++++++++++++-------
>  fs/xfs/xfs_reflink.c | 141 +++++++++++++++++++++------------------------------
>  fs/xfs/xfs_reflink.h |   4 +-
>  fs/xfs/xfs_trace.h   |   3 +-
>  4 files changed, 100 insertions(+), 104 deletions(-)
> 
> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> index 1dabf2e..436e109 100644
> --- a/fs/xfs/xfs_iomap.c
> +++ b/fs/xfs/xfs_iomap.c
> @@ -566,6 +566,17 @@ xfs_file_iomap_begin_delay(
>  	xfs_bmap_search_extents(ip, offset_fsb, XFS_DATA_FORK, &eof, &idx,
>  			&got, &prev);
>  	if (!eof && got.br_startoff <= offset_fsb) {
> +		if (xfs_is_reflink_inode(ip)) {
> +			bool		shared;
> +
> +			end_fsb = min(XFS_B_TO_FSB(mp, offset + count),
> +					maxbytes_fsb);
> +			xfs_trim_extent(&got, offset_fsb, end_fsb - offset_fsb);
> +			error = xfs_reflink_reserve_cow(ip, &got, &shared);
> +			if (error)
> +				goto out_unlock;
> +		}
> +
>  		trace_xfs_iomap_found(ip, offset, count, 0, &got);
>  		goto done;
>  	}
> @@ -961,19 +972,13 @@ xfs_file_iomap_begin(
>  	struct xfs_mount	*mp = ip->i_mount;
>  	struct xfs_bmbt_irec	imap;
>  	xfs_fileoff_t		offset_fsb, end_fsb;
> -	bool			shared, trimmed;
>  	int			nimaps = 1, error = 0;
> +	bool			shared = false, trimmed = false;
>  	unsigned		lockmode;
>  
>  	if (XFS_FORCED_SHUTDOWN(mp))
>  		return -EIO;
>  
> -	if ((flags & (IOMAP_WRITE | IOMAP_ZERO)) && xfs_is_reflink_inode(ip)) {
> -		error = xfs_reflink_reserve_cow_range(ip, offset, length);
> -		if (error < 0)
> -			return error;
> -	}
> -
>  	if ((flags & IOMAP_WRITE) && !IS_DAX(inode) &&
>  		   !xfs_get_extsz_hint(ip)) {
>  		/* Reserve delalloc blocks for regular writeback. */
> @@ -981,7 +986,16 @@ xfs_file_iomap_begin(
>  				iomap);
>  	}
>  
> -	lockmode = xfs_ilock_data_map_shared(ip);
> +	/*
> +	 * COW writes will allocate delalloc space, so we need to make sure
> +	 * to take the lock exclusively here.
> +	 */
> +	if ((flags & (IOMAP_WRITE | IOMAP_ZERO)) && xfs_is_reflink_inode(ip)) {
> +		lockmode = XFS_ILOCK_EXCL;
> +		xfs_ilock(ip, XFS_ILOCK_EXCL);
> +	} else {
> +		lockmode = xfs_ilock_data_map_shared(ip);
> +	}
>  
>  	ASSERT(offset <= mp->m_super->s_maxbytes);
>  	if ((xfs_fsize_t)offset + length > mp->m_super->s_maxbytes)
> @@ -991,19 +1005,24 @@ xfs_file_iomap_begin(
>  
>  	error = xfs_bmapi_read(ip, offset_fsb, end_fsb - offset_fsb, &imap,
>  			       &nimaps, 0);
> -	if (error) {
> -		xfs_iunlock(ip, lockmode);
> -		return error;
> -	}
> +	if (error)
> +		goto out_unlock;
>  
> -	if (flags & (IOMAP_WRITE | IOMAP_ZERO | IOMAP_REPORT)) {
> +	if (flags & IOMAP_REPORT) {
>  		/* Trim the mapping to the nearest shared extent boundary. */
>  		error = xfs_reflink_trim_around_shared(ip, &imap, &shared,
>  				&trimmed);
> -		if (error) {
> -			xfs_iunlock(ip, lockmode);
> -			return error;
> -		}
> +		if (error)
> +			goto out_unlock;
> +	}
> +
> +	if ((flags & (IOMAP_WRITE | IOMAP_ZERO)) && xfs_is_reflink_inode(ip)) {
> +		error = xfs_reflink_reserve_cow(ip, &imap, &shared);
> +		if (error)
> +			goto out_unlock;
> +
> +		end_fsb = imap.br_startoff + imap.br_blockcount;
> +		length = XFS_FSB_TO_B(mp, end_fsb) - offset;
>  	}
>  
>  	if ((flags & IOMAP_WRITE) && imap_needs_alloc(inode, &imap, nimaps)) {
> @@ -1042,6 +1061,9 @@ xfs_file_iomap_begin(
>  	if (shared)
>  		iomap->flags |= IOMAP_F_SHARED;
>  	return 0;
> +out_unlock:
> +	xfs_iunlock(ip, lockmode);
> +	return error;
>  }
>  
>  static int
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index 46c824c..5d230ea 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -228,50 +228,54 @@ xfs_reflink_trim_around_shared(
>  	}
>  }
>  
> -/* Create a CoW reservation for a range of blocks within a file. */
> -static int
> -__xfs_reflink_reserve_cow(
> +/*
> + * Trim the passed in imap to the next shared/unshared extent boundary, and
> + * if imap->br_startoff points to a shared extent reserve space for it in the
> + * COW fork.  In this case *shared is set to true, else to false.
> + *
> + * Note that imap will always contain the block numbers for the existing blocks
> + * in the data fork, as the upper layers need them for read-modify-write
> + * operations.
> + */
> +int
> +xfs_reflink_reserve_cow(
>  	struct xfs_inode	*ip,
> -	xfs_fileoff_t		*offset_fsb,
> -	xfs_fileoff_t		end_fsb,
> -	bool			*skipped)
> +	struct xfs_bmbt_irec	*imap,
> +	bool			*shared)
>  {
> -	struct xfs_bmbt_irec	got, prev, imap;
> -	xfs_fileoff_t		orig_end_fsb;
> -	int			nimaps, eof = 0, error = 0;
> -	bool			shared = false, trimmed = false;
> +	struct xfs_bmbt_irec	got, prev;
> +	xfs_fileoff_t		end_fsb, orig_end_fsb;
> +	int			eof = 0, error = 0;
> +	bool			trimmed;
>  	xfs_extnum_t		idx;
>  	xfs_extlen_t		align;
>  
> -	/* Already reserved?  Skip the refcount btree access. */
> -	xfs_bmap_search_extents(ip, *offset_fsb, XFS_COW_FORK, &eof, &idx,
> +	/*
> +	 * Search the COW fork extent list first.  This serves two purposes:
> +	 * first this implement the speculative preallocation using cowextisze,
> +	 * so that we also unshared block adjacent to shared blocks instead
> +	 * of just the shared blocks themselves.  Second the lookup in the
> +	 * extent list is generally faster than going out to the shared extent
> +	 * tree.
> +	 */
> +	xfs_bmap_search_extents(ip, imap->br_startoff, XFS_COW_FORK, &eof, &idx,
>  			&got, &prev);
> -	if (!eof && got.br_startoff <= *offset_fsb) {
> -		end_fsb = orig_end_fsb = got.br_startoff + got.br_blockcount;
> -		trace_xfs_reflink_cow_found(ip, &got);
> -		goto done;
> -	}
> +	if (!eof && got.br_startoff <= imap->br_startoff) {
> +		trace_xfs_reflink_cow_found(ip, imap);
> +		xfs_trim_extent(imap, got.br_startoff, got.br_blockcount);
>  
> -	/* Read extent from the source file. */
> -	nimaps = 1;
> -	error = xfs_bmapi_read(ip, *offset_fsb, end_fsb - *offset_fsb,
> -			&imap, &nimaps, 0);
> -	if (error)
> -		goto out_unlock;
> -	ASSERT(nimaps == 1);
> +		*shared = true;
> +		return 0;
> +	}
>  
>  	/* Trim the mapping to the nearest shared extent boundary. */
> -	error = xfs_reflink_trim_around_shared(ip, &imap, &shared, &trimmed);
> +	error = xfs_reflink_trim_around_shared(ip, imap, shared, &trimmed);
>  	if (error)
> -		goto out_unlock;
> -
> -	end_fsb = orig_end_fsb = imap.br_startoff + imap.br_blockcount;
> +		return error;
>  
>  	/* Not shared?  Just report the (potentially capped) extent. */
> -	if (!shared) {
> -		*skipped = true;
> -		goto done;
> -	}
> +	if (!*shared)
> +		return 0;
>  
>  	/*
>  	 * Fork all the shared blocks from our write offset until the end of
> @@ -279,73 +283,40 @@ __xfs_reflink_reserve_cow(
>  	 */
>  	error = xfs_qm_dqattach_locked(ip, 0);
>  	if (error)
> -		goto out_unlock;
> +		return error;
> +
> +	end_fsb = orig_end_fsb = imap->br_startoff + imap->br_blockcount;
>  
>  	align = xfs_eof_alignment(ip, xfs_get_cowextsz_hint(ip));
>  	if (align)
>  		end_fsb = roundup_64(end_fsb, align);
>  
>  retry:
> -	error = xfs_bmapi_reserve_delalloc(ip, XFS_COW_FORK, *offset_fsb,
> -			end_fsb - *offset_fsb, &got,
> -			&prev, &idx, eof);
> +	error = xfs_bmapi_reserve_delalloc(ip, XFS_COW_FORK, imap->br_startoff,
> +			end_fsb - imap->br_startoff, &got, &prev, &idx, eof);
>  	switch (error) {
>  	case 0:
>  		break;
>  	case -ENOSPC:
>  	case -EDQUOT:
>  		/* retry without any preallocation */
> -		trace_xfs_reflink_cow_enospc(ip, &imap);
> +		trace_xfs_reflink_cow_enospc(ip, imap);
>  		if (end_fsb != orig_end_fsb) {
>  			end_fsb = orig_end_fsb;
>  			goto retry;
>  		}
>  		/*FALLTHRU*/
>  	default:
> -		goto out_unlock;
> +		return error;
>  	}
>  
>  	if (end_fsb != orig_end_fsb)
>  		xfs_inode_set_cowblocks_tag(ip);
>  
>  	trace_xfs_reflink_cow_alloc(ip, &got);
> -done:
> -	*offset_fsb = end_fsb;
> -out_unlock:
> -	return error;
> +	return 0;
>  }
>  
> -/* Create a CoW reservation for part of a file. */
> -int
> -xfs_reflink_reserve_cow_range(
> -	struct xfs_inode	*ip,
> -	xfs_off_t		offset,
> -	xfs_off_t		count)
> -{
> -	struct xfs_mount	*mp = ip->i_mount;
> -	xfs_fileoff_t		offset_fsb, end_fsb;
> -	bool			skipped = false;
> -	int			error;
> -
> -	trace_xfs_reflink_reserve_cow_range(ip, offset, count);
> -
> -	offset_fsb = XFS_B_TO_FSBT(mp, offset);
> -	end_fsb = XFS_B_TO_FSB(mp, offset + count);
> -
> -	xfs_ilock(ip, XFS_ILOCK_EXCL);
> -	while (offset_fsb < end_fsb) {
> -		error = __xfs_reflink_reserve_cow(ip, &offset_fsb, end_fsb,
> -				&skipped);
> -		if (error) {
> -			trace_xfs_reflink_reserve_cow_range_error(ip, error,
> -				_RET_IP_);
> -			break;
> -		}
> -	}
> -	xfs_iunlock(ip, XFS_ILOCK_EXCL);
> -
> -	return error;
> -}
>  
>  /* Allocate all CoW reservations covering a range of blocks in a file. */
>  static int
> @@ -359,9 +330,8 @@ __xfs_reflink_allocate_cow(
>  	struct xfs_defer_ops	dfops;
>  	struct xfs_trans	*tp;
>  	xfs_fsblock_t		first_block;
> -	xfs_fileoff_t		next_fsb;
>  	int			nimaps = 1, error;
> -	bool			skipped = false;
> +	bool			shared;
>  
>  	xfs_defer_init(&dfops, &first_block);
>  
> @@ -372,33 +342,38 @@ __xfs_reflink_allocate_cow(
>  
>  	xfs_ilock(ip, XFS_ILOCK_EXCL);
>  
> -	next_fsb = *offset_fsb;
> -	error = __xfs_reflink_reserve_cow(ip, &next_fsb, end_fsb, &skipped);
> +	/* Read extent from the source file. */
> +	nimaps = 1;
> +	error = xfs_bmapi_read(ip, *offset_fsb, end_fsb - *offset_fsb,
> +			&imap, &nimaps, 0);
> +	if (error)
> +		goto out_unlock;
> +	ASSERT(nimaps == 1);
> +
> +	error = xfs_reflink_reserve_cow(ip, &imap, &shared);
>  	if (error)
>  		goto out_trans_cancel;
>  
> -	if (skipped) {
> -		*offset_fsb = next_fsb;
> +	if (!shared) {
> +		*offset_fsb = imap.br_startoff + imap.br_blockcount;
>  		goto out_trans_cancel;
>  	}
>  
>  	xfs_trans_ijoin(tp, ip, 0);
> -	error = xfs_bmapi_write(tp, ip, *offset_fsb, next_fsb - *offset_fsb,
> +	error = xfs_bmapi_write(tp, ip, imap.br_startoff, imap.br_blockcount,
>  			XFS_BMAPI_COWFORK, &first_block,
>  			XFS_EXTENTADD_SPACE_RES(mp, XFS_DATA_FORK),
>  			&imap, &nimaps, &dfops);
>  	if (error)
>  		goto out_trans_cancel;
>  
> -	/* We might not have been able to map the whole delalloc extent */
> -	*offset_fsb = min(*offset_fsb + imap.br_blockcount, next_fsb);
> -
>  	error = xfs_defer_finish(&tp, &dfops, NULL);
>  	if (error)
>  		goto out_trans_cancel;
>  
>  	error = xfs_trans_commit(tp);
>  
> +	*offset_fsb = imap.br_startoff + imap.br_blockcount;
>  out_unlock:
>  	xfs_iunlock(ip, XFS_ILOCK_EXCL);
>  	return error;
> diff --git a/fs/xfs/xfs_reflink.h b/fs/xfs/xfs_reflink.h
> index 5dc3c8a..9f76924 100644
> --- a/fs/xfs/xfs_reflink.h
> +++ b/fs/xfs/xfs_reflink.h
> @@ -26,8 +26,8 @@ extern int xfs_reflink_find_shared(struct xfs_mount *mp, xfs_agnumber_t agno,
>  extern int xfs_reflink_trim_around_shared(struct xfs_inode *ip,
>  		struct xfs_bmbt_irec *irec, bool *shared, bool *trimmed);
>  
> -extern int xfs_reflink_reserve_cow_range(struct xfs_inode *ip,
> -		xfs_off_t offset, xfs_off_t count);
> +extern int xfs_reflink_reserve_cow(struct xfs_inode *ip,
> +		struct xfs_bmbt_irec *imap, bool *shared);
>  extern int xfs_reflink_allocate_cow_range(struct xfs_inode *ip,
>  		xfs_off_t offset, xfs_off_t count);
>  extern bool xfs_reflink_find_cow_mapping(struct xfs_inode *ip, xfs_off_t offset,
> diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> index ad188d3..72f9f6b 100644
> --- a/fs/xfs/xfs_trace.h
> +++ b/fs/xfs/xfs_trace.h
> @@ -3346,7 +3346,7 @@ DEFINE_INODE_IREC_EVENT(xfs_reflink_cow_alloc);
>  DEFINE_INODE_IREC_EVENT(xfs_reflink_cow_found);
>  DEFINE_INODE_IREC_EVENT(xfs_reflink_cow_enospc);
>  
> -DEFINE_RW_EVENT(xfs_reflink_reserve_cow_range);
> +DEFINE_RW_EVENT(xfs_reflink_reserve_cow);
>  DEFINE_RW_EVENT(xfs_reflink_allocate_cow_range);
>  
>  DEFINE_INODE_IREC_EVENT(xfs_reflink_bounce_dio_write);
> @@ -3358,7 +3358,6 @@ DEFINE_SIMPLE_IO_EVENT(xfs_reflink_end_cow);
>  DEFINE_INODE_IREC_EVENT(xfs_reflink_cow_remap);
>  DEFINE_INODE_IREC_EVENT(xfs_reflink_cow_remap_piece);
>  
> -DEFINE_INODE_ERROR_EVENT(xfs_reflink_reserve_cow_range_error);
>  DEFINE_INODE_ERROR_EVENT(xfs_reflink_allocate_cow_range_error);
>  DEFINE_INODE_ERROR_EVENT(xfs_reflink_cancel_cow_range_error);
>  DEFINE_INODE_ERROR_EVENT(xfs_reflink_end_cow_error);
> -- 
> 2.1.4
> 
> --
> 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

Re: [PATCH 5/9] xfs: optimize writes to reflink files

[Darrick J. Wong]

On Sat, Oct 15, 2016 at 10:52:30AM +0200, Christoph Hellwig wrote:
> Instead of reserving space as the first thing in write_begin move it past
> reading the extent in the data fork.  That way we only have to read from
> the data fork once and can reuse that information for trimming the extent
> to the shared/unshared boundary.  Additionally this allows to easily
> limit the actual write size to said boundary, and avoid a roundtrip on the
> ilock.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

> ---
>  fs/xfs/xfs_iomap.c   |  56 +++++++++++++-------
>  fs/xfs/xfs_reflink.c | 141 +++++++++++++++++++++------------------------------
>  fs/xfs/xfs_reflink.h |   4 +-
>  fs/xfs/xfs_trace.h   |   3 +-
>  4 files changed, 100 insertions(+), 104 deletions(-)
> 
> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> index 1dabf2e..436e109 100644
> --- a/fs/xfs/xfs_iomap.c
> +++ b/fs/xfs/xfs_iomap.c
> @@ -566,6 +566,17 @@ xfs_file_iomap_begin_delay(
>  	xfs_bmap_search_extents(ip, offset_fsb, XFS_DATA_FORK, &eof, &idx,
>  			&got, &prev);
>  	if (!eof && got.br_startoff <= offset_fsb) {
> +		if (xfs_is_reflink_inode(ip)) {
> +			bool		shared;
> +
> +			end_fsb = min(XFS_B_TO_FSB(mp, offset + count),
> +					maxbytes_fsb);
> +			xfs_trim_extent(&got, offset_fsb, end_fsb - offset_fsb);
> +			error = xfs_reflink_reserve_cow(ip, &got, &shared);
> +			if (error)
> +				goto out_unlock;
> +		}
> +
>  		trace_xfs_iomap_found(ip, offset, count, 0, &got);
>  		goto done;
>  	}
> @@ -961,19 +972,13 @@ xfs_file_iomap_begin(
>  	struct xfs_mount	*mp = ip->i_mount;
>  	struct xfs_bmbt_irec	imap;
>  	xfs_fileoff_t		offset_fsb, end_fsb;
> -	bool			shared, trimmed;
>  	int			nimaps = 1, error = 0;
> +	bool			shared = false, trimmed = false;
>  	unsigned		lockmode;
>  
>  	if (XFS_FORCED_SHUTDOWN(mp))
>  		return -EIO;
>  
> -	if ((flags & (IOMAP_WRITE | IOMAP_ZERO)) && xfs_is_reflink_inode(ip)) {
> -		error = xfs_reflink_reserve_cow_range(ip, offset, length);
> -		if (error < 0)
> -			return error;
> -	}
> -
>  	if ((flags & IOMAP_WRITE) && !IS_DAX(inode) &&
>  		   !xfs_get_extsz_hint(ip)) {
>  		/* Reserve delalloc blocks for regular writeback. */
> @@ -981,7 +986,16 @@ xfs_file_iomap_begin(
>  				iomap);
>  	}
>  
> -	lockmode = xfs_ilock_data_map_shared(ip);
> +	/*
> +	 * COW writes will allocate delalloc space, so we need to make sure
> +	 * to take the lock exclusively here.
> +	 */
> +	if ((flags & (IOMAP_WRITE | IOMAP_ZERO)) && xfs_is_reflink_inode(ip)) {
> +		lockmode = XFS_ILOCK_EXCL;
> +		xfs_ilock(ip, XFS_ILOCK_EXCL);
> +	} else {
> +		lockmode = xfs_ilock_data_map_shared(ip);
> +	}
>  
>  	ASSERT(offset <= mp->m_super->s_maxbytes);
>  	if ((xfs_fsize_t)offset + length > mp->m_super->s_maxbytes)
> @@ -991,19 +1005,24 @@ xfs_file_iomap_begin(
>  
>  	error = xfs_bmapi_read(ip, offset_fsb, end_fsb - offset_fsb, &imap,
>  			       &nimaps, 0);
> -	if (error) {
> -		xfs_iunlock(ip, lockmode);
> -		return error;
> -	}
> +	if (error)
> +		goto out_unlock;
>  
> -	if (flags & (IOMAP_WRITE | IOMAP_ZERO | IOMAP_REPORT)) {
> +	if (flags & IOMAP_REPORT) {
>  		/* Trim the mapping to the nearest shared extent boundary. */
>  		error = xfs_reflink_trim_around_shared(ip, &imap, &shared,
>  				&trimmed);
> -		if (error) {
> -			xfs_iunlock(ip, lockmode);
> -			return error;
> -		}
> +		if (error)
> +			goto out_unlock;
> +	}
> +
> +	if ((flags & (IOMAP_WRITE | IOMAP_ZERO)) && xfs_is_reflink_inode(ip)) {
> +		error = xfs_reflink_reserve_cow(ip, &imap, &shared);
> +		if (error)
> +			goto out_unlock;
> +
> +		end_fsb = imap.br_startoff + imap.br_blockcount;
> +		length = XFS_FSB_TO_B(mp, end_fsb) - offset;
>  	}
>  
>  	if ((flags & IOMAP_WRITE) && imap_needs_alloc(inode, &imap, nimaps)) {
> @@ -1042,6 +1061,9 @@ xfs_file_iomap_begin(
>  	if (shared)
>  		iomap->flags |= IOMAP_F_SHARED;
>  	return 0;
> +out_unlock:
> +	xfs_iunlock(ip, lockmode);
> +	return error;
>  }
>  
>  static int
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index 46c824c..5d230ea 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -228,50 +228,54 @@ xfs_reflink_trim_around_shared(
>  	}
>  }
>  
> -/* Create a CoW reservation for a range of blocks within a file. */
> -static int
> -__xfs_reflink_reserve_cow(
> +/*
> + * Trim the passed in imap to the next shared/unshared extent boundary, and
> + * if imap->br_startoff points to a shared extent reserve space for it in the
> + * COW fork.  In this case *shared is set to true, else to false.
> + *
> + * Note that imap will always contain the block numbers for the existing blocks
> + * in the data fork, as the upper layers need them for read-modify-write
> + * operations.
> + */
> +int
> +xfs_reflink_reserve_cow(
>  	struct xfs_inode	*ip,
> -	xfs_fileoff_t		*offset_fsb,
> -	xfs_fileoff_t		end_fsb,
> -	bool			*skipped)
> +	struct xfs_bmbt_irec	*imap,
> +	bool			*shared)
>  {
> -	struct xfs_bmbt_irec	got, prev, imap;
> -	xfs_fileoff_t		orig_end_fsb;
> -	int			nimaps, eof = 0, error = 0;
> -	bool			shared = false, trimmed = false;
> +	struct xfs_bmbt_irec	got, prev;
> +	xfs_fileoff_t		end_fsb, orig_end_fsb;
> +	int			eof = 0, error = 0;
> +	bool			trimmed;
>  	xfs_extnum_t		idx;
>  	xfs_extlen_t		align;
>  
> -	/* Already reserved?  Skip the refcount btree access. */
> -	xfs_bmap_search_extents(ip, *offset_fsb, XFS_COW_FORK, &eof, &idx,
> +	/*
> +	 * Search the COW fork extent list first.  This serves two purposes:
> +	 * first this implement the speculative preallocation using cowextisze,
> +	 * so that we also unshared block adjacent to shared blocks instead
> +	 * of just the shared blocks themselves.  Second the lookup in the
> +	 * extent list is generally faster than going out to the shared extent
> +	 * tree.
> +	 */
> +	xfs_bmap_search_extents(ip, imap->br_startoff, XFS_COW_FORK, &eof, &idx,
>  			&got, &prev);
> -	if (!eof && got.br_startoff <= *offset_fsb) {
> -		end_fsb = orig_end_fsb = got.br_startoff + got.br_blockcount;
> -		trace_xfs_reflink_cow_found(ip, &got);
> -		goto done;
> -	}
> +	if (!eof && got.br_startoff <= imap->br_startoff) {
> +		trace_xfs_reflink_cow_found(ip, imap);
> +		xfs_trim_extent(imap, got.br_startoff, got.br_blockcount);
>  
> -	/* Read extent from the source file. */
> -	nimaps = 1;
> -	error = xfs_bmapi_read(ip, *offset_fsb, end_fsb - *offset_fsb,
> -			&imap, &nimaps, 0);
> -	if (error)
> -		goto out_unlock;
> -	ASSERT(nimaps == 1);
> +		*shared = true;
> +		return 0;
> +	}
>  
>  	/* Trim the mapping to the nearest shared extent boundary. */
> -	error = xfs_reflink_trim_around_shared(ip, &imap, &shared, &trimmed);
> +	error = xfs_reflink_trim_around_shared(ip, imap, shared, &trimmed);
>  	if (error)
> -		goto out_unlock;
> -
> -	end_fsb = orig_end_fsb = imap.br_startoff + imap.br_blockcount;
> +		return error;
>  
>  	/* Not shared?  Just report the (potentially capped) extent. */
> -	if (!shared) {
> -		*skipped = true;
> -		goto done;
> -	}
> +	if (!*shared)
> +		return 0;
>  
>  	/*
>  	 * Fork all the shared blocks from our write offset until the end of
> @@ -279,73 +283,40 @@ __xfs_reflink_reserve_cow(
>  	 */
>  	error = xfs_qm_dqattach_locked(ip, 0);
>  	if (error)
> -		goto out_unlock;
> +		return error;
> +
> +	end_fsb = orig_end_fsb = imap->br_startoff + imap->br_blockcount;
>  
>  	align = xfs_eof_alignment(ip, xfs_get_cowextsz_hint(ip));
>  	if (align)
>  		end_fsb = roundup_64(end_fsb, align);
>  
>  retry:
> -	error = xfs_bmapi_reserve_delalloc(ip, XFS_COW_FORK, *offset_fsb,
> -			end_fsb - *offset_fsb, &got,
> -			&prev, &idx, eof);
> +	error = xfs_bmapi_reserve_delalloc(ip, XFS_COW_FORK, imap->br_startoff,
> +			end_fsb - imap->br_startoff, &got, &prev, &idx, eof);
>  	switch (error) {
>  	case 0:
>  		break;
>  	case -ENOSPC:
>  	case -EDQUOT:
>  		/* retry without any preallocation */
> -		trace_xfs_reflink_cow_enospc(ip, &imap);
> +		trace_xfs_reflink_cow_enospc(ip, imap);
>  		if (end_fsb != orig_end_fsb) {
>  			end_fsb = orig_end_fsb;
>  			goto retry;
>  		}
>  		/*FALLTHRU*/
>  	default:
> -		goto out_unlock;
> +		return error;
>  	}
>  
>  	if (end_fsb != orig_end_fsb)
>  		xfs_inode_set_cowblocks_tag(ip);
>  
>  	trace_xfs_reflink_cow_alloc(ip, &got);
> -done:
> -	*offset_fsb = end_fsb;
> -out_unlock:
> -	return error;
> +	return 0;
>  }
>  
> -/* Create a CoW reservation for part of a file. */
> -int
> -xfs_reflink_reserve_cow_range(
> -	struct xfs_inode	*ip,
> -	xfs_off_t		offset,
> -	xfs_off_t		count)
> -{
> -	struct xfs_mount	*mp = ip->i_mount;
> -	xfs_fileoff_t		offset_fsb, end_fsb;
> -	bool			skipped = false;
> -	int			error;
> -
> -	trace_xfs_reflink_reserve_cow_range(ip, offset, count);
> -
> -	offset_fsb = XFS_B_TO_FSBT(mp, offset);
> -	end_fsb = XFS_B_TO_FSB(mp, offset + count);
> -
> -	xfs_ilock(ip, XFS_ILOCK_EXCL);
> -	while (offset_fsb < end_fsb) {
> -		error = __xfs_reflink_reserve_cow(ip, &offset_fsb, end_fsb,
> -				&skipped);
> -		if (error) {
> -			trace_xfs_reflink_reserve_cow_range_error(ip, error,
> -				_RET_IP_);
> -			break;
> -		}
> -	}
> -	xfs_iunlock(ip, XFS_ILOCK_EXCL);
> -
> -	return error;
> -}
>  
>  /* Allocate all CoW reservations covering a range of blocks in a file. */
>  static int
> @@ -359,9 +330,8 @@ __xfs_reflink_allocate_cow(
>  	struct xfs_defer_ops	dfops;
>  	struct xfs_trans	*tp;
>  	xfs_fsblock_t		first_block;
> -	xfs_fileoff_t		next_fsb;
>  	int			nimaps = 1, error;
> -	bool			skipped = false;
> +	bool			shared;
>  
>  	xfs_defer_init(&dfops, &first_block);
>  
> @@ -372,33 +342,38 @@ __xfs_reflink_allocate_cow(
>  
>  	xfs_ilock(ip, XFS_ILOCK_EXCL);
>  
> -	next_fsb = *offset_fsb;
> -	error = __xfs_reflink_reserve_cow(ip, &next_fsb, end_fsb, &skipped);
> +	/* Read extent from the source file. */
> +	nimaps = 1;
> +	error = xfs_bmapi_read(ip, *offset_fsb, end_fsb - *offset_fsb,
> +			&imap, &nimaps, 0);
> +	if (error)
> +		goto out_unlock;
> +	ASSERT(nimaps == 1);
> +
> +	error = xfs_reflink_reserve_cow(ip, &imap, &shared);
>  	if (error)
>  		goto out_trans_cancel;
>  
> -	if (skipped) {
> -		*offset_fsb = next_fsb;
> +	if (!shared) {
> +		*offset_fsb = imap.br_startoff + imap.br_blockcount;
>  		goto out_trans_cancel;
>  	}
>  
>  	xfs_trans_ijoin(tp, ip, 0);
> -	error = xfs_bmapi_write(tp, ip, *offset_fsb, next_fsb - *offset_fsb,
> +	error = xfs_bmapi_write(tp, ip, imap.br_startoff, imap.br_blockcount,
>  			XFS_BMAPI_COWFORK, &first_block,
>  			XFS_EXTENTADD_SPACE_RES(mp, XFS_DATA_FORK),
>  			&imap, &nimaps, &dfops);
>  	if (error)
>  		goto out_trans_cancel;
>  
> -	/* We might not have been able to map the whole delalloc extent */
> -	*offset_fsb = min(*offset_fsb + imap.br_blockcount, next_fsb);
> -
>  	error = xfs_defer_finish(&tp, &dfops, NULL);
>  	if (error)
>  		goto out_trans_cancel;
>  
>  	error = xfs_trans_commit(tp);
>  
> +	*offset_fsb = imap.br_startoff + imap.br_blockcount;
>  out_unlock:
>  	xfs_iunlock(ip, XFS_ILOCK_EXCL);
>  	return error;
> diff --git a/fs/xfs/xfs_reflink.h b/fs/xfs/xfs_reflink.h
> index 5dc3c8a..9f76924 100644
> --- a/fs/xfs/xfs_reflink.h
> +++ b/fs/xfs/xfs_reflink.h
> @@ -26,8 +26,8 @@ extern int xfs_reflink_find_shared(struct xfs_mount *mp, xfs_agnumber_t agno,
>  extern int xfs_reflink_trim_around_shared(struct xfs_inode *ip,
>  		struct xfs_bmbt_irec *irec, bool *shared, bool *trimmed);
>  
> -extern int xfs_reflink_reserve_cow_range(struct xfs_inode *ip,
> -		xfs_off_t offset, xfs_off_t count);
> +extern int xfs_reflink_reserve_cow(struct xfs_inode *ip,
> +		struct xfs_bmbt_irec *imap, bool *shared);
>  extern int xfs_reflink_allocate_cow_range(struct xfs_inode *ip,
>  		xfs_off_t offset, xfs_off_t count);
>  extern bool xfs_reflink_find_cow_mapping(struct xfs_inode *ip, xfs_off_t offset,
> diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> index ad188d3..72f9f6b 100644
> --- a/fs/xfs/xfs_trace.h
> +++ b/fs/xfs/xfs_trace.h
> @@ -3346,7 +3346,7 @@ DEFINE_INODE_IREC_EVENT(xfs_reflink_cow_alloc);
>  DEFINE_INODE_IREC_EVENT(xfs_reflink_cow_found);
>  DEFINE_INODE_IREC_EVENT(xfs_reflink_cow_enospc);
>  
> -DEFINE_RW_EVENT(xfs_reflink_reserve_cow_range);
> +DEFINE_RW_EVENT(xfs_reflink_reserve_cow);
>  DEFINE_RW_EVENT(xfs_reflink_allocate_cow_range);
>  
>  DEFINE_INODE_IREC_EVENT(xfs_reflink_bounce_dio_write);
> @@ -3358,7 +3358,6 @@ DEFINE_SIMPLE_IO_EVENT(xfs_reflink_end_cow);
>  DEFINE_INODE_IREC_EVENT(xfs_reflink_cow_remap);
>  DEFINE_INODE_IREC_EVENT(xfs_reflink_cow_remap_piece);
>  
> -DEFINE_INODE_ERROR_EVENT(xfs_reflink_reserve_cow_range_error);
>  DEFINE_INODE_ERROR_EVENT(xfs_reflink_allocate_cow_range_error);
>  DEFINE_INODE_ERROR_EVENT(xfs_reflink_cancel_cow_range_error);
>  DEFINE_INODE_ERROR_EVENT(xfs_reflink_end_cow_error);
> -- 
> 2.1.4
> 

[PATCH 6/9] xfs: refactor xfs_bunmapi_cow

[Christoph Hellwig]

Split out two helpers for deleting delayed or real extents from the COW fork.
This allows to call them directly from xfs_reflink_cow_end_io once that
function is refactored to iterate the extent tree.  It will also allow
to reuse the delalloc deletion from xfs_bunmapi in the future.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/libxfs/xfs_bmap.c | 374 ++++++++++++++++++++++++++++-------------------
 fs/xfs/libxfs/xfs_bmap.h |   5 +
 fs/xfs/xfs_reflink.c     |   5 -
 3 files changed, 225 insertions(+), 159 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 016dacc..64f82c8 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -4862,6 +4862,219 @@ xfs_bmap_split_indlen(
 	return stolen;
 }
 
+int
+xfs_bmap_del_extent_delay(
+	struct xfs_inode	*ip,
+	int			whichfork,
+	xfs_extnum_t		*idx,
+	struct xfs_bmbt_irec	*got,
+	struct xfs_bmbt_irec	*del)
+{
+	struct xfs_mount	*mp = ip->i_mount;
+	struct xfs_ifork	*ifp = XFS_IFORK_PTR(ip, whichfork);
+	struct xfs_bmbt_irec	new;
+	int64_t			da_old, da_new, da_diff = 0;
+	xfs_fileoff_t		del_endoff, got_endoff;
+	xfs_filblks_t		got_indlen, new_indlen, stolen;
+	int			error = 0, state = 0;
+	bool			isrt;
+
+	XFS_STATS_INC(mp, xs_del_exlist);
+
+	isrt = (whichfork == XFS_DATA_FORK) && XFS_IS_REALTIME_INODE(ip);
+	del_endoff = del->br_startoff + del->br_blockcount;
+	got_endoff = got->br_startoff + got->br_blockcount;
+	da_old = startblockval(got->br_startblock);
+	da_new = 0;
+
+	ASSERT(*idx >= 0);
+	ASSERT(*idx < ifp->if_bytes / sizeof(struct xfs_bmbt_rec));
+	ASSERT(del->br_blockcount > 0);
+	ASSERT(got->br_startoff <= del->br_startoff);
+	ASSERT(got_endoff >= del_endoff);
+
+	if (isrt) {
+		int64_t rtexts = XFS_FSB_TO_B(mp, del->br_blockcount);
+
+		do_div(rtexts, mp->m_sb.sb_rextsize);
+		xfs_mod_frextents(mp, rtexts);
+	}
+
+	/*
+	 * Update the inode delalloc counter now and wait to update the
+	 * sb counters as we might have to borrow some blocks for the
+	 * indirect block accounting.
+	 */
+	xfs_trans_reserve_quota_nblks(NULL, ip, -((long)del->br_blockcount), 0,
+			isrt ? XFS_QMOPT_RES_RTBLKS : XFS_QMOPT_RES_REGBLKS);
+	ip->i_delayed_blks -= del->br_blockcount;
+
+	if (whichfork == XFS_COW_FORK)
+		state |= BMAP_COWFORK;
+
+	if (got->br_startoff == del->br_startoff)
+		state |= BMAP_LEFT_CONTIG;
+	if (got_endoff == del_endoff)
+		state |= BMAP_RIGHT_CONTIG;
+
+	switch (state & (BMAP_LEFT_CONTIG | BMAP_RIGHT_CONTIG)) {
+	case BMAP_LEFT_CONTIG | BMAP_RIGHT_CONTIG:
+		/*
+		 * Matches the whole extent.  Delete the entry.
+		 */
+		xfs_iext_remove(ip, *idx, 1, state);
+		--*idx;
+		break;
+	case BMAP_LEFT_CONTIG:
+		/*
+		 * Deleting the first part of the extent.
+		 */
+		trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
+		got->br_startoff = del_endoff;
+		got->br_blockcount -= del->br_blockcount;
+		da_new = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(ip,
+				got->br_blockcount), da_old);
+		got->br_startblock = nullstartblock((int)da_new);
+		xfs_bmbt_set_all(xfs_iext_get_ext(ifp, *idx), got);
+		trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
+		break;
+	case BMAP_RIGHT_CONTIG:
+		/*
+		 * Deleting the last part of the extent.
+		 */
+		trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
+		got->br_blockcount = got->br_blockcount - del->br_blockcount;
+		da_new = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(ip,
+				got->br_blockcount), da_old);
+		got->br_startblock = nullstartblock((int)da_new);
+		xfs_bmbt_set_all(xfs_iext_get_ext(ifp, *idx), got);
+		trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
+		break;
+	case 0:
+		/*
+		 * Deleting the middle of the extent.
+		 *
+		 * Distribute the original indlen reservation across the two new
+		 * extents.  Steal blocks from the deleted extent if necessary.
+		 * Stealing blocks simply fudges the fdblocks accounting below.
+		 * Warn if either of the new indlen reservations is zero as this
+		 * can lead to delalloc problems.
+		 */
+		trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
+
+		got->br_blockcount = del->br_startoff - got->br_startoff;
+		got_indlen = xfs_bmap_worst_indlen(ip, got->br_blockcount);
+
+		new.br_blockcount = got_endoff - del_endoff;
+		new_indlen = xfs_bmap_worst_indlen(ip, new.br_blockcount);
+
+		WARN_ON_ONCE(!got_indlen || !new_indlen);
+		stolen = xfs_bmap_split_indlen(da_old, &got_indlen, &new_indlen,
+						       del->br_blockcount);
+
+		got->br_startblock = nullstartblock((int)got_indlen);
+		xfs_bmbt_set_all(xfs_iext_get_ext(ifp, *idx), got);
+		trace_xfs_bmap_post_update(ip, *idx, 0, _THIS_IP_);
+
+		new.br_startoff = del_endoff;
+		new.br_state = got->br_state;
+		new.br_startblock = nullstartblock((int)new_indlen);
+
+		++*idx;
+		xfs_iext_insert(ip, *idx, 1, &new, state);
+
+		da_new = got_indlen + new_indlen - stolen;
+		del->br_blockcount -= stolen;
+		break;
+	}
+
+	ASSERT(da_old >= da_new);
+	da_diff = da_old - da_new;
+	if (!isrt)
+		da_diff += del->br_blockcount;
+	if (da_diff)
+		xfs_mod_fdblocks(mp, da_diff, false);
+	return error;
+}
+
+void
+xfs_bmap_del_extent_cow(
+	struct xfs_inode	*ip,
+	xfs_extnum_t		*idx,
+	struct xfs_bmbt_irec	*got,
+	struct xfs_bmbt_irec	*del)
+{
+	struct xfs_mount	*mp = ip->i_mount;
+	struct xfs_ifork	*ifp = XFS_IFORK_PTR(ip, XFS_COW_FORK);
+	struct xfs_bmbt_irec	new;
+	xfs_fileoff_t		del_endoff, got_endoff;
+	int			state = BMAP_COWFORK;
+
+	XFS_STATS_INC(mp, xs_del_exlist);
+
+	del_endoff = del->br_startoff + del->br_blockcount;
+	got_endoff = got->br_startoff + got->br_blockcount;
+
+	ASSERT(*idx >= 0);
+	ASSERT(*idx < ifp->if_bytes / sizeof(struct xfs_bmbt_rec));
+	ASSERT(del->br_blockcount > 0);
+	ASSERT(got->br_startoff <= del->br_startoff);
+	ASSERT(got_endoff >= del_endoff);
+	ASSERT(!isnullstartblock(got->br_startblock));
+
+	if (got->br_startoff == del->br_startoff)
+		state |= BMAP_LEFT_CONTIG;
+	if (got_endoff == del_endoff)
+		state |= BMAP_RIGHT_CONTIG;
+
+	switch (state & (BMAP_LEFT_CONTIG | BMAP_RIGHT_CONTIG)) {
+	case BMAP_LEFT_CONTIG | BMAP_RIGHT_CONTIG:
+		/*
+		 * Matches the whole extent.  Delete the entry.
+		 */
+		xfs_iext_remove(ip, *idx, 1, state);
+		--*idx;
+		break;
+	case BMAP_LEFT_CONTIG:
+		/*
+		 * Deleting the first part of the extent.
+		 */
+		trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
+		got->br_startoff = del_endoff;
+		got->br_blockcount -= del->br_blockcount;
+		got->br_startblock = del->br_startblock + del->br_blockcount;
+		xfs_bmbt_set_all(xfs_iext_get_ext(ifp, *idx), got);
+		trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
+		break;
+	case BMAP_RIGHT_CONTIG:
+		/*
+		 * Deleting the last part of the extent.
+		 */
+		trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
+		got->br_blockcount -= del->br_blockcount;
+		xfs_bmbt_set_all(xfs_iext_get_ext(ifp, *idx), got);
+		trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
+		break;
+	case 0:
+		/*
+		 * Deleting the middle of the extent.
+		 */
+		trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
+		got->br_blockcount = del->br_startoff - got->br_startoff;
+		xfs_bmbt_set_all(xfs_iext_get_ext(ifp, *idx), got);
+		trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
+
+		new.br_startoff = del_endoff;
+		new.br_blockcount = got_endoff - del_endoff;
+		new.br_state = got->br_state;
+		new.br_startblock = del->br_startblock + del->br_blockcount;
+
+		++*idx;
+		xfs_iext_insert(ip, *idx, 1, &new, state);
+		break;
+	}
+}
+
 /*
  * Called by xfs_bmapi to update file extent records and the btree
  * after removing space (or undoing a delayed allocation).
@@ -5210,167 +5423,20 @@ xfs_bunmapi_cow(
 	struct xfs_inode		*ip,
 	struct xfs_bmbt_irec		*del)
 {
-	xfs_filblks_t			da_new;
-	xfs_filblks_t			da_old;
-	xfs_fsblock_t			del_endblock = 0;
-	xfs_fileoff_t			del_endoff;
-	int				delay;
 	struct xfs_bmbt_rec_host	*ep;
-	int				error;
 	struct xfs_bmbt_irec		got;
-	xfs_fileoff_t			got_endoff;
-	struct xfs_ifork		*ifp;
-	struct xfs_mount		*mp;
-	xfs_filblks_t			nblks;
 	struct xfs_bmbt_irec		new;
-	/* REFERENCED */
-	uint				qfield;
-	xfs_filblks_t			temp;
-	xfs_filblks_t			temp2;
-	int				state = BMAP_COWFORK;
 	int				eof;
 	xfs_extnum_t			eidx;
 
-	mp = ip->i_mount;
-	XFS_STATS_INC(mp, xs_del_exlist);
-
 	ep = xfs_bmap_search_extents(ip, del->br_startoff, XFS_COW_FORK, &eof,
-			&eidx, &got, &new);
-
-	ifp = XFS_IFORK_PTR(ip, XFS_COW_FORK); ifp = ifp;
-	ASSERT((eidx >= 0) && (eidx < ifp->if_bytes /
-		(uint)sizeof(xfs_bmbt_rec_t)));
-	ASSERT(del->br_blockcount > 0);
-	ASSERT(got.br_startoff <= del->br_startoff);
-	del_endoff = del->br_startoff + del->br_blockcount;
-	got_endoff = got.br_startoff + got.br_blockcount;
-	ASSERT(got_endoff >= del_endoff);
-	delay = isnullstartblock(got.br_startblock);
-	ASSERT(isnullstartblock(del->br_startblock) == delay);
-	qfield = 0;
-	error = 0;
-	/*
-	 * If deleting a real allocation, must free up the disk space.
-	 */
-	if (!delay) {
-		nblks = del->br_blockcount;
-		qfield = XFS_TRANS_DQ_BCOUNT;
-		/*
-		 * Set up del_endblock and cur for later.
-		 */
-		del_endblock = del->br_startblock + del->br_blockcount;
-		da_old = da_new = 0;
-	} else {
-		da_old = startblockval(got.br_startblock);
-		da_new = 0;
-		nblks = 0;
-	}
-	qfield = qfield;
-	nblks = nblks;
-
-	/*
-	 * Set flag value to use in switch statement.
-	 * Left-contig is 2, right-contig is 1.
-	 */
-	switch (((got.br_startoff == del->br_startoff) << 1) |
-		(got_endoff == del_endoff)) {
-	case 3:
-		/*
-		 * Matches the whole extent.  Delete the entry.
-		 */
-		xfs_iext_remove(ip, eidx, 1, BMAP_COWFORK);
-		--eidx;
-		break;
-
-	case 2:
-		/*
-		 * Deleting the first part of the extent.
-		 */
-		trace_xfs_bmap_pre_update(ip, eidx, state, _THIS_IP_);
-		xfs_bmbt_set_startoff(ep, del_endoff);
-		temp = got.br_blockcount - del->br_blockcount;
-		xfs_bmbt_set_blockcount(ep, temp);
-		if (delay) {
-			temp = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(ip, temp),
-				da_old);
-			xfs_bmbt_set_startblock(ep, nullstartblock((int)temp));
-			trace_xfs_bmap_post_update(ip, eidx, state, _THIS_IP_);
-			da_new = temp;
-			break;
-		}
-		xfs_bmbt_set_startblock(ep, del_endblock);
-		trace_xfs_bmap_post_update(ip, eidx, state, _THIS_IP_);
-		break;
-
-	case 1:
-		/*
-		 * Deleting the last part of the extent.
-		 */
-		temp = got.br_blockcount - del->br_blockcount;
-		trace_xfs_bmap_pre_update(ip, eidx, state, _THIS_IP_);
-		xfs_bmbt_set_blockcount(ep, temp);
-		if (delay) {
-			temp = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(ip, temp),
-				da_old);
-			xfs_bmbt_set_startblock(ep, nullstartblock((int)temp));
-			trace_xfs_bmap_post_update(ip, eidx, state, _THIS_IP_);
-			da_new = temp;
-			break;
-		}
-		trace_xfs_bmap_post_update(ip, eidx, state, _THIS_IP_);
-		break;
-
-	case 0:
-		/*
-		 * Deleting the middle of the extent.
-		 */
-		temp = del->br_startoff - got.br_startoff;
-		trace_xfs_bmap_pre_update(ip, eidx, state, _THIS_IP_);
-		xfs_bmbt_set_blockcount(ep, temp);
-		new.br_startoff = del_endoff;
-		temp2 = got_endoff - del_endoff;
-		new.br_blockcount = temp2;
-		new.br_state = got.br_state;
-		if (!delay) {
-			new.br_startblock = del_endblock;
-		} else {
-			temp = xfs_bmap_worst_indlen(ip, temp);
-			xfs_bmbt_set_startblock(ep, nullstartblock((int)temp));
-			temp2 = xfs_bmap_worst_indlen(ip, temp2);
-			new.br_startblock = nullstartblock((int)temp2);
-			da_new = temp + temp2;
-			while (da_new > da_old) {
-				if (temp) {
-					temp--;
-					da_new--;
-					xfs_bmbt_set_startblock(ep,
-						nullstartblock((int)temp));
-				}
-				if (da_new == da_old)
-					break;
-				if (temp2) {
-					temp2--;
-					da_new--;
-					new.br_startblock =
-						nullstartblock((int)temp2);
-				}
-			}
-		}
-		trace_xfs_bmap_post_update(ip, eidx, state, _THIS_IP_);
-		xfs_iext_insert(ip, eidx + 1, 1, &new, state);
-		++eidx;
-		break;
-	}
-
-	/*
-	 * Account for change in delayed indirect blocks.
-	 * Nothing to do for disk quota accounting here.
-	 */
-	ASSERT(da_old >= da_new);
-	if (da_old > da_new)
-		xfs_mod_fdblocks(mp, (int64_t)(da_old - da_new), false);
-
-	return error;
+				&eidx, &got, &new);
+	ASSERT(ep);
+	if (isnullstartblock(got.br_startblock))
+		xfs_bmap_del_extent_delay(ip, XFS_COW_FORK, &eidx, &got, del);
+	else
+		xfs_bmap_del_extent_cow(ip, &eidx, &got, del);
+	return 0;
 }
 
 /*
diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
index eb86af0..5f18248 100644
--- a/fs/xfs/libxfs/xfs_bmap.h
+++ b/fs/xfs/libxfs/xfs_bmap.h
@@ -224,6 +224,11 @@ int	xfs_bunmapi(struct xfs_trans *tp, struct xfs_inode *ip,
 		xfs_extnum_t nexts, xfs_fsblock_t *firstblock,
 		struct xfs_defer_ops *dfops, int *done);
 int	xfs_bunmapi_cow(struct xfs_inode *ip, struct xfs_bmbt_irec *del);
+int	xfs_bmap_del_extent_delay(struct xfs_inode *ip, int whichfork,
+		xfs_extnum_t *idx, struct xfs_bmbt_irec *got,
+		struct xfs_bmbt_irec *del);
+void	xfs_bmap_del_extent_cow(struct xfs_inode *ip, xfs_extnum_t *idx,
+		struct xfs_bmbt_irec *got, struct xfs_bmbt_irec *del);
 int	xfs_check_nostate_extents(struct xfs_ifork *ifp, xfs_extnum_t idx,
 		xfs_extnum_t num);
 uint	xfs_default_attroffset(struct xfs_inode *ip);
diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index 5d230ea..f33f737 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -535,11 +535,6 @@ xfs_reflink_cancel_cow_blocks(
 		trace_xfs_reflink_cancel_cow(ip, &irec);
 
 		if (irec.br_startblock == DELAYSTARTBLOCK) {
-			/* Free a delayed allocation. */
-			xfs_mod_fdblocks(ip->i_mount, irec.br_blockcount,
-					false);
-			ip->i_delayed_blks -= irec.br_blockcount;
-
 			/* Remove the mapping from the CoW fork. */
 			error = xfs_bunmapi_cow(ip, &irec);
 			if (error)
-- 
2.1.4

Re: [PATCH 6/9] xfs: refactor xfs_bunmapi_cow

[Brian Foster]

On Sat, Oct 15, 2016 at 10:52:31AM +0200, Christoph Hellwig wrote:
> Split out two helpers for deleting delayed or real extents from the COW fork.
> This allows to call them directly from xfs_reflink_cow_end_io once that
> function is refactored to iterate the extent tree.  It will also allow
> to reuse the delalloc deletion from xfs_bunmapi in the future.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Brian Foster <bfoster@redhat.com>
> ---
>  fs/xfs/libxfs/xfs_bmap.c | 374 ++++++++++++++++++++++++++++-------------------
>  fs/xfs/libxfs/xfs_bmap.h |   5 +
>  fs/xfs/xfs_reflink.c     |   5 -
>  3 files changed, 225 insertions(+), 159 deletions(-)
> 
...
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index 5d230ea..f33f737 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -535,11 +535,6 @@ xfs_reflink_cancel_cow_blocks(
>  		trace_xfs_reflink_cancel_cow(ip, &irec);
>  
>  		if (irec.br_startblock == DELAYSTARTBLOCK) {
> -			/* Free a delayed allocation. */
> -			xfs_mod_fdblocks(ip->i_mount, irec.br_blockcount,
> -					false);

Don't we still need this call, or am I missing where it exists
elsewhere? Also, IIRC we might need to make sure this occurs after
xfs_bunmapi_cow() since the latter can steal blocks from the deleted
extent for indirect blocks (e.g., where we call
xfs_bmap_split_indlen()).

Brian

> -			ip->i_delayed_blks -= irec.br_blockcount;
> -
>  			/* Remove the mapping from the CoW fork. */
>  			error = xfs_bunmapi_cow(ip, &irec);
>  			if (error)
> -- 
> 2.1.4
> 
> --
> 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

Re: [PATCH 6/9] xfs: refactor xfs_bunmapi_cow

[Christoph Hellwig]

On Mon, Oct 17, 2016 at 01:21:03PM -0400, Brian Foster wrote:
> On Sat, Oct 15, 2016 at 10:52:31AM +0200, Christoph Hellwig wrote:
> > Split out two helpers for deleting delayed or real extents from the COW fork.
> > This allows to call them directly from xfs_reflink_cow_end_io once that
> > function is refactored to iterate the extent tree.  It will also allow
> > to reuse the delalloc deletion from xfs_bunmapi in the future.
> > 
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > Reviewed-by: Brian Foster <bfoster@redhat.com>
> > ---
> >  fs/xfs/libxfs/xfs_bmap.c | 374 ++++++++++++++++++++++++++++-------------------
> >  fs/xfs/libxfs/xfs_bmap.h |   5 +
> >  fs/xfs/xfs_reflink.c     |   5 -
> >  3 files changed, 225 insertions(+), 159 deletions(-)
> > 
> ...
> > diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> > index 5d230ea..f33f737 100644
> > --- a/fs/xfs/xfs_reflink.c
> > +++ b/fs/xfs/xfs_reflink.c
> > @@ -535,11 +535,6 @@ xfs_reflink_cancel_cow_blocks(
> >  		trace_xfs_reflink_cancel_cow(ip, &irec);
> >  
> >  		if (irec.br_startblock == DELAYSTARTBLOCK) {
> > -			/* Free a delayed allocation. */
> > -			xfs_mod_fdblocks(ip->i_mount, irec.br_blockcount,
> > -					false);
> 
> Don't we still need this call, or am I missing where it exists
> elsewhere? Also, IIRC we might need to make sure this occurs after
> xfs_bunmapi_cow() since the latter can steal blocks from the deleted
> extent for indirect blocks (e.g., where we call
> xfs_bmap_split_indlen()).

xfs_bmap_del_extent_delay is now taking care of this:

+       ASSERT(da_old >= da_new);
+       da_diff = da_old - da_new;
+       if (!isrt)
+               da_diff += del->br_blockcount;
+       if (da_diff)
+               xfs_mod_fdblocks(mp, da_diff, false);
+       return error;

Note that the old code also was buggy if for some reason
the cancellation was for a partial cow extent.

Re: [PATCH 6/9] xfs: refactor xfs_bunmapi_cow

[Brian Foster]

On Mon, Oct 17, 2016 at 07:44:11PM +0200, Christoph Hellwig wrote:
> On Mon, Oct 17, 2016 at 01:21:03PM -0400, Brian Foster wrote:
> > On Sat, Oct 15, 2016 at 10:52:31AM +0200, Christoph Hellwig wrote:
> > > Split out two helpers for deleting delayed or real extents from the COW fork.
> > > This allows to call them directly from xfs_reflink_cow_end_io once that
> > > function is refactored to iterate the extent tree.  It will also allow
> > > to reuse the delalloc deletion from xfs_bunmapi in the future.
> > > 
> > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > > Reviewed-by: Brian Foster <bfoster@redhat.com>
> > > ---
> > >  fs/xfs/libxfs/xfs_bmap.c | 374 ++++++++++++++++++++++++++++-------------------
> > >  fs/xfs/libxfs/xfs_bmap.h |   5 +
> > >  fs/xfs/xfs_reflink.c     |   5 -
> > >  3 files changed, 225 insertions(+), 159 deletions(-)
> > > 
> > ...
> > > diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> > > index 5d230ea..f33f737 100644
> > > --- a/fs/xfs/xfs_reflink.c
> > > +++ b/fs/xfs/xfs_reflink.c
> > > @@ -535,11 +535,6 @@ xfs_reflink_cancel_cow_blocks(
> > >  		trace_xfs_reflink_cancel_cow(ip, &irec);
> > >  
> > >  		if (irec.br_startblock == DELAYSTARTBLOCK) {
> > > -			/* Free a delayed allocation. */
> > > -			xfs_mod_fdblocks(ip->i_mount, irec.br_blockcount,
> > > -					false);
> > 
> > Don't we still need this call, or am I missing where it exists
> > elsewhere? Also, IIRC we might need to make sure this occurs after
> > xfs_bunmapi_cow() since the latter can steal blocks from the deleted
> > extent for indirect blocks (e.g., where we call
> > xfs_bmap_split_indlen()).
> 
> xfs_bmap_del_extent_delay is now taking care of this:
> 
> +       ASSERT(da_old >= da_new);
> +       da_diff = da_old - da_new;
> +       if (!isrt)
> +               da_diff += del->br_blockcount;
> +       if (da_diff)
> +               xfs_mod_fdblocks(mp, da_diff, false);
> +       return error;
> 
> Note that the old code also was buggy if for some reason
> the cancellation was for a partial cow extent.

Ah, missed that. Thanks.

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

> --
> 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

Re: [PATCH 6/9] xfs: refactor xfs_bunmapi_cow

[Darrick J. Wong]

On Sat, Oct 15, 2016 at 10:52:31AM +0200, Christoph Hellwig wrote:
> Split out two helpers for deleting delayed or real extents from the COW fork.
> This allows to call them directly from xfs_reflink_cow_end_io once that
> function is refactored to iterate the extent tree.  It will also allow
> to reuse the delalloc deletion from xfs_bunmapi in the future.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Brian Foster <bfoster@redhat.com>

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

> ---
>  fs/xfs/libxfs/xfs_bmap.c | 374 ++++++++++++++++++++++++++++-------------------
>  fs/xfs/libxfs/xfs_bmap.h |   5 +
>  fs/xfs/xfs_reflink.c     |   5 -
>  3 files changed, 225 insertions(+), 159 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 016dacc..64f82c8 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -4862,6 +4862,219 @@ xfs_bmap_split_indlen(
>  	return stolen;
>  }
>  
> +int
> +xfs_bmap_del_extent_delay(
> +	struct xfs_inode	*ip,
> +	int			whichfork,
> +	xfs_extnum_t		*idx,
> +	struct xfs_bmbt_irec	*got,
> +	struct xfs_bmbt_irec	*del)
> +{
> +	struct xfs_mount	*mp = ip->i_mount;
> +	struct xfs_ifork	*ifp = XFS_IFORK_PTR(ip, whichfork);
> +	struct xfs_bmbt_irec	new;
> +	int64_t			da_old, da_new, da_diff = 0;
> +	xfs_fileoff_t		del_endoff, got_endoff;
> +	xfs_filblks_t		got_indlen, new_indlen, stolen;
> +	int			error = 0, state = 0;
> +	bool			isrt;
> +
> +	XFS_STATS_INC(mp, xs_del_exlist);
> +
> +	isrt = (whichfork == XFS_DATA_FORK) && XFS_IS_REALTIME_INODE(ip);
> +	del_endoff = del->br_startoff + del->br_blockcount;
> +	got_endoff = got->br_startoff + got->br_blockcount;
> +	da_old = startblockval(got->br_startblock);
> +	da_new = 0;
> +
> +	ASSERT(*idx >= 0);
> +	ASSERT(*idx < ifp->if_bytes / sizeof(struct xfs_bmbt_rec));
> +	ASSERT(del->br_blockcount > 0);
> +	ASSERT(got->br_startoff <= del->br_startoff);
> +	ASSERT(got_endoff >= del_endoff);
> +
> +	if (isrt) {
> +		int64_t rtexts = XFS_FSB_TO_B(mp, del->br_blockcount);
> +
> +		do_div(rtexts, mp->m_sb.sb_rextsize);
> +		xfs_mod_frextents(mp, rtexts);
> +	}
> +
> +	/*
> +	 * Update the inode delalloc counter now and wait to update the
> +	 * sb counters as we might have to borrow some blocks for the
> +	 * indirect block accounting.
> +	 */
> +	xfs_trans_reserve_quota_nblks(NULL, ip, -((long)del->br_blockcount), 0,
> +			isrt ? XFS_QMOPT_RES_RTBLKS : XFS_QMOPT_RES_REGBLKS);
> +	ip->i_delayed_blks -= del->br_blockcount;
> +
> +	if (whichfork == XFS_COW_FORK)
> +		state |= BMAP_COWFORK;
> +
> +	if (got->br_startoff == del->br_startoff)
> +		state |= BMAP_LEFT_CONTIG;
> +	if (got_endoff == del_endoff)
> +		state |= BMAP_RIGHT_CONTIG;
> +
> +	switch (state & (BMAP_LEFT_CONTIG | BMAP_RIGHT_CONTIG)) {
> +	case BMAP_LEFT_CONTIG | BMAP_RIGHT_CONTIG:
> +		/*
> +		 * Matches the whole extent.  Delete the entry.
> +		 */
> +		xfs_iext_remove(ip, *idx, 1, state);
> +		--*idx;
> +		break;
> +	case BMAP_LEFT_CONTIG:
> +		/*
> +		 * Deleting the first part of the extent.
> +		 */
> +		trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
> +		got->br_startoff = del_endoff;
> +		got->br_blockcount -= del->br_blockcount;
> +		da_new = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(ip,
> +				got->br_blockcount), da_old);
> +		got->br_startblock = nullstartblock((int)da_new);
> +		xfs_bmbt_set_all(xfs_iext_get_ext(ifp, *idx), got);
> +		trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
> +		break;
> +	case BMAP_RIGHT_CONTIG:
> +		/*
> +		 * Deleting the last part of the extent.
> +		 */
> +		trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
> +		got->br_blockcount = got->br_blockcount - del->br_blockcount;
> +		da_new = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(ip,
> +				got->br_blockcount), da_old);
> +		got->br_startblock = nullstartblock((int)da_new);
> +		xfs_bmbt_set_all(xfs_iext_get_ext(ifp, *idx), got);
> +		trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
> +		break;
> +	case 0:
> +		/*
> +		 * Deleting the middle of the extent.
> +		 *
> +		 * Distribute the original indlen reservation across the two new
> +		 * extents.  Steal blocks from the deleted extent if necessary.
> +		 * Stealing blocks simply fudges the fdblocks accounting below.
> +		 * Warn if either of the new indlen reservations is zero as this
> +		 * can lead to delalloc problems.
> +		 */
> +		trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
> +
> +		got->br_blockcount = del->br_startoff - got->br_startoff;
> +		got_indlen = xfs_bmap_worst_indlen(ip, got->br_blockcount);
> +
> +		new.br_blockcount = got_endoff - del_endoff;
> +		new_indlen = xfs_bmap_worst_indlen(ip, new.br_blockcount);
> +
> +		WARN_ON_ONCE(!got_indlen || !new_indlen);
> +		stolen = xfs_bmap_split_indlen(da_old, &got_indlen, &new_indlen,
> +						       del->br_blockcount);
> +
> +		got->br_startblock = nullstartblock((int)got_indlen);
> +		xfs_bmbt_set_all(xfs_iext_get_ext(ifp, *idx), got);
> +		trace_xfs_bmap_post_update(ip, *idx, 0, _THIS_IP_);
> +
> +		new.br_startoff = del_endoff;
> +		new.br_state = got->br_state;
> +		new.br_startblock = nullstartblock((int)new_indlen);
> +
> +		++*idx;
> +		xfs_iext_insert(ip, *idx, 1, &new, state);
> +
> +		da_new = got_indlen + new_indlen - stolen;
> +		del->br_blockcount -= stolen;
> +		break;
> +	}
> +
> +	ASSERT(da_old >= da_new);
> +	da_diff = da_old - da_new;
> +	if (!isrt)
> +		da_diff += del->br_blockcount;
> +	if (da_diff)
> +		xfs_mod_fdblocks(mp, da_diff, false);
> +	return error;
> +}
> +
> +void
> +xfs_bmap_del_extent_cow(
> +	struct xfs_inode	*ip,
> +	xfs_extnum_t		*idx,
> +	struct xfs_bmbt_irec	*got,
> +	struct xfs_bmbt_irec	*del)
> +{
> +	struct xfs_mount	*mp = ip->i_mount;
> +	struct xfs_ifork	*ifp = XFS_IFORK_PTR(ip, XFS_COW_FORK);
> +	struct xfs_bmbt_irec	new;
> +	xfs_fileoff_t		del_endoff, got_endoff;
> +	int			state = BMAP_COWFORK;
> +
> +	XFS_STATS_INC(mp, xs_del_exlist);
> +
> +	del_endoff = del->br_startoff + del->br_blockcount;
> +	got_endoff = got->br_startoff + got->br_blockcount;
> +
> +	ASSERT(*idx >= 0);
> +	ASSERT(*idx < ifp->if_bytes / sizeof(struct xfs_bmbt_rec));
> +	ASSERT(del->br_blockcount > 0);
> +	ASSERT(got->br_startoff <= del->br_startoff);
> +	ASSERT(got_endoff >= del_endoff);
> +	ASSERT(!isnullstartblock(got->br_startblock));
> +
> +	if (got->br_startoff == del->br_startoff)
> +		state |= BMAP_LEFT_CONTIG;
> +	if (got_endoff == del_endoff)
> +		state |= BMAP_RIGHT_CONTIG;
> +
> +	switch (state & (BMAP_LEFT_CONTIG | BMAP_RIGHT_CONTIG)) {
> +	case BMAP_LEFT_CONTIG | BMAP_RIGHT_CONTIG:
> +		/*
> +		 * Matches the whole extent.  Delete the entry.
> +		 */
> +		xfs_iext_remove(ip, *idx, 1, state);
> +		--*idx;
> +		break;
> +	case BMAP_LEFT_CONTIG:
> +		/*
> +		 * Deleting the first part of the extent.
> +		 */
> +		trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
> +		got->br_startoff = del_endoff;
> +		got->br_blockcount -= del->br_blockcount;
> +		got->br_startblock = del->br_startblock + del->br_blockcount;
> +		xfs_bmbt_set_all(xfs_iext_get_ext(ifp, *idx), got);
> +		trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
> +		break;
> +	case BMAP_RIGHT_CONTIG:
> +		/*
> +		 * Deleting the last part of the extent.
> +		 */
> +		trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
> +		got->br_blockcount -= del->br_blockcount;
> +		xfs_bmbt_set_all(xfs_iext_get_ext(ifp, *idx), got);
> +		trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
> +		break;
> +	case 0:
> +		/*
> +		 * Deleting the middle of the extent.
> +		 */
> +		trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
> +		got->br_blockcount = del->br_startoff - got->br_startoff;
> +		xfs_bmbt_set_all(xfs_iext_get_ext(ifp, *idx), got);
> +		trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
> +
> +		new.br_startoff = del_endoff;
> +		new.br_blockcount = got_endoff - del_endoff;
> +		new.br_state = got->br_state;
> +		new.br_startblock = del->br_startblock + del->br_blockcount;
> +
> +		++*idx;
> +		xfs_iext_insert(ip, *idx, 1, &new, state);
> +		break;
> +	}
> +}
> +
>  /*
>   * Called by xfs_bmapi to update file extent records and the btree
>   * after removing space (or undoing a delayed allocation).
> @@ -5210,167 +5423,20 @@ xfs_bunmapi_cow(
>  	struct xfs_inode		*ip,
>  	struct xfs_bmbt_irec		*del)
>  {
> -	xfs_filblks_t			da_new;
> -	xfs_filblks_t			da_old;
> -	xfs_fsblock_t			del_endblock = 0;
> -	xfs_fileoff_t			del_endoff;
> -	int				delay;
>  	struct xfs_bmbt_rec_host	*ep;
> -	int				error;
>  	struct xfs_bmbt_irec		got;
> -	xfs_fileoff_t			got_endoff;
> -	struct xfs_ifork		*ifp;
> -	struct xfs_mount		*mp;
> -	xfs_filblks_t			nblks;
>  	struct xfs_bmbt_irec		new;
> -	/* REFERENCED */
> -	uint				qfield;
> -	xfs_filblks_t			temp;
> -	xfs_filblks_t			temp2;
> -	int				state = BMAP_COWFORK;
>  	int				eof;
>  	xfs_extnum_t			eidx;
>  
> -	mp = ip->i_mount;
> -	XFS_STATS_INC(mp, xs_del_exlist);
> -
>  	ep = xfs_bmap_search_extents(ip, del->br_startoff, XFS_COW_FORK, &eof,
> -			&eidx, &got, &new);
> -
> -	ifp = XFS_IFORK_PTR(ip, XFS_COW_FORK); ifp = ifp;
> -	ASSERT((eidx >= 0) && (eidx < ifp->if_bytes /
> -		(uint)sizeof(xfs_bmbt_rec_t)));
> -	ASSERT(del->br_blockcount > 0);
> -	ASSERT(got.br_startoff <= del->br_startoff);
> -	del_endoff = del->br_startoff + del->br_blockcount;
> -	got_endoff = got.br_startoff + got.br_blockcount;
> -	ASSERT(got_endoff >= del_endoff);
> -	delay = isnullstartblock(got.br_startblock);
> -	ASSERT(isnullstartblock(del->br_startblock) == delay);
> -	qfield = 0;
> -	error = 0;
> -	/*
> -	 * If deleting a real allocation, must free up the disk space.
> -	 */
> -	if (!delay) {
> -		nblks = del->br_blockcount;
> -		qfield = XFS_TRANS_DQ_BCOUNT;
> -		/*
> -		 * Set up del_endblock and cur for later.
> -		 */
> -		del_endblock = del->br_startblock + del->br_blockcount;
> -		da_old = da_new = 0;
> -	} else {
> -		da_old = startblockval(got.br_startblock);
> -		da_new = 0;
> -		nblks = 0;
> -	}
> -	qfield = qfield;
> -	nblks = nblks;
> -
> -	/*
> -	 * Set flag value to use in switch statement.
> -	 * Left-contig is 2, right-contig is 1.
> -	 */
> -	switch (((got.br_startoff == del->br_startoff) << 1) |
> -		(got_endoff == del_endoff)) {
> -	case 3:
> -		/*
> -		 * Matches the whole extent.  Delete the entry.
> -		 */
> -		xfs_iext_remove(ip, eidx, 1, BMAP_COWFORK);
> -		--eidx;
> -		break;
> -
> -	case 2:
> -		/*
> -		 * Deleting the first part of the extent.
> -		 */
> -		trace_xfs_bmap_pre_update(ip, eidx, state, _THIS_IP_);
> -		xfs_bmbt_set_startoff(ep, del_endoff);
> -		temp = got.br_blockcount - del->br_blockcount;
> -		xfs_bmbt_set_blockcount(ep, temp);
> -		if (delay) {
> -			temp = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(ip, temp),
> -				da_old);
> -			xfs_bmbt_set_startblock(ep, nullstartblock((int)temp));
> -			trace_xfs_bmap_post_update(ip, eidx, state, _THIS_IP_);
> -			da_new = temp;
> -			break;
> -		}
> -		xfs_bmbt_set_startblock(ep, del_endblock);
> -		trace_xfs_bmap_post_update(ip, eidx, state, _THIS_IP_);
> -		break;
> -
> -	case 1:
> -		/*
> -		 * Deleting the last part of the extent.
> -		 */
> -		temp = got.br_blockcount - del->br_blockcount;
> -		trace_xfs_bmap_pre_update(ip, eidx, state, _THIS_IP_);
> -		xfs_bmbt_set_blockcount(ep, temp);
> -		if (delay) {
> -			temp = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(ip, temp),
> -				da_old);
> -			xfs_bmbt_set_startblock(ep, nullstartblock((int)temp));
> -			trace_xfs_bmap_post_update(ip, eidx, state, _THIS_IP_);
> -			da_new = temp;
> -			break;
> -		}
> -		trace_xfs_bmap_post_update(ip, eidx, state, _THIS_IP_);
> -		break;
> -
> -	case 0:
> -		/*
> -		 * Deleting the middle of the extent.
> -		 */
> -		temp = del->br_startoff - got.br_startoff;
> -		trace_xfs_bmap_pre_update(ip, eidx, state, _THIS_IP_);
> -		xfs_bmbt_set_blockcount(ep, temp);
> -		new.br_startoff = del_endoff;
> -		temp2 = got_endoff - del_endoff;
> -		new.br_blockcount = temp2;
> -		new.br_state = got.br_state;
> -		if (!delay) {
> -			new.br_startblock = del_endblock;
> -		} else {
> -			temp = xfs_bmap_worst_indlen(ip, temp);
> -			xfs_bmbt_set_startblock(ep, nullstartblock((int)temp));
> -			temp2 = xfs_bmap_worst_indlen(ip, temp2);
> -			new.br_startblock = nullstartblock((int)temp2);
> -			da_new = temp + temp2;
> -			while (da_new > da_old) {
> -				if (temp) {
> -					temp--;
> -					da_new--;
> -					xfs_bmbt_set_startblock(ep,
> -						nullstartblock((int)temp));
> -				}
> -				if (da_new == da_old)
> -					break;
> -				if (temp2) {
> -					temp2--;
> -					da_new--;
> -					new.br_startblock =
> -						nullstartblock((int)temp2);
> -				}
> -			}
> -		}
> -		trace_xfs_bmap_post_update(ip, eidx, state, _THIS_IP_);
> -		xfs_iext_insert(ip, eidx + 1, 1, &new, state);
> -		++eidx;
> -		break;
> -	}
> -
> -	/*
> -	 * Account for change in delayed indirect blocks.
> -	 * Nothing to do for disk quota accounting here.
> -	 */
> -	ASSERT(da_old >= da_new);
> -	if (da_old > da_new)
> -		xfs_mod_fdblocks(mp, (int64_t)(da_old - da_new), false);
> -
> -	return error;
> +				&eidx, &got, &new);
> +	ASSERT(ep);
> +	if (isnullstartblock(got.br_startblock))
> +		xfs_bmap_del_extent_delay(ip, XFS_COW_FORK, &eidx, &got, del);
> +	else
> +		xfs_bmap_del_extent_cow(ip, &eidx, &got, del);
> +	return 0;
>  }
>  
>  /*
> diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
> index eb86af0..5f18248 100644
> --- a/fs/xfs/libxfs/xfs_bmap.h
> +++ b/fs/xfs/libxfs/xfs_bmap.h
> @@ -224,6 +224,11 @@ int	xfs_bunmapi(struct xfs_trans *tp, struct xfs_inode *ip,
>  		xfs_extnum_t nexts, xfs_fsblock_t *firstblock,
>  		struct xfs_defer_ops *dfops, int *done);
>  int	xfs_bunmapi_cow(struct xfs_inode *ip, struct xfs_bmbt_irec *del);
> +int	xfs_bmap_del_extent_delay(struct xfs_inode *ip, int whichfork,
> +		xfs_extnum_t *idx, struct xfs_bmbt_irec *got,
> +		struct xfs_bmbt_irec *del);
> +void	xfs_bmap_del_extent_cow(struct xfs_inode *ip, xfs_extnum_t *idx,
> +		struct xfs_bmbt_irec *got, struct xfs_bmbt_irec *del);
>  int	xfs_check_nostate_extents(struct xfs_ifork *ifp, xfs_extnum_t idx,
>  		xfs_extnum_t num);
>  uint	xfs_default_attroffset(struct xfs_inode *ip);
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index 5d230ea..f33f737 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -535,11 +535,6 @@ xfs_reflink_cancel_cow_blocks(
>  		trace_xfs_reflink_cancel_cow(ip, &irec);
>  
>  		if (irec.br_startblock == DELAYSTARTBLOCK) {
> -			/* Free a delayed allocation. */
> -			xfs_mod_fdblocks(ip->i_mount, irec.br_blockcount,
> -					false);
> -			ip->i_delayed_blks -= irec.br_blockcount;
> -
>  			/* Remove the mapping from the CoW fork. */
>  			error = xfs_bunmapi_cow(ip, &irec);
>  			if (error)
> -- 
> 2.1.4
> 

[PATCH 7/9] xfs: optimize xfs_reflink_cancel_cow_blocks

[Christoph Hellwig]

Rewrite xfs_reflink_cancel_cow_blocks so that we only do a search for
the first extent in the extent list and then iterate over the remaining
extents using the extent index, passing the extent we operate on
directly to xfs_bmap_del_extent_delay or xfs_bmap_del_extent_cow instead
of going through xfs_bunmapi and doing yet another extent list lookup.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_reflink.c | 51 +++++++++++++++++++++++----------------------------
 1 file changed, 23 insertions(+), 28 deletions(-)

diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index f33f737..7979d46 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -512,53 +512,49 @@ xfs_reflink_cancel_cow_blocks(
 	xfs_fileoff_t			offset_fsb,
 	xfs_fileoff_t			end_fsb)
 {
-	struct xfs_bmbt_irec		irec;
-	xfs_filblks_t			count_fsb;
+	struct xfs_ifork		*ifp = XFS_IFORK_PTR(ip, XFS_COW_FORK);
+	struct xfs_bmbt_irec		got, prev, del;
+	xfs_extnum_t			idx;
 	xfs_fsblock_t			firstfsb;
 	struct xfs_defer_ops		dfops;
-	int				error = 0;
-	int				nimaps;
+	int				error = 0, eof = 0;
 
 	if (!xfs_is_reflink_inode(ip))
 		return 0;
 
-	/* Go find the old extent in the CoW fork. */
-	while (offset_fsb < end_fsb) {
-		nimaps = 1;
-		count_fsb = (xfs_filblks_t)(end_fsb - offset_fsb);
-		error = xfs_bmapi_read(ip, offset_fsb, count_fsb, &irec,
-				&nimaps, XFS_BMAPI_COWFORK);
-		if (error)
-			break;
-		ASSERT(nimaps == 1);
+	xfs_bmap_search_extents(ip, offset_fsb, XFS_COW_FORK, &eof, &idx,
+			&got, &prev);
+	if (eof)
+		return 0;
 
-		trace_xfs_reflink_cancel_cow(ip, &irec);
+	while (got.br_startoff < end_fsb) {
+		del = got;
+		xfs_trim_extent(&del, offset_fsb, end_fsb - offset_fsb);
+		trace_xfs_reflink_cancel_cow(ip, &del);
 
-		if (irec.br_startblock == DELAYSTARTBLOCK) {
-			/* Remove the mapping from the CoW fork. */
-			error = xfs_bunmapi_cow(ip, &irec);
+		if (isnullstartblock(del.br_startblock)) {
+			error = xfs_bmap_del_extent_delay(ip, XFS_COW_FORK,
+					&idx, &got, &del);
 			if (error)
 				break;
-		} else if (irec.br_startblock == HOLESTARTBLOCK) {
-			/* empty */
 		} else {
 			xfs_trans_ijoin(*tpp, ip, 0);
 			xfs_defer_init(&dfops, &firstfsb);
 
 			/* Free the CoW orphan record. */
 			error = xfs_refcount_free_cow_extent(ip->i_mount,
-					&dfops, irec.br_startblock,
-					irec.br_blockcount);
+					&dfops, del.br_startblock,
+					del.br_blockcount);
 			if (error)
 				break;
 
 			xfs_bmap_add_free(ip->i_mount, &dfops,
-					irec.br_startblock, irec.br_blockcount,
+					del.br_startblock, del.br_blockcount,
 					NULL);
 
 			/* Update quota accounting */
 			xfs_trans_mod_dquot_byino(*tpp, ip, XFS_TRANS_DQ_BCOUNT,
-					-(long)irec.br_blockcount);
+					-(long)del.br_blockcount);
 
 			/* Roll the transaction */
 			error = xfs_defer_finish(tpp, &dfops, ip);
@@ -568,13 +564,12 @@ xfs_reflink_cancel_cow_blocks(
 			}
 
 			/* Remove the mapping from the CoW fork. */
-			error = xfs_bunmapi_cow(ip, &irec);
-			if (error)
-				break;
+			xfs_bmap_del_extent_cow(ip, &idx, &got, &del);
 		}
 
-		/* Roll on... */
-		offset_fsb = irec.br_startoff + irec.br_blockcount;
+		if (++idx >= ifp->if_bytes / sizeof(struct xfs_bmbt_rec))
+			return 0;
+		xfs_bmbt_get_all(xfs_iext_get_ext(ifp, idx), &got);
 	}
 
 	return error;
-- 
2.1.4

Re: [PATCH 7/9] xfs: optimize xfs_reflink_cancel_cow_blocks

[Brian Foster]

On Sat, Oct 15, 2016 at 10:52:32AM +0200, Christoph Hellwig wrote:
> Rewrite xfs_reflink_cancel_cow_blocks so that we only do a search for
> the first extent in the extent list and then iterate over the remaining
> extents using the extent index, passing the extent we operate on
> directly to xfs_bmap_del_extent_delay or xfs_bmap_del_extent_cow instead
> of going through xfs_bunmapi and doing yet another extent list lookup.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---

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

>  fs/xfs/xfs_reflink.c | 51 +++++++++++++++++++++++----------------------------
>  1 file changed, 23 insertions(+), 28 deletions(-)
> 
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index f33f737..7979d46 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -512,53 +512,49 @@ xfs_reflink_cancel_cow_blocks(
>  	xfs_fileoff_t			offset_fsb,
>  	xfs_fileoff_t			end_fsb)
>  {
> -	struct xfs_bmbt_irec		irec;
> -	xfs_filblks_t			count_fsb;
> +	struct xfs_ifork		*ifp = XFS_IFORK_PTR(ip, XFS_COW_FORK);
> +	struct xfs_bmbt_irec		got, prev, del;
> +	xfs_extnum_t			idx;
>  	xfs_fsblock_t			firstfsb;
>  	struct xfs_defer_ops		dfops;
> -	int				error = 0;
> -	int				nimaps;
> +	int				error = 0, eof = 0;
>  
>  	if (!xfs_is_reflink_inode(ip))
>  		return 0;
>  
> -	/* Go find the old extent in the CoW fork. */
> -	while (offset_fsb < end_fsb) {
> -		nimaps = 1;
> -		count_fsb = (xfs_filblks_t)(end_fsb - offset_fsb);
> -		error = xfs_bmapi_read(ip, offset_fsb, count_fsb, &irec,
> -				&nimaps, XFS_BMAPI_COWFORK);
> -		if (error)
> -			break;
> -		ASSERT(nimaps == 1);
> +	xfs_bmap_search_extents(ip, offset_fsb, XFS_COW_FORK, &eof, &idx,
> +			&got, &prev);
> +	if (eof)
> +		return 0;
>  
> -		trace_xfs_reflink_cancel_cow(ip, &irec);
> +	while (got.br_startoff < end_fsb) {
> +		del = got;
> +		xfs_trim_extent(&del, offset_fsb, end_fsb - offset_fsb);
> +		trace_xfs_reflink_cancel_cow(ip, &del);
>  
> -		if (irec.br_startblock == DELAYSTARTBLOCK) {
> -			/* Remove the mapping from the CoW fork. */
> -			error = xfs_bunmapi_cow(ip, &irec);
> +		if (isnullstartblock(del.br_startblock)) {
> +			error = xfs_bmap_del_extent_delay(ip, XFS_COW_FORK,
> +					&idx, &got, &del);
>  			if (error)
>  				break;
> -		} else if (irec.br_startblock == HOLESTARTBLOCK) {
> -			/* empty */
>  		} else {
>  			xfs_trans_ijoin(*tpp, ip, 0);
>  			xfs_defer_init(&dfops, &firstfsb);
>  
>  			/* Free the CoW orphan record. */
>  			error = xfs_refcount_free_cow_extent(ip->i_mount,
> -					&dfops, irec.br_startblock,
> -					irec.br_blockcount);
> +					&dfops, del.br_startblock,
> +					del.br_blockcount);
>  			if (error)
>  				break;
>  
>  			xfs_bmap_add_free(ip->i_mount, &dfops,
> -					irec.br_startblock, irec.br_blockcount,
> +					del.br_startblock, del.br_blockcount,
>  					NULL);
>  
>  			/* Update quota accounting */
>  			xfs_trans_mod_dquot_byino(*tpp, ip, XFS_TRANS_DQ_BCOUNT,
> -					-(long)irec.br_blockcount);
> +					-(long)del.br_blockcount);
>  
>  			/* Roll the transaction */
>  			error = xfs_defer_finish(tpp, &dfops, ip);
> @@ -568,13 +564,12 @@ xfs_reflink_cancel_cow_blocks(
>  			}
>  
>  			/* Remove the mapping from the CoW fork. */
> -			error = xfs_bunmapi_cow(ip, &irec);
> -			if (error)
> -				break;
> +			xfs_bmap_del_extent_cow(ip, &idx, &got, &del);
>  		}
>  
> -		/* Roll on... */
> -		offset_fsb = irec.br_startoff + irec.br_blockcount;
> +		if (++idx >= ifp->if_bytes / sizeof(struct xfs_bmbt_rec))
> +			return 0;
> +		xfs_bmbt_get_all(xfs_iext_get_ext(ifp, idx), &got);
>  	}
>  
>  	return error;
> -- 
> 2.1.4
> 
> --
> 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

Re: [PATCH 7/9] xfs: optimize xfs_reflink_cancel_cow_blocks

[Darrick J. Wong]

On Sat, Oct 15, 2016 at 10:52:32AM +0200, Christoph Hellwig wrote:
> Rewrite xfs_reflink_cancel_cow_blocks so that we only do a search for
> the first extent in the extent list and then iterate over the remaining
> extents using the extent index, passing the extent we operate on
> directly to xfs_bmap_del_extent_delay or xfs_bmap_del_extent_cow instead
> of going through xfs_bunmapi and doing yet another extent list lookup.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

> ---
>  fs/xfs/xfs_reflink.c | 51 +++++++++++++++++++++++----------------------------
>  1 file changed, 23 insertions(+), 28 deletions(-)
> 
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index f33f737..7979d46 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -512,53 +512,49 @@ xfs_reflink_cancel_cow_blocks(
>  	xfs_fileoff_t			offset_fsb,
>  	xfs_fileoff_t			end_fsb)
>  {
> -	struct xfs_bmbt_irec		irec;
> -	xfs_filblks_t			count_fsb;
> +	struct xfs_ifork		*ifp = XFS_IFORK_PTR(ip, XFS_COW_FORK);
> +	struct xfs_bmbt_irec		got, prev, del;
> +	xfs_extnum_t			idx;
>  	xfs_fsblock_t			firstfsb;
>  	struct xfs_defer_ops		dfops;
> -	int				error = 0;
> -	int				nimaps;
> +	int				error = 0, eof = 0;
>  
>  	if (!xfs_is_reflink_inode(ip))
>  		return 0;
>  
> -	/* Go find the old extent in the CoW fork. */
> -	while (offset_fsb < end_fsb) {
> -		nimaps = 1;
> -		count_fsb = (xfs_filblks_t)(end_fsb - offset_fsb);
> -		error = xfs_bmapi_read(ip, offset_fsb, count_fsb, &irec,
> -				&nimaps, XFS_BMAPI_COWFORK);
> -		if (error)
> -			break;
> -		ASSERT(nimaps == 1);
> +	xfs_bmap_search_extents(ip, offset_fsb, XFS_COW_FORK, &eof, &idx,
> +			&got, &prev);
> +	if (eof)
> +		return 0;
>  
> -		trace_xfs_reflink_cancel_cow(ip, &irec);
> +	while (got.br_startoff < end_fsb) {
> +		del = got;
> +		xfs_trim_extent(&del, offset_fsb, end_fsb - offset_fsb);
> +		trace_xfs_reflink_cancel_cow(ip, &del);
>  
> -		if (irec.br_startblock == DELAYSTARTBLOCK) {
> -			/* Remove the mapping from the CoW fork. */
> -			error = xfs_bunmapi_cow(ip, &irec);
> +		if (isnullstartblock(del.br_startblock)) {
> +			error = xfs_bmap_del_extent_delay(ip, XFS_COW_FORK,
> +					&idx, &got, &del);
>  			if (error)
>  				break;
> -		} else if (irec.br_startblock == HOLESTARTBLOCK) {
> -			/* empty */
>  		} else {
>  			xfs_trans_ijoin(*tpp, ip, 0);
>  			xfs_defer_init(&dfops, &firstfsb);
>  
>  			/* Free the CoW orphan record. */
>  			error = xfs_refcount_free_cow_extent(ip->i_mount,
> -					&dfops, irec.br_startblock,
> -					irec.br_blockcount);
> +					&dfops, del.br_startblock,
> +					del.br_blockcount);
>  			if (error)
>  				break;
>  
>  			xfs_bmap_add_free(ip->i_mount, &dfops,
> -					irec.br_startblock, irec.br_blockcount,
> +					del.br_startblock, del.br_blockcount,
>  					NULL);
>  
>  			/* Update quota accounting */
>  			xfs_trans_mod_dquot_byino(*tpp, ip, XFS_TRANS_DQ_BCOUNT,
> -					-(long)irec.br_blockcount);
> +					-(long)del.br_blockcount);
>  
>  			/* Roll the transaction */
>  			error = xfs_defer_finish(tpp, &dfops, ip);
> @@ -568,13 +564,12 @@ xfs_reflink_cancel_cow_blocks(
>  			}
>  
>  			/* Remove the mapping from the CoW fork. */
> -			error = xfs_bunmapi_cow(ip, &irec);
> -			if (error)
> -				break;
> +			xfs_bmap_del_extent_cow(ip, &idx, &got, &del);
>  		}
>  
> -		/* Roll on... */
> -		offset_fsb = irec.br_startoff + irec.br_blockcount;
> +		if (++idx >= ifp->if_bytes / sizeof(struct xfs_bmbt_rec))
> +			return 0;
> +		xfs_bmbt_get_all(xfs_iext_get_ext(ifp, idx), &got);
>  	}
>  
>  	return error;
> -- 
> 2.1.4
> 

[PATCH 8/9] xfs: optimize xfs_reflink_end_cow

[Christoph Hellwig]

Instead of doing a full extent list search for each extent that is to be
deleted using xfs_bmapi_read and then doing another one inside of
xfs_bunmapi_cow use the same scheme that xfs_bumapi uses:  look up the
last extent to be deleted and then use the extent index to walk downward
until we are outside the range to be deleted.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_reflink.c | 119 ++++++++++++++++++++++++---------------------------
 fs/xfs/xfs_trace.h   |   1 -
 2 files changed, 56 insertions(+), 64 deletions(-)

diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index 7979d46..93a863e 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -634,25 +634,26 @@ xfs_reflink_end_cow(
 	xfs_off_t			offset,
 	xfs_off_t			count)
 {
-	struct xfs_bmbt_irec		irec;
-	struct xfs_bmbt_irec		uirec;
+	struct xfs_ifork		*ifp = XFS_IFORK_PTR(ip, XFS_COW_FORK);
+	struct xfs_bmbt_irec		got, prev, del;
 	struct xfs_trans		*tp;
 	xfs_fileoff_t			offset_fsb;
 	xfs_fileoff_t			end_fsb;
-	xfs_filblks_t			count_fsb;
 	xfs_fsblock_t			firstfsb;
 	struct xfs_defer_ops		dfops;
-	int				error;
+	int				error, eof = 0;
 	unsigned int			resblks;
-	xfs_filblks_t			ilen;
 	xfs_filblks_t			rlen;
-	int				nimaps;
+	xfs_extnum_t			idx;
 
 	trace_xfs_reflink_end_cow(ip, offset, count);
 
+	/* No COW extents?  That's easy! */
+	if (ifp->if_bytes == 0)
+		return 0;
+
 	offset_fsb = XFS_B_TO_FSBT(ip->i_mount, offset);
 	end_fsb = XFS_B_TO_FSB(ip->i_mount, offset + count);
-	count_fsb = (xfs_filblks_t)(end_fsb - offset_fsb);
 
 	/* Start a rolling transaction to switch the mappings */
 	resblks = XFS_EXTENTADD_SPACE_RES(ip->i_mount, XFS_DATA_FORK);
@@ -664,72 +665,65 @@ xfs_reflink_end_cow(
 	xfs_ilock(ip, XFS_ILOCK_EXCL);
 	xfs_trans_ijoin(tp, ip, 0);
 
-	/* Go find the old extent in the CoW fork. */
-	while (offset_fsb < end_fsb) {
-		/* Read extent from the source file */
-		nimaps = 1;
-		count_fsb = (xfs_filblks_t)(end_fsb - offset_fsb);
-		error = xfs_bmapi_read(ip, offset_fsb, count_fsb, &irec,
-				&nimaps, XFS_BMAPI_COWFORK);
-		if (error)
-			goto out_cancel;
-		ASSERT(nimaps == 1);
+	xfs_bmap_search_extents(ip, end_fsb - 1, XFS_COW_FORK, &eof, &idx,
+			&got, &prev);
 
-		ASSERT(irec.br_startblock != DELAYSTARTBLOCK);
-		trace_xfs_reflink_cow_remap(ip, &irec);
+	/* If there is a hole at end_fsb - 1 go to the previous extent */
+	if (eof || got.br_startoff > end_fsb) {
+		ASSERT(idx > 0);
+		xfs_bmbt_get_all(xfs_iext_get_ext(ifp, --idx), &got);
+	}
 
-		/*
-		 * We can have a hole in the CoW fork if part of a directio
-		 * write is CoW but part of it isn't.
-		 */
-		rlen = ilen = irec.br_blockcount;
-		if (irec.br_startblock == HOLESTARTBLOCK)
+	/* Walk backwards until we're out of the I/O range... */
+	while (got.br_startoff + got.br_blockcount > offset_fsb) {
+		del = got;
+		xfs_trim_extent(&del, offset_fsb, end_fsb - offset_fsb);
+
+		/* Extent delete may have bumped idx forward */
+		if (!del.br_blockcount) {
+			idx--;
 			goto next_extent;
+		}
+
+		ASSERT(!isnullstartblock(got.br_startblock));
 
 		/* Unmap the old blocks in the data fork. */
-		while (rlen) {
-			xfs_defer_init(&dfops, &firstfsb);
-			error = __xfs_bunmapi(tp, ip, irec.br_startoff,
-					&rlen, 0, 1, &firstfsb, &dfops);
-			if (error)
-				goto out_defer;
-
-			/*
-			 * Trim the extent to whatever got unmapped.
-			 * Remember, bunmapi works backwards.
-			 */
-			uirec.br_startblock = irec.br_startblock + rlen;
-			uirec.br_startoff = irec.br_startoff + rlen;
-			uirec.br_blockcount = irec.br_blockcount - rlen;
-			irec.br_blockcount = rlen;
-			trace_xfs_reflink_cow_remap_piece(ip, &uirec);
+		xfs_defer_init(&dfops, &firstfsb);
+		rlen = del.br_blockcount;
+		error = __xfs_bunmapi(tp, ip, del.br_startoff, &rlen, 0, 1,
+				&firstfsb, &dfops);
+		if (error)
+			goto out_defer;
 
-			/* Free the CoW orphan record. */
-			error = xfs_refcount_free_cow_extent(tp->t_mountp,
-					&dfops, uirec.br_startblock,
-					uirec.br_blockcount);
-			if (error)
-				goto out_defer;
+		/* Trim the extent to whatever got unmapped. */
+		if (rlen) {
+			xfs_trim_extent(&del, del.br_startoff + rlen,
+				del.br_blockcount - rlen);
+		}
+		trace_xfs_reflink_cow_remap(ip, &del);
 
-			/* Map the new blocks into the data fork. */
-			error = xfs_bmap_map_extent(tp->t_mountp, &dfops,
-					ip, &uirec);
-			if (error)
-				goto out_defer;
+		/* Free the CoW orphan record. */
+		error = xfs_refcount_free_cow_extent(tp->t_mountp, &dfops,
+				del.br_startblock, del.br_blockcount);
+		if (error)
+			goto out_defer;
 
-			/* Remove the mapping from the CoW fork. */
-			error = xfs_bunmapi_cow(ip, &uirec);
-			if (error)
-				goto out_defer;
+		/* Map the new blocks into the data fork. */
+		error = xfs_bmap_map_extent(tp->t_mountp, &dfops, ip, &del);
+		if (error)
+			goto out_defer;
 
-			error = xfs_defer_finish(&tp, &dfops, ip);
-			if (error)
-				goto out_defer;
-		}
+		/* Remove the mapping from the CoW fork. */
+		xfs_bmap_del_extent_cow(ip, &idx, &got, &del);
+
+		error = xfs_defer_finish(&tp, &dfops, ip);
+		if (error)
+			goto out_defer;
 
 next_extent:
-		/* Roll on... */
-		offset_fsb = irec.br_startoff + ilen;
+		if (idx < 0)
+			break;
+		xfs_bmbt_get_all(xfs_iext_get_ext(ifp, idx), &got);
 	}
 
 	error = xfs_trans_commit(tp);
@@ -740,7 +734,6 @@ xfs_reflink_end_cow(
 
 out_defer:
 	xfs_defer_cancel(&dfops);
-out_cancel:
 	xfs_trans_cancel(tp);
 	xfs_iunlock(ip, XFS_ILOCK_EXCL);
 out:
diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index 72f9f6b..0907752 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -3356,7 +3356,6 @@ DEFINE_INODE_IREC_EVENT(xfs_reflink_trim_irec);
 DEFINE_SIMPLE_IO_EVENT(xfs_reflink_cancel_cow_range);
 DEFINE_SIMPLE_IO_EVENT(xfs_reflink_end_cow);
 DEFINE_INODE_IREC_EVENT(xfs_reflink_cow_remap);
-DEFINE_INODE_IREC_EVENT(xfs_reflink_cow_remap_piece);
 
 DEFINE_INODE_ERROR_EVENT(xfs_reflink_allocate_cow_range_error);
 DEFINE_INODE_ERROR_EVENT(xfs_reflink_cancel_cow_range_error);
-- 
2.1.4

Re: [PATCH 8/9] xfs: optimize xfs_reflink_end_cow

[Brian Foster]

On Sat, Oct 15, 2016 at 10:52:33AM +0200, Christoph Hellwig wrote:
> Instead of doing a full extent list search for each extent that is to be
> deleted using xfs_bmapi_read and then doing another one inside of
> xfs_bunmapi_cow use the same scheme that xfs_bumapi uses:  look up the
> last extent to be deleted and then use the extent index to walk downward
> until we are outside the range to be deleted.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---

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

>  fs/xfs/xfs_reflink.c | 119 ++++++++++++++++++++++++---------------------------
>  fs/xfs/xfs_trace.h   |   1 -
>  2 files changed, 56 insertions(+), 64 deletions(-)
> 
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index 7979d46..93a863e 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -634,25 +634,26 @@ xfs_reflink_end_cow(
>  	xfs_off_t			offset,
>  	xfs_off_t			count)
>  {
> -	struct xfs_bmbt_irec		irec;
> -	struct xfs_bmbt_irec		uirec;
> +	struct xfs_ifork		*ifp = XFS_IFORK_PTR(ip, XFS_COW_FORK);
> +	struct xfs_bmbt_irec		got, prev, del;
>  	struct xfs_trans		*tp;
>  	xfs_fileoff_t			offset_fsb;
>  	xfs_fileoff_t			end_fsb;
> -	xfs_filblks_t			count_fsb;
>  	xfs_fsblock_t			firstfsb;
>  	struct xfs_defer_ops		dfops;
> -	int				error;
> +	int				error, eof = 0;
>  	unsigned int			resblks;
> -	xfs_filblks_t			ilen;
>  	xfs_filblks_t			rlen;
> -	int				nimaps;
> +	xfs_extnum_t			idx;
>  
>  	trace_xfs_reflink_end_cow(ip, offset, count);
>  
> +	/* No COW extents?  That's easy! */
> +	if (ifp->if_bytes == 0)
> +		return 0;
> +
>  	offset_fsb = XFS_B_TO_FSBT(ip->i_mount, offset);
>  	end_fsb = XFS_B_TO_FSB(ip->i_mount, offset + count);
> -	count_fsb = (xfs_filblks_t)(end_fsb - offset_fsb);
>  
>  	/* Start a rolling transaction to switch the mappings */
>  	resblks = XFS_EXTENTADD_SPACE_RES(ip->i_mount, XFS_DATA_FORK);
> @@ -664,72 +665,65 @@ xfs_reflink_end_cow(
>  	xfs_ilock(ip, XFS_ILOCK_EXCL);
>  	xfs_trans_ijoin(tp, ip, 0);
>  
> -	/* Go find the old extent in the CoW fork. */
> -	while (offset_fsb < end_fsb) {
> -		/* Read extent from the source file */
> -		nimaps = 1;
> -		count_fsb = (xfs_filblks_t)(end_fsb - offset_fsb);
> -		error = xfs_bmapi_read(ip, offset_fsb, count_fsb, &irec,
> -				&nimaps, XFS_BMAPI_COWFORK);
> -		if (error)
> -			goto out_cancel;
> -		ASSERT(nimaps == 1);
> +	xfs_bmap_search_extents(ip, end_fsb - 1, XFS_COW_FORK, &eof, &idx,
> +			&got, &prev);
>  
> -		ASSERT(irec.br_startblock != DELAYSTARTBLOCK);
> -		trace_xfs_reflink_cow_remap(ip, &irec);
> +	/* If there is a hole at end_fsb - 1 go to the previous extent */
> +	if (eof || got.br_startoff > end_fsb) {
> +		ASSERT(idx > 0);
> +		xfs_bmbt_get_all(xfs_iext_get_ext(ifp, --idx), &got);
> +	}
>  
> -		/*
> -		 * We can have a hole in the CoW fork if part of a directio
> -		 * write is CoW but part of it isn't.
> -		 */
> -		rlen = ilen = irec.br_blockcount;
> -		if (irec.br_startblock == HOLESTARTBLOCK)
> +	/* Walk backwards until we're out of the I/O range... */
> +	while (got.br_startoff + got.br_blockcount > offset_fsb) {
> +		del = got;
> +		xfs_trim_extent(&del, offset_fsb, end_fsb - offset_fsb);
> +
> +		/* Extent delete may have bumped idx forward */
> +		if (!del.br_blockcount) {
> +			idx--;
>  			goto next_extent;
> +		}
> +
> +		ASSERT(!isnullstartblock(got.br_startblock));
>  
>  		/* Unmap the old blocks in the data fork. */
> -		while (rlen) {
> -			xfs_defer_init(&dfops, &firstfsb);
> -			error = __xfs_bunmapi(tp, ip, irec.br_startoff,
> -					&rlen, 0, 1, &firstfsb, &dfops);
> -			if (error)
> -				goto out_defer;
> -
> -			/*
> -			 * Trim the extent to whatever got unmapped.
> -			 * Remember, bunmapi works backwards.
> -			 */
> -			uirec.br_startblock = irec.br_startblock + rlen;
> -			uirec.br_startoff = irec.br_startoff + rlen;
> -			uirec.br_blockcount = irec.br_blockcount - rlen;
> -			irec.br_blockcount = rlen;
> -			trace_xfs_reflink_cow_remap_piece(ip, &uirec);
> +		xfs_defer_init(&dfops, &firstfsb);
> +		rlen = del.br_blockcount;
> +		error = __xfs_bunmapi(tp, ip, del.br_startoff, &rlen, 0, 1,
> +				&firstfsb, &dfops);
> +		if (error)
> +			goto out_defer;
>  
> -			/* Free the CoW orphan record. */
> -			error = xfs_refcount_free_cow_extent(tp->t_mountp,
> -					&dfops, uirec.br_startblock,
> -					uirec.br_blockcount);
> -			if (error)
> -				goto out_defer;
> +		/* Trim the extent to whatever got unmapped. */
> +		if (rlen) {
> +			xfs_trim_extent(&del, del.br_startoff + rlen,
> +				del.br_blockcount - rlen);
> +		}
> +		trace_xfs_reflink_cow_remap(ip, &del);
>  
> -			/* Map the new blocks into the data fork. */
> -			error = xfs_bmap_map_extent(tp->t_mountp, &dfops,
> -					ip, &uirec);
> -			if (error)
> -				goto out_defer;
> +		/* Free the CoW orphan record. */
> +		error = xfs_refcount_free_cow_extent(tp->t_mountp, &dfops,
> +				del.br_startblock, del.br_blockcount);
> +		if (error)
> +			goto out_defer;
>  
> -			/* Remove the mapping from the CoW fork. */
> -			error = xfs_bunmapi_cow(ip, &uirec);
> -			if (error)
> -				goto out_defer;
> +		/* Map the new blocks into the data fork. */
> +		error = xfs_bmap_map_extent(tp->t_mountp, &dfops, ip, &del);
> +		if (error)
> +			goto out_defer;
>  
> -			error = xfs_defer_finish(&tp, &dfops, ip);
> -			if (error)
> -				goto out_defer;
> -		}
> +		/* Remove the mapping from the CoW fork. */
> +		xfs_bmap_del_extent_cow(ip, &idx, &got, &del);
> +
> +		error = xfs_defer_finish(&tp, &dfops, ip);
> +		if (error)
> +			goto out_defer;
>  
>  next_extent:
> -		/* Roll on... */
> -		offset_fsb = irec.br_startoff + ilen;
> +		if (idx < 0)
> +			break;
> +		xfs_bmbt_get_all(xfs_iext_get_ext(ifp, idx), &got);
>  	}
>  
>  	error = xfs_trans_commit(tp);
> @@ -740,7 +734,6 @@ xfs_reflink_end_cow(
>  
>  out_defer:
>  	xfs_defer_cancel(&dfops);
> -out_cancel:
>  	xfs_trans_cancel(tp);
>  	xfs_iunlock(ip, XFS_ILOCK_EXCL);
>  out:
> diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> index 72f9f6b..0907752 100644
> --- a/fs/xfs/xfs_trace.h
> +++ b/fs/xfs/xfs_trace.h
> @@ -3356,7 +3356,6 @@ DEFINE_INODE_IREC_EVENT(xfs_reflink_trim_irec);
>  DEFINE_SIMPLE_IO_EVENT(xfs_reflink_cancel_cow_range);
>  DEFINE_SIMPLE_IO_EVENT(xfs_reflink_end_cow);
>  DEFINE_INODE_IREC_EVENT(xfs_reflink_cow_remap);
> -DEFINE_INODE_IREC_EVENT(xfs_reflink_cow_remap_piece);
>  
>  DEFINE_INODE_ERROR_EVENT(xfs_reflink_allocate_cow_range_error);
>  DEFINE_INODE_ERROR_EVENT(xfs_reflink_cancel_cow_range_error);
> -- 
> 2.1.4
> 
> --
> 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

Re: [PATCH 8/9] xfs: optimize xfs_reflink_end_cow

[Darrick J. Wong]

On Sat, Oct 15, 2016 at 10:52:33AM +0200, Christoph Hellwig wrote:
> Instead of doing a full extent list search for each extent that is to be
> deleted using xfs_bmapi_read and then doing another one inside of
> xfs_bunmapi_cow use the same scheme that xfs_bumapi uses:  look up the
> last extent to be deleted and then use the extent index to walk downward
> until we are outside the range to be deleted.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

> ---
>  fs/xfs/xfs_reflink.c | 119 ++++++++++++++++++++++++---------------------------
>  fs/xfs/xfs_trace.h   |   1 -
>  2 files changed, 56 insertions(+), 64 deletions(-)
> 
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index 7979d46..93a863e 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -634,25 +634,26 @@ xfs_reflink_end_cow(
>  	xfs_off_t			offset,
>  	xfs_off_t			count)
>  {
> -	struct xfs_bmbt_irec		irec;
> -	struct xfs_bmbt_irec		uirec;
> +	struct xfs_ifork		*ifp = XFS_IFORK_PTR(ip, XFS_COW_FORK);
> +	struct xfs_bmbt_irec		got, prev, del;
>  	struct xfs_trans		*tp;
>  	xfs_fileoff_t			offset_fsb;
>  	xfs_fileoff_t			end_fsb;
> -	xfs_filblks_t			count_fsb;
>  	xfs_fsblock_t			firstfsb;
>  	struct xfs_defer_ops		dfops;
> -	int				error;
> +	int				error, eof = 0;
>  	unsigned int			resblks;
> -	xfs_filblks_t			ilen;
>  	xfs_filblks_t			rlen;
> -	int				nimaps;
> +	xfs_extnum_t			idx;
>  
>  	trace_xfs_reflink_end_cow(ip, offset, count);
>  
> +	/* No COW extents?  That's easy! */
> +	if (ifp->if_bytes == 0)
> +		return 0;
> +
>  	offset_fsb = XFS_B_TO_FSBT(ip->i_mount, offset);
>  	end_fsb = XFS_B_TO_FSB(ip->i_mount, offset + count);
> -	count_fsb = (xfs_filblks_t)(end_fsb - offset_fsb);
>  
>  	/* Start a rolling transaction to switch the mappings */
>  	resblks = XFS_EXTENTADD_SPACE_RES(ip->i_mount, XFS_DATA_FORK);
> @@ -664,72 +665,65 @@ xfs_reflink_end_cow(
>  	xfs_ilock(ip, XFS_ILOCK_EXCL);
>  	xfs_trans_ijoin(tp, ip, 0);
>  
> -	/* Go find the old extent in the CoW fork. */
> -	while (offset_fsb < end_fsb) {
> -		/* Read extent from the source file */
> -		nimaps = 1;
> -		count_fsb = (xfs_filblks_t)(end_fsb - offset_fsb);
> -		error = xfs_bmapi_read(ip, offset_fsb, count_fsb, &irec,
> -				&nimaps, XFS_BMAPI_COWFORK);
> -		if (error)
> -			goto out_cancel;
> -		ASSERT(nimaps == 1);
> +	xfs_bmap_search_extents(ip, end_fsb - 1, XFS_COW_FORK, &eof, &idx,
> +			&got, &prev);
>  
> -		ASSERT(irec.br_startblock != DELAYSTARTBLOCK);
> -		trace_xfs_reflink_cow_remap(ip, &irec);
> +	/* If there is a hole at end_fsb - 1 go to the previous extent */
> +	if (eof || got.br_startoff > end_fsb) {
> +		ASSERT(idx > 0);
> +		xfs_bmbt_get_all(xfs_iext_get_ext(ifp, --idx), &got);
> +	}
>  
> -		/*
> -		 * We can have a hole in the CoW fork if part of a directio
> -		 * write is CoW but part of it isn't.
> -		 */
> -		rlen = ilen = irec.br_blockcount;
> -		if (irec.br_startblock == HOLESTARTBLOCK)
> +	/* Walk backwards until we're out of the I/O range... */
> +	while (got.br_startoff + got.br_blockcount > offset_fsb) {
> +		del = got;
> +		xfs_trim_extent(&del, offset_fsb, end_fsb - offset_fsb);
> +
> +		/* Extent delete may have bumped idx forward */
> +		if (!del.br_blockcount) {
> +			idx--;
>  			goto next_extent;
> +		}
> +
> +		ASSERT(!isnullstartblock(got.br_startblock));
>  
>  		/* Unmap the old blocks in the data fork. */
> -		while (rlen) {
> -			xfs_defer_init(&dfops, &firstfsb);
> -			error = __xfs_bunmapi(tp, ip, irec.br_startoff,
> -					&rlen, 0, 1, &firstfsb, &dfops);
> -			if (error)
> -				goto out_defer;
> -
> -			/*
> -			 * Trim the extent to whatever got unmapped.
> -			 * Remember, bunmapi works backwards.
> -			 */
> -			uirec.br_startblock = irec.br_startblock + rlen;
> -			uirec.br_startoff = irec.br_startoff + rlen;
> -			uirec.br_blockcount = irec.br_blockcount - rlen;
> -			irec.br_blockcount = rlen;
> -			trace_xfs_reflink_cow_remap_piece(ip, &uirec);
> +		xfs_defer_init(&dfops, &firstfsb);
> +		rlen = del.br_blockcount;
> +		error = __xfs_bunmapi(tp, ip, del.br_startoff, &rlen, 0, 1,
> +				&firstfsb, &dfops);
> +		if (error)
> +			goto out_defer;
>  
> -			/* Free the CoW orphan record. */
> -			error = xfs_refcount_free_cow_extent(tp->t_mountp,
> -					&dfops, uirec.br_startblock,
> -					uirec.br_blockcount);
> -			if (error)
> -				goto out_defer;
> +		/* Trim the extent to whatever got unmapped. */
> +		if (rlen) {
> +			xfs_trim_extent(&del, del.br_startoff + rlen,
> +				del.br_blockcount - rlen);
> +		}
> +		trace_xfs_reflink_cow_remap(ip, &del);
>  
> -			/* Map the new blocks into the data fork. */
> -			error = xfs_bmap_map_extent(tp->t_mountp, &dfops,
> -					ip, &uirec);
> -			if (error)
> -				goto out_defer;
> +		/* Free the CoW orphan record. */
> +		error = xfs_refcount_free_cow_extent(tp->t_mountp, &dfops,
> +				del.br_startblock, del.br_blockcount);
> +		if (error)
> +			goto out_defer;
>  
> -			/* Remove the mapping from the CoW fork. */
> -			error = xfs_bunmapi_cow(ip, &uirec);
> -			if (error)
> -				goto out_defer;
> +		/* Map the new blocks into the data fork. */
> +		error = xfs_bmap_map_extent(tp->t_mountp, &dfops, ip, &del);
> +		if (error)
> +			goto out_defer;
>  
> -			error = xfs_defer_finish(&tp, &dfops, ip);
> -			if (error)
> -				goto out_defer;
> -		}
> +		/* Remove the mapping from the CoW fork. */
> +		xfs_bmap_del_extent_cow(ip, &idx, &got, &del);
> +
> +		error = xfs_defer_finish(&tp, &dfops, ip);
> +		if (error)
> +			goto out_defer;
>  
>  next_extent:
> -		/* Roll on... */
> -		offset_fsb = irec.br_startoff + ilen;
> +		if (idx < 0)
> +			break;
> +		xfs_bmbt_get_all(xfs_iext_get_ext(ifp, idx), &got);
>  	}
>  
>  	error = xfs_trans_commit(tp);
> @@ -740,7 +734,6 @@ xfs_reflink_end_cow(
>  
>  out_defer:
>  	xfs_defer_cancel(&dfops);
> -out_cancel:
>  	xfs_trans_cancel(tp);
>  	xfs_iunlock(ip, XFS_ILOCK_EXCL);
>  out:
> diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> index 72f9f6b..0907752 100644
> --- a/fs/xfs/xfs_trace.h
> +++ b/fs/xfs/xfs_trace.h
> @@ -3356,7 +3356,6 @@ DEFINE_INODE_IREC_EVENT(xfs_reflink_trim_irec);
>  DEFINE_SIMPLE_IO_EVENT(xfs_reflink_cancel_cow_range);
>  DEFINE_SIMPLE_IO_EVENT(xfs_reflink_end_cow);
>  DEFINE_INODE_IREC_EVENT(xfs_reflink_cow_remap);
> -DEFINE_INODE_IREC_EVENT(xfs_reflink_cow_remap_piece);
>  
>  DEFINE_INODE_ERROR_EVENT(xfs_reflink_allocate_cow_range_error);
>  DEFINE_INODE_ERROR_EVENT(xfs_reflink_cancel_cow_range_error);
> -- 
> 2.1.4
> 

[PATCH 9/9] xfs: remove xfs_bunmapi_cow

[Christoph Hellwig]

Since no one uses it anymore.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_bmap.c | 22 ----------------------
 fs/xfs/libxfs/xfs_bmap.h |  1 -
 2 files changed, 23 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 64f82c8..cc193e6 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -5417,28 +5417,6 @@ xfs_bmap_del_extent(
 	return error;
 }
 
-/* Remove an extent from the CoW fork.  Similar to xfs_bmap_del_extent. */
-int
-xfs_bunmapi_cow(
-	struct xfs_inode		*ip,
-	struct xfs_bmbt_irec		*del)
-{
-	struct xfs_bmbt_rec_host	*ep;
-	struct xfs_bmbt_irec		got;
-	struct xfs_bmbt_irec		new;
-	int				eof;
-	xfs_extnum_t			eidx;
-
-	ep = xfs_bmap_search_extents(ip, del->br_startoff, XFS_COW_FORK, &eof,
-				&eidx, &got, &new);
-	ASSERT(ep);
-	if (isnullstartblock(got.br_startblock))
-		xfs_bmap_del_extent_delay(ip, XFS_COW_FORK, &eidx, &got, del);
-	else
-		xfs_bmap_del_extent_cow(ip, &eidx, &got, del);
-	return 0;
-}
-
 /*
  * Unmap (remove) blocks from a file.
  * If nexts is nonzero then the number of extents to remove is limited to
diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
index 5f18248..7cae6ec 100644
--- a/fs/xfs/libxfs/xfs_bmap.h
+++ b/fs/xfs/libxfs/xfs_bmap.h
@@ -223,7 +223,6 @@ int	xfs_bunmapi(struct xfs_trans *tp, struct xfs_inode *ip,
 		xfs_fileoff_t bno, xfs_filblks_t len, int flags,
 		xfs_extnum_t nexts, xfs_fsblock_t *firstblock,
 		struct xfs_defer_ops *dfops, int *done);
-int	xfs_bunmapi_cow(struct xfs_inode *ip, struct xfs_bmbt_irec *del);
 int	xfs_bmap_del_extent_delay(struct xfs_inode *ip, int whichfork,
 		xfs_extnum_t *idx, struct xfs_bmbt_irec *got,
 		struct xfs_bmbt_irec *del);
-- 
2.1.4

Re: [PATCH 9/9] xfs: remove xfs_bunmapi_cow

[Brian Foster]

On Sat, Oct 15, 2016 at 10:52:34AM +0200, Christoph Hellwig wrote:
> Since no one uses it anymore.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---

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

>  fs/xfs/libxfs/xfs_bmap.c | 22 ----------------------
>  fs/xfs/libxfs/xfs_bmap.h |  1 -
>  2 files changed, 23 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 64f82c8..cc193e6 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -5417,28 +5417,6 @@ xfs_bmap_del_extent(
>  	return error;
>  }
>  
> -/* Remove an extent from the CoW fork.  Similar to xfs_bmap_del_extent. */
> -int
> -xfs_bunmapi_cow(
> -	struct xfs_inode		*ip,
> -	struct xfs_bmbt_irec		*del)
> -{
> -	struct xfs_bmbt_rec_host	*ep;
> -	struct xfs_bmbt_irec		got;
> -	struct xfs_bmbt_irec		new;
> -	int				eof;
> -	xfs_extnum_t			eidx;
> -
> -	ep = xfs_bmap_search_extents(ip, del->br_startoff, XFS_COW_FORK, &eof,
> -				&eidx, &got, &new);
> -	ASSERT(ep);
> -	if (isnullstartblock(got.br_startblock))
> -		xfs_bmap_del_extent_delay(ip, XFS_COW_FORK, &eidx, &got, del);
> -	else
> -		xfs_bmap_del_extent_cow(ip, &eidx, &got, del);
> -	return 0;
> -}
> -
>  /*
>   * Unmap (remove) blocks from a file.
>   * If nexts is nonzero then the number of extents to remove is limited to
> diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
> index 5f18248..7cae6ec 100644
> --- a/fs/xfs/libxfs/xfs_bmap.h
> +++ b/fs/xfs/libxfs/xfs_bmap.h
> @@ -223,7 +223,6 @@ int	xfs_bunmapi(struct xfs_trans *tp, struct xfs_inode *ip,
>  		xfs_fileoff_t bno, xfs_filblks_t len, int flags,
>  		xfs_extnum_t nexts, xfs_fsblock_t *firstblock,
>  		struct xfs_defer_ops *dfops, int *done);
> -int	xfs_bunmapi_cow(struct xfs_inode *ip, struct xfs_bmbt_irec *del);
>  int	xfs_bmap_del_extent_delay(struct xfs_inode *ip, int whichfork,
>  		xfs_extnum_t *idx, struct xfs_bmbt_irec *got,
>  		struct xfs_bmbt_irec *del);
> -- 
> 2.1.4
> 
> --
> 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

Re: [PATCH 9/9] xfs: remove xfs_bunmapi_cow

[Darrick J. Wong]

On Sat, Oct 15, 2016 at 10:52:34AM +0200, Christoph Hellwig wrote:
> Since no one uses it anymore.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

> ---
>  fs/xfs/libxfs/xfs_bmap.c | 22 ----------------------
>  fs/xfs/libxfs/xfs_bmap.h |  1 -
>  2 files changed, 23 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 64f82c8..cc193e6 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -5417,28 +5417,6 @@ xfs_bmap_del_extent(
>  	return error;
>  }
>  
> -/* Remove an extent from the CoW fork.  Similar to xfs_bmap_del_extent. */
> -int
> -xfs_bunmapi_cow(
> -	struct xfs_inode		*ip,
> -	struct xfs_bmbt_irec		*del)
> -{
> -	struct xfs_bmbt_rec_host	*ep;
> -	struct xfs_bmbt_irec		got;
> -	struct xfs_bmbt_irec		new;
> -	int				eof;
> -	xfs_extnum_t			eidx;
> -
> -	ep = xfs_bmap_search_extents(ip, del->br_startoff, XFS_COW_FORK, &eof,
> -				&eidx, &got, &new);
> -	ASSERT(ep);
> -	if (isnullstartblock(got.br_startblock))
> -		xfs_bmap_del_extent_delay(ip, XFS_COW_FORK, &eidx, &got, del);
> -	else
> -		xfs_bmap_del_extent_cow(ip, &eidx, &got, del);
> -	return 0;
> -}
> -
>  /*
>   * Unmap (remove) blocks from a file.
>   * If nexts is nonzero then the number of extents to remove is limited to
> diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
> index 5f18248..7cae6ec 100644
> --- a/fs/xfs/libxfs/xfs_bmap.h
> +++ b/fs/xfs/libxfs/xfs_bmap.h
> @@ -223,7 +223,6 @@ int	xfs_bunmapi(struct xfs_trans *tp, struct xfs_inode *ip,
>  		xfs_fileoff_t bno, xfs_filblks_t len, int flags,
>  		xfs_extnum_t nexts, xfs_fsblock_t *firstblock,
>  		struct xfs_defer_ops *dfops, int *done);
> -int	xfs_bunmapi_cow(struct xfs_inode *ip, struct xfs_bmbt_irec *del);
>  int	xfs_bmap_del_extent_delay(struct xfs_inode *ip, int whichfork,
>  		xfs_extnum_t *idx, struct xfs_bmbt_irec *got,
>  		struct xfs_bmbt_irec *del);
> -- 
> 2.1.4
> 

optimize the COW I/O path

[Christoph Hellwig]

This series contains a couple optimizations for the COW I/O path:

 - avoid any overhead for reads from COW files
 - streamline the COW handling in write_begin
 - reduce the number of extent lookups during COW I/O completion

It does not touch the direct I/O path (yet), I still have a few
unsolved problems there.

[PATCH 4/9] xfs: don't bother looking at the refcount tree for reads

[Christoph Hellwig]

There is no need to trim an extent into a shared or non-shared one, or
report any flags for plain old reads.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_iomap.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index d907eb9..1dabf2e 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -996,11 +996,14 @@ xfs_file_iomap_begin(
 		return error;
 	}
 
-	/* Trim the mapping to the nearest shared extent boundary. */
-	error = xfs_reflink_trim_around_shared(ip, &imap, &shared, &trimmed);
-	if (error) {
-		xfs_iunlock(ip, lockmode);
-		return error;
+	if (flags & (IOMAP_WRITE | IOMAP_ZERO | IOMAP_REPORT)) {
+		/* Trim the mapping to the nearest shared extent boundary. */
+		error = xfs_reflink_trim_around_shared(ip, &imap, &shared,
+				&trimmed);
+		if (error) {
+			xfs_iunlock(ip, lockmode);
+			return error;
+		}
 	}
 
 	if ((flags & IOMAP_WRITE) && imap_needs_alloc(inode, &imap, nimaps)) {
-- 
2.10.1.382.ga23ca1b