[PATCH] xfs: factor out nodiscard helpers

[PATCH] xfs: factor out nodiscard helpers

[Brian Foster]

The changes to skip discards of speculative preallocation and
unwritten extents introduced several new wrapper functions through
the bunmapi -> extent free codepath to reduce churn in all of the
associated callers. In several cases, these wrappers simply toggle a
single flag to skip or not skip discards for the resulting blocks.

The explicit _nodiscard() wrappers for such an isolated set of
callers is a bit overkill. Kill off these wrappers and replace with
the calls to the underlying functions in the contexts that need to
control discard behavior. Retain the wrappers that preserve the
original calling conventions to serve the original purpose of
reducing code churn.

This is a refactoring patch and does not change behavior.

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

Hi all,

Darrick had previously indicated that he was going to pull in the v2 of
the skip discard set rather than hold it up on refactoring churn. Given
that I was playing around with some of Christoph's compromised feedback,
here's a followup patch with the associated cleanups that I generally
agree with. I actually like this factoring a bit better than that of the
v2 because while it still avoids changing all callers of the associated
functions, I do prefer to control discard behavior with a parameter
rather than if/else statements with calls to completely different
functions. The additional benefit is that this removes more code than it
adds.

I left struct xfs_free_extent_item alone because I don't really agree
that XFS_EXTENT_BUSY_ belong anywhere outside of the busy extent code.
IOW, I'd change the xfs_extent_busy_insert() parameter before I'd change
xfs_free_extent_item because, despite the flags name, the param really
only has two sane values (skip discard or don't). The affected
xfs_extent_busy->flags field is used as more of a state than a control
flag.

Brian

 fs/xfs/libxfs/xfs_alloc.h  | 11 -----------
 fs/xfs/libxfs/xfs_bmap.c   | 16 ++++++++--------
 fs/xfs/libxfs/xfs_bmap.h   | 11 -----------
 fs/xfs/xfs_bmap_util.c     |  4 ++--
 fs/xfs/xfs_inode.c         |  9 +++------
 fs/xfs/xfs_inode.h         | 16 +++-------------
 fs/xfs/xfs_trans_extfree.c |  9 ++-------
 7 files changed, 18 insertions(+), 58 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_alloc.h b/fs/xfs/libxfs/xfs_alloc.h
index 4a8e8dcaf352..21e5c0662a92 100644
--- a/fs/xfs/libxfs/xfs_alloc.h
+++ b/fs/xfs/libxfs/xfs_alloc.h
@@ -210,17 +210,6 @@ xfs_free_extent(
 	return __xfs_free_extent(tp, bno, len, oinfo, type, false);
 }
 
-static inline int
-xfs_free_extent_nodiscard(
-	struct xfs_trans	*tp,
-	xfs_fsblock_t		bno,
-	xfs_extlen_t		len,
-	struct xfs_owner_info	*oinfo,
-	enum xfs_ag_resv_type	type)
-{
-	return __xfs_free_extent(tp, bno, len, oinfo, type, true);
-}
-
 int				/* error */
 xfs_alloc_lookup_le(
 	struct xfs_btree_cur	*cur,	/* btree cursor */
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 24f60ee810e4..fd3c3be4d64a 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -5107,15 +5107,15 @@ xfs_bmap_del_extent_real(
 			if (error)
 				goto done;
 		} else {
+			bool	skip_discard = false;
+
 			if ((bflags & XFS_BMAPI_NODISCARD) ||
-			    (del->br_state == XFS_EXT_UNWRITTEN)) {
-				xfs_bmap_add_free_nodiscard(mp, dfops,
-					del->br_startblock, del->br_blockcount,
-					NULL);
-			} else {
-				xfs_bmap_add_free(mp, dfops, del->br_startblock,
-					del->br_blockcount, NULL);
-			}
+			    (del->br_state == XFS_EXT_UNWRITTEN))
+				skip_discard = true;
+
+			__xfs_bmap_add_free(mp, dfops, del->br_startblock,
+					    del->br_blockcount, NULL,
+					    skip_discard);
 		}
 	}
 
diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
index 8d8946bfdd8c..8affd3915a11 100644
--- a/fs/xfs/libxfs/xfs_bmap.h
+++ b/fs/xfs/libxfs/xfs_bmap.h
@@ -256,17 +256,6 @@ xfs_bmap_add_free(
 	__xfs_bmap_add_free(mp, dfops, bno, len, oinfo, false);
 }
 
-static inline void
-xfs_bmap_add_free_nodiscard(
-	struct xfs_mount		*mp,
-	struct xfs_defer_ops		*dfops,
-	xfs_fsblock_t			bno,
-	xfs_filblks_t			len,
-	struct xfs_owner_info		*oinfo)
-{
-	__xfs_bmap_add_free(mp, dfops, bno, len, oinfo, true);
-}
-
 enum xfs_bmap_intent_type {
 	XFS_BMAP_MAP = 1,
 	XFS_BMAP_UNMAP,
diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index 696c3b6bd2c9..11058459ea98 100644
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -871,8 +871,8 @@ xfs_free_eofblocks(
 		 * contents of the file are flushed to disk then the files
 		 * may be full of holes (ie NULL files bug).
 		 */
-		error = xfs_itruncate_extents_nodiscard(&tp, ip, XFS_DATA_FORK,
-							XFS_ISIZE(ip));
+		error = xfs_itruncate_extents_flags(&tp, ip, XFS_DATA_FORK,
+					XFS_ISIZE(ip), XFS_BMAPI_NODISCARD);
 		if (error) {
 			/*
 			 * If we get an error at this point we simply don't
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index d63ddd806074..3c54f09fee4e 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -1534,12 +1534,12 @@ xfs_itruncate_clear_reflink_flags(
  * dirty on error so that transactions can be easily aborted if possible.
  */
 int
-__xfs_itruncate_extents(
+xfs_itruncate_extents_flags(
 	struct xfs_trans	**tpp,
 	struct xfs_inode	*ip,
 	int			whichfork,
 	xfs_fsize_t		new_size,
-	bool			skip_discard)
+	int			flags)
 {
 	struct xfs_mount	*mp = ip->i_mount;
 	struct xfs_trans	*tp = *tpp;
@@ -1550,7 +1550,6 @@ __xfs_itruncate_extents(
 	xfs_filblks_t		unmap_len;
 	int			error = 0;
 	int			done = 0;
-	int			flags;
 
 	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
 	ASSERT(!atomic_read(&VFS_I(ip)->i_count) ||
@@ -1563,9 +1562,7 @@ __xfs_itruncate_extents(
 
 	trace_xfs_itruncate_extents_start(ip, new_size);
 
-	flags = xfs_bmapi_aflag(whichfork);
-	if (skip_discard)
-		flags |= XFS_BMAPI_NODISCARD;
+	flags |= xfs_bmapi_aflag(whichfork);
 
 	/*
 	 * Since it is possible for space to become allocated beyond
diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
index 3dd3201f4409..00fee6824745 100644
--- a/fs/xfs/xfs_inode.h
+++ b/fs/xfs/xfs_inode.h
@@ -415,8 +415,8 @@ uint		xfs_ilock_attr_map_shared(struct xfs_inode *);
 uint		xfs_ip2xflags(struct xfs_inode *);
 int		xfs_ifree(struct xfs_trans *, xfs_inode_t *,
 			   struct xfs_defer_ops *);
-int		__xfs_itruncate_extents(struct xfs_trans **, struct xfs_inode *,
-				        int, xfs_fsize_t, bool);
+int		xfs_itruncate_extents_flags(struct xfs_trans **,
+				struct xfs_inode *, int, xfs_fsize_t, int);
 void		xfs_iext_realloc(xfs_inode_t *, int, int);
 
 void		xfs_iunpin_wait(xfs_inode_t *);
@@ -440,17 +440,7 @@ xfs_itruncate_extents(
 	int			whichfork,
 	xfs_fsize_t		new_size)
 {
-	return __xfs_itruncate_extents(tpp, ip, whichfork, new_size, false);
-}
-
-static inline int
-xfs_itruncate_extents_nodiscard(
-	struct xfs_trans	**tpp,
-	struct xfs_inode	*ip,
-	int			whichfork,
-	xfs_fsize_t		new_size)
-{
-	return __xfs_itruncate_extents(tpp, ip, whichfork, new_size, true);
+	return xfs_itruncate_extents_flags(tpp, ip, whichfork, new_size, 0);
 }
 
 /* from xfs_file.c */
diff --git a/fs/xfs/xfs_trans_extfree.c b/fs/xfs/xfs_trans_extfree.c
index dab8b3b7a9b8..5e137ee826ff 100644
--- a/fs/xfs/xfs_trans_extfree.c
+++ b/fs/xfs/xfs_trans_extfree.c
@@ -80,13 +80,8 @@ xfs_trans_free_extent(
 
 	trace_xfs_bmap_free_deferred(tp->t_mountp, agno, 0, agbno, ext_len);
 
-	if (skip_discard)
-		error = xfs_free_extent_nodiscard(tp, start_block, ext_len,
-						  oinfo, XFS_AG_RESV_NONE);
-	else
-		error = xfs_free_extent(tp, start_block, ext_len, oinfo,
-					XFS_AG_RESV_NONE);
-
+	error = __xfs_free_extent(tp, start_block, ext_len,
+				  oinfo, XFS_AG_RESV_NONE, skip_discard);
 	/*
 	 * Mark the transaction dirty, even on error. This ensures the
 	 * transaction is aborted, which:
-- 
2.14.3

Re: [PATCH] xfs: factor out nodiscard helpers

[Christoph Hellwig]

This looks much better, and I think the diffstat speaks for itself..

Reviewed-by: Christoph Hellwig <hch@lst.de>

> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 24f60ee810e4..fd3c3be4d64a 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -5107,15 +5107,15 @@ xfs_bmap_del_extent_real(
>  			if (error)
>  				goto done;
>  		} else {
> +			bool	skip_discard = false;
> +
>  			if ((bflags & XFS_BMAPI_NODISCARD) ||
> +			    (del->br_state == XFS_EXT_UNWRITTEN))

No need for the inner braces in the second statement.

> +				skip_discard = true;
> +
> +			__xfs_bmap_add_free(mp, dfops, del->br_startblock,
> +					    del->br_blockcount, NULL,
> +					    skip_discard);

In fact I'd avoid the variable entirely:

			__xfs_bmap_add_free(mp, dfops, del->br_startblock,
					del->br_blockcount, NULL,
					del->br_state == XFS_EXT_UNWRITTEN ||
					(bflags & XFS_BMAPI_NODISCARD));

[PATCH v2] xfs: factor out nodiscard helpers

[Brian Foster]

The changes to skip discards of speculative preallocation and
unwritten extents introduced several new wrapper functions through
the bunmapi -> extent free codepath to reduce churn in all of the
associated callers. In several cases, these wrappers simply toggle a
single flag to skip or not skip discards for the resulting blocks.

The explicit _nodiscard() wrappers for such an isolated set of
callers is a bit overkill. Kill off these wrappers and replace with
the calls to the underlying functions in the contexts that need to
control discard behavior. Retain the wrappers that preserve the
original calling conventions to serve the original purpose of
reducing code churn.

This is a refactoring patch and does not change behavior.

Signed-off-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---

v2:
- Clean up xfs_bmap_del_extent_real().
v1: https://marc.info/?l=linux-xfs&m=152595674924579&w=2

 fs/xfs/libxfs/xfs_alloc.h  | 11 -----------
 fs/xfs/libxfs/xfs_bmap.c   | 13 ++++---------
 fs/xfs/libxfs/xfs_bmap.h   | 11 -----------
 fs/xfs/xfs_bmap_util.c     |  4 ++--
 fs/xfs/xfs_inode.c         |  9 +++------
 fs/xfs/xfs_inode.h         | 16 +++-------------
 fs/xfs/xfs_trans_extfree.c |  9 ++-------
 7 files changed, 14 insertions(+), 59 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_alloc.h b/fs/xfs/libxfs/xfs_alloc.h
index 4a8e8dcaf352..21e5c0662a92 100644
--- a/fs/xfs/libxfs/xfs_alloc.h
+++ b/fs/xfs/libxfs/xfs_alloc.h
@@ -210,17 +210,6 @@ xfs_free_extent(
 	return __xfs_free_extent(tp, bno, len, oinfo, type, false);
 }
 
-static inline int
-xfs_free_extent_nodiscard(
-	struct xfs_trans	*tp,
-	xfs_fsblock_t		bno,
-	xfs_extlen_t		len,
-	struct xfs_owner_info	*oinfo,
-	enum xfs_ag_resv_type	type)
-{
-	return __xfs_free_extent(tp, bno, len, oinfo, type, true);
-}
-
 int				/* error */
 xfs_alloc_lookup_le(
 	struct xfs_btree_cur	*cur,	/* btree cursor */
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 24f60ee810e4..91659dfe93ee 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -5107,15 +5107,10 @@ xfs_bmap_del_extent_real(
 			if (error)
 				goto done;
 		} else {
-			if ((bflags & XFS_BMAPI_NODISCARD) ||
-			    (del->br_state == XFS_EXT_UNWRITTEN)) {
-				xfs_bmap_add_free_nodiscard(mp, dfops,
-					del->br_startblock, del->br_blockcount,
-					NULL);
-			} else {
-				xfs_bmap_add_free(mp, dfops, del->br_startblock,
-					del->br_blockcount, NULL);
-			}
+			__xfs_bmap_add_free(mp, dfops, del->br_startblock,
+					del->br_blockcount, NULL,
+					(bflags & XFS_BMAPI_NODISCARD) ||
+					del->br_state == XFS_EXT_UNWRITTEN);
 		}
 	}
 
diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
index 8d8946bfdd8c..8affd3915a11 100644
--- a/fs/xfs/libxfs/xfs_bmap.h
+++ b/fs/xfs/libxfs/xfs_bmap.h
@@ -256,17 +256,6 @@ xfs_bmap_add_free(
 	__xfs_bmap_add_free(mp, dfops, bno, len, oinfo, false);
 }
 
-static inline void
-xfs_bmap_add_free_nodiscard(
-	struct xfs_mount		*mp,
-	struct xfs_defer_ops		*dfops,
-	xfs_fsblock_t			bno,
-	xfs_filblks_t			len,
-	struct xfs_owner_info		*oinfo)
-{
-	__xfs_bmap_add_free(mp, dfops, bno, len, oinfo, true);
-}
-
 enum xfs_bmap_intent_type {
 	XFS_BMAP_MAP = 1,
 	XFS_BMAP_UNMAP,
diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index 696c3b6bd2c9..11058459ea98 100644
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -871,8 +871,8 @@ xfs_free_eofblocks(
 		 * contents of the file are flushed to disk then the files
 		 * may be full of holes (ie NULL files bug).
 		 */
-		error = xfs_itruncate_extents_nodiscard(&tp, ip, XFS_DATA_FORK,
-							XFS_ISIZE(ip));
+		error = xfs_itruncate_extents_flags(&tp, ip, XFS_DATA_FORK,
+					XFS_ISIZE(ip), XFS_BMAPI_NODISCARD);
 		if (error) {
 			/*
 			 * If we get an error at this point we simply don't
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index d63ddd806074..3c54f09fee4e 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -1534,12 +1534,12 @@ xfs_itruncate_clear_reflink_flags(
  * dirty on error so that transactions can be easily aborted if possible.
  */
 int
-__xfs_itruncate_extents(
+xfs_itruncate_extents_flags(
 	struct xfs_trans	**tpp,
 	struct xfs_inode	*ip,
 	int			whichfork,
 	xfs_fsize_t		new_size,
-	bool			skip_discard)
+	int			flags)
 {
 	struct xfs_mount	*mp = ip->i_mount;
 	struct xfs_trans	*tp = *tpp;
@@ -1550,7 +1550,6 @@ __xfs_itruncate_extents(
 	xfs_filblks_t		unmap_len;
 	int			error = 0;
 	int			done = 0;
-	int			flags;
 
 	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
 	ASSERT(!atomic_read(&VFS_I(ip)->i_count) ||
@@ -1563,9 +1562,7 @@ __xfs_itruncate_extents(
 
 	trace_xfs_itruncate_extents_start(ip, new_size);
 
-	flags = xfs_bmapi_aflag(whichfork);
-	if (skip_discard)
-		flags |= XFS_BMAPI_NODISCARD;
+	flags |= xfs_bmapi_aflag(whichfork);
 
 	/*
 	 * Since it is possible for space to become allocated beyond
diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
index 3dd3201f4409..00fee6824745 100644
--- a/fs/xfs/xfs_inode.h
+++ b/fs/xfs/xfs_inode.h
@@ -415,8 +415,8 @@ uint		xfs_ilock_attr_map_shared(struct xfs_inode *);
 uint		xfs_ip2xflags(struct xfs_inode *);
 int		xfs_ifree(struct xfs_trans *, xfs_inode_t *,
 			   struct xfs_defer_ops *);
-int		__xfs_itruncate_extents(struct xfs_trans **, struct xfs_inode *,
-				        int, xfs_fsize_t, bool);
+int		xfs_itruncate_extents_flags(struct xfs_trans **,
+				struct xfs_inode *, int, xfs_fsize_t, int);
 void		xfs_iext_realloc(xfs_inode_t *, int, int);
 
 void		xfs_iunpin_wait(xfs_inode_t *);
@@ -440,17 +440,7 @@ xfs_itruncate_extents(
 	int			whichfork,
 	xfs_fsize_t		new_size)
 {
-	return __xfs_itruncate_extents(tpp, ip, whichfork, new_size, false);
-}
-
-static inline int
-xfs_itruncate_extents_nodiscard(
-	struct xfs_trans	**tpp,
-	struct xfs_inode	*ip,
-	int			whichfork,
-	xfs_fsize_t		new_size)
-{
-	return __xfs_itruncate_extents(tpp, ip, whichfork, new_size, true);
+	return xfs_itruncate_extents_flags(tpp, ip, whichfork, new_size, 0);
 }
 
 /* from xfs_file.c */
diff --git a/fs/xfs/xfs_trans_extfree.c b/fs/xfs/xfs_trans_extfree.c
index dab8b3b7a9b8..5e137ee826ff 100644
--- a/fs/xfs/xfs_trans_extfree.c
+++ b/fs/xfs/xfs_trans_extfree.c
@@ -80,13 +80,8 @@ xfs_trans_free_extent(
 
 	trace_xfs_bmap_free_deferred(tp->t_mountp, agno, 0, agbno, ext_len);
 
-	if (skip_discard)
-		error = xfs_free_extent_nodiscard(tp, start_block, ext_len,
-						  oinfo, XFS_AG_RESV_NONE);
-	else
-		error = xfs_free_extent(tp, start_block, ext_len, oinfo,
-					XFS_AG_RESV_NONE);
-
+	error = __xfs_free_extent(tp, start_block, ext_len,
+				  oinfo, XFS_AG_RESV_NONE, skip_discard);
 	/*
 	 * Mark the transaction dirty, even on error. This ensures the
 	 * transaction is aborted, which:
-- 
2.14.3

Re: [PATCH v2] xfs: factor out nodiscard helpers

[Darrick J. Wong]

On Thu, May 10, 2018 at 10:07:32AM -0400, Brian Foster wrote:
> The changes to skip discards of speculative preallocation and
> unwritten extents introduced several new wrapper functions through
> the bunmapi -> extent free codepath to reduce churn in all of the
> associated callers. In several cases, these wrappers simply toggle a
> single flag to skip or not skip discards for the resulting blocks.
> 
> The explicit _nodiscard() wrappers for such an isolated set of
> callers is a bit overkill. Kill off these wrappers and replace with
> the calls to the underlying functions in the contexts that need to
> control discard behavior. Retain the wrappers that preserve the
> original calling conventions to serve the original purpose of
> reducing code churn.
> 
> This is a refactoring patch and does not change behavior.
> 
> Signed-off-by: Brian Foster <bfoster@redhat.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>

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

--D

> ---
> 
> v2:
> - Clean up xfs_bmap_del_extent_real().
> v1: https://marc.info/?l=linux-xfs&m=152595674924579&w=2
> 
>  fs/xfs/libxfs/xfs_alloc.h  | 11 -----------
>  fs/xfs/libxfs/xfs_bmap.c   | 13 ++++---------
>  fs/xfs/libxfs/xfs_bmap.h   | 11 -----------
>  fs/xfs/xfs_bmap_util.c     |  4 ++--
>  fs/xfs/xfs_inode.c         |  9 +++------
>  fs/xfs/xfs_inode.h         | 16 +++-------------
>  fs/xfs/xfs_trans_extfree.c |  9 ++-------
>  7 files changed, 14 insertions(+), 59 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_alloc.h b/fs/xfs/libxfs/xfs_alloc.h
> index 4a8e8dcaf352..21e5c0662a92 100644
> --- a/fs/xfs/libxfs/xfs_alloc.h
> +++ b/fs/xfs/libxfs/xfs_alloc.h
> @@ -210,17 +210,6 @@ xfs_free_extent(
>  	return __xfs_free_extent(tp, bno, len, oinfo, type, false);
>  }
>  
> -static inline int
> -xfs_free_extent_nodiscard(
> -	struct xfs_trans	*tp,
> -	xfs_fsblock_t		bno,
> -	xfs_extlen_t		len,
> -	struct xfs_owner_info	*oinfo,
> -	enum xfs_ag_resv_type	type)
> -{
> -	return __xfs_free_extent(tp, bno, len, oinfo, type, true);
> -}
> -
>  int				/* error */
>  xfs_alloc_lookup_le(
>  	struct xfs_btree_cur	*cur,	/* btree cursor */
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 24f60ee810e4..91659dfe93ee 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -5107,15 +5107,10 @@ xfs_bmap_del_extent_real(
>  			if (error)
>  				goto done;
>  		} else {
> -			if ((bflags & XFS_BMAPI_NODISCARD) ||
> -			    (del->br_state == XFS_EXT_UNWRITTEN)) {
> -				xfs_bmap_add_free_nodiscard(mp, dfops,
> -					del->br_startblock, del->br_blockcount,
> -					NULL);
> -			} else {
> -				xfs_bmap_add_free(mp, dfops, del->br_startblock,
> -					del->br_blockcount, NULL);
> -			}
> +			__xfs_bmap_add_free(mp, dfops, del->br_startblock,
> +					del->br_blockcount, NULL,
> +					(bflags & XFS_BMAPI_NODISCARD) ||
> +					del->br_state == XFS_EXT_UNWRITTEN);
>  		}
>  	}
>  
> diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
> index 8d8946bfdd8c..8affd3915a11 100644
> --- a/fs/xfs/libxfs/xfs_bmap.h
> +++ b/fs/xfs/libxfs/xfs_bmap.h
> @@ -256,17 +256,6 @@ xfs_bmap_add_free(
>  	__xfs_bmap_add_free(mp, dfops, bno, len, oinfo, false);
>  }
>  
> -static inline void
> -xfs_bmap_add_free_nodiscard(
> -	struct xfs_mount		*mp,
> -	struct xfs_defer_ops		*dfops,
> -	xfs_fsblock_t			bno,
> -	xfs_filblks_t			len,
> -	struct xfs_owner_info		*oinfo)
> -{
> -	__xfs_bmap_add_free(mp, dfops, bno, len, oinfo, true);
> -}
> -
>  enum xfs_bmap_intent_type {
>  	XFS_BMAP_MAP = 1,
>  	XFS_BMAP_UNMAP,
> diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
> index 696c3b6bd2c9..11058459ea98 100644
> --- a/fs/xfs/xfs_bmap_util.c
> +++ b/fs/xfs/xfs_bmap_util.c
> @@ -871,8 +871,8 @@ xfs_free_eofblocks(
>  		 * contents of the file are flushed to disk then the files
>  		 * may be full of holes (ie NULL files bug).
>  		 */
> -		error = xfs_itruncate_extents_nodiscard(&tp, ip, XFS_DATA_FORK,
> -							XFS_ISIZE(ip));
> +		error = xfs_itruncate_extents_flags(&tp, ip, XFS_DATA_FORK,
> +					XFS_ISIZE(ip), XFS_BMAPI_NODISCARD);
>  		if (error) {
>  			/*
>  			 * If we get an error at this point we simply don't
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index d63ddd806074..3c54f09fee4e 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -1534,12 +1534,12 @@ xfs_itruncate_clear_reflink_flags(
>   * dirty on error so that transactions can be easily aborted if possible.
>   */
>  int
> -__xfs_itruncate_extents(
> +xfs_itruncate_extents_flags(
>  	struct xfs_trans	**tpp,
>  	struct xfs_inode	*ip,
>  	int			whichfork,
>  	xfs_fsize_t		new_size,
> -	bool			skip_discard)
> +	int			flags)
>  {
>  	struct xfs_mount	*mp = ip->i_mount;
>  	struct xfs_trans	*tp = *tpp;
> @@ -1550,7 +1550,6 @@ __xfs_itruncate_extents(
>  	xfs_filblks_t		unmap_len;
>  	int			error = 0;
>  	int			done = 0;
> -	int			flags;
>  
>  	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
>  	ASSERT(!atomic_read(&VFS_I(ip)->i_count) ||
> @@ -1563,9 +1562,7 @@ __xfs_itruncate_extents(
>  
>  	trace_xfs_itruncate_extents_start(ip, new_size);
>  
> -	flags = xfs_bmapi_aflag(whichfork);
> -	if (skip_discard)
> -		flags |= XFS_BMAPI_NODISCARD;
> +	flags |= xfs_bmapi_aflag(whichfork);
>  
>  	/*
>  	 * Since it is possible for space to become allocated beyond
> diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
> index 3dd3201f4409..00fee6824745 100644
> --- a/fs/xfs/xfs_inode.h
> +++ b/fs/xfs/xfs_inode.h
> @@ -415,8 +415,8 @@ uint		xfs_ilock_attr_map_shared(struct xfs_inode *);
>  uint		xfs_ip2xflags(struct xfs_inode *);
>  int		xfs_ifree(struct xfs_trans *, xfs_inode_t *,
>  			   struct xfs_defer_ops *);
> -int		__xfs_itruncate_extents(struct xfs_trans **, struct xfs_inode *,
> -				        int, xfs_fsize_t, bool);
> +int		xfs_itruncate_extents_flags(struct xfs_trans **,
> +				struct xfs_inode *, int, xfs_fsize_t, int);
>  void		xfs_iext_realloc(xfs_inode_t *, int, int);
>  
>  void		xfs_iunpin_wait(xfs_inode_t *);
> @@ -440,17 +440,7 @@ xfs_itruncate_extents(
>  	int			whichfork,
>  	xfs_fsize_t		new_size)
>  {
> -	return __xfs_itruncate_extents(tpp, ip, whichfork, new_size, false);
> -}
> -
> -static inline int
> -xfs_itruncate_extents_nodiscard(
> -	struct xfs_trans	**tpp,
> -	struct xfs_inode	*ip,
> -	int			whichfork,
> -	xfs_fsize_t		new_size)
> -{
> -	return __xfs_itruncate_extents(tpp, ip, whichfork, new_size, true);
> +	return xfs_itruncate_extents_flags(tpp, ip, whichfork, new_size, 0);
>  }
>  
>  /* from xfs_file.c */
> diff --git a/fs/xfs/xfs_trans_extfree.c b/fs/xfs/xfs_trans_extfree.c
> index dab8b3b7a9b8..5e137ee826ff 100644
> --- a/fs/xfs/xfs_trans_extfree.c
> +++ b/fs/xfs/xfs_trans_extfree.c
> @@ -80,13 +80,8 @@ xfs_trans_free_extent(
>  
>  	trace_xfs_bmap_free_deferred(tp->t_mountp, agno, 0, agbno, ext_len);
>  
> -	if (skip_discard)
> -		error = xfs_free_extent_nodiscard(tp, start_block, ext_len,
> -						  oinfo, XFS_AG_RESV_NONE);
> -	else
> -		error = xfs_free_extent(tp, start_block, ext_len, oinfo,
> -					XFS_AG_RESV_NONE);
> -
> +	error = __xfs_free_extent(tp, start_block, ext_len,
> +				  oinfo, XFS_AG_RESV_NONE, skip_discard);
>  	/*
>  	 * Mark the transaction dirty, even on error. This ensures the
>  	 * transaction is aborted, which:
> -- 
> 2.14.3
> 
> --
> 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