[PATCH v2 0/6] xfsprogs-4.19: transaction cleanups

[PATCH v2 0/6] xfsprogs-4.19: transaction cleanups

[Darrick J. Wong]

Here are six 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.  They should apply to Eric's
libxfs-4.19-sync branch.

--D

[PATCH 1/6] 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/6] 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/6] 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>
---
 mkfs/proto.c    |   19 +++++++++----------
 mkfs/xfs_mkfs.c |    4 ++--
 repair/phase5.c |    4 ++--
 repair/phase6.c |   20 ++++++++------------
 repair/rmap.c   |    7 +++----
 5 files changed, 24 insertions(+), 30 deletions(-)


diff --git a/mkfs/proto.c b/mkfs/proto.c
index 07d019d6..9da0587e 100644
--- a/mkfs/proto.c
+++ b/mkfs/proto.c
@@ -123,9 +123,8 @@ 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(mp, &M_RES(mp)->tr_itruncate,
+				r, 0, 0, &tp);
 		if (i == 0)
 			return tp;
 	}
@@ -180,7 +179,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 +190,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(mp, &M_RES(mp)->tr_itruncate, 0, 0, 0, &tp);
 	if (error)
 		fail(_("allocating transaction for a file"), error);
 	libxfs_trans_ijoin(tp, ip, 0);
@@ -610,12 +608,12 @@ rtinit(
 	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(mp, &M_RES(mp)->tr_itruncate,
+			MKFS_BLOCKRES_INODE, 0, 0, &tp);
 	if (i)
 		res_failed(i);
 
@@ -652,7 +650,7 @@ rtinit(
 	/*
 	 * Next, give the bitmap file some zero-filled blocks.
 	 */
-	i = -libxfs_trans_alloc(mp, &tres,
+	i = -libxfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate,
 		mp->m_sb.sb_rbmblocks + (XFS_BM_MAXLEVELS(mp,XFS_DATA_FORK) - 1),
 				0, 0, &tp);
 	if (i)
@@ -683,7 +681,7 @@ rtinit(
 	 * Give the summary file some zero-filled blocks.
 	 */
 	nsumblocks = mp->m_rsumsize >> mp->m_sb.sb_blocklog;
-	i = -libxfs_trans_alloc(mp, &tres,
+	i = -libxfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate,
 			nsumblocks + (XFS_BM_MAXLEVELS(mp,XFS_DATA_FORK) - 1),
 				0, 0, &tp);
 	if (i)
@@ -713,7 +711,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..9ef6e84a 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -3674,10 +3674,10 @@ 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(mp, &M_RES(mp)->tr_itruncate,
+			worst_freelist, 0, 0, &tp);
 	if (c)
 		res_failed(c);
 
diff --git a/repair/phase5.c b/repair/phase5.c
index 64f7b6e8..ac2eea54 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,8 @@ 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(mp, &M_RES(mp)->tr_itruncate,
+				16, 0, 0, &tp);
 		if (error)
 			goto out_cancel;
 
diff --git a/repair/phase6.c b/repair/phase6.c
index d4b6a5cf..ab125d6c 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};
 
 	/*
 	 * first set up inode
 	 */
-	i = -libxfs_trans_alloc(mp, &tres, 10, 0, 0, &tp);
+	i = -libxfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate,
+			10, 0, 0, &tp);
 	if (i)
 		res_failed(i);
 
@@ -579,7 +579,7 @@ mk_rbmino(xfs_mount_t *mp)
 	 * then allocate blocks for file and fill with zeroes (stolen
 	 * from mkfs)
 	 */
-	error = -libxfs_trans_alloc(mp, &tres,
+	error = -libxfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate,
 		mp->m_sb.sb_rbmblocks + (XFS_BM_MAXLEVELS(mp,XFS_DATA_FORK) - 1),
 				   0, 0, &tp);
 	if (error)
@@ -619,12 +619,12 @@ 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(mp, &M_RES(mp)->tr_itruncate,
+			10, 0, 0, &tp);
 	if (error)
 		res_failed(error);
 
@@ -686,13 +686,13 @@ 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(mp, &M_RES(mp)->tr_itruncate,
+			10, 0, 0, &tp);
 	if (error)
 		res_failed(error);
 
@@ -757,7 +757,6 @@ 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};
 
 	/*
 	 * first set up inode
@@ -811,10 +810,7 @@ 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,
+	error = -libxfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate,
 		mp->m_sb.sb_rbmblocks + (XFS_BM_MAXLEVELS(mp,XFS_DATA_FORK) - 1),
 				    0, 0, &tp);
 	if (error)
diff --git a/repair/rmap.c b/repair/rmap.c
index f991144a..dbe5b2c8 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,8 @@ 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(mp, &M_RES(mp)->tr_itruncate,
+				16, 0, 0, &tp);
 		if (error)
 			goto err_slab;
 
@@ -1366,7 +1366,6 @@ fix_freelist(
 {
 	xfs_alloc_arg_t		args;
 	xfs_trans_t		*tp;
-	struct xfs_trans_res	tres = {0};
 	int			flags;
 	int			error;
 
@@ -1375,7 +1374,7 @@ fix_freelist(
 	args.agno = agno;
 	args.alignment = 1;
 	args.pag = libxfs_perag_get(mp, agno);
-	error = -libxfs_trans_alloc(mp, &tres,
+	error = -libxfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate,
 			libxfs_alloc_min_freelist(mp, args.pag), 0, 0, &tp);
 	if (error)
 		do_error(_("failed to fix AGFL on AG %d, error %d\n"),

[PATCH 4/6] 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 9da0587e..5999fd2e 100644
--- a/mkfs/proto.c
+++ b/mkfs/proto.c
@@ -212,7 +212,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
@@ -474,7 +476,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;
@@ -552,7 +556,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.
@@ -575,7 +581,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);
 }
 
@@ -645,7 +655,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.
@@ -675,7 +688,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.
@@ -704,7 +720,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.
@@ -723,7 +742,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 9ef6e84a..b04a705f 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -3690,7 +3690,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 ab125d6c..0a52cf96 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);
 }
 
@@ -669,7 +676,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);
 }
@@ -738,7 +747,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);
 }
@@ -803,7 +814,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
@@ -835,7 +848,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);
 }
 
@@ -900,7 +918,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),
@@ -1030,7 +1051,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);
@@ -1128,7 +1153,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);
@@ -1168,7 +1196,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  {
@@ -1199,7 +1230,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);
@@ -1335,7 +1369,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;
@@ -1366,7 +1403,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;
@@ -1412,7 +1452,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);
 }
 
 /*
@@ -1885,7 +1928,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);
@@ -2901,7 +2947,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);
 			}
@@ -2942,7 +2990,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)  {
@@ -2995,7 +3046,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 dbe5b2c8..bd24b527 100644
--- a/repair/rmap.c
+++ b/repair/rmap.c
@@ -1411,7 +1411,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 5/6] 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 5999fd2e..d5ffe969 100644
--- a/mkfs/proto.c
+++ b/mkfs/proto.c
@@ -480,7 +480,7 @@ parseproto(
 		if (error)
 			fail(_("Space preallocation failed."), error);
 		rsvfile(mp, ip, llen);
-		IRELE(ip);
+		libxfs_irele(ip);
 		return;
 
 	case IF_BLOCK:
@@ -574,7 +574,7 @@ parseproto(
 				break;
 			parseproto(mp, ip, fsxp, pp, name);
 		}
-		IRELE(ip);
+		libxfs_irele(ip);
 		return;
 	default:
 		ASSERT(0);
@@ -586,7 +586,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 0a52cf96..3d43935c 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
@@ -679,7 +679,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);
 }
 
@@ -735,7 +735,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);
 		}
 
@@ -750,7 +750,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);
 }
 
@@ -854,7 +854,7 @@ mk_rsumino(xfs_mount_t *mp)
 	_("allocation of the realtime summary ino failed, error = %d\n"),
 			error);
 	}
-	IRELE(ip);
+	libxfs_irele(ip);
 }
 
 /*
@@ -922,7 +922,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));
@@ -1056,8 +1056,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);
@@ -1235,8 +1235,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
@@ -3052,7 +3052,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 6/6] 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 54546da8..d0fc2a84 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 11a2098d..5ebca844 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);
 
@@ -775,6 +778,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:

Re: [PATCH 3/6] libxfs: fix xfs_trans_alloc reservation abuse

[Dave Chinner]

On Thu, Sep 27, 2018 at 06:04:49PM -0700, Darrick J. Wong wrote:
> 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>
> ---
>  mkfs/proto.c    |   19 +++++++++----------
>  mkfs/xfs_mkfs.c |    4 ++--
>  repair/phase5.c |    4 ++--
>  repair/phase6.c |   20 ++++++++------------
>  repair/rmap.c   |    7 +++----
>  5 files changed, 24 insertions(+), 30 deletions(-)
> 
> 
> diff --git a/mkfs/proto.c b/mkfs/proto.c
> index 07d019d6..9da0587e 100644
> --- a/mkfs/proto.c
> +++ b/mkfs/proto.c
> @@ -123,9 +123,8 @@ 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(mp, &M_RES(mp)->tr_itruncate,

I'm wondering if this should explicitly call out that it's a dummy
reservation rather than using the itruncate reservation? e.g. these
places use:

	i = -libxfs_trans_alloc_perm(mp, blks, rtblks, flags, &tp);

And the implementation of this function then goes and uses the
itruncate reservation with a comment explaining what thay is used

(open to a better name - "dummy" doesn't seem right - perm, rolling,
deferred, etc all seem appropriate to indicate that it's an
allocation for a permanent transaction type for rolling/defered
transactions).

Cheers,

Dave.

-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 3/6] libxfs: fix xfs_trans_alloc reservation abuse

[Darrick J. Wong]

On Sun, Sep 30, 2018 at 08:31:31AM +1000, Dave Chinner wrote:
> On Thu, Sep 27, 2018 at 06:04:49PM -0700, Darrick J. Wong wrote:
> > 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>
> > ---
> >  mkfs/proto.c    |   19 +++++++++----------
> >  mkfs/xfs_mkfs.c |    4 ++--
> >  repair/phase5.c |    4 ++--
> >  repair/phase6.c |   20 ++++++++------------
> >  repair/rmap.c   |    7 +++----
> >  5 files changed, 24 insertions(+), 30 deletions(-)
> > 
> > 
> > diff --git a/mkfs/proto.c b/mkfs/proto.c
> > index 07d019d6..9da0587e 100644
> > --- a/mkfs/proto.c
> > +++ b/mkfs/proto.c
> > @@ -123,9 +123,8 @@ 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(mp, &M_RES(mp)->tr_itruncate,
> 
> I'm wondering if this should explicitly call out that it's a dummy
> reservation rather than using the itruncate reservation? e.g. these
> places use:
> 
> 	i = -libxfs_trans_alloc_perm(mp, blks, rtblks, flags, &tp);
> 
> And the implementation of this function then goes and uses the
> itruncate reservation with a comment explaining what thay is used
> 
> (open to a better name - "dummy" doesn't seem right - perm, rolling,
> deferred, etc all seem appropriate to indicate that it's an
> allocation for a permanent transaction type for rolling/defered
> transactions).

I don't necessarily like the long name, but
"libxfs_trans_alloc_rollable" seems the most descriptive to me.

I do like using a helper instead of opencoding tr_itruncate
everywhere.

--D

> 
> Cheers,
> 
> Dave.
> 
> -- 
> Dave Chinner
> david@fromorbit.com