[PATCH v3 0/8] xfsprogs-4.19: transaction cleanups

[PATCH v3 0/8] xfsprogs-4.19: transaction cleanups

[Darrick J. Wong]

Here are seven cleanups to the userspace transaction code that make the
commit and roll code more closely resemble their kernel counterparts,
and fix a number of problems where client code did not check the return
values of the transaction functions.  The eighth patch fixes a bug in
xfs_scrub_all.  The patches should apply to Eric's libxfs-4.19-sync
branch.

--D

[PATCH 1/8] libxfs: port kernel transaction code

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Restructure the userspace transaction code to resemble the kernel code
more closely.  This will make deferred operations behave the same (with
respect to transaction lifetimes) as the kernel.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 libxfs/trans.c |  238 ++++++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 187 insertions(+), 51 deletions(-)


diff --git a/libxfs/trans.c b/libxfs/trans.c
index b1543b0c..11a2098d 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -20,6 +20,10 @@
 #include "xfs_defer.h"
 
 static void xfs_trans_free_items(struct xfs_trans *tp);
+STATIC struct xfs_trans *xfs_trans_dup(struct xfs_trans *tp);
+static int xfs_trans_reserve(struct xfs_trans *tp, struct xfs_trans_res *resp,
+		uint blocks, uint rtextents);
+static int __xfs_trans_commit(struct xfs_trans *tp, bool regrant);
 
 /*
  * Simple transaction interface
@@ -76,24 +80,17 @@ int
 libxfs_trans_roll(
 	struct xfs_trans	**tpp)
 {
-	struct xfs_mount	*mp;
 	struct xfs_trans	*trans = *tpp;
 	struct xfs_trans_res	tres;
-	unsigned int		old_blk_res;
-	xfs_fsblock_t		old_firstblock;
-	struct list_head	old_dfops;
 	int			error;
 
 	/*
 	 * Copy the critical parameters from one trans to the next.
 	 */
-	mp = trans->t_mountp;
 	tres.tr_logres = trans->t_log_res;
 	tres.tr_logcount = trans->t_log_count;
-	old_blk_res = trans->t_blk_res;
-	old_firstblock = trans->t_firstblock;
-	/* structure copy */
-	old_dfops = trans->t_dfops;
+
+	*tpp = xfs_trans_dup(trans);
 
 	/*
 	 * Commit the current transaction.
@@ -102,7 +99,7 @@ libxfs_trans_roll(
 	 * is in progress. The caller takes the responsibility to cancel
 	 * the duplicate transaction that gets returned.
 	 */
-	error = xfs_trans_commit(trans);
+	error = __xfs_trans_commit(trans, true);
 	if (error)
 		return error;
 
@@ -115,14 +112,7 @@ libxfs_trans_roll(
 	 * the prior and the next transactions.
 	 */
 	tres.tr_logflags = XFS_TRANS_PERM_LOG_RES;
-	error = libxfs_trans_alloc(mp, &tres, 0, 0, 0, tpp);
-	trans = *tpp;
-	trans->t_blk_res = old_blk_res;
-	trans->t_firstblock = old_firstblock;
-	/* structure copy */
-	trans->t_dfops = old_dfops;
-
-	return 0;
+	return xfs_trans_reserve(*tpp, &tres, 0, 0);
 }
 
 /*
@@ -136,40 +126,150 @@ xfs_trans_free(
 	kmem_zone_free(xfs_trans_zone, tp);
 }
 
-int
-libxfs_trans_alloc(
-	struct xfs_mount	*mp,
-	struct xfs_trans_res	*resp,
-	unsigned int		blocks,
-	unsigned int		rtextents,
-	unsigned int		flags,
-	struct xfs_trans	**tpp)
+/*
+ * This is called to create a new transaction which will share the
+ * permanent log reservation of the given transaction.  The remaining
+ * unused block and rt extent reservations are also inherited.  This
+ * implies that the original transaction is no longer allowed to allocate
+ * blocks.  Locks and log items, however, are no inherited.  They must
+ * be added to the new transaction explicitly.
+ */
+STATIC struct xfs_trans *
+xfs_trans_dup(
+	struct xfs_trans	*tp)
+{
+	struct xfs_trans	*ntp;
 
+	ntp = kmem_zone_zalloc(xfs_trans_zone, KM_SLEEP);
+
+	/*
+	 * Initialize the new transaction structure.
+	 */
+	ntp->t_mountp = tp->t_mountp;
+	INIT_LIST_HEAD(&ntp->t_items);
+	INIT_LIST_HEAD(&ntp->t_dfops);
+	ntp->t_firstblock = NULLFSBLOCK;
+
+	ASSERT(tp->t_flags & XFS_TRANS_PERM_LOG_RES);
+
+	ntp->t_flags = XFS_TRANS_PERM_LOG_RES |
+		       (tp->t_flags & XFS_TRANS_RESERVE) |
+		       (tp->t_flags & XFS_TRANS_NO_WRITECOUNT);
+	/* We gave our writer reference to the new transaction */
+	tp->t_flags |= XFS_TRANS_NO_WRITECOUNT;
+
+	/* move deferred ops over to the new tp */
+	xfs_defer_move(ntp, tp);
+
+	return ntp;
+}
+
+/*
+ * This is called to reserve free disk blocks and log space for the
+ * given transaction.  This must be done before allocating any resources
+ * within the transaction.
+ *
+ * This will return ENOSPC if there are not enough blocks available.
+ * It will sleep waiting for available log space.
+ * The only valid value for the flags parameter is XFS_RES_LOG_PERM, which
+ * is used by long running transactions.  If any one of the reservations
+ * fails then they will all be backed out.
+ *
+ * This does not do quota reservations. That typically is done by the
+ * caller afterwards.
+ */
+static int
+xfs_trans_reserve(
+	struct xfs_trans	*tp,
+	struct xfs_trans_res	*resp,
+	uint			blocks,
+	uint			rtextents)
 {
-	struct xfs_sb	*sb = &mp->m_sb;
-	struct xfs_trans *ptr;
+	int			error = 0;
 
 	/*
 	 * Attempt to reserve the needed disk blocks by decrementing
-	 * the number needed from the number available.	 This will
+	 * the number needed from the number available.  This will
 	 * fail if the count would go below zero.
 	 */
 	if (blocks > 0) {
-		if (sb->sb_fdblocks < blocks)
+		if (tp->t_mountp->m_sb.sb_fdblocks < blocks)
 			return -ENOSPC;
+		tp->t_blk_res += blocks;
+	}
+
+	/*
+	 * Reserve the log space needed for this transaction.
+	 */
+	if (resp->tr_logres > 0) {
+		ASSERT(tp->t_log_res == 0 ||
+		       tp->t_log_res == resp->tr_logres);
+		ASSERT(tp->t_log_count == 0 ||
+		       tp->t_log_count == resp->tr_logcount);
+
+		if (resp->tr_logflags & XFS_TRANS_PERM_LOG_RES)
+			tp->t_flags |= XFS_TRANS_PERM_LOG_RES;
+		else
+			ASSERT(!(tp->t_flags & XFS_TRANS_PERM_LOG_RES));
+
+		tp->t_log_res = resp->tr_logres;
+		tp->t_log_count = resp->tr_logcount;
 	}
 
-	ptr = kmem_zone_zalloc(xfs_trans_zone,
+	/*
+	 * Attempt to reserve the needed realtime extents by decrementing
+	 * the number needed from the number available.  This will
+	 * fail if the count would go below zero.
+	 */
+	if (rtextents > 0) {
+		if (tp->t_mountp->m_sb.sb_rextents < rtextents) {
+			error = -ENOSPC;
+			goto undo_blocks;
+		}
+	}
+
+	return 0;
+
+	/*
+	 * Error cases jump to one of these labels to undo any
+	 * reservations which have already been performed.
+	 */
+undo_blocks:
+	if (blocks > 0)
+		tp->t_blk_res = 0;
+
+	return error;
+}
+
+int
+libxfs_trans_alloc(
+	struct xfs_mount	*mp,
+	struct xfs_trans_res	*resp,
+	unsigned int		blocks,
+	unsigned int		rtextents,
+	unsigned int		flags,
+	struct xfs_trans	**tpp)
+
+{
+	struct xfs_trans	*tp;
+	int			error;
+
+	tp = kmem_zone_zalloc(xfs_trans_zone,
 		(flags & XFS_TRANS_NOFS) ? KM_NOFS : KM_SLEEP);
-	ptr->t_mountp = mp;
-	ptr->t_blk_res = blocks;
-	INIT_LIST_HEAD(&ptr->t_items);
-	INIT_LIST_HEAD(&ptr->t_dfops);
-	ptr->t_firstblock = NULLFSBLOCK;
+	tp->t_mountp = mp;
+	INIT_LIST_HEAD(&tp->t_items);
+	INIT_LIST_HEAD(&tp->t_dfops);
+	tp->t_firstblock = NULLFSBLOCK;
+
+	error = xfs_trans_reserve(tp, resp, blocks, rtextents);
+	if (error) {
+		xfs_trans_cancel(tp);
+		return error;
+	}
 #ifdef XACT_DEBUG
-	fprintf(stderr, "allocated new transaction %p\n", ptr);
+	fprintf(stderr, "allocated new transaction %p\n", tp);
 #endif
-	*tpp = ptr;
+	*tpp = tp;
 	return 0;
 }
 
@@ -197,18 +297,25 @@ libxfs_trans_alloc_empty(
 
 void
 libxfs_trans_cancel(
-	xfs_trans_t	*tp)
+	struct xfs_trans	*tp)
 {
 #ifdef XACT_DEBUG
-	xfs_trans_t	*otp = tp;
+	struct xfs_trans	*otp = tp;
 #endif
-	if (tp != NULL) {
-		xfs_trans_free_items(tp);
-		xfs_trans_free(tp);
-	}
+	if (tp == NULL)
+		goto out;
+
+	if (tp->t_flags & XFS_TRANS_PERM_LOG_RES)
+		xfs_defer_cancel(tp);
+
+	xfs_trans_free_items(tp);
+	xfs_trans_free(tp);
+
+out:
 #ifdef XACT_DEBUG
 	fprintf(stderr, "## cancelled transaction %p\n", otp);
 #endif
+	return;
 }
 
 int
@@ -261,6 +368,9 @@ libxfs_trans_ijoin(
 	ASSERT(iip->ili_flags == 0);
 	ASSERT(iip->ili_inode != NULL);
 
+	ASSERT(iip->ili_lock_flags == 0);
+	iip->ili_lock_flags = lock_flags;
+
 	xfs_trans_add_item(tp, (xfs_log_item_t *)(iip));
 
 	ip->i_transp = tp;
@@ -839,22 +949,36 @@ xfs_trans_free_items(
 /*
  * Commit the changes represented by this transaction
  */
-int
-libxfs_trans_commit(
-	xfs_trans_t	*tp)
+static int
+__xfs_trans_commit(
+	struct xfs_trans	*tp,
+	bool			regrant)
 {
-	xfs_sb_t	*sbp;
+	struct xfs_sb		*sbp;
+	int			error = 0;
 
 	if (tp == NULL)
 		return 0;
 
+	/*
+	 * Finish deferred items on final commit. Only permanent transactions
+	 * should ever have deferred ops.
+	 */
+	WARN_ON_ONCE(!list_empty(&tp->t_dfops) &&
+		     !(tp->t_flags & XFS_TRANS_PERM_LOG_RES));
+	if (!regrant && (tp->t_flags & XFS_TRANS_PERM_LOG_RES)) {
+		error = xfs_defer_finish_noroll(&tp);
+		if (error) {
+			xfs_defer_cancel(tp);
+			goto out_unreserve;
+		}
+	}
+
 	if (!(tp->t_flags & XFS_TRANS_DIRTY)) {
 #ifdef XACT_DEBUG
 		fprintf(stderr, "committed clean transaction %p\n", tp);
 #endif
-		xfs_trans_free_items(tp);
-		xfs_trans_free(tp);
-		return 0;
+		goto out_unreserve;
 	}
 
 	if (tp->t_flags & XFS_TRANS_SB_DIRTY) {
@@ -878,4 +1002,16 @@ libxfs_trans_commit(
 	/* That's it for the transaction structure.  Free it. */
 	xfs_trans_free(tp);
 	return 0;
+
+out_unreserve:
+	xfs_trans_free_items(tp);
+	xfs_trans_free(tp);
+	return error;
+}
+
+int
+libxfs_trans_commit(
+	struct xfs_trans	*tp)
+{
+	return __xfs_trans_commit(tp, false);
 }

[PATCH 2/8] libxfs: fix libxfs_trans_alloc callsite problems

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Fix some incorrect libxfs_trans_alloc callers to check return values
correctly.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 mkfs/proto.c    |    4 +++-
 mkfs/xfs_mkfs.c |    2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)


diff --git a/mkfs/proto.c b/mkfs/proto.c
index c13e3644..07d019d6 100644
--- a/mkfs/proto.c
+++ b/mkfs/proto.c
@@ -192,7 +192,9 @@ rsvfile(
 	/*
 	 * update the inode timestamp, mode, and prealloc flag bits
 	 */
-	libxfs_trans_alloc(mp, &tres, 0, 0, 0, &tp);
+	error = -libxfs_trans_alloc(mp, &tres, 0, 0, 0, &tp);
+	if (error)
+		fail(_("allocating transaction for a file"), error);
 	libxfs_trans_ijoin(tp, ip, 0);
 
 	VFS_I(ip)->i_mode &= ~S_ISUID;
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 2e53c1e8..c6ef3a71 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -3677,7 +3677,7 @@ initialise_ag_freespace(
 	struct xfs_trans_res tres = {0};
 	int			c;
 
-	c = libxfs_trans_alloc(mp, &tres, worst_freelist, 0, 0, &tp);
+	c = -libxfs_trans_alloc(mp, &tres, worst_freelist, 0, 0, &tp);
 	if (c)
 		res_failed(c);
 

[PATCH 3/8] xfs_repair: fix block reservation in mk_rsumino

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

The functions mk_rsumino and rtinit both allocate transactions to create
the realtime summary inode.  In order to allocate and map blocks to the
rtsummary file, these transactions require a block reservation.
However, despite the comments in mk_rsumino about lifting the code from
mkfs, it doesn't actually copy the same reservation calculation that
mkfs uses in rtinit().  Practically speaking this has no effect since
userspace doesn't care about transaction block reservations, but fix
this logic bomb anyway.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 repair/phase6.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/repair/phase6.c b/repair/phase6.c
index d4b6a5cf..afa65c51 100644
--- a/repair/phase6.c
+++ b/repair/phase6.c
@@ -815,7 +815,7 @@ mk_rsumino(xfs_mount_t *mp)
 	tres.tr_logcount = XFS_DEFAULT_PERM_LOG_COUNT;
 	tres.tr_logflags = XFS_TRANS_PERM_LOG_RES;
 	error = -libxfs_trans_alloc(mp, &tres,
-		mp->m_sb.sb_rbmblocks + (XFS_BM_MAXLEVELS(mp,XFS_DATA_FORK) - 1),
+		nsumblocks + (XFS_BM_MAXLEVELS(mp, XFS_DATA_FORK) - 1),
 				    0, 0, &tp);
 	if (error)
 		res_failed(error);

[PATCH 4/8] libxfs: fix xfs_trans_alloc reservation abuse

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Various xfsprogs tools have been abusing the transaction reservation
system by allocating the transaction with zero reservation.  This has
always worked in the past because userspace transactions do not require
reservations.  However, once we merge deferred ops into the transaction
structure, we will need to use a permanent reservation type to set up
any transaction that can roll.  tr_itruncate has all we need, so use
that as the reservation dummy.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 include/xfs_trans.h |    2 ++
 libxfs/trans.c      |   15 +++++++++++++++
 mkfs/proto.c        |   25 +++++++++++--------------
 mkfs/xfs_mkfs.c     |    3 +--
 repair/phase5.c     |    3 +--
 repair/phase6.c     |   26 ++++++++++----------------
 repair/rmap.c       |    8 +++-----
 7 files changed, 43 insertions(+), 39 deletions(-)


diff --git a/include/xfs_trans.h b/include/xfs_trans.h
index 54546da8..eda2f894 100644
--- a/include/xfs_trans.h
+++ b/include/xfs_trans.h
@@ -86,6 +86,8 @@ int	xfs_trans_roll(struct xfs_trans **);
 int	libxfs_trans_alloc(struct xfs_mount *mp, struct xfs_trans_res *resp,
 			   uint blocks, uint rtextents, uint flags,
 			   struct xfs_trans **tpp);
+int	libxfs_trans_alloc_rollable(struct xfs_mount *mp, uint blocks,
+				    struct xfs_trans **tpp);
 int	libxfs_trans_alloc_empty(struct xfs_mount *mp, struct xfs_trans **tpp);
 int	libxfs_trans_commit(struct xfs_trans *);
 void	libxfs_trans_cancel(struct xfs_trans *);
diff --git a/libxfs/trans.c b/libxfs/trans.c
index 11a2098d..508d12d7 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -295,6 +295,21 @@ libxfs_trans_alloc_empty(
 	return xfs_trans_alloc(mp, &resv, 0, 0, XFS_TRANS_NO_WRITECOUNT, tpp);
 }
 
+/*
+ * Allocate a transaction that can be rolled.  Since userspace doesn't have
+ * a need for log reservations, we really only tr_itruncate to get the
+ * permanent log reservation flag to avoid blowing asserts.
+ */
+int
+libxfs_trans_alloc_rollable(
+	struct xfs_mount	*mp,
+	unsigned int		blocks,
+	struct xfs_trans	**tpp)
+{
+	return libxfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate, blocks,
+			0, 0, tpp);
+}
+
 void
 libxfs_trans_cancel(
 	struct xfs_trans	*tp)
diff --git a/mkfs/proto.c b/mkfs/proto.c
index 07d019d6..50a1bc54 100644
--- a/mkfs/proto.c
+++ b/mkfs/proto.c
@@ -123,9 +123,7 @@ getres(
 	uint		r;
 
 	for (i = 0, r = MKFS_BLOCKRES(blocks); r >= blocks; r--) {
-		struct xfs_trans_res    tres = {0};
-
-		i = -libxfs_trans_alloc(mp, &tres, r, 0, 0, &tp);
+		i = -libxfs_trans_alloc_rollable(mp, r, &tp);
 		if (i == 0)
 			return tp;
 	}
@@ -180,7 +178,6 @@ rsvfile(
 {
 	int		error;
 	xfs_trans_t	*tp;
-	struct xfs_trans_res tres = {0};
 
 	error = -libxfs_alloc_file_space(ip, 0, llen, 1, 0);
 
@@ -192,7 +189,7 @@ rsvfile(
 	/*
 	 * update the inode timestamp, mode, and prealloc flag bits
 	 */
-	error = -libxfs_trans_alloc(mp, &tres, 0, 0, 0, &tp);
+	error = -libxfs_trans_alloc_rollable(mp, 0, &tp);
 	if (error)
 		fail(_("allocating transaction for a file"), error);
 	libxfs_trans_ijoin(tp, ip, 0);
@@ -604,18 +601,18 @@ rtinit(
 	int		i;
 	xfs_bmbt_irec_t	map[XFS_BMAP_MAX_NMAP];
 	xfs_extlen_t	nsumblocks;
+	uint		blocks;
 	int		nmap;
 	xfs_inode_t	*rbmip;
 	xfs_inode_t	*rsumip;
 	xfs_trans_t	*tp;
 	struct cred	creds;
 	struct fsxattr	fsxattrs;
-	struct xfs_trans_res tres = {0};
 
 	/*
 	 * First, allocate the inodes.
 	 */
-	i = -libxfs_trans_alloc(mp, &tres, MKFS_BLOCKRES_INODE, 0, 0, &tp);
+	i = -libxfs_trans_alloc_rollable(mp, MKFS_BLOCKRES_INODE, &tp);
 	if (i)
 		res_failed(i);
 
@@ -652,9 +649,9 @@ rtinit(
 	/*
 	 * Next, give the bitmap file some zero-filled blocks.
 	 */
-	i = -libxfs_trans_alloc(mp, &tres,
-		mp->m_sb.sb_rbmblocks + (XFS_BM_MAXLEVELS(mp,XFS_DATA_FORK) - 1),
-				0, 0, &tp);
+	blocks = mp->m_sb.sb_rbmblocks +
+			XFS_BM_MAXLEVELS(mp, XFS_DATA_FORK) - 1;
+	i = -libxfs_trans_alloc_rollable(mp, blocks, &tp);
 	if (i)
 		res_failed(i);
 
@@ -683,9 +680,8 @@ rtinit(
 	 * Give the summary file some zero-filled blocks.
 	 */
 	nsumblocks = mp->m_rsumsize >> mp->m_sb.sb_blocklog;
-	i = -libxfs_trans_alloc(mp, &tres,
-			nsumblocks + (XFS_BM_MAXLEVELS(mp,XFS_DATA_FORK) - 1),
-				0, 0, &tp);
+	blocks = nsumblocks + XFS_BM_MAXLEVELS(mp, XFS_DATA_FORK) - 1;
+	i = -libxfs_trans_alloc_rollable(mp, blocks, &tp);
 	if (i)
 		res_failed(i);
 	libxfs_trans_ijoin(tp, rsumip, 0);
@@ -713,7 +709,8 @@ rtinit(
 	 * Do one transaction per bitmap block.
 	 */
 	for (bno = 0; bno < mp->m_sb.sb_rextents; bno = ebno) {
-		i = -libxfs_trans_alloc(mp, &tres, 0, 0, 0, &tp);
+		i = -libxfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate,
+				0, 0, 0, &tp);
 		if (i)
 			res_failed(i);
 		libxfs_trans_ijoin(tp, rbmip, 0);
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index c6ef3a71..982d3871 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -3674,10 +3674,9 @@ initialise_ag_freespace(
 {
 	struct xfs_alloc_arg	args;
 	struct xfs_trans	*tp;
-	struct xfs_trans_res tres = {0};
 	int			c;
 
-	c = -libxfs_trans_alloc(mp, &tres, worst_freelist, 0, 0, &tp);
+	c = -libxfs_trans_alloc_rollable(mp, worst_freelist, &tp);
 	if (c)
 		res_failed(c);
 
diff --git a/repair/phase5.c b/repair/phase5.c
index 64f7b6e8..85d1f4fb 100644
--- a/repair/phase5.c
+++ b/repair/phase5.c
@@ -2421,7 +2421,6 @@ inject_lost_blocks(
 	struct xfs_trans	*tp = NULL;
 	struct xfs_slab_cursor	*cur = NULL;
 	xfs_fsblock_t		*fsb;
-	struct xfs_trans_res	tres = {0};
 	struct xfs_owner_info	oinfo;
 	int			error;
 
@@ -2431,7 +2430,7 @@ inject_lost_blocks(
 		return error;
 
 	while ((fsb = pop_slab_cursor(cur)) != NULL) {
-		error = -libxfs_trans_alloc(mp, &tres, 16, 0, 0, &tp);
+		error = -libxfs_trans_alloc_rollable(mp, 16, &tp);
 		if (error)
 			goto out_cancel;
 
diff --git a/repair/phase6.c b/repair/phase6.c
index afa65c51..d6cb0a5a 100644
--- a/repair/phase6.c
+++ b/repair/phase6.c
@@ -526,12 +526,12 @@ mk_rbmino(xfs_mount_t *mp)
 	xfs_bmbt_irec_t	map[XFS_BMAP_MAX_NMAP];
 	int		vers;
 	int		times;
-	struct xfs_trans_res tres = {0};
+	uint		blocks;
 
 	/*
 	 * first set up inode
 	 */
-	i = -libxfs_trans_alloc(mp, &tres, 10, 0, 0, &tp);
+	i = -libxfs_trans_alloc_rollable(mp, 10, &tp);
 	if (i)
 		res_failed(i);
 
@@ -579,9 +579,9 @@ mk_rbmino(xfs_mount_t *mp)
 	 * then allocate blocks for file and fill with zeroes (stolen
 	 * from mkfs)
 	 */
-	error = -libxfs_trans_alloc(mp, &tres,
-		mp->m_sb.sb_rbmblocks + (XFS_BM_MAXLEVELS(mp,XFS_DATA_FORK) - 1),
-				   0, 0, &tp);
+	blocks = mp->m_sb.sb_rbmblocks +
+			XFS_BM_MAXLEVELS(mp, XFS_DATA_FORK) - 1;
+	error = -libxfs_trans_alloc_rollable(mp, blocks, &tp);
 	if (error)
 		res_failed(error);
 
@@ -619,12 +619,11 @@ fill_rbmino(xfs_mount_t *mp)
 	int		error;
 	xfs_fileoff_t	bno;
 	xfs_bmbt_irec_t	map;
-	struct xfs_trans_res tres = {0};
 
 	bmp = btmcompute;
 	bno = 0;
 
-	error = -libxfs_trans_alloc(mp, &tres, 10, 0, 0, &tp);
+	error = -libxfs_trans_alloc_rollable(mp, 10, &tp);
 	if (error)
 		res_failed(error);
 
@@ -686,13 +685,12 @@ fill_rsumino(xfs_mount_t *mp)
 	xfs_fileoff_t	bno;
 	xfs_fileoff_t	end_bno;
 	xfs_bmbt_irec_t	map;
-	struct xfs_trans_res tres = {0};
 
 	smp = sumcompute;
 	bno = 0;
 	end_bno = mp->m_rsumsize >> mp->m_sb.sb_blocklog;
 
-	error = -libxfs_trans_alloc(mp, &tres, 10, 0, 0, &tp);
+	error = -libxfs_trans_alloc_rollable(mp, 10, &tp);
 	if (error)
 		res_failed(error);
 
@@ -757,7 +755,7 @@ mk_rsumino(xfs_mount_t *mp)
 	xfs_bmbt_irec_t	map[XFS_BMAP_MAX_NMAP];
 	int		vers;
 	int		times;
-	struct xfs_trans_res tres = {0};
+	uint		blocks;
 
 	/*
 	 * first set up inode
@@ -811,12 +809,8 @@ mk_rsumino(xfs_mount_t *mp)
 	 * from mkfs)
 	 */
 	nsumblocks = mp->m_rsumsize >> mp->m_sb.sb_blocklog;
-	tres.tr_logres = BBTOB(128);
-	tres.tr_logcount = XFS_DEFAULT_PERM_LOG_COUNT;
-	tres.tr_logflags = XFS_TRANS_PERM_LOG_RES;
-	error = -libxfs_trans_alloc(mp, &tres,
-		nsumblocks + (XFS_BM_MAXLEVELS(mp, XFS_DATA_FORK) - 1),
-				    0, 0, &tp);
+	blocks = nsumblocks + XFS_BM_MAXLEVELS(mp, XFS_DATA_FORK) - 1;
+	error = -libxfs_trans_alloc_rollable(mp, blocks, &tp);
 	if (error)
 		res_failed(error);
 
diff --git a/repair/rmap.c b/repair/rmap.c
index f991144a..d596d0ff 100644
--- a/repair/rmap.c
+++ b/repair/rmap.c
@@ -449,7 +449,6 @@ rmap_store_ag_btree_rec(
 	struct xfs_buf		*agbp = NULL;
 	struct xfs_buf		*agflbp = NULL;
 	struct xfs_trans	*tp;
-	struct xfs_trans_res tres = {0};
 	__be32			*agfl_bno, *b;
 	int			error = 0;
 	struct xfs_owner_info	oinfo;
@@ -507,7 +506,7 @@ rmap_store_ag_btree_rec(
 	/* Insert rmaps into the btree one at a time */
 	rm_rec = pop_slab_cursor(rm_cur);
 	while (rm_rec) {
-		error = -libxfs_trans_alloc(mp, &tres, 16, 0, 0, &tp);
+		error = -libxfs_trans_alloc_rollable(mp, 16, &tp);
 		if (error)
 			goto err_slab;
 
@@ -1366,7 +1365,6 @@ fix_freelist(
 {
 	xfs_alloc_arg_t		args;
 	xfs_trans_t		*tp;
-	struct xfs_trans_res	tres = {0};
 	int			flags;
 	int			error;
 
@@ -1375,8 +1373,8 @@ fix_freelist(
 	args.agno = agno;
 	args.alignment = 1;
 	args.pag = libxfs_perag_get(mp, agno);
-	error = -libxfs_trans_alloc(mp, &tres,
-			libxfs_alloc_min_freelist(mp, args.pag), 0, 0, &tp);
+	error = -libxfs_trans_alloc_rollable(mp,
+			libxfs_alloc_min_freelist(mp, args.pag), &tp);
 	if (error)
 		do_error(_("failed to fix AGFL on AG %d, error %d\n"),
 				agno, error);

[PATCH 5/8] libxfs: check libxfs_trans_commit return values

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Check the return value from libxfs_trans_commit since it can now return
error codes.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 mkfs/proto.c    |   38 ++++++++++++++++++-----
 mkfs/xfs_mkfs.c |    4 ++
 repair/phase6.c |   90 ++++++++++++++++++++++++++++++++++++++++++++-----------
 repair/rmap.c   |    4 ++
 4 files changed, 108 insertions(+), 28 deletions(-)


diff --git a/mkfs/proto.c b/mkfs/proto.c
index 50a1bc54..dc82f093 100644
--- a/mkfs/proto.c
+++ b/mkfs/proto.c
@@ -211,7 +211,9 @@ rsvfile(
 	ip->i_d.di_flags |= XFS_DIFLAG_PREALLOC;
 
 	libxfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
-	libxfs_trans_commit(tp);
+	error = -libxfs_trans_commit(tp);
+	if (error)
+		fail(_("committing space for a file failed"), error);
 }
 
 static int
@@ -473,7 +475,9 @@ parseproto(
 		xname.type = XFS_DIR3_FT_REG_FILE;
 		newdirent(mp, tp, pip, &xname, ip->i_ino);
 		libxfs_trans_log_inode(tp, ip, flags);
-		libxfs_trans_commit(tp);
+		error = -libxfs_trans_commit(tp);
+		if (error)
+			fail(_("Space preallocation failed."), error);
 		rsvfile(mp, ip, llen);
 		IRELE(ip);
 		return;
@@ -551,7 +555,9 @@ parseproto(
 		}
 		newdirectory(mp, tp, ip, pip);
 		libxfs_trans_log_inode(tp, ip, flags);
-		libxfs_trans_commit(tp);
+		error = -libxfs_trans_commit(tp);
+		if (error)
+			fail(_("Directory inode allocation failed."), error);
 		/*
 		 * RT initialization.  Do this here to ensure that
 		 * the RT inodes get placed after the root inode.
@@ -574,7 +580,11 @@ parseproto(
 		fail(_("Unknown format"), EINVAL);
 	}
 	libxfs_trans_log_inode(tp, ip, flags);
-	libxfs_trans_commit(tp);
+	error = -libxfs_trans_commit(tp);
+	if (error) {
+		fail(_("Error encountered creating file from prototype file"),
+			error);
+	}
 	IRELE(ip);
 }
 
@@ -644,7 +654,10 @@ rtinit(
 	rsumip->i_d.di_size = mp->m_rsumsize;
 	libxfs_trans_log_inode(tp, rsumip, XFS_ILOG_CORE);
 	libxfs_log_sb(tp);
-	libxfs_trans_commit(tp);
+	error = -libxfs_trans_commit(tp);
+	if (error)
+		fail(_("Completion of the realtime summary inode failed"),
+				error);
 	mp->m_rsumip = rsumip;
 	/*
 	 * Next, give the bitmap file some zero-filled blocks.
@@ -674,7 +687,10 @@ rtinit(
 		}
 	}
 
-	libxfs_trans_commit(tp);
+	error = -libxfs_trans_commit(tp);
+	if (error)
+		fail(_("Block allocation of the realtime bitmap inode failed"),
+				error);
 
 	/*
 	 * Give the summary file some zero-filled blocks.
@@ -702,7 +718,10 @@ rtinit(
 			bno += ep->br_blockcount;
 		}
 	}
-	libxfs_trans_commit(tp);
+	error = -libxfs_trans_commit(tp);
+	if (error)
+		fail(_("Block allocation of the realtime summary inode failed"),
+				error);
 
 	/*
 	 * Free the whole area using transactions.
@@ -721,7 +740,10 @@ rtinit(
 			fail(_("Error initializing the realtime space"),
 				error);
 		}
-		libxfs_trans_commit(tp);
+		error = -libxfs_trans_commit(tp);
+		if (error)
+			fail(_("Initialization of the realtime space failed"),
+					error);
 	}
 }
 
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 982d3871..7c05c6f6 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -3689,7 +3689,9 @@ initialise_ag_freespace(
 
 	libxfs_alloc_fix_freelist(&args, 0);
 	libxfs_perag_put(args.pag);
-	libxfs_trans_commit(tp);
+	c = -libxfs_trans_commit(tp);
+	if (c)
+		res_failed(c);
 }
 
 /*
diff --git a/repair/phase6.c b/repair/phase6.c
index d6cb0a5a..8c0165ca 100644
--- a/repair/phase6.c
+++ b/repair/phase6.c
@@ -573,7 +573,9 @@ mk_rbmino(xfs_mount_t *mp)
 	 * commit changes
 	 */
 	libxfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
-	libxfs_trans_commit(tp);
+	error = -libxfs_trans_commit(tp);
+	if (error)
+		do_error(_("%s: commit failed, error %d\n"), __func__, error);
 
 	/*
 	 * then allocate blocks for file and fill with zeroes (stolen
@@ -604,7 +606,12 @@ mk_rbmino(xfs_mount_t *mp)
 			bno += ep->br_blockcount;
 		}
 	}
-	libxfs_trans_commit(tp);
+	error = -libxfs_trans_commit(tp);
+	if (error) {
+		do_error(
+		_("allocation of the realtime bitmap failed, error = %d\n"),
+			error);
+	}
 	IRELE(ip);
 }
 
@@ -668,7 +675,9 @@ _("can't access block %" PRIu64 " (fsbno %" PRIu64 ") of realtime bitmap inode %
 		bno++;
 	}
 
-	libxfs_trans_commit(tp);
+	error = -libxfs_trans_commit(tp);
+	if (error)
+		do_error(_("%s: commit failed, error %d\n"), __func__, error);
 	IRELE(ip);
 	return(0);
 }
@@ -736,7 +745,9 @@ _("can't access block %" PRIu64 " (fsbno %" PRIu64 ") of realtime summary inode
 		bno++;
 	}
 
-	libxfs_trans_commit(tp);
+	error = -libxfs_trans_commit(tp);
+	if (error)
+		do_error(_("%s: commit failed, error %d\n"), __func__, error);
 	IRELE(ip);
 	return(0);
 }
@@ -802,7 +813,9 @@ mk_rsumino(xfs_mount_t *mp)
 	 * commit changes
 	 */
 	libxfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
-	libxfs_trans_commit(tp);
+	error = -libxfs_trans_commit(tp);
+	if (error)
+		do_error(_("%s: commit failed, error %d\n"), __func__, error);
 
 	/*
 	 * then allocate blocks for file and fill with zeroes (stolen
@@ -833,7 +846,12 @@ mk_rsumino(xfs_mount_t *mp)
 			bno += ep->br_blockcount;
 		}
 	}
-	libxfs_trans_commit(tp);
+	error = -libxfs_trans_commit(tp);
+	if (error) {
+		do_error(
+	_("allocation of the realtime summary ino failed, error = %d\n"),
+			error);
+	}
 	IRELE(ip);
 }
 
@@ -898,7 +916,10 @@ mk_root_dir(xfs_mount_t *mp)
 	ip->d_ops = mp->m_dir_inode_ops;
 	libxfs_dir_init(tp, ip, ip);
 
-	libxfs_trans_commit(tp);
+	error = -libxfs_trans_commit(tp);
+	if (error)
+		do_error(_("%s: commit failed, error %d\n"), __func__, error);
+
 	IRELE(ip);
 
 	irec = find_inode_rec(mp, XFS_INO_TO_AGNO(mp, mp->m_sb.sb_rootino),
@@ -1028,7 +1049,11 @@ mk_orphanage(xfs_mount_t *mp)
 	libxfs_trans_log_inode(tp, pip, XFS_ILOG_CORE);
 	libxfs_dir_init(tp, ip, pip);
 	libxfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
-	libxfs_trans_commit(tp);
+	error = -libxfs_trans_commit(tp);
+	if (error) {
+		do_error(_("%s directory creation failed -- bmapf error %d\n"),
+			ORPHANAGE, error);
+	}
 	IRELE(ip);
 	IRELE(pip);
 	add_inode_reached(irec,ino_offset);
@@ -1126,7 +1151,10 @@ mv_orphanage(
 
 			inc_nlink(VFS_I(ino_p));
 			libxfs_trans_log_inode(tp, ino_p, XFS_ILOG_CORE);
-			libxfs_trans_commit(tp);
+			err = -libxfs_trans_commit(tp);
+			if (err)
+				do_error(
+	_("creation of .. entry failed (%d)\n"), err);
 		} else  {
 			err = -libxfs_trans_alloc(mp, &M_RES(mp)->tr_rename,
 						  nres, 0, 0, &tp);
@@ -1166,7 +1194,10 @@ mv_orphanage(
 						err);
 			}
 
-			libxfs_trans_commit(tp);
+			err = -libxfs_trans_commit(tp);
+			if (err)
+				do_error(
+	_("orphanage name replace op failed (%d)\n"), err);
 		}
 
 	} else  {
@@ -1197,7 +1228,10 @@ mv_orphanage(
 
 		set_nlink(VFS_I(ino_p), 1);
 		libxfs_trans_log_inode(tp, ino_p, XFS_ILOG_CORE);
-		libxfs_trans_commit(tp);
+		err = -libxfs_trans_commit(tp);
+		if (err)
+			do_error(
+	_("orphanage name create failed (%d)\n"), err);
 	}
 	IRELE(ino_p);
 	IRELE(orphanage_ip);
@@ -1333,7 +1367,10 @@ longform_dir2_rebuild(
 		goto out_bmap_cancel;
 	}
 
-	libxfs_trans_commit(tp);
+	error = -libxfs_trans_commit(tp);
+	if (error)
+		do_error(
+	_("dir init failed (%d)\n"), error);
 
 	if (ino == mp->m_sb.sb_rootino)
 		need_root_dotdot = 0;
@@ -1364,7 +1401,10 @@ _("name create failed in ino %" PRIu64 " (%d), filesystem may be out of space\n"
 			goto out_bmap_cancel;
 		}
 
-		libxfs_trans_commit(tp);
+		error = -libxfs_trans_commit(tp);
+		if (error)
+			do_error(
+_("name create failed (%d) during rebuild\n"), error);
 	}
 
 	return;
@@ -1410,7 +1450,10 @@ dir2_kill_block(
 	if (error)
 		do_error(_("shrink_inode failed inode %" PRIu64 " block %u\n"),
 			ip->i_ino, da_bno);
-	libxfs_trans_commit(tp);
+	error = -libxfs_trans_commit(tp);
+	if (error)
+		do_error(
+_("directory shrink failed (%d)\n"), error);
 }
 
 /*
@@ -1883,7 +1926,10 @@ _("entry \"%s\" in dir inode %" PRIu64 " inconsistent with .. value (%" PRIu64 "
 				d, &i);
 	if (needlog)
 		libxfs_dir2_data_log_header(&da, bp);
-	libxfs_trans_commit(tp);
+	error = -libxfs_trans_commit(tp);
+	if (error)
+		do_error(
+_("directory block fixing failed (%d)\n"), error);
 
 	/* record the largest free space in the freetab for later checking */
 	bf = M_DIROPS(mp)->data_bestfree_p(d);
@@ -2899,7 +2945,9 @@ process_dir_inode(
 			if (dirty)  {
 				libxfs_trans_log_inode(tp, ip,
 					XFS_ILOG_CORE | XFS_ILOG_DDATA);
-				libxfs_trans_commit(tp);
+				error = -libxfs_trans_commit(tp);
+				if (error)
+					res_failed(error);
 			} else  {
 				libxfs_trans_cancel(tp);
 			}
@@ -2940,7 +2988,10 @@ process_dir_inode(
 	_("can't make \"..\" entry in root inode %" PRIu64 ", createname error %d\n"), ino, error);
 
 		libxfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
-		libxfs_trans_commit(tp);
+		error = -libxfs_trans_commit(tp);
+		if (error)
+			do_error(
+	_("root inode \"..\" entry recreation failed (%d)\n"), error);
 
 		need_root_dotdot = 0;
 	} else if (need_root_dotdot && ino == mp->m_sb.sb_rootino)  {
@@ -2993,7 +3044,10 @@ process_dir_inode(
 					ino, error);
 
 			libxfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
-			libxfs_trans_commit(tp);
+			error = -libxfs_trans_commit(tp);
+			if (error)
+				do_error(
+	_("root inode \".\" entry recreation failed (%d)\n"), error);
 		}
 	}
 	IRELE(ip);
diff --git a/repair/rmap.c b/repair/rmap.c
index d596d0ff..6de4a105 100644
--- a/repair/rmap.c
+++ b/repair/rmap.c
@@ -1410,7 +1410,9 @@ fix_freelist(
 		do_error(_("failed to fix AGFL on AG %d, error %d\n"),
 				agno, error);
 	}
-	libxfs_trans_commit(tp);
+	error = -libxfs_trans_commit(tp);
+	if (error)
+		do_error(_("%s: commit failed, error %d\n"), __func__, error);
 }
 
 /*

[PATCH 6/8] libxfs: clean up IRELE/iput callsites

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Replace the IRELE macro with a proper function so that we can do proper
typechecking.  This is the userspace cleanup in the same vein as the
kernel patch with the same subject.
---
 db/attrset.c        |    4 ++--
 include/xfs_inode.h |    4 +---
 libxfs/init.c       |    4 ++--
 libxfs/rdwr.c       |    5 +++--
 mkfs/proto.c        |    6 +++---
 repair/phase6.c     |   22 +++++++++++-----------
 repair/phase7.c     |    2 +-
 7 files changed, 23 insertions(+), 24 deletions(-)


diff --git a/db/attrset.c b/db/attrset.c
index dc82a6c6..56972506 100644
--- a/db/attrset.c
+++ b/db/attrset.c
@@ -159,7 +159,7 @@ attr_set_f(
 out:
 	mp->m_flags &= ~LIBXFS_MOUNT_COMPAT_ATTR;
 	if (ip)
-		IRELE(ip);
+		libxfs_irele(ip);
 	if (value)
 		free(value);
 	return 0;
@@ -234,6 +234,6 @@ attr_remove_f(
 out:
 	mp->m_flags &= ~LIBXFS_MOUNT_COMPAT_ATTR;
 	if (ip)
-		IRELE(ip);
+		libxfs_irele(ip);
 	return 0;
 }
diff --git a/include/xfs_inode.h b/include/xfs_inode.h
index f573f23b..79ec3a2d 100644
--- a/include/xfs_inode.h
+++ b/include/xfs_inode.h
@@ -155,8 +155,6 @@ extern bool	libxfs_inode_verify_forks(struct xfs_inode *ip,
 extern int	libxfs_iget(struct xfs_mount *, struct xfs_trans *, xfs_ino_t,
 				uint, struct xfs_inode **,
 				struct xfs_ifork_ops *);
-extern void	libxfs_iput(struct xfs_inode *);
-
-#define IRELE(ip) libxfs_iput(ip)
+extern void	libxfs_irele(struct xfs_inode *ip);
 
 #endif /* __XFS_INODE_H__ */
diff --git a/libxfs/init.c b/libxfs/init.c
index 36d637c5..d7543d4a 100644
--- a/libxfs/init.c
+++ b/libxfs/init.c
@@ -838,9 +838,9 @@ void
 libxfs_rtmount_destroy(xfs_mount_t *mp)
 {
 	if (mp->m_rsumip)
-		IRELE(mp->m_rsumip);
+		libxfs_irele(mp->m_rsumip);
 	if (mp->m_rbmip)
-		IRELE(mp->m_rbmip);
+		libxfs_irele(mp->m_rbmip);
 	mp->m_rsumip = mp->m_rbmip = NULL;
 }
 
diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
index 14a4633e..0ee3ba86 100644
--- a/libxfs/rdwr.c
+++ b/libxfs/rdwr.c
@@ -1402,7 +1402,7 @@ libxfs_iget(
 	}
 
 	if (!libxfs_inode_verify_forks(ip, ifork_ops)) {
-		libxfs_iput(ip);
+		libxfs_irele(ip);
 		return -EFSCORRUPTED;
 	}
 
@@ -1435,7 +1435,8 @@ libxfs_idestroy(xfs_inode_t *ip)
 }
 
 void
-libxfs_iput(xfs_inode_t *ip)
+libxfs_irele(
+	struct xfs_inode	*ip)
 {
 	if (ip->i_itemp)
 		kmem_zone_free(xfs_ili_zone, ip->i_itemp);
diff --git a/mkfs/proto.c b/mkfs/proto.c
index dc82f093..687c53ab 100644
--- a/mkfs/proto.c
+++ b/mkfs/proto.c
@@ -479,7 +479,7 @@ parseproto(
 		if (error)
 			fail(_("Space preallocation failed."), error);
 		rsvfile(mp, ip, llen);
-		IRELE(ip);
+		libxfs_irele(ip);
 		return;
 
 	case IF_BLOCK:
@@ -573,7 +573,7 @@ parseproto(
 				break;
 			parseproto(mp, ip, fsxp, pp, name);
 		}
-		IRELE(ip);
+		libxfs_irele(ip);
 		return;
 	default:
 		ASSERT(0);
@@ -585,7 +585,7 @@ parseproto(
 		fail(_("Error encountered creating file from prototype file"),
 			error);
 	}
-	IRELE(ip);
+	libxfs_irele(ip);
 }
 
 void
diff --git a/repair/phase6.c b/repair/phase6.c
index 8c0165ca..e2e4446a 100644
--- a/repair/phase6.c
+++ b/repair/phase6.c
@@ -612,7 +612,7 @@ mk_rbmino(xfs_mount_t *mp)
 		_("allocation of the realtime bitmap failed, error = %d\n"),
 			error);
 	}
-	IRELE(ip);
+	libxfs_irele(ip);
 }
 
 static int
@@ -678,7 +678,7 @@ _("can't access block %" PRIu64 " (fsbno %" PRIu64 ") of realtime bitmap inode %
 	error = -libxfs_trans_commit(tp);
 	if (error)
 		do_error(_("%s: commit failed, error %d\n"), __func__, error);
-	IRELE(ip);
+	libxfs_irele(ip);
 	return(0);
 }
 
@@ -733,7 +733,7 @@ fill_rsumino(xfs_mount_t *mp)
 			do_warn(
 _("can't access block %" PRIu64 " (fsbno %" PRIu64 ") of realtime summary inode %" PRIu64 "\n"),
 				bno, map.br_startblock, mp->m_sb.sb_rsumino);
-			IRELE(ip);
+			libxfs_irele(ip);
 			return(1);
 		}
 
@@ -748,7 +748,7 @@ _("can't access block %" PRIu64 " (fsbno %" PRIu64 ") of realtime summary inode
 	error = -libxfs_trans_commit(tp);
 	if (error)
 		do_error(_("%s: commit failed, error %d\n"), __func__, error);
-	IRELE(ip);
+	libxfs_irele(ip);
 	return(0);
 }
 
@@ -852,7 +852,7 @@ mk_rsumino(xfs_mount_t *mp)
 	_("allocation of the realtime summary ino failed, error = %d\n"),
 			error);
 	}
-	IRELE(ip);
+	libxfs_irele(ip);
 }
 
 /*
@@ -920,7 +920,7 @@ mk_root_dir(xfs_mount_t *mp)
 	if (error)
 		do_error(_("%s: commit failed, error %d\n"), __func__, error);
 
-	IRELE(ip);
+	libxfs_irele(ip);
 
 	irec = find_inode_rec(mp, XFS_INO_TO_AGNO(mp, mp->m_sb.sb_rootino),
 				XFS_INO_TO_AGINO(mp, mp->m_sb.sb_rootino));
@@ -1054,8 +1054,8 @@ mk_orphanage(xfs_mount_t *mp)
 		do_error(_("%s directory creation failed -- bmapf error %d\n"),
 			ORPHANAGE, error);
 	}
-	IRELE(ip);
-	IRELE(pip);
+	libxfs_irele(ip);
+	libxfs_irele(pip);
 	add_inode_reached(irec,ino_offset);
 
 	return(ino);
@@ -1233,8 +1233,8 @@ mv_orphanage(
 			do_error(
 	_("orphanage name create failed (%d)\n"), err);
 	}
-	IRELE(ino_p);
-	IRELE(orphanage_ip);
+	libxfs_irele(ino_p);
+	libxfs_irele(orphanage_ip);
 }
 
 static int
@@ -3050,7 +3050,7 @@ process_dir_inode(
 	_("root inode \".\" entry recreation failed (%d)\n"), error);
 		}
 	}
-	IRELE(ip);
+	libxfs_irele(ip);
 }
 
 /*
diff --git a/repair/phase7.c b/repair/phase7.c
index 09d11fcd..c2a60a93 100644
--- a/repair/phase7.c
+++ b/repair/phase7.c
@@ -77,7 +77,7 @@ update_inode_nlinks(
 
 		ASSERT(error == 0);
 	}
-	IRELE(ip);
+	libxfs_irele(ip);
 }
 
 /*

[PATCH 7/8] libxfs: track transaction block reservation usage like the kernel

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Currently, block reservations in userspace transactions are not carried
over across transaction rolls.  This will lead to ENOSPC failures inside
libxfs code which checks for reservation overruns in an upcoming patch
that borrows the bmbt repair code from the kernel because it makes
extensive use of transaction rolling.

Therefore, port t_blk_res_used from the kernel so that block
reservations work the same way in userspace.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 include/xfs_trans.h |    1 +
 libxfs/trans.c      |   12 ++++++++++++
 2 files changed, 13 insertions(+)


diff --git a/include/xfs_trans.h b/include/xfs_trans.h
index eda2f894..31df1172 100644
--- a/include/xfs_trans.h
+++ b/include/xfs_trans.h
@@ -71,6 +71,7 @@ typedef struct xfs_trans {
 	unsigned int	t_blk_res;		/* # of blocks resvd */
 	xfs_fsblock_t	t_firstblock;		/* first block allocated */
 	struct xfs_mount *t_mountp;		/* ptr to fs mount struct */
+	unsigned int	t_blk_res_used;		/* # of resvd blocks used */
 	unsigned int	t_flags;		/* misc flags */
 	long		t_icount_delta;		/* superblock icount change */
 	long		t_ifree_delta;		/* superblock ifree change */
diff --git a/libxfs/trans.c b/libxfs/trans.c
index 508d12d7..7058ecef 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -158,6 +158,9 @@ xfs_trans_dup(
 	/* We gave our writer reference to the new transaction */
 	tp->t_flags |= XFS_TRANS_NO_WRITECOUNT;
 
+	ntp->t_blk_res = tp->t_blk_res - tp->t_blk_res_used;
+	tp->t_blk_res = tp->t_blk_res_used;
+
 	/* move deferred ops over to the new tp */
 	xfs_defer_move(ntp, tp);
 
@@ -790,6 +793,15 @@ libxfs_trans_mod_sb(
 	case XFS_TRANS_SB_RES_FDBLOCKS:
 		return;
 	case XFS_TRANS_SB_FDBLOCKS:
+		if (delta < 0) {
+			tp->t_blk_res_used += (uint)-delta;
+			if (tp->t_blk_res_used > tp->t_blk_res) {
+				fprintf(stderr,
+_("Transaction block reservation exceeded! %u > %u\n"),
+					tp->t_blk_res_used, tp->t_blk_res);
+				ASSERT(0);
+			}
+		}
 		tp->t_fdblocks_delta += delta;
 		break;
 	case XFS_TRANS_SB_ICOUNT:

[PATCH 8/8] xfs_scrub_all: fix systemd escaping again

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Apparently newer versions of systemd than the one on this author's
laptop <cough> now complain about lack of (path) escaping in unit instance
variable contents:

 # xfs_scrub_all
 Scrubbing /home...
  Invalid unit name "xfs_scrub@/home" was escaped as "xfs_scrub@-home"
 (maybe you should use systemd-escape?)
 Starting Online XFS Metadata Check for /home...

So change the systemd_escape() function to escape paths unconditionally
to make the warning go away.

Reported-by: Matthias Bodenbinder <matthias@bodenbinder.de>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 scrub/xfs_scrub_all.in |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)


diff --git a/scrub/xfs_scrub_all.in b/scrub/xfs_scrub_all.in
index f7d9e6c7..c4e9899d 100644
--- a/scrub/xfs_scrub_all.in
+++ b/scrub/xfs_scrub_all.in
@@ -82,11 +82,16 @@ def run_killable(cmd, stdout, killfuncs, kill_fn):
 # actually /can/ escape the dashes correctly if it is told that this is a path
 # (and not a unit name), but it didn't do this prior to January 2017, so fix
 # this for them.
+#
+# systemd path escaping also drops the initial slash so we add that back in so
+# that log messages from the service units preserve the full path and users can
+# look up log messages using full paths.  However, for "/" the escaping rules
+# do /not/ drop the initial slash, so we have to special-case that here.
 def systemd_escape(path):
 	'''Escape a path to avoid mangled systemd mangling.'''
 
-	if '-' not in path:
-		return path
+	if path == '/':
+		return '-'
 	cmd = ['systemd-escape', '--path', path]
 	try:
 		proc = subprocess.Popen(cmd, stdout = subprocess.PIPE)

Re: [PATCH v3 0/8] xfsprogs-4.19: transaction cleanups

[Eric Sandeen]

On 10/1/18 12:04 PM, Darrick J. Wong wrote:
> Here are seven cleanups to the userspace transaction code that make the
> commit and roll code more closely resemble their kernel counterparts,
> and fix a number of problems where client code did not check the return
> values of the transaction functions.  The eighth patch fixes a bug in
> xfs_scrub_all.  The patches should apply to Eric's libxfs-4.19-sync
> branch.
> 
> --D
> 

You're going to hate me (or maybe not) but in the end I decided to stop
being stubborn and bring in these patches before the libxfs-4.19 up[date,
as I should have from the start.  You don't have to rebase them again tho,
I already got through it.  I'll diff to your tree to make sure nothing went
haywire.

For the series with my out of order backporting tweaks,

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

Thanks, this was great work.

-Eric

Re: [PATCH v3 0/8] xfsprogs-4.19: transaction cleanups

[Darrick J. Wong]

On Thu, Oct 04, 2018 at 02:13:19PM -0500, Eric Sandeen wrote:
> 
> 
> On 10/1/18 12:04 PM, Darrick J. Wong wrote:
> > Here are seven cleanups to the userspace transaction code that make the
> > commit and roll code more closely resemble their kernel counterparts,
> > and fix a number of problems where client code did not check the return
> > values of the transaction functions.  The eighth patch fixes a bug in
> > xfs_scrub_all.  The patches should apply to Eric's libxfs-4.19-sync
> > branch.
> > 
> > --D
> > 
> 
> You're going to hate me (or maybe not) but in the end I decided to stop
> being stubborn and bring in these patches before the libxfs-4.19 up[date,
> as I should have from the start.  You don't have to rebase them again tho,
> I already got through it.  I'll diff to your tree to make sure nothing went
> haywire.

But a lot of stuff went 'haywire'... most of which I think is in patch 3
and patch 4 of this series.  Patch 8 is also new, though not related.
Oh well, see libxfs-4.19-sync-3 in my djwong/xfsprogs-dev.git tree on
korg.

(Oh and I found another annoying bug in xfs_io's dedupe command; will
send patch for that when the dust settles...)

--D

> For the series with my out of order backporting tweaks,
> 
> Reviewed-by: Eric Sandeen <sandeen@redhat.com>
> 
> Thanks, this was great work.
> 
> -Eric

[PATCH 9/8] xfs_io: dedupe command should only complain if we don't dedupe anything

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

The dedupe command should only complain about non-matching extents if
the kernel hasn't managed to dedupe /any/ of the input range.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 io/reflink.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/io/reflink.c b/io/reflink.c
index 26eb2e32..72dfe32d 100644
--- a/io/reflink.c
+++ b/io/reflink.c
@@ -70,7 +70,8 @@ dedupe_ioctl(
 					_(strerror(-info->status)));
 			goto done;
 		}
-		if (info->status == XFS_EXTENT_DATA_DIFFERS) {
+		if (deduped == 0 &&
+		    info->status == XFS_EXTENT_DATA_DIFFERS) {
 			fprintf(stderr, "XFS_IOC_FILE_EXTENT_SAME: %s\n",
 					_("Extents did not match."));
 			goto done;

Re: [PATCH 9/8] xfs_io: dedupe command should only complain if we don't dedupe anything

[Eric Sandeen]

On 10/4/18 5:27 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> The dedupe command should only complain about non-matching extents if
> the kernel hasn't managed to dedupe /any/ of the input range.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Should it be "no extents matched" then perhaps?

I mean xfs_io is not exactly-quite a purpose-built dedupe tool, but should
we be a bit more specific if we're issuing a message at all?

if (info->status == XFS_EXTENT_DATA_DIFFERS) {
	if (deduped == 0)
		fprintf(stderr, "XFS_IOC_FILE_EXTENT_SAME: %s\n",
  			_("No extents matched."));
	else
		fprintf(stderr, "XFS_IOC_FILE_EXTENT_SAME: %s\n",
  			_("Some extents did not match."));
}

And the manpage implies that any difference will make it fail:

> map length bytes at offset dst_offset in the open file to the same physical
> blocks that are mapped at offset src_offset in the file src_file, but only
> if the contents of both ranges are identical.

now you're telling me it'll make a best effort?  ;)  I think the manpage
needs clarification too ...

> ---


>  io/reflink.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/io/reflink.c b/io/reflink.c
> index 26eb2e32..72dfe32d 100644
> --- a/io/reflink.c
> +++ b/io/reflink.c
> @@ -70,7 +70,8 @@ dedupe_ioctl(
>  					_(strerror(-info->status)));
>  			goto done;
>  		}
> -		if (info->status == XFS_EXTENT_DATA_DIFFERS) {
> +		if (deduped == 0 &&
> +		    info->status == XFS_EXTENT_DATA_DIFFERS) {
>  			fprintf(stderr, "XFS_IOC_FILE_EXTENT_SAME: %s\n",
>  					_("Extents did not match."));
>  			goto done;
> 

Re: [PATCH 9/8] xfs_io: dedupe command should only complain if we don't dedupe anything

[Darrick J. Wong]

On Fri, Oct 05, 2018 at 04:37:14PM -0500, Eric Sandeen wrote:
> On 10/4/18 5:27 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > The dedupe command should only complain about non-matching extents if
> > the kernel hasn't managed to dedupe /any/ of the input range.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Should it be "no extents matched" then perhaps?
> 
> I mean xfs_io is not exactly-quite a purpose-built dedupe tool, but should
> we be a bit more specific if we're issuing a message at all?
> 
> if (info->status == XFS_EXTENT_DATA_DIFFERS) {
> 	if (deduped == 0)
> 		fprintf(stderr, "XFS_IOC_FILE_EXTENT_SAME: %s\n",
>   			_("No extents matched."));
> 	else
> 		fprintf(stderr, "XFS_IOC_FILE_EXTENT_SAME: %s\n",
>   			_("Some extents did not match."));

We don't need to tell the user that /some/ extents did not match.

If we have two files containing "moo cow" and "moo bow", you'd agree
that the first four bytes match, right?  So the output should be:

$ xfs_io -c 'dedupe /a 0 0 7' /b
linked 4/7 bytes at offset 0
4 bytes, 1 ops; 0.0000 sec

The "4/7" tells us that some extents didn't match, because we didn't
fulfill the entire range requested.  Pretend that this fs can dedupe at
byte alignment.

Now if the files contained "boo cow" and "aaaaaaa", you'd expect:

$ xfs_io -c 'dedupe /a 0 0 7' /b
XFS_IOC_EXTENT_SAME: No extents matched.

And if the files contained "moo cow" and "moo cow":

$ xfs_io -c 'dedupe /a 0 0 7' /b
linked 7/7 bytes at offset 0
7 bytes, 1 ops; 0.0000 sec

> }
> 
> And the manpage implies that any difference will make it fail:
> 
> > map length bytes at offset dst_offset in the open file to the same physical
> > blocks that are mapped at offset src_offset in the file src_file, but only
> > if the contents of both ranges are identical.

It also says:

"Upon  successful  completion  of  this ioctl, the number of bytes
successfully deduplicated is returned in bytes_deduped...."

Which contradicts:

"If even a single byte in the range does not match, the deduplication
request will be ignored and status set to FILE_DEDUPE_RANGE_DIFFERS.

But that makes no sense because if our only choices were really "all the
bytes" or "none of the bytes" then there wouldn't be a bytes_deduped.

> now you're telling me it'll make a best effort?  ;)  I think the manpage
> needs clarification too ...

Yep.

--D

> 
> > ---
> 
> 
> >  io/reflink.c |    3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/io/reflink.c b/io/reflink.c
> > index 26eb2e32..72dfe32d 100644
> > --- a/io/reflink.c
> > +++ b/io/reflink.c
> > @@ -70,7 +70,8 @@ dedupe_ioctl(
> >  					_(strerror(-info->status)));
> >  			goto done;
> >  		}
> > -		if (info->status == XFS_EXTENT_DATA_DIFFERS) {
> > +		if (deduped == 0 &&
> > +		    info->status == XFS_EXTENT_DATA_DIFFERS) {
> >  			fprintf(stderr, "XFS_IOC_FILE_EXTENT_SAME: %s\n",
> >  					_("Extents did not match."));
> >  			goto done;
> >