* [PATCH 1/9] iomap: add IOMAP_REPORT
2016-10-15 8:52 optimize the COW I/O path V2 Christoph Hellwig
@ 2016-10-15 8:52 ` Christoph Hellwig
2016-10-17 15:35 ` Brian Foster
2016-10-17 16:18 ` Darrick J. Wong
2016-10-15 8:52 ` [PATCH 2/9] xfs: add xfs_trim_extent Christoph Hellwig
` (7 subsequent siblings)
8 siblings, 2 replies; 33+ messages in thread
From: Christoph Hellwig @ 2016-10-15 8:52 UTC (permalink / raw)
To: linux-xfs; +Cc: darrick.wong, bfoster
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
^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH 1/9] iomap: add IOMAP_REPORT
2016-10-15 8:52 ` [PATCH 1/9] iomap: add IOMAP_REPORT Christoph Hellwig
@ 2016-10-17 15:35 ` Brian Foster
2016-10-17 16:18 ` Darrick J. Wong
1 sibling, 0 replies; 33+ messages in thread
From: Brian Foster @ 2016-10-17 15:35 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs, darrick.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: 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
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 1/9] iomap: add IOMAP_REPORT
2016-10-15 8:52 ` [PATCH 1/9] iomap: add IOMAP_REPORT Christoph Hellwig
2016-10-17 15:35 ` Brian Foster
@ 2016-10-17 16:18 ` Darrick J. Wong
1 sibling, 0 replies; 33+ messages in thread
From: Darrick J. Wong @ 2016-10-17 16:18 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs, bfoster
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
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 2/9] xfs: add xfs_trim_extent
2016-10-15 8:52 optimize the COW I/O path V2 Christoph Hellwig
2016-10-15 8:52 ` [PATCH 1/9] iomap: add IOMAP_REPORT Christoph Hellwig
@ 2016-10-15 8:52 ` Christoph Hellwig
2016-10-17 15:35 ` Brian Foster
2016-10-17 16:27 ` Darrick J. Wong
2016-10-15 8:52 ` [PATCH 3/9] xfs: handle "raw" delayed extents xfs_reflink_trim_around_shared Christoph Hellwig
` (6 subsequent siblings)
8 siblings, 2 replies; 33+ messages in thread
From: Christoph Hellwig @ 2016-10-15 8:52 UTC (permalink / raw)
To: linux-xfs; +Cc: darrick.wong, bfoster
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
^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH 2/9] xfs: add xfs_trim_extent
2016-10-15 8:52 ` [PATCH 2/9] xfs: add xfs_trim_extent Christoph Hellwig
@ 2016-10-17 15:35 ` Brian Foster
2016-10-17 16:27 ` Darrick J. Wong
1 sibling, 0 replies; 33+ messages in thread
From: Brian Foster @ 2016-10-17 15:35 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs, darrick.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: 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
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/9] xfs: add xfs_trim_extent
2016-10-15 8:52 ` [PATCH 2/9] xfs: add xfs_trim_extent Christoph Hellwig
2016-10-17 15:35 ` Brian Foster
@ 2016-10-17 16:27 ` Darrick J. Wong
1 sibling, 0 replies; 33+ messages in thread
From: Darrick J. Wong @ 2016-10-17 16:27 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs, bfoster
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
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 3/9] xfs: handle "raw" delayed extents xfs_reflink_trim_around_shared
2016-10-15 8:52 optimize the COW I/O path V2 Christoph Hellwig
2016-10-15 8:52 ` [PATCH 1/9] iomap: add IOMAP_REPORT Christoph Hellwig
2016-10-15 8:52 ` [PATCH 2/9] xfs: add xfs_trim_extent Christoph Hellwig
@ 2016-10-15 8:52 ` Christoph Hellwig
2016-10-17 15:35 ` Brian Foster
2016-10-17 16:27 ` Darrick J. Wong
2016-10-15 8:52 ` [PATCH 4/9] xfs: don't bother looking at the refcount tree for reads Christoph Hellwig
` (5 subsequent siblings)
8 siblings, 2 replies; 33+ messages in thread
From: Christoph Hellwig @ 2016-10-15 8:52 UTC (permalink / raw)
To: linux-xfs; +Cc: darrick.wong, bfoster
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
^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH 3/9] xfs: handle "raw" delayed extents xfs_reflink_trim_around_shared
2016-10-15 8:52 ` [PATCH 3/9] xfs: handle "raw" delayed extents xfs_reflink_trim_around_shared Christoph Hellwig
@ 2016-10-17 15:35 ` Brian Foster
2016-10-17 16:27 ` Darrick J. Wong
1 sibling, 0 replies; 33+ messages in thread
From: Brian Foster @ 2016-10-17 15:35 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs, darrick.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: 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
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 3/9] xfs: handle "raw" delayed extents xfs_reflink_trim_around_shared
2016-10-15 8:52 ` [PATCH 3/9] xfs: handle "raw" delayed extents xfs_reflink_trim_around_shared Christoph Hellwig
2016-10-17 15:35 ` Brian Foster
@ 2016-10-17 16:27 ` Darrick J. Wong
1 sibling, 0 replies; 33+ messages in thread
From: Darrick J. Wong @ 2016-10-17 16:27 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs, bfoster
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
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 4/9] xfs: don't bother looking at the refcount tree for reads
2016-10-15 8:52 optimize the COW I/O path V2 Christoph Hellwig
` (2 preceding siblings ...)
2016-10-15 8:52 ` [PATCH 3/9] xfs: handle "raw" delayed extents xfs_reflink_trim_around_shared Christoph Hellwig
@ 2016-10-15 8:52 ` Christoph Hellwig
2016-10-17 15:36 ` Brian Foster
2016-10-17 16:41 ` Darrick J. Wong
2016-10-15 8:52 ` [PATCH 5/9] xfs: optimize writes to reflink files Christoph Hellwig
` (4 subsequent siblings)
8 siblings, 2 replies; 33+ messages in thread
From: Christoph Hellwig @ 2016-10-15 8:52 UTC (permalink / raw)
To: linux-xfs; +Cc: darrick.wong, bfoster
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
^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH 4/9] xfs: don't bother looking at the refcount tree for reads
2016-10-15 8:52 ` [PATCH 4/9] xfs: don't bother looking at the refcount tree for reads Christoph Hellwig
@ 2016-10-17 15:36 ` Brian Foster
2016-10-17 16:41 ` Darrick J. Wong
1 sibling, 0 replies; 33+ messages in thread
From: Brian Foster @ 2016-10-17 15:36 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs, darrick.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>
> ---
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
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 4/9] xfs: don't bother looking at the refcount tree for reads
2016-10-15 8:52 ` [PATCH 4/9] xfs: don't bother looking at the refcount tree for reads Christoph Hellwig
2016-10-17 15:36 ` Brian Foster
@ 2016-10-17 16:41 ` Darrick J. Wong
1 sibling, 0 replies; 33+ messages in thread
From: Darrick J. Wong @ 2016-10-17 16:41 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs, bfoster
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
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 5/9] xfs: optimize writes to reflink files
2016-10-15 8:52 optimize the COW I/O path V2 Christoph Hellwig
` (3 preceding siblings ...)
2016-10-15 8:52 ` [PATCH 4/9] xfs: don't bother looking at the refcount tree for reads Christoph Hellwig
@ 2016-10-15 8:52 ` Christoph Hellwig
2016-10-17 17:19 ` Brian Foster
2016-10-17 17:34 ` Darrick J. Wong
2016-10-15 8:52 ` [PATCH 6/9] xfs: refactor xfs_bunmapi_cow Christoph Hellwig
` (3 subsequent siblings)
8 siblings, 2 replies; 33+ messages in thread
From: Christoph Hellwig @ 2016-10-15 8:52 UTC (permalink / raw)
To: linux-xfs; +Cc: darrick.wong, bfoster
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
^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH 5/9] xfs: optimize writes to reflink files
2016-10-15 8:52 ` [PATCH 5/9] xfs: optimize writes to reflink files Christoph Hellwig
@ 2016-10-17 17:19 ` Brian Foster
2016-10-17 17:34 ` Darrick J. Wong
1 sibling, 0 replies; 33+ messages in thread
From: Brian Foster @ 2016-10-17 17:19 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs, darrick.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: 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
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 5/9] xfs: optimize writes to reflink files
2016-10-15 8:52 ` [PATCH 5/9] xfs: optimize writes to reflink files Christoph Hellwig
2016-10-17 17:19 ` Brian Foster
@ 2016-10-17 17:34 ` Darrick J. Wong
1 sibling, 0 replies; 33+ messages in thread
From: Darrick J. Wong @ 2016-10-17 17:34 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs, bfoster
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
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 6/9] xfs: refactor xfs_bunmapi_cow
2016-10-15 8:52 optimize the COW I/O path V2 Christoph Hellwig
` (4 preceding siblings ...)
2016-10-15 8:52 ` [PATCH 5/9] xfs: optimize writes to reflink files Christoph Hellwig
@ 2016-10-15 8:52 ` Christoph Hellwig
2016-10-17 17:21 ` Brian Foster
2016-10-17 18:32 ` Darrick J. Wong
2016-10-15 8:52 ` [PATCH 7/9] xfs: optimize xfs_reflink_cancel_cow_blocks Christoph Hellwig
` (2 subsequent siblings)
8 siblings, 2 replies; 33+ messages in thread
From: Christoph Hellwig @ 2016-10-15 8:52 UTC (permalink / raw)
To: linux-xfs; +Cc: darrick.wong, bfoster
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
^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH 6/9] xfs: refactor xfs_bunmapi_cow
2016-10-15 8:52 ` [PATCH 6/9] xfs: refactor xfs_bunmapi_cow Christoph Hellwig
@ 2016-10-17 17:21 ` Brian Foster
2016-10-17 17:44 ` Christoph Hellwig
2016-10-17 18:32 ` Darrick J. Wong
1 sibling, 1 reply; 33+ messages in thread
From: Brian Foster @ 2016-10-17 17:21 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs, darrick.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>
> ---
> 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
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 6/9] xfs: refactor xfs_bunmapi_cow
2016-10-17 17:21 ` Brian Foster
@ 2016-10-17 17:44 ` Christoph Hellwig
2016-10-17 17:53 ` Brian Foster
0 siblings, 1 reply; 33+ messages in thread
From: Christoph Hellwig @ 2016-10-17 17:44 UTC (permalink / raw)
To: Brian Foster; +Cc: Christoph Hellwig, linux-xfs, darrick.wong
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.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 6/9] xfs: refactor xfs_bunmapi_cow
2016-10-17 17:44 ` Christoph Hellwig
@ 2016-10-17 17:53 ` Brian Foster
0 siblings, 0 replies; 33+ messages in thread
From: Brian Foster @ 2016-10-17 17:53 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs, darrick.wong
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
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 6/9] xfs: refactor xfs_bunmapi_cow
2016-10-15 8:52 ` [PATCH 6/9] xfs: refactor xfs_bunmapi_cow Christoph Hellwig
2016-10-17 17:21 ` Brian Foster
@ 2016-10-17 18:32 ` Darrick J. Wong
1 sibling, 0 replies; 33+ messages in thread
From: Darrick J. Wong @ 2016-10-17 18:32 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs, bfoster
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
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 7/9] xfs: optimize xfs_reflink_cancel_cow_blocks
2016-10-15 8:52 optimize the COW I/O path V2 Christoph Hellwig
` (5 preceding siblings ...)
2016-10-15 8:52 ` [PATCH 6/9] xfs: refactor xfs_bunmapi_cow Christoph Hellwig
@ 2016-10-15 8:52 ` Christoph Hellwig
2016-10-17 17:52 ` Brian Foster
2016-10-17 18:33 ` Darrick J. Wong
2016-10-15 8:52 ` [PATCH 8/9] xfs: optimize xfs_reflink_end_cow Christoph Hellwig
2016-10-15 8:52 ` [PATCH 9/9] xfs: remove xfs_bunmapi_cow Christoph Hellwig
8 siblings, 2 replies; 33+ messages in thread
From: Christoph Hellwig @ 2016-10-15 8:52 UTC (permalink / raw)
To: linux-xfs; +Cc: darrick.wong, bfoster
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
^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH 7/9] xfs: optimize xfs_reflink_cancel_cow_blocks
2016-10-15 8:52 ` [PATCH 7/9] xfs: optimize xfs_reflink_cancel_cow_blocks Christoph Hellwig
@ 2016-10-17 17:52 ` Brian Foster
2016-10-17 18:33 ` Darrick J. Wong
1 sibling, 0 replies; 33+ messages in thread
From: Brian Foster @ 2016-10-17 17:52 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs, darrick.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: 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
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 7/9] xfs: optimize xfs_reflink_cancel_cow_blocks
2016-10-15 8:52 ` [PATCH 7/9] xfs: optimize xfs_reflink_cancel_cow_blocks Christoph Hellwig
2016-10-17 17:52 ` Brian Foster
@ 2016-10-17 18:33 ` Darrick J. Wong
1 sibling, 0 replies; 33+ messages in thread
From: Darrick J. Wong @ 2016-10-17 18:33 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs, bfoster
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
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 8/9] xfs: optimize xfs_reflink_end_cow
2016-10-15 8:52 optimize the COW I/O path V2 Christoph Hellwig
` (6 preceding siblings ...)
2016-10-15 8:52 ` [PATCH 7/9] xfs: optimize xfs_reflink_cancel_cow_blocks Christoph Hellwig
@ 2016-10-15 8:52 ` Christoph Hellwig
2016-10-17 17:53 ` Brian Foster
2016-10-17 18:34 ` Darrick J. Wong
2016-10-15 8:52 ` [PATCH 9/9] xfs: remove xfs_bunmapi_cow Christoph Hellwig
8 siblings, 2 replies; 33+ messages in thread
From: Christoph Hellwig @ 2016-10-15 8:52 UTC (permalink / raw)
To: linux-xfs; +Cc: darrick.wong, bfoster
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
^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH 8/9] xfs: optimize xfs_reflink_end_cow
2016-10-15 8:52 ` [PATCH 8/9] xfs: optimize xfs_reflink_end_cow Christoph Hellwig
@ 2016-10-17 17:53 ` Brian Foster
2016-10-17 18:34 ` Darrick J. Wong
1 sibling, 0 replies; 33+ messages in thread
From: Brian Foster @ 2016-10-17 17:53 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs, darrick.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: 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
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 8/9] xfs: optimize xfs_reflink_end_cow
2016-10-15 8:52 ` [PATCH 8/9] xfs: optimize xfs_reflink_end_cow Christoph Hellwig
2016-10-17 17:53 ` Brian Foster
@ 2016-10-17 18:34 ` Darrick J. Wong
1 sibling, 0 replies; 33+ messages in thread
From: Darrick J. Wong @ 2016-10-17 18:34 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs, bfoster
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
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 9/9] xfs: remove xfs_bunmapi_cow
2016-10-15 8:52 optimize the COW I/O path V2 Christoph Hellwig
` (7 preceding siblings ...)
2016-10-15 8:52 ` [PATCH 8/9] xfs: optimize xfs_reflink_end_cow Christoph Hellwig
@ 2016-10-15 8:52 ` Christoph Hellwig
2016-10-17 17:53 ` Brian Foster
2016-10-17 18:34 ` Darrick J. Wong
8 siblings, 2 replies; 33+ messages in thread
From: Christoph Hellwig @ 2016-10-15 8:52 UTC (permalink / raw)
To: linux-xfs; +Cc: darrick.wong, bfoster
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
^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH 9/9] xfs: remove xfs_bunmapi_cow
2016-10-15 8:52 ` [PATCH 9/9] xfs: remove xfs_bunmapi_cow Christoph Hellwig
@ 2016-10-17 17:53 ` Brian Foster
2016-10-17 18:34 ` Darrick J. Wong
1 sibling, 0 replies; 33+ messages in thread
From: Brian Foster @ 2016-10-17 17:53 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs, darrick.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: 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
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 9/9] xfs: remove xfs_bunmapi_cow
2016-10-15 8:52 ` [PATCH 9/9] xfs: remove xfs_bunmapi_cow Christoph Hellwig
2016-10-17 17:53 ` Brian Foster
@ 2016-10-17 18:34 ` Darrick J. Wong
1 sibling, 0 replies; 33+ messages in thread
From: Darrick J. Wong @ 2016-10-17 18:34 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs, bfoster
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
>
^ permalink raw reply [flat|nested] 33+ messages in thread