a reflink fix

a reflink fix

[Christoph Hellwig]

This fixes the issues I saw with generic/168 over NFS.  Turns out it
wasn't anything specific to that particular test cases, we just needed
a sufficiently aged refcount btree to trigger it.

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

[PATCH] xfs: always invalidate buffer in xfs_refcountbt_free_block

[Christoph Hellwig]

Simplify the return value from xfs_perag_pool_free_block to a bool so that
we can easily call xfs_trans_binval for both the per-AG pool and the real
freeing case.  Without this we fail to invalidate the btree buffer and
will trip over the write verifier on a shrinking refcount btree.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_perag_pool.c     | 22 ++++++++++------------
 fs/xfs/libxfs/xfs_perag_pool.h     |  2 +-
 fs/xfs/libxfs/xfs_refcount_btree.c | 20 +++++++-------------
 3 files changed, 18 insertions(+), 26 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_perag_pool.c b/fs/xfs/libxfs/xfs_perag_pool.c
index b49ffd2..41c4e5e 100644
--- a/fs/xfs/libxfs/xfs_perag_pool.c
+++ b/fs/xfs/libxfs/xfs_perag_pool.c
@@ -338,7 +338,7 @@ err:
  * @tp: Transaction to record the free operation.
  * @bno: Block to put back.
  */
-int
+bool
 xfs_perag_pool_free_block(
 	struct xfs_perag_pool		*p,
 	struct xfs_trans		*tp,
@@ -348,7 +348,7 @@ xfs_perag_pool_free_block(
 	struct xfs_perag_pool_entry	*ppe;
 
 	if (p == NULL)
-		return -EINVAL;
+		return false;
 
 	mp = p->pp_mount;
 	mp = mp;
@@ -356,18 +356,12 @@ xfs_perag_pool_free_block(
 
 	list_for_each_entry(ppe, &p->pp_entries, ppe_list) {
 		if (ppe->ppe_bno - 1 == bno) {
-
 			/* Adjust bookkeeping. */
-			p->pp_inuse--;
 			ppe->ppe_bno--;
-			ppe->ppe_len++;
-			return 0;
-		}
-		if (ppe->ppe_bno + ppe->ppe_len == bno) {
-			p->pp_inuse--;
-			ppe->ppe_len++;
-			return 0;
+			goto adjust;
 		}
+		if (ppe->ppe_bno + ppe->ppe_len == bno)
+			goto adjust;
 	}
 	ppe = kmem_alloc(sizeof(struct xfs_perag_pool_entry), KM_SLEEP);
 	ppe->ppe_bno = bno;
@@ -375,5 +369,9 @@ xfs_perag_pool_free_block(
 	p->pp_inuse--;
 
 	list_add_tail(&ppe->ppe_list, &p->pp_entries);
-	return 0;
+	return true;
+adjust:
+	p->pp_inuse--;
+	ppe->ppe_len++;
+	return true;
 }
diff --git a/fs/xfs/libxfs/xfs_perag_pool.h b/fs/xfs/libxfs/xfs_perag_pool.h
index ecdcd2a..665ea16 100644
--- a/fs/xfs/libxfs/xfs_perag_pool.h
+++ b/fs/xfs/libxfs/xfs_perag_pool.h
@@ -43,5 +43,5 @@ int xfs_perag_pool_ensure_capacity(struct xfs_perag_pool *p, xfs_extlen_t sz);
 
 int xfs_perag_pool_alloc_block(struct xfs_perag_pool *p, struct xfs_trans *tp,
 		xfs_agblock_t *bno);
-int xfs_perag_pool_free_block(struct xfs_perag_pool *p, struct xfs_trans *tp,
+bool xfs_perag_pool_free_block(struct xfs_perag_pool *p, struct xfs_trans *tp,
 		xfs_agblock_t bno);
diff --git a/fs/xfs/libxfs/xfs_refcount_btree.c b/fs/xfs/libxfs/xfs_refcount_btree.c
index 2379488..4a31164 100644
--- a/fs/xfs/libxfs/xfs_refcount_btree.c
+++ b/fs/xfs/libxfs/xfs_refcount_btree.c
@@ -141,27 +141,21 @@ xfs_refcountbt_free_block(
 	struct xfs_perag	*pag;
 	xfs_fsblock_t		fsbno = XFS_DADDR_TO_FSB(mp, XFS_BUF_ADDR(bp));
 	struct xfs_owner_info	oinfo;
-	int			error;
+	bool			freed;
 
 	/* Try to give it back to the pool. */
 	pag = xfs_perag_get(cur->bc_mp, cur->bc_private.a.agno);
-	error = xfs_perag_pool_free_block(pag->pagf_refcountbt_pool, cur->bc_tp,
+	freed = xfs_perag_pool_free_block(pag->pagf_refcountbt_pool, cur->bc_tp,
 			XFS_FSB_TO_AGBNO(cur->bc_mp, fsbno));
 	xfs_perag_put(pag);
 
-	switch (error) {
-	case 0:
-		return 0;
-	case -EINVAL:
-		break;
-	default:
-		return error;
+	if (!freed) {
+		/* Return it to the AG. */
+		XFS_RMAP_AG_OWNER(&oinfo, XFS_RMAP_OWN_REFC);
+		xfs_bmap_add_free(mp, cur->bc_private.a.flist, fsbno, 1,
+				&oinfo);
 	}
 
-	/* Return it to the AG. */
-	XFS_RMAP_AG_OWNER(&oinfo, XFS_RMAP_OWN_REFC);
-	xfs_bmap_add_free(mp, cur->bc_private.a.flist, fsbno, 1,
-			&oinfo);
 	xfs_trans_binval(tp, bp);
 	return 0;
 }
-- 
1.9.1

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

Re: a reflink fix

[Darrick J. Wong]

On Wed, Jan 13, 2016 at 05:13:48PM +0100, Christoph Hellwig wrote:
> This fixes the issues I saw with generic/168 over NFS.  Turns out it
> wasn't anything specific to that particular test cases, we just needed
> a sufficiently aged refcount btree to trigger it.

Thanks for the patch; I've worked the fixes into the overall patchset.

I also wrote a quick reproducer which will appear as xfs/807 in my next
xfstests drop.

--D

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