a couple minor btree free_block patches

a couple minor btree free_block patches

[Christoph Hellwig]

The intent of this series is to invalidate buffers for freed btree
blocks in common code instead of the methods after I found a reflink
bug due to improper buffer invalidation.  The first two patches are
things I noticed while doing that move.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 1/3] xfs: handle errors from ->free_blocks in xfs_btree_kill_iroot

[Christoph Hellwig]

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

diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
index a0eb18c..3143577 100644
--- a/fs/xfs/libxfs/xfs_btree.c
+++ b/fs/xfs/libxfs/xfs_btree.c
@@ -3209,6 +3209,7 @@ xfs_btree_kill_iroot(
 	int			level;
 	int			index;
 	int			numrecs;
+	int			error;
 #ifdef DEBUG
 	union xfs_btree_ptr	ptr;
 	int			i;
@@ -3272,8 +3273,6 @@ xfs_btree_kill_iroot(
 	cpp = xfs_btree_ptr_addr(cur, 1, cblock);
 #ifdef DEBUG
 	for (i = 0; i < numrecs; i++) {
-		int		error;
-
 		error = xfs_btree_check_ptr(cur, cpp, i, level - 1);
 		if (error) {
 			XFS_BTREE_TRACE_CURSOR(cur, XBT_ERROR);
@@ -3283,7 +3282,11 @@ xfs_btree_kill_iroot(
 #endif
 	xfs_btree_copy_ptrs(cur, pp, cpp, numrecs);
 
-	cur->bc_ops->free_block(cur, cbp);
+	error = cur->bc_ops->free_block(cur, cbp);
+	if (error) {
+		XFS_BTREE_TRACE_CURSOR(cur, XBT_ERROR);
+		return error;
+	}
 	XFS_BTREE_STATS_INC(cur, free);
 
 	cur->bc_bufs[level - 1] = NULL;
-- 
1.9.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 1/3] xfs: handle errors from ->free_blocks in xfs_btree_kill_iroot

[Brian Foster]

On Wed, Jan 13, 2016 at 06:50:52PM +0100, Christoph Hellwig wrote:
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---

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

>  fs/xfs/libxfs/xfs_btree.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
> index a0eb18c..3143577 100644
> --- a/fs/xfs/libxfs/xfs_btree.c
> +++ b/fs/xfs/libxfs/xfs_btree.c
> @@ -3209,6 +3209,7 @@ xfs_btree_kill_iroot(
>  	int			level;
>  	int			index;
>  	int			numrecs;
> +	int			error;
>  #ifdef DEBUG
>  	union xfs_btree_ptr	ptr;
>  	int			i;
> @@ -3272,8 +3273,6 @@ xfs_btree_kill_iroot(
>  	cpp = xfs_btree_ptr_addr(cur, 1, cblock);
>  #ifdef DEBUG
>  	for (i = 0; i < numrecs; i++) {
> -		int		error;
> -
>  		error = xfs_btree_check_ptr(cur, cpp, i, level - 1);
>  		if (error) {
>  			XFS_BTREE_TRACE_CURSOR(cur, XBT_ERROR);
> @@ -3283,7 +3282,11 @@ xfs_btree_kill_iroot(
>  #endif
>  	xfs_btree_copy_ptrs(cur, pp, cpp, numrecs);
>  
> -	cur->bc_ops->free_block(cur, cbp);
> +	error = cur->bc_ops->free_block(cur, cbp);
> +	if (error) {
> +		XFS_BTREE_TRACE_CURSOR(cur, XBT_ERROR);
> +		return error;
> +	}
>  	XFS_BTREE_STATS_INC(cur, free);
>  
>  	cur->bc_bufs[level - 1] = NULL;
> -- 
> 1.9.1
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 2/3] xfs: factor btree block freeing into a helper

[Christoph Hellwig]

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

diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
index 3143577..77afb4a 100644
--- a/fs/xfs/libxfs/xfs_btree.c
+++ b/fs/xfs/libxfs/xfs_btree.c
@@ -294,6 +294,19 @@ xfs_btree_sblock_verify_crc(
 	return true;
 }
 
+static int
+xfs_btree_free_block(
+	struct xfs_btree_cur	*cur,
+	struct xfs_buf		*bp)
+{
+	int			error;
+
+	error = cur->bc_ops->free_block(cur, bp);
+	if (!error)
+		XFS_BTREE_STATS_INC(cur, free);
+	return error;
+}
+
 /*
  * Delete the btree cursor.
  */
@@ -3282,12 +3295,11 @@ xfs_btree_kill_iroot(
 #endif
 	xfs_btree_copy_ptrs(cur, pp, cpp, numrecs);
 
-	error = cur->bc_ops->free_block(cur, cbp);
+	error = xfs_btree_free_block(cur, cbp);
 	if (error) {
 		XFS_BTREE_TRACE_CURSOR(cur, XBT_ERROR);
 		return error;
 	}
-	XFS_BTREE_STATS_INC(cur, free);
 
 	cur->bc_bufs[level - 1] = NULL;
 	be16_add_cpu(&block->bb_level, -1);
@@ -3320,14 +3332,12 @@ xfs_btree_kill_root(
 	 */
 	cur->bc_ops->set_root(cur, newroot, -1);
 
-	error = cur->bc_ops->free_block(cur, bp);
+	error = xfs_btree_free_block(cur, bp);
 	if (error) {
 		XFS_BTREE_TRACE_CURSOR(cur, XBT_ERROR);
 		return error;
 	}
 
-	XFS_BTREE_STATS_INC(cur, free);
-
 	cur->bc_bufs[level] = NULL;
 	cur->bc_ra[level] = 0;
 	cur->bc_nlevels--;
@@ -3833,10 +3843,9 @@ xfs_btree_delrec(
 	}
 
 	/* Free the deleted block. */
-	error = cur->bc_ops->free_block(cur, rbp);
+	error = xfs_btree_free_block(cur, rbp);
 	if (error)
 		goto error0;
-	XFS_BTREE_STATS_INC(cur, free);
 
 	/*
 	 * If we joined with the left neighbor, set the buffer in the
-- 
1.9.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 2/3] xfs: factor btree block freeing into a helper

[Brian Foster]

On Wed, Jan 13, 2016 at 06:50:53PM +0100, Christoph Hellwig wrote:
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---

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

>  fs/xfs/libxfs/xfs_btree.c | 23 ++++++++++++++++-------
>  1 file changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
> index 3143577..77afb4a 100644
> --- a/fs/xfs/libxfs/xfs_btree.c
> +++ b/fs/xfs/libxfs/xfs_btree.c
> @@ -294,6 +294,19 @@ xfs_btree_sblock_verify_crc(
>  	return true;
>  }
>  
> +static int
> +xfs_btree_free_block(
> +	struct xfs_btree_cur	*cur,
> +	struct xfs_buf		*bp)
> +{
> +	int			error;
> +
> +	error = cur->bc_ops->free_block(cur, bp);
> +	if (!error)
> +		XFS_BTREE_STATS_INC(cur, free);
> +	return error;
> +}
> +
>  /*
>   * Delete the btree cursor.
>   */
> @@ -3282,12 +3295,11 @@ xfs_btree_kill_iroot(
>  #endif
>  	xfs_btree_copy_ptrs(cur, pp, cpp, numrecs);
>  
> -	error = cur->bc_ops->free_block(cur, cbp);
> +	error = xfs_btree_free_block(cur, cbp);
>  	if (error) {
>  		XFS_BTREE_TRACE_CURSOR(cur, XBT_ERROR);
>  		return error;
>  	}
> -	XFS_BTREE_STATS_INC(cur, free);
>  
>  	cur->bc_bufs[level - 1] = NULL;
>  	be16_add_cpu(&block->bb_level, -1);
> @@ -3320,14 +3332,12 @@ xfs_btree_kill_root(
>  	 */
>  	cur->bc_ops->set_root(cur, newroot, -1);
>  
> -	error = cur->bc_ops->free_block(cur, bp);
> +	error = xfs_btree_free_block(cur, bp);
>  	if (error) {
>  		XFS_BTREE_TRACE_CURSOR(cur, XBT_ERROR);
>  		return error;
>  	}
>  
> -	XFS_BTREE_STATS_INC(cur, free);
> -
>  	cur->bc_bufs[level] = NULL;
>  	cur->bc_ra[level] = 0;
>  	cur->bc_nlevels--;
> @@ -3833,10 +3843,9 @@ xfs_btree_delrec(
>  	}
>  
>  	/* Free the deleted block. */
> -	error = cur->bc_ops->free_block(cur, rbp);
> +	error = xfs_btree_free_block(cur, rbp);
>  	if (error)
>  		goto error0;
> -	XFS_BTREE_STATS_INC(cur, free);
>  
>  	/*
>  	 * If we joined with the left neighbor, set the buffer in the
> -- 
> 1.9.1
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 3/3] xfs: move buffer invalidation to xfs_btree_free_block

[Christoph Hellwig]

... instead of leaving it in the methods.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_alloc_btree.c  |  2 --
 fs/xfs/libxfs/xfs_bmap_btree.c   |  1 -
 fs/xfs/libxfs/xfs_btree.c        |  4 +++-
 fs/xfs/libxfs/xfs_ialloc_btree.c | 12 ++----------
 4 files changed, 5 insertions(+), 14 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_alloc_btree.c b/fs/xfs/libxfs/xfs_alloc_btree.c
index 444626d..d9b42425 100644
--- a/fs/xfs/libxfs/xfs_alloc_btree.c
+++ b/fs/xfs/libxfs/xfs_alloc_btree.c
@@ -118,8 +118,6 @@ xfs_allocbt_free_block(
 	xfs_extent_busy_insert(cur->bc_tp, be32_to_cpu(agf->agf_seqno), bno, 1,
 			      XFS_EXTENT_BUSY_SKIP_DISCARD);
 	xfs_trans_agbtree_delta(cur->bc_tp, -1);
-
-	xfs_trans_binval(cur->bc_tp, bp);
 	return 0;
 }
 
diff --git a/fs/xfs/libxfs/xfs_bmap_btree.c b/fs/xfs/libxfs/xfs_bmap_btree.c
index 1637c37..e37508a 100644
--- a/fs/xfs/libxfs/xfs_bmap_btree.c
+++ b/fs/xfs/libxfs/xfs_bmap_btree.c
@@ -531,7 +531,6 @@ xfs_bmbt_free_block(
 
 	xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
 	xfs_trans_mod_dquot_byino(tp, ip, XFS_TRANS_DQ_BCOUNT, -1L);
-	xfs_trans_binval(tp, bp);
 	return 0;
 }
 
diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
index 77afb4a..1f88e1c 100644
--- a/fs/xfs/libxfs/xfs_btree.c
+++ b/fs/xfs/libxfs/xfs_btree.c
@@ -302,8 +302,10 @@ xfs_btree_free_block(
 	int			error;
 
 	error = cur->bc_ops->free_block(cur, bp);
-	if (!error)
+	if (!error) {
+		xfs_trans_binval(cur->bc_tp, bp);
 		XFS_BTREE_STATS_INC(cur, free);
+	}
 	return error;
 }
 
diff --git a/fs/xfs/libxfs/xfs_ialloc_btree.c b/fs/xfs/libxfs/xfs_ialloc_btree.c
index c679f3c..89c21d7 100644
--- a/fs/xfs/libxfs/xfs_ialloc_btree.c
+++ b/fs/xfs/libxfs/xfs_ialloc_btree.c
@@ -125,16 +125,8 @@ xfs_inobt_free_block(
 	struct xfs_btree_cur	*cur,
 	struct xfs_buf		*bp)
 {
-	xfs_fsblock_t		fsbno;
-	int			error;
-
-	fsbno = XFS_DADDR_TO_FSB(cur->bc_mp, XFS_BUF_ADDR(bp));
-	error = xfs_free_extent(cur->bc_tp, fsbno, 1);
-	if (error)
-		return error;
-
-	xfs_trans_binval(cur->bc_tp, bp);
-	return error;
+	return xfs_free_extent(cur->bc_tp,
+			XFS_DADDR_TO_FSB(cur->bc_mp, XFS_BUF_ADDR(bp)), 1);
 }
 
 STATIC int
-- 
1.9.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 3/3] xfs: move buffer invalidation to xfs_btree_free_block

[Brian Foster]

On Wed, Jan 13, 2016 at 06:50:54PM +0100, Christoph Hellwig wrote:
> ... instead of leaving it in the methods.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---

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

>  fs/xfs/libxfs/xfs_alloc_btree.c  |  2 --
>  fs/xfs/libxfs/xfs_bmap_btree.c   |  1 -
>  fs/xfs/libxfs/xfs_btree.c        |  4 +++-
>  fs/xfs/libxfs/xfs_ialloc_btree.c | 12 ++----------
>  4 files changed, 5 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_alloc_btree.c b/fs/xfs/libxfs/xfs_alloc_btree.c
> index 444626d..d9b42425 100644
> --- a/fs/xfs/libxfs/xfs_alloc_btree.c
> +++ b/fs/xfs/libxfs/xfs_alloc_btree.c
> @@ -118,8 +118,6 @@ xfs_allocbt_free_block(
>  	xfs_extent_busy_insert(cur->bc_tp, be32_to_cpu(agf->agf_seqno), bno, 1,
>  			      XFS_EXTENT_BUSY_SKIP_DISCARD);
>  	xfs_trans_agbtree_delta(cur->bc_tp, -1);
> -
> -	xfs_trans_binval(cur->bc_tp, bp);
>  	return 0;
>  }
>  
> diff --git a/fs/xfs/libxfs/xfs_bmap_btree.c b/fs/xfs/libxfs/xfs_bmap_btree.c
> index 1637c37..e37508a 100644
> --- a/fs/xfs/libxfs/xfs_bmap_btree.c
> +++ b/fs/xfs/libxfs/xfs_bmap_btree.c
> @@ -531,7 +531,6 @@ xfs_bmbt_free_block(
>  
>  	xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
>  	xfs_trans_mod_dquot_byino(tp, ip, XFS_TRANS_DQ_BCOUNT, -1L);
> -	xfs_trans_binval(tp, bp);
>  	return 0;
>  }
>  
> diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
> index 77afb4a..1f88e1c 100644
> --- a/fs/xfs/libxfs/xfs_btree.c
> +++ b/fs/xfs/libxfs/xfs_btree.c
> @@ -302,8 +302,10 @@ xfs_btree_free_block(
>  	int			error;
>  
>  	error = cur->bc_ops->free_block(cur, bp);
> -	if (!error)
> +	if (!error) {
> +		xfs_trans_binval(cur->bc_tp, bp);
>  		XFS_BTREE_STATS_INC(cur, free);
> +	}
>  	return error;
>  }
>  
> diff --git a/fs/xfs/libxfs/xfs_ialloc_btree.c b/fs/xfs/libxfs/xfs_ialloc_btree.c
> index c679f3c..89c21d7 100644
> --- a/fs/xfs/libxfs/xfs_ialloc_btree.c
> +++ b/fs/xfs/libxfs/xfs_ialloc_btree.c
> @@ -125,16 +125,8 @@ xfs_inobt_free_block(
>  	struct xfs_btree_cur	*cur,
>  	struct xfs_buf		*bp)
>  {
> -	xfs_fsblock_t		fsbno;
> -	int			error;
> -
> -	fsbno = XFS_DADDR_TO_FSB(cur->bc_mp, XFS_BUF_ADDR(bp));
> -	error = xfs_free_extent(cur->bc_tp, fsbno, 1);
> -	if (error)
> -		return error;
> -
> -	xfs_trans_binval(cur->bc_tp, bp);
> -	return error;
> +	return xfs_free_extent(cur->bc_tp,
> +			XFS_DADDR_TO_FSB(cur->bc_mp, XFS_BUF_ADDR(bp)), 1);
>  }
>  
>  STATIC int
> -- 
> 1.9.1
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs