misc fixes and cleanups

misc fixes and cleanups

[Christoph Hellwig]

Below are a set of small fixes and cleanups that were the fallout of
not yet finished feature development.

[PATCH 1/8] xfs: fix the check for COW extents in xfs_swap_extents

[Christoph Hellwig]

i_cnextents does not include delayed allocated extents, so switch
to the inode fork size check that we already use in other places
instead.

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

diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index e0a442f504e5..19ea7d086cf8 100644
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -2014,11 +2014,11 @@ xfs_swap_extents(
 		ip->i_cowfp = tip->i_cowfp;
 		tip->i_cowfp = cowfp;
 
-		if (ip->i_cowfp && ip->i_cnextents)
+		if (ip->i_cowfp && ip->i_cowfp->if_bytes)
 			xfs_inode_set_cowblocks_tag(ip);
 		else
 			xfs_inode_clear_cowblocks_tag(ip);
-		if (tip->i_cowfp && tip->i_cnextents)
+		if (tip->i_cowfp && tip->i_cowfp->if_bytes)
 			xfs_inode_set_cowblocks_tag(tip);
 		else
 			xfs_inode_clear_cowblocks_tag(tip);
-- 
2.14.2

Re: [PATCH 1/8] xfs: fix the check for COW extents in xfs_swap_extents

[Darrick J. Wong]

On Tue, Mar 13, 2018 at 03:35:54PM +0100, Christoph Hellwig wrote:
> i_cnextents does not include delayed allocated extents, so switch
> to the inode fork size check that we already use in other places
> instead.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

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

--D

> ---
>  fs/xfs/xfs_bmap_util.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
> index e0a442f504e5..19ea7d086cf8 100644
> --- a/fs/xfs/xfs_bmap_util.c
> +++ b/fs/xfs/xfs_bmap_util.c
> @@ -2014,11 +2014,11 @@ xfs_swap_extents(
>  		ip->i_cowfp = tip->i_cowfp;
>  		tip->i_cowfp = cowfp;
>  
> -		if (ip->i_cowfp && ip->i_cnextents)
> +		if (ip->i_cowfp && ip->i_cowfp->if_bytes)
>  			xfs_inode_set_cowblocks_tag(ip);
>  		else
>  			xfs_inode_clear_cowblocks_tag(ip);
> -		if (tip->i_cowfp && tip->i_cnextents)
> +		if (tip->i_cowfp && tip->i_cowfp->if_bytes)
>  			xfs_inode_set_cowblocks_tag(tip);
>  		else
>  			xfs_inode_clear_cowblocks_tag(tip);
> -- 
> 2.14.2
> 
> --
> 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

[PATCH 2/8] xfs: don't use XFS_BMAPI_ENTRIRE in xfs_get_blocks

[Christoph Hellwig]

There is no reason to get a mapping bigger than what we were asked for.

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

diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
index a0afb6411417..c79a3ca20ef8 100644
--- a/fs/xfs/xfs_aops.c
+++ b/fs/xfs/xfs_aops.c
@@ -1331,8 +1331,8 @@ xfs_get_blocks(
 	end_fsb = XFS_B_TO_FSB(mp, (xfs_ufsize_t)offset + size);
 	offset_fsb = XFS_B_TO_FSBT(mp, offset);
 
-	error = xfs_bmapi_read(ip, offset_fsb, end_fsb - offset_fsb,
-				&imap, &nimaps, XFS_BMAPI_ENTIRE);
+	error = xfs_bmapi_read(ip, offset_fsb, end_fsb - offset_fsb, &imap,
+			&nimaps, 0);
 	if (error)
 		goto out_unlock;
 
-- 
2.14.2

Re: [PATCH 2/8] xfs: don't use XFS_BMAPI_ENTRIRE in xfs_get_blocks

[Darrick J. Wong]

On Tue, Mar 13, 2018 at 03:35:55PM +0100, Christoph Hellwig wrote:
> There is no reason to get a mapping bigger than what we were asked for.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

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

--D

> ---
>  fs/xfs/xfs_aops.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
> index a0afb6411417..c79a3ca20ef8 100644
> --- a/fs/xfs/xfs_aops.c
> +++ b/fs/xfs/xfs_aops.c
> @@ -1331,8 +1331,8 @@ xfs_get_blocks(
>  	end_fsb = XFS_B_TO_FSB(mp, (xfs_ufsize_t)offset + size);
>  	offset_fsb = XFS_B_TO_FSBT(mp, offset);
>  
> -	error = xfs_bmapi_read(ip, offset_fsb, end_fsb - offset_fsb,
> -				&imap, &nimaps, XFS_BMAPI_ENTIRE);
> +	error = xfs_bmapi_read(ip, offset_fsb, end_fsb - offset_fsb, &imap,
> +			&nimaps, 0);
>  	if (error)
>  		goto out_unlock;
>  
> -- 
> 2.14.2
> 
> --
> 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

[PATCH 3/8] xfs: assert that xfs_reflink_allocate_cow is called with XFS_ILOCK_EXCL

[Christoph Hellwig]

Now that we convert COW preallocations from unwritten to real on every
call this function needs to be called with the ilock held exclusively.

Fortunately we already do that, but update the assert to match.

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

diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index d81c4f868d69..90aac8889dd9 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -394,7 +394,7 @@ xfs_reflink_allocate_cow(
 
 retry:
 	ASSERT(xfs_is_reflink_inode(ip));
-	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL | XFS_ILOCK_SHARED));
+	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
 
 	/*
 	 * Even if the extent is not shared we might have a preallocation for
-- 
2.14.2

Re: [PATCH 3/8] xfs: assert that xfs_reflink_allocate_cow is called with XFS_ILOCK_EXCL

[Darrick J. Wong]

On Tue, Mar 13, 2018 at 03:35:56PM +0100, Christoph Hellwig wrote:
> Now that we convert COW preallocations from unwritten to real on every
> call this function needs to be called with the ilock held exclusively.
> 
> Fortunately we already do that, but update the assert to match.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

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

--D

> ---
>  fs/xfs/xfs_reflink.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index d81c4f868d69..90aac8889dd9 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -394,7 +394,7 @@ xfs_reflink_allocate_cow(
>  
>  retry:
>  	ASSERT(xfs_is_reflink_inode(ip));
> -	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL | XFS_ILOCK_SHARED));
> +	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
>  
>  	/*
>  	 * Even if the extent is not shared we might have a preallocation for
> -- 
> 2.14.2
> 
> --
> 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

[PATCH 4/8] xfs: remove xfs_zero_range

[Christoph Hellwig]

This helper doesn't add any real value over just calling iomap_zero_range
directly, so remove it.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_bmap_util.c | 11 ++++-------
 fs/xfs/xfs_file.c      | 48 +++++++-----------------------------------------
 fs/xfs/xfs_inode.h     |  4 ----
 fs/xfs/xfs_iops.c      |  4 +++-
 4 files changed, 14 insertions(+), 53 deletions(-)

diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index 19ea7d086cf8..05dee8fdd895 100644
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -1208,18 +1208,15 @@ xfs_free_file_space(
 
 	/*
 	 * Now that we've unmap all full blocks we'll have to zero out any
-	 * partial block at the beginning and/or end.  xfs_zero_range is
-	 * smart enough to skip any holes, including those we just created,
-	 * but we must take care not to zero beyond EOF and enlarge i_size.
+	 * partial block at the beginning and/or end.  iomap_zero_range is smart
+	 * enough to skip any holes, including those we just created, but we
+	 * must take care not to zero beyond EOF and enlarge i_size.
 	 */
-
 	if (offset >= XFS_ISIZE(ip))
 		return 0;
-
 	if (offset + len > XFS_ISIZE(ip))
 		len = XFS_ISIZE(ip) - offset;
-
-	return xfs_zero_range(ip, offset, len, NULL);
+	return iomap_zero_range(VFS_I(ip), offset, len, NULL, &xfs_iomap_ops);
 }
 
 /*
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 9ea08326f876..d2666ddf7381 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -48,20 +48,6 @@
 
 static const struct vm_operations_struct xfs_file_vm_ops;
 
-/*
- * Clear the specified ranges to zero through either the pagecache or DAX.
- * Holes and unwritten extents will be left as-is as they already are zeroed.
- */
-int
-xfs_zero_range(
-	struct xfs_inode	*ip,
-	xfs_off_t		pos,
-	xfs_off_t		count,
-	bool			*did_zero)
-{
-	return iomap_zero_range(VFS_I(ip), pos, count, did_zero, &xfs_iomap_ops);
-}
-
 int
 xfs_update_prealloc_flags(
 	struct xfs_inode	*ip,
@@ -300,31 +286,6 @@ xfs_file_read_iter(
 	return ret;
 }
 
-/*
- * Zero any on disk space between the current EOF and the new, larger EOF.
- *
- * This handles the normal case of zeroing the remainder of the last block in
- * the file and the unusual case of zeroing blocks out beyond the size of the
- * file.  This second case only happens with fixed size extents and when the
- * system crashes before the inode size was updated but after blocks were
- * allocated.
- *
- * Expects the iolock to be held exclusive, and will take the ilock internally.
- */
-int					/* error (positive) */
-xfs_zero_eof(
-	struct xfs_inode	*ip,
-	xfs_off_t		offset,		/* starting I/O offset */
-	xfs_fsize_t		isize,		/* current inode size */
-	bool			*did_zeroing)
-{
-	ASSERT(xfs_isilocked(ip, XFS_IOLOCK_EXCL));
-	ASSERT(offset > isize);
-
-	trace_xfs_zero_eof(ip, isize, offset - isize);
-	return xfs_zero_range(ip, isize, offset - isize, did_zeroing);
-}
-
 /*
  * Common pre-write limit and setup checks.
  *
@@ -344,6 +305,7 @@ xfs_file_aio_write_checks(
 	ssize_t			error = 0;
 	size_t			count = iov_iter_count(from);
 	bool			drained_dio = false;
+	loff_t			isize;
 
 restart:
 	error = generic_write_checks(iocb, from);
@@ -380,7 +342,8 @@ xfs_file_aio_write_checks(
 	 * and hence be able to correctly determine if we need to run zeroing.
 	 */
 	spin_lock(&ip->i_flags_lock);
-	if (iocb->ki_pos > i_size_read(inode)) {
+	isize = i_size_read(inode);
+	if (iocb->ki_pos > isize) {
 		spin_unlock(&ip->i_flags_lock);
 		if (!drained_dio) {
 			if (*iolock == XFS_IOLOCK_SHARED) {
@@ -401,7 +364,10 @@ xfs_file_aio_write_checks(
 			drained_dio = true;
 			goto restart;
 		}
-		error = xfs_zero_eof(ip, iocb->ki_pos, i_size_read(inode), NULL);
+	
+		trace_xfs_zero_eof(ip, isize, iocb->ki_pos - isize);
+		error = iomap_zero_range(inode, isize, iocb->ki_pos - isize,
+				NULL, &xfs_iomap_ops);
 		if (error)
 			return error;
 	} else
diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
index 3e8dc990d41c..132d8aa2afc4 100644
--- a/fs/xfs/xfs_inode.h
+++ b/fs/xfs/xfs_inode.h
@@ -443,10 +443,6 @@ enum xfs_prealloc_flags {
 
 int	xfs_update_prealloc_flags(struct xfs_inode *ip,
 				  enum xfs_prealloc_flags flags);
-int	xfs_zero_eof(struct xfs_inode *ip, xfs_off_t offset,
-		     xfs_fsize_t isize, bool *did_zeroing);
-int	xfs_zero_range(struct xfs_inode *ip, xfs_off_t pos, xfs_off_t count,
-		bool *did_zero);
 
 /* from xfs_iops.c */
 extern void xfs_setup_inode(struct xfs_inode *ip);
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index 8567951eff10..e0307fbff911 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -875,7 +875,9 @@ xfs_setattr_size(
 	 * truncate.
 	 */
 	if (newsize > oldsize) {
-		error = xfs_zero_eof(ip, newsize, oldsize, &did_zeroing);
+		trace_xfs_zero_eof(ip, oldsize, newsize - oldsize);
+		error = iomap_zero_range(inode, oldsize, newsize - oldsize,
+				&did_zeroing, &xfs_iomap_ops);
 	} else {
 		error = iomap_truncate_page(inode, newsize, &did_zeroing,
 				&xfs_iomap_ops);
-- 
2.14.2

Re: [PATCH 4/8] xfs: remove xfs_zero_range

[Darrick J. Wong]

On Tue, Mar 13, 2018 at 03:35:57PM +0100, Christoph Hellwig wrote:
> This helper doesn't add any real value over just calling iomap_zero_range
> directly, so remove it.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

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

--D

> ---
>  fs/xfs/xfs_bmap_util.c | 11 ++++-------
>  fs/xfs/xfs_file.c      | 48 +++++++-----------------------------------------
>  fs/xfs/xfs_inode.h     |  4 ----
>  fs/xfs/xfs_iops.c      |  4 +++-
>  4 files changed, 14 insertions(+), 53 deletions(-)
> 
> diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
> index 19ea7d086cf8..05dee8fdd895 100644
> --- a/fs/xfs/xfs_bmap_util.c
> +++ b/fs/xfs/xfs_bmap_util.c
> @@ -1208,18 +1208,15 @@ xfs_free_file_space(
>  
>  	/*
>  	 * Now that we've unmap all full blocks we'll have to zero out any
> -	 * partial block at the beginning and/or end.  xfs_zero_range is
> -	 * smart enough to skip any holes, including those we just created,
> -	 * but we must take care not to zero beyond EOF and enlarge i_size.
> +	 * partial block at the beginning and/or end.  iomap_zero_range is smart
> +	 * enough to skip any holes, including those we just created, but we
> +	 * must take care not to zero beyond EOF and enlarge i_size.
>  	 */
> -
>  	if (offset >= XFS_ISIZE(ip))
>  		return 0;
> -
>  	if (offset + len > XFS_ISIZE(ip))
>  		len = XFS_ISIZE(ip) - offset;
> -
> -	return xfs_zero_range(ip, offset, len, NULL);
> +	return iomap_zero_range(VFS_I(ip), offset, len, NULL, &xfs_iomap_ops);
>  }
>  
>  /*
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index 9ea08326f876..d2666ddf7381 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -48,20 +48,6 @@
>  
>  static const struct vm_operations_struct xfs_file_vm_ops;
>  
> -/*
> - * Clear the specified ranges to zero through either the pagecache or DAX.
> - * Holes and unwritten extents will be left as-is as they already are zeroed.
> - */
> -int
> -xfs_zero_range(
> -	struct xfs_inode	*ip,
> -	xfs_off_t		pos,
> -	xfs_off_t		count,
> -	bool			*did_zero)
> -{
> -	return iomap_zero_range(VFS_I(ip), pos, count, did_zero, &xfs_iomap_ops);
> -}
> -
>  int
>  xfs_update_prealloc_flags(
>  	struct xfs_inode	*ip,
> @@ -300,31 +286,6 @@ xfs_file_read_iter(
>  	return ret;
>  }
>  
> -/*
> - * Zero any on disk space between the current EOF and the new, larger EOF.
> - *
> - * This handles the normal case of zeroing the remainder of the last block in
> - * the file and the unusual case of zeroing blocks out beyond the size of the
> - * file.  This second case only happens with fixed size extents and when the
> - * system crashes before the inode size was updated but after blocks were
> - * allocated.
> - *
> - * Expects the iolock to be held exclusive, and will take the ilock internally.
> - */
> -int					/* error (positive) */
> -xfs_zero_eof(
> -	struct xfs_inode	*ip,
> -	xfs_off_t		offset,		/* starting I/O offset */
> -	xfs_fsize_t		isize,		/* current inode size */
> -	bool			*did_zeroing)
> -{
> -	ASSERT(xfs_isilocked(ip, XFS_IOLOCK_EXCL));
> -	ASSERT(offset > isize);
> -
> -	trace_xfs_zero_eof(ip, isize, offset - isize);
> -	return xfs_zero_range(ip, isize, offset - isize, did_zeroing);
> -}
> -
>  /*
>   * Common pre-write limit and setup checks.
>   *
> @@ -344,6 +305,7 @@ xfs_file_aio_write_checks(
>  	ssize_t			error = 0;
>  	size_t			count = iov_iter_count(from);
>  	bool			drained_dio = false;
> +	loff_t			isize;
>  
>  restart:
>  	error = generic_write_checks(iocb, from);
> @@ -380,7 +342,8 @@ xfs_file_aio_write_checks(
>  	 * and hence be able to correctly determine if we need to run zeroing.
>  	 */
>  	spin_lock(&ip->i_flags_lock);
> -	if (iocb->ki_pos > i_size_read(inode)) {
> +	isize = i_size_read(inode);
> +	if (iocb->ki_pos > isize) {
>  		spin_unlock(&ip->i_flags_lock);
>  		if (!drained_dio) {
>  			if (*iolock == XFS_IOLOCK_SHARED) {
> @@ -401,7 +364,10 @@ xfs_file_aio_write_checks(
>  			drained_dio = true;
>  			goto restart;
>  		}
> -		error = xfs_zero_eof(ip, iocb->ki_pos, i_size_read(inode), NULL);
> +	
> +		trace_xfs_zero_eof(ip, isize, iocb->ki_pos - isize);
> +		error = iomap_zero_range(inode, isize, iocb->ki_pos - isize,
> +				NULL, &xfs_iomap_ops);
>  		if (error)
>  			return error;
>  	} else
> diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
> index 3e8dc990d41c..132d8aa2afc4 100644
> --- a/fs/xfs/xfs_inode.h
> +++ b/fs/xfs/xfs_inode.h
> @@ -443,10 +443,6 @@ enum xfs_prealloc_flags {
>  
>  int	xfs_update_prealloc_flags(struct xfs_inode *ip,
>  				  enum xfs_prealloc_flags flags);
> -int	xfs_zero_eof(struct xfs_inode *ip, xfs_off_t offset,
> -		     xfs_fsize_t isize, bool *did_zeroing);
> -int	xfs_zero_range(struct xfs_inode *ip, xfs_off_t pos, xfs_off_t count,
> -		bool *did_zero);
>  
>  /* from xfs_iops.c */
>  extern void xfs_setup_inode(struct xfs_inode *ip);
> diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
> index 8567951eff10..e0307fbff911 100644
> --- a/fs/xfs/xfs_iops.c
> +++ b/fs/xfs/xfs_iops.c
> @@ -875,7 +875,9 @@ xfs_setattr_size(
>  	 * truncate.
>  	 */
>  	if (newsize > oldsize) {
> -		error = xfs_zero_eof(ip, newsize, oldsize, &did_zeroing);
> +		trace_xfs_zero_eof(ip, oldsize, newsize - oldsize);
> +		error = iomap_zero_range(inode, oldsize, newsize - oldsize,
> +				&did_zeroing, &xfs_iomap_ops);
>  	} else {
>  		error = iomap_truncate_page(inode, newsize, &did_zeroing,
>  				&xfs_iomap_ops);
> -- 
> 2.14.2
> 
> --
> 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

[PATCH 5/8] xfs: minor cleanup for xfs_get_blocks

[Christoph Hellwig]

Simplify the control flow a bit in preparation for O_ATOMIC-related
changes.

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

diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
index c79a3ca20ef8..19eadc807056 100644
--- a/fs/xfs/xfs_aops.c
+++ b/fs/xfs/xfs_aops.c
@@ -1335,17 +1335,16 @@ xfs_get_blocks(
 			&nimaps, 0);
 	if (error)
 		goto out_unlock;
-
-	if (nimaps) {
-		trace_xfs_get_blocks_found(ip, offset, size,
-			imap.br_state == XFS_EXT_UNWRITTEN ?
-				XFS_IO_UNWRITTEN : XFS_IO_OVERWRITE, &imap);
-		xfs_iunlock(ip, lockmode);
-	} else {
+	if (!nimaps) {
 		trace_xfs_get_blocks_notfound(ip, offset, size);
 		goto out_unlock;
 	}
 
+	trace_xfs_get_blocks_found(ip, offset, size,
+		imap.br_state == XFS_EXT_UNWRITTEN ?
+			XFS_IO_UNWRITTEN : XFS_IO_OVERWRITE, &imap);
+	xfs_iunlock(ip, lockmode);
+
 	/* trim mapping down to size requested */
 	xfs_map_trim_size(inode, iblock, bh_result, &imap, offset, size);
 
-- 
2.14.2

Re: [PATCH 5/8] xfs: minor cleanup for xfs_get_blocks

[Darrick J. Wong]

On Tue, Mar 13, 2018 at 03:35:58PM +0100, Christoph Hellwig wrote:
> Simplify the control flow a bit in preparation for O_ATOMIC-related
> changes.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

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

--D

> ---
>  fs/xfs/xfs_aops.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
> index c79a3ca20ef8..19eadc807056 100644
> --- a/fs/xfs/xfs_aops.c
> +++ b/fs/xfs/xfs_aops.c
> @@ -1335,17 +1335,16 @@ xfs_get_blocks(
>  			&nimaps, 0);
>  	if (error)
>  		goto out_unlock;
> -
> -	if (nimaps) {
> -		trace_xfs_get_blocks_found(ip, offset, size,
> -			imap.br_state == XFS_EXT_UNWRITTEN ?
> -				XFS_IO_UNWRITTEN : XFS_IO_OVERWRITE, &imap);
> -		xfs_iunlock(ip, lockmode);
> -	} else {
> +	if (!nimaps) {
>  		trace_xfs_get_blocks_notfound(ip, offset, size);
>  		goto out_unlock;
>  	}
>  
> +	trace_xfs_get_blocks_found(ip, offset, size,
> +		imap.br_state == XFS_EXT_UNWRITTEN ?
> +			XFS_IO_UNWRITTEN : XFS_IO_OVERWRITE, &imap);
> +	xfs_iunlock(ip, lockmode);
> +
>  	/* trim mapping down to size requested */
>  	xfs_map_trim_size(inode, iblock, bh_result, &imap, offset, size);
>  
> -- 
> 2.14.2
> 
> --
> 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

[PATCH 6/8] xfs: minor cleanup for xfs_reflink_end_cow

[Christoph Hellwig]

Use xfs_iext_prev_extent to skip to the previous extent instead of
opencoding it.

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

diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index 90aac8889dd9..cdbd342a5249 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -762,10 +762,8 @@ xfs_reflink_end_cow(
 		xfs_trim_extent(&del, offset_fsb, end_fsb - offset_fsb);
 
 		/* Extent delete may have bumped ext forward */
-		if (!del.br_blockcount) {
-			xfs_iext_prev(ifp, &icur);
-			goto next_extent;
-		}
+		if (!del.br_blockcount)
+			goto prev_extent;
 
 		ASSERT(!isnullstartblock(got.br_startblock));
 
@@ -774,10 +772,8 @@ xfs_reflink_end_cow(
 		 * speculatively preallocated CoW extents that have been
 		 * allocated but have not yet been involved in a write.
 		 */
-		if (got.br_state == XFS_EXT_UNWRITTEN) {
-			xfs_iext_prev(ifp, &icur);
-			goto next_extent;
-		}
+		if (got.br_state == XFS_EXT_UNWRITTEN)
+			goto prev_extent;
 
 		/* Unmap the old blocks in the data fork. */
 		xfs_defer_init(&dfops, &firstfsb);
@@ -816,9 +812,12 @@ xfs_reflink_end_cow(
 		error = xfs_defer_finish(&tp, &dfops);
 		if (error)
 			goto out_defer;
-next_extent:
 		if (!xfs_iext_get_extent(ifp, &icur, &got))
 			break;
+		continue;
+prev_extent:
+		if (!xfs_iext_prev_extent(ifp, &icur, &got))
+			break;
 	}
 
 	error = xfs_trans_commit(tp);
-- 
2.14.2

Re: [PATCH 6/8] xfs: minor cleanup for xfs_reflink_end_cow

[Darrick J. Wong]

On Tue, Mar 13, 2018 at 03:35:59PM +0100, Christoph Hellwig wrote:
> Use xfs_iext_prev_extent to skip to the previous extent instead of
> opencoding it.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

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

--D

> ---
>  fs/xfs/xfs_reflink.c | 17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index 90aac8889dd9..cdbd342a5249 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -762,10 +762,8 @@ xfs_reflink_end_cow(
>  		xfs_trim_extent(&del, offset_fsb, end_fsb - offset_fsb);
>  
>  		/* Extent delete may have bumped ext forward */
> -		if (!del.br_blockcount) {
> -			xfs_iext_prev(ifp, &icur);
> -			goto next_extent;
> -		}
> +		if (!del.br_blockcount)
> +			goto prev_extent;
>  
>  		ASSERT(!isnullstartblock(got.br_startblock));
>  
> @@ -774,10 +772,8 @@ xfs_reflink_end_cow(
>  		 * speculatively preallocated CoW extents that have been
>  		 * allocated but have not yet been involved in a write.
>  		 */
> -		if (got.br_state == XFS_EXT_UNWRITTEN) {
> -			xfs_iext_prev(ifp, &icur);
> -			goto next_extent;
> -		}
> +		if (got.br_state == XFS_EXT_UNWRITTEN)
> +			goto prev_extent;
>  
>  		/* Unmap the old blocks in the data fork. */
>  		xfs_defer_init(&dfops, &firstfsb);
> @@ -816,9 +812,12 @@ xfs_reflink_end_cow(
>  		error = xfs_defer_finish(&tp, &dfops);
>  		if (error)
>  			goto out_defer;
> -next_extent:
>  		if (!xfs_iext_get_extent(ifp, &icur, &got))
>  			break;
> +		continue;
> +prev_extent:
> +		if (!xfs_iext_prev_extent(ifp, &icur, &got))
> +			break;
>  	}
>  
>  	error = xfs_trans_commit(tp);
> -- 
> 2.14.2
> 
> --
> 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

[PATCH 7/8] xfs: mark xfs_reflink_clear_inode_flag static

[Christoph Hellwig]

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

diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index cdbd342a5249..85d3379a81df 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -1552,7 +1552,7 @@ xfs_reflink_inode_has_shared_extents(
 }
 
 /* Clear the inode reflink flag if there are no shared extents. */
-int
+static int
 xfs_reflink_clear_inode_flag(
 	struct xfs_inode	*ip,
 	struct xfs_trans	**tpp)
diff --git a/fs/xfs/xfs_reflink.h b/fs/xfs/xfs_reflink.h
index 701487bab468..7d6d34a30200 100644
--- a/fs/xfs/xfs_reflink.h
+++ b/fs/xfs/xfs_reflink.h
@@ -49,8 +49,6 @@ extern int xfs_reflink_remap_range(struct file *file_in, loff_t pos_in,
 		struct file *file_out, loff_t pos_out, u64 len, bool is_dedupe);
 extern int xfs_reflink_inode_has_shared_extents(struct xfs_trans *tp,
 		struct xfs_inode *ip, bool *has_shared);
-extern int xfs_reflink_clear_inode_flag(struct xfs_inode *ip,
-		struct xfs_trans **tpp);
 extern int xfs_reflink_unshare(struct xfs_inode *ip, xfs_off_t offset,
 		xfs_off_t len);
 
-- 
2.14.2

Re: [PATCH 7/8] xfs: mark xfs_reflink_clear_inode_flag static

[Darrick J. Wong]

On Tue, Mar 13, 2018 at 03:36:00PM +0100, Christoph Hellwig wrote:
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_reflink.c | 2 +-
>  fs/xfs/xfs_reflink.h | 2 --
>  2 files changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index cdbd342a5249..85d3379a81df 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -1552,7 +1552,7 @@ xfs_reflink_inode_has_shared_extents(
>  }
>  
>  /* Clear the inode reflink flag if there are no shared extents. */
> -int
> +static int

Online inode repair/optimization will call this function (see
xfs_repair_inode in scrub/inode_repair.c in the online repair series),
so I'd prefer not to add static only to have to pull it out later.

(Granted, nobody's commented on any of online repair afaict...)

--D

>  xfs_reflink_clear_inode_flag(
>  	struct xfs_inode	*ip,
>  	struct xfs_trans	**tpp)
> diff --git a/fs/xfs/xfs_reflink.h b/fs/xfs/xfs_reflink.h
> index 701487bab468..7d6d34a30200 100644
> --- a/fs/xfs/xfs_reflink.h
> +++ b/fs/xfs/xfs_reflink.h
> @@ -49,8 +49,6 @@ extern int xfs_reflink_remap_range(struct file *file_in, loff_t pos_in,
>  		struct file *file_out, loff_t pos_out, u64 len, bool is_dedupe);
>  extern int xfs_reflink_inode_has_shared_extents(struct xfs_trans *tp,
>  		struct xfs_inode *ip, bool *has_shared);
> -extern int xfs_reflink_clear_inode_flag(struct xfs_inode *ip,
> -		struct xfs_trans **tpp);
>  extern int xfs_reflink_unshare(struct xfs_inode *ip, xfs_off_t offset,
>  		xfs_off_t len);
>  
> -- 
> 2.14.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 7/8] xfs: mark xfs_reflink_clear_inode_flag static

[Christoph Hellwig]

On Tue, Mar 13, 2018 at 01:50:53PM -0700, Darrick J. Wong wrote:
> Online inode repair/optimization will call this function (see
> xfs_repair_inode in scrub/inode_repair.c in the online repair series),
> so I'd prefer not to add static only to have to pull it out later.

Ok, just skip this patch then.

[PATCH 8/8] xfs: only clear preallocated COW blocks in xfs_reflink_clear_inode_flag

[Christoph Hellwig]

Currently these are the only ones that should exist at this point anyway,
but with O_ATOMIC writes we might have legit extents around in the COW
fork even when clearing the reflink flag.

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

diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index 85d3379a81df..8ab9c73b430e 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -1570,7 +1570,7 @@ xfs_reflink_clear_inode_flag(
 	 * We didn't find any shared blocks so turn off the reflink flag.
 	 * First, get rid of any leftover CoW mappings.
 	 */
-	error = xfs_reflink_cancel_cow_blocks(ip, tpp, 0, NULLFILEOFF, true);
+	error = xfs_reflink_cancel_cow_blocks(ip, tpp, 0, NULLFILEOFF, false);
 	if (error)
 		return error;
 
-- 
2.14.2

Re: [PATCH 8/8] xfs: only clear preallocated COW blocks in xfs_reflink_clear_inode_flag

[Darrick J. Wong]

On Tue, Mar 13, 2018 at 03:36:01PM +0100, Christoph Hellwig wrote:
> Currently these are the only ones that should exist at this point anyway,
> but with O_ATOMIC writes we might have legit extents around in the COW
> fork even when clearing the reflink flag.

Hmmm... so I assume that the premise of the O_ATOMIC write series is
still that we use the cow fork to stage writes until fsync, and the
presence of a cow fork is now separate from the reflink inode flag?

Therefore, it's perfectly legit to have a !reflink inode with real
extents sitting in the cow fork and legit to be clearing the reflink
iflag with real extents sitting around.

I think this is ok but I'd sure like to see the atomic writes patches
before I pull this one in.

--D

> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_reflink.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index 85d3379a81df..8ab9c73b430e 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -1570,7 +1570,7 @@ xfs_reflink_clear_inode_flag(
>  	 * We didn't find any shared blocks so turn off the reflink flag.
>  	 * First, get rid of any leftover CoW mappings.
>  	 */
> -	error = xfs_reflink_cancel_cow_blocks(ip, tpp, 0, NULLFILEOFF, true);
> +	error = xfs_reflink_cancel_cow_blocks(ip, tpp, 0, NULLFILEOFF, false);
>  	if (error)
>  		return error;
>  
> -- 
> 2.14.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 8/8] xfs: only clear preallocated COW blocks in xfs_reflink_clear_inode_flag

[Christoph Hellwig]

On Tue, Mar 13, 2018 at 01:56:04PM -0700, Darrick J. Wong wrote:
> On Tue, Mar 13, 2018 at 03:36:01PM +0100, Christoph Hellwig wrote:
> > Currently these are the only ones that should exist at this point anyway,
> > but with O_ATOMIC writes we might have legit extents around in the COW
> > fork even when clearing the reflink flag.
> 
> Hmmm... so I assume that the premise of the O_ATOMIC write series is
> still that we use the cow fork to stage writes until fsync, and the
> presence of a cow fork is now separate from the reflink inode flag?

Yes.
> 
> Therefore, it's perfectly legit to have a !reflink inode with real
> extents sitting in the cow fork and legit to be clearing the reflink
> iflag with real extents sitting around.
> 
> I think this is ok but I'd sure like to see the atomic writes patches
> before I pull this one in.

Ok.  Skip if for now then.