[PATCH 3/8] xfs: decouple xfs_trans_alloc_empty from xfs_trans_alloc

[PATCH 3/8] xfs: decouple xfs_trans_alloc_empty from xfs_trans_alloc

[Christoph Hellwig]

xfs_trans_alloc_empty only shares the very basic transaction structure
allocation and initialization with xfs_trans_alloc.

Split out a new __xfs_trans_alloc helper for that and otherwise decouple
xfs_trans_alloc_empty from xfs_trans_alloc.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_trans.c | 52 +++++++++++++++++++++++++---------------------
 1 file changed, 28 insertions(+), 24 deletions(-)

diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
index 1fddea8d761a..e43f44f62c5f 100644
--- a/fs/xfs/xfs_trans.c
+++ b/fs/xfs/xfs_trans.c
@@ -241,6 +241,28 @@ xfs_trans_reserve(
 	return error;
 }
 
+static struct xfs_trans *
+__xfs_trans_alloc(
+	struct xfs_mount	*mp,
+	uint			flags)
+{
+	struct xfs_trans	*tp;
+
+	ASSERT(!(flags & XFS_TRANS_RES_FDBLKS) || xfs_has_lazysbcount(mp));
+
+	tp = kmem_cache_zalloc(xfs_trans_cache, GFP_KERNEL | __GFP_NOFAIL);
+	if (!(flags & XFS_TRANS_NO_WRITECOUNT))
+		sb_start_intwrite(mp->m_super);
+	xfs_trans_set_context(tp);
+	tp->t_flags = flags;
+	tp->t_mountp = mp;
+	INIT_LIST_HEAD(&tp->t_items);
+	INIT_LIST_HEAD(&tp->t_busy);
+	INIT_LIST_HEAD(&tp->t_dfops);
+	tp->t_highest_agno = NULLAGNUMBER;
+	return tp;
+}
+
 int
 xfs_trans_alloc(
 	struct xfs_mount	*mp,
@@ -254,33 +276,16 @@ xfs_trans_alloc(
 	bool			want_retry = true;
 	int			error;
 
+	ASSERT(resp->tr_logres > 0);
+
 	/*
 	 * Allocate the handle before we do our freeze accounting and setting up
 	 * GFP_NOFS allocation context so that we avoid lockdep false positives
 	 * by doing GFP_KERNEL allocations inside sb_start_intwrite().
 	 */
 retry:
-	tp = kmem_cache_zalloc(xfs_trans_cache, GFP_KERNEL | __GFP_NOFAIL);
-	if (!(flags & XFS_TRANS_NO_WRITECOUNT))
-		sb_start_intwrite(mp->m_super);
-	xfs_trans_set_context(tp);
-
-	/*
-	 * Zero-reservation ("empty") transactions can't modify anything, so
-	 * they're allowed to run while we're frozen.
-	 */
-	WARN_ON(resp->tr_logres > 0 &&
-		mp->m_super->s_writers.frozen == SB_FREEZE_COMPLETE);
-	ASSERT(!(flags & XFS_TRANS_RES_FDBLKS) ||
-	       xfs_has_lazysbcount(mp));
-
-	tp->t_flags = flags;
-	tp->t_mountp = mp;
-	INIT_LIST_HEAD(&tp->t_items);
-	INIT_LIST_HEAD(&tp->t_busy);
-	INIT_LIST_HEAD(&tp->t_dfops);
-	tp->t_highest_agno = NULLAGNUMBER;
-
+	WARN_ON(mp->m_super->s_writers.frozen == SB_FREEZE_COMPLETE);
+	tp = __xfs_trans_alloc(mp, flags);
 	error = xfs_trans_reserve(tp, resp, blocks, rtextents);
 	if (error == -ENOSPC && want_retry) {
 		xfs_trans_cancel(tp);
@@ -329,9 +334,8 @@ xfs_trans_alloc_empty(
 	struct xfs_mount		*mp,
 	struct xfs_trans		**tpp)
 {
-	struct xfs_trans_res		resv = {0};
-
-	return xfs_trans_alloc(mp, &resv, 0, 0, XFS_TRANS_NO_WRITECOUNT, tpp);
+	*tpp = __xfs_trans_alloc(mp, XFS_TRANS_NO_WRITECOUNT);
+	return 0;
 }
 
 /*
-- 
2.47.2

Re: [PATCH 3/8] xfs: decouple xfs_trans_alloc_empty from xfs_trans_alloc

[Darrick J. Wong]

On Tue, Jul 15, 2025 at 02:25:36PM +0200, Christoph Hellwig wrote:
> xfs_trans_alloc_empty only shares the very basic transaction structure
> allocation and initialization with xfs_trans_alloc.
> 
> Split out a new __xfs_trans_alloc helper for that and otherwise decouple
> xfs_trans_alloc_empty from xfs_trans_alloc.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

That makes sense, especially because all the _empty callsites become so
much cleaner later on...

Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/xfs/xfs_trans.c | 52 +++++++++++++++++++++++++---------------------
>  1 file changed, 28 insertions(+), 24 deletions(-)
> 
> diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
> index 1fddea8d761a..e43f44f62c5f 100644
> --- a/fs/xfs/xfs_trans.c
> +++ b/fs/xfs/xfs_trans.c
> @@ -241,6 +241,28 @@ xfs_trans_reserve(
>  	return error;
>  }
>  
> +static struct xfs_trans *
> +__xfs_trans_alloc(
> +	struct xfs_mount	*mp,
> +	uint			flags)
> +{
> +	struct xfs_trans	*tp;
> +
> +	ASSERT(!(flags & XFS_TRANS_RES_FDBLKS) || xfs_has_lazysbcount(mp));
> +
> +	tp = kmem_cache_zalloc(xfs_trans_cache, GFP_KERNEL | __GFP_NOFAIL);
> +	if (!(flags & XFS_TRANS_NO_WRITECOUNT))
> +		sb_start_intwrite(mp->m_super);
> +	xfs_trans_set_context(tp);
> +	tp->t_flags = flags;
> +	tp->t_mountp = mp;
> +	INIT_LIST_HEAD(&tp->t_items);
> +	INIT_LIST_HEAD(&tp->t_busy);
> +	INIT_LIST_HEAD(&tp->t_dfops);
> +	tp->t_highest_agno = NULLAGNUMBER;
> +	return tp;
> +}
> +
>  int
>  xfs_trans_alloc(
>  	struct xfs_mount	*mp,
> @@ -254,33 +276,16 @@ xfs_trans_alloc(
>  	bool			want_retry = true;
>  	int			error;
>  
> +	ASSERT(resp->tr_logres > 0);
> +
>  	/*
>  	 * Allocate the handle before we do our freeze accounting and setting up
>  	 * GFP_NOFS allocation context so that we avoid lockdep false positives
>  	 * by doing GFP_KERNEL allocations inside sb_start_intwrite().
>  	 */
>  retry:
> -	tp = kmem_cache_zalloc(xfs_trans_cache, GFP_KERNEL | __GFP_NOFAIL);
> -	if (!(flags & XFS_TRANS_NO_WRITECOUNT))
> -		sb_start_intwrite(mp->m_super);
> -	xfs_trans_set_context(tp);
> -
> -	/*
> -	 * Zero-reservation ("empty") transactions can't modify anything, so
> -	 * they're allowed to run while we're frozen.
> -	 */
> -	WARN_ON(resp->tr_logres > 0 &&
> -		mp->m_super->s_writers.frozen == SB_FREEZE_COMPLETE);
> -	ASSERT(!(flags & XFS_TRANS_RES_FDBLKS) ||
> -	       xfs_has_lazysbcount(mp));
> -
> -	tp->t_flags = flags;
> -	tp->t_mountp = mp;
> -	INIT_LIST_HEAD(&tp->t_items);
> -	INIT_LIST_HEAD(&tp->t_busy);
> -	INIT_LIST_HEAD(&tp->t_dfops);
> -	tp->t_highest_agno = NULLAGNUMBER;
> -
> +	WARN_ON(mp->m_super->s_writers.frozen == SB_FREEZE_COMPLETE);
> +	tp = __xfs_trans_alloc(mp, flags);
>  	error = xfs_trans_reserve(tp, resp, blocks, rtextents);
>  	if (error == -ENOSPC && want_retry) {
>  		xfs_trans_cancel(tp);
> @@ -329,9 +334,8 @@ xfs_trans_alloc_empty(
>  	struct xfs_mount		*mp,
>  	struct xfs_trans		**tpp)
>  {
> -	struct xfs_trans_res		resv = {0};
> -
> -	return xfs_trans_alloc(mp, &resv, 0, 0, XFS_TRANS_NO_WRITECOUNT, tpp);
> +	*tpp = __xfs_trans_alloc(mp, XFS_TRANS_NO_WRITECOUNT);
> +	return 0;
>  }
>  
>  /*
> -- 
> 2.47.2
> 
> 

cleanup transaction allocation v2

[Christoph Hellwig]

Hi all,

this series cleans up the xfs_trans_alloc* and xfs_trans_reserve*
interfaces by keeping different code paths more separate and thus
removing redundant arguments and error returns.

A git tree is also available here:

    git://git.infradead.org/users/hch/xfs.git xfs-trans-cleanups

Gitweb:

    https://git.infradead.org/?p=users/hch/xfs.git;a=shortlog;h=refs/heads/xfs-trans-cleanups

Changes since v1:
 - drop the scrub handling for ->journal_info
 - clean up xfs_trans_roll a bit more
 - clean up xfs_trans_reserve_more a bit more
 - fix an potentially uninitialized error variable

Diffstat:
 libxfs/xfs_refcount.c |    4 
 scrub/common.c        |    7 -
 scrub/common.h        |    2 
 scrub/dir_repair.c    |    8 -
 scrub/fscounters.c    |    3 
 scrub/metapath.c      |    4 
 scrub/nlinks.c        |    8 -
 scrub/nlinks_repair.c |    4 
 scrub/parent_repair.c |   12 --
 scrub/quotacheck.c    |    4 
 scrub/repair.c        |   36 --------
 scrub/repair.h        |    4 
 scrub/rmap_repair.c   |   14 ---
 scrub/rtrmap_repair.c |   14 ---
 scrub/scrub.c         |    5 -
 xfs_attr_item.c       |    5 -
 xfs_discard.c         |   12 --
 xfs_fsmap.c           |    4 
 xfs_icache.c          |    5 -
 xfs_inode.c           |    7 -
 xfs_itable.c          |   18 ----
 xfs_iwalk.c           |   11 --
 xfs_log.c             |    6 -
 xfs_log_priv.h        |    4 
 xfs_notify_failure.c  |    5 -
 xfs_qm.c              |   10 --
 xfs_rtalloc.c         |   13 ---
 xfs_trans.c           |  208 +++++++++++++++++++++++---------------------------
 xfs_trans.h           |    3 
 xfs_zone_gc.c         |    5 -
 30 files changed, 151 insertions(+), 294 deletions(-)

[PATCH 1/8] xfs: use xfs_trans_reserve_more in xfs_trans_reserve_more_inode

[Christoph Hellwig]

Instead of duplicating the empty transacaction reservation
definition.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/xfs_trans.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
index b4a07af513ba..8b15bfe68774 100644
--- a/fs/xfs/xfs_trans.c
+++ b/fs/xfs/xfs_trans.c
@@ -1163,14 +1163,13 @@ xfs_trans_reserve_more_inode(
 	unsigned int		rblocks,
 	bool			force_quota)
 {
-	struct xfs_trans_res	resv = { };
 	struct xfs_mount	*mp = ip->i_mount;
 	unsigned int		rtx = xfs_extlen_to_rtxlen(mp, rblocks);
 	int			error;
 
 	xfs_assert_ilocked(ip, XFS_ILOCK_EXCL);
 
-	error = xfs_trans_reserve(tp, &resv, dblocks, rtx);
+	error = xfs_trans_reserve_more(tp, dblocks, rtx);
 	if (error)
 		return error;
 
-- 
2.47.2

[PATCH 2/8] xfs: don't use xfs_trans_reserve in xfs_trans_reserve_more

[Christoph Hellwig]

xfs_trans_reserve_more just tries to allocate additional blocks and/or
rtextents and is otherwise unrelated to the transaction reservation
logic.  Open code the block and rtextent reservation in
xfs_trans_reserve_more to prepare for simplifying xfs_trans_reserve.

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

diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
index 8b15bfe68774..2213cb2278d2 100644
--- a/fs/xfs/xfs_trans.c
+++ b/fs/xfs/xfs_trans.c
@@ -1146,9 +1146,18 @@ xfs_trans_reserve_more(
 	unsigned int		blocks,
 	unsigned int		rtextents)
 {
-	struct xfs_trans_res	resv = { };
-
-	return xfs_trans_reserve(tp, &resv, blocks, rtextents);
+	bool			rsvd = tp->t_flags & XFS_TRANS_RESERVE;
+
+	if (blocks && xfs_dec_fdblocks(tp->t_mountp, blocks, rsvd))
+		return -ENOSPC;
+	if (rtextents && xfs_dec_frextents(tp->t_mountp, rtextents)) {
+		if (blocks)
+			xfs_add_fdblocks(tp->t_mountp, blocks);
+		return -ENOSPC;
+	}
+	tp->t_blk_res += blocks;
+	tp->t_rtx_res += rtextents;
+	return 0;
 }
 
 /*
-- 
2.47.2

[PATCH 3/8] xfs: decouple xfs_trans_alloc_empty from xfs_trans_alloc

[Christoph Hellwig]

xfs_trans_alloc_empty only shares the very basic transaction structure
allocation and initialization with xfs_trans_alloc.

Split out a new __xfs_trans_alloc helper for that and otherwise decouple
xfs_trans_alloc_empty from xfs_trans_alloc.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/xfs_trans.c | 52 +++++++++++++++++++++++++---------------------
 1 file changed, 28 insertions(+), 24 deletions(-)

diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
index 2213cb2278d2..e5edb89909ed 100644
--- a/fs/xfs/xfs_trans.c
+++ b/fs/xfs/xfs_trans.c
@@ -241,6 +241,28 @@ xfs_trans_reserve(
 	return error;
 }
 
+static struct xfs_trans *
+__xfs_trans_alloc(
+	struct xfs_mount	*mp,
+	uint			flags)
+{
+	struct xfs_trans	*tp;
+
+	ASSERT(!(flags & XFS_TRANS_RES_FDBLKS) || xfs_has_lazysbcount(mp));
+
+	tp = kmem_cache_zalloc(xfs_trans_cache, GFP_KERNEL | __GFP_NOFAIL);
+	if (!(flags & XFS_TRANS_NO_WRITECOUNT))
+		sb_start_intwrite(mp->m_super);
+	xfs_trans_set_context(tp);
+	tp->t_flags = flags;
+	tp->t_mountp = mp;
+	INIT_LIST_HEAD(&tp->t_items);
+	INIT_LIST_HEAD(&tp->t_busy);
+	INIT_LIST_HEAD(&tp->t_dfops);
+	tp->t_highest_agno = NULLAGNUMBER;
+	return tp;
+}
+
 int
 xfs_trans_alloc(
 	struct xfs_mount	*mp,
@@ -254,33 +276,16 @@ xfs_trans_alloc(
 	bool			want_retry = true;
 	int			error;
 
+	ASSERT(resp->tr_logres > 0);
+
 	/*
 	 * Allocate the handle before we do our freeze accounting and setting up
 	 * GFP_NOFS allocation context so that we avoid lockdep false positives
 	 * by doing GFP_KERNEL allocations inside sb_start_intwrite().
 	 */
 retry:
-	tp = kmem_cache_zalloc(xfs_trans_cache, GFP_KERNEL | __GFP_NOFAIL);
-	if (!(flags & XFS_TRANS_NO_WRITECOUNT))
-		sb_start_intwrite(mp->m_super);
-	xfs_trans_set_context(tp);
-
-	/*
-	 * Zero-reservation ("empty") transactions can't modify anything, so
-	 * they're allowed to run while we're frozen.
-	 */
-	WARN_ON(resp->tr_logres > 0 &&
-		mp->m_super->s_writers.frozen == SB_FREEZE_COMPLETE);
-	ASSERT(!(flags & XFS_TRANS_RES_FDBLKS) ||
-	       xfs_has_lazysbcount(mp));
-
-	tp->t_flags = flags;
-	tp->t_mountp = mp;
-	INIT_LIST_HEAD(&tp->t_items);
-	INIT_LIST_HEAD(&tp->t_busy);
-	INIT_LIST_HEAD(&tp->t_dfops);
-	tp->t_highest_agno = NULLAGNUMBER;
-
+	WARN_ON(mp->m_super->s_writers.frozen == SB_FREEZE_COMPLETE);
+	tp = __xfs_trans_alloc(mp, flags);
 	error = xfs_trans_reserve(tp, resp, blocks, rtextents);
 	if (error == -ENOSPC && want_retry) {
 		xfs_trans_cancel(tp);
@@ -329,9 +334,8 @@ xfs_trans_alloc_empty(
 	struct xfs_mount		*mp,
 	struct xfs_trans		**tpp)
 {
-	struct xfs_trans_res		resv = {0};
-
-	return xfs_trans_alloc(mp, &resv, 0, 0, XFS_TRANS_NO_WRITECOUNT, tpp);
+	*tpp = __xfs_trans_alloc(mp, XFS_TRANS_NO_WRITECOUNT);
+	return 0;
 }
 
 /*
-- 
2.47.2

[PATCH 4/8] xfs: don't use xfs_trans_reserve in xfs_trans_roll

[Christoph Hellwig]

xfs_trans_roll uses xfs_trans_reserve to basically just call into
xfs_log_regrant while bypassing the reset of xfs_trans_reserve.

Open code the call to xfs_log_regrant in xfs_trans_roll and simplify
xfs_trans_reserve now that it never regrants and always asks for a log
reservation.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_trans.c | 133 ++++++++++++++++++---------------------------
 1 file changed, 54 insertions(+), 79 deletions(-)

diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
index e5edb89909ed..8d0aee747f72 100644
--- a/fs/xfs/xfs_trans.c
+++ b/fs/xfs/xfs_trans.c
@@ -134,18 +134,14 @@ xfs_trans_dup(
 }
 
 /*
- * 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 is called to reserve free disk blocks and log space for the given
+ * transaction 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.
+ * This does not do quota reservations. That typically is done by the caller
+ * afterwards.
  */
 static int
 xfs_trans_reserve(
@@ -158,10 +154,12 @@ xfs_trans_reserve(
 	int			error = 0;
 	bool			rsvd = (tp->t_flags & XFS_TRANS_RESERVE) != 0;
 
+	ASSERT(resp->tr_logres > 0);
+
 	/*
-	 * Attempt to reserve the needed disk blocks by decrementing
-	 * the number needed from the number available.  This will
-	 * fail if the count would go below zero.
+	 * Attempt to reserve the needed disk blocks by decrementing the number
+	 * needed from the number available.  This will fail if the count would
+	 * go below zero.
 	 */
 	if (blocks > 0) {
 		error = xfs_dec_fdblocks(mp, blocks, rsvd);
@@ -173,42 +171,20 @@ xfs_trans_reserve(
 	/*
 	 * Reserve the log space needed for this transaction.
 	 */
-	if (resp->tr_logres > 0) {
-		bool	permanent = false;
-
-		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;
-			permanent = true;
-		} else {
-			ASSERT(tp->t_ticket == NULL);
-			ASSERT(!(tp->t_flags & XFS_TRANS_PERM_LOG_RES));
-		}
-
-		if (tp->t_ticket != NULL) {
-			ASSERT(resp->tr_logflags & XFS_TRANS_PERM_LOG_RES);
-			error = xfs_log_regrant(mp, tp->t_ticket);
-		} else {
-			error = xfs_log_reserve(mp, resp->tr_logres,
-						resp->tr_logcount,
-						&tp->t_ticket, permanent);
-		}
-
-		if (error)
-			goto undo_blocks;
+	if (resp->tr_logflags & XFS_TRANS_PERM_LOG_RES)
+		tp->t_flags |= XFS_TRANS_PERM_LOG_RES;
+	error = xfs_log_reserve(mp, resp->tr_logres, resp->tr_logcount,
+			&tp->t_ticket, (tp->t_flags & XFS_TRANS_PERM_LOG_RES));
+	if (error)
+		goto undo_blocks;
 
-		tp->t_log_res = resp->tr_logres;
-		tp->t_log_count = resp->tr_logcount;
-	}
+	tp->t_log_res = resp->tr_logres;
+	tp->t_log_count = resp->tr_logcount;
 
 	/*
-	 * 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.
+	 * 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) {
 		error = xfs_dec_frextents(mp, rtextents);
@@ -221,18 +197,11 @@ xfs_trans_reserve(
 
 	return 0;
 
-	/*
-	 * Error cases jump to one of these labels to undo any
-	 * reservations which have already been performed.
-	 */
 undo_log:
-	if (resp->tr_logres > 0) {
-		xfs_log_ticket_ungrant(mp->m_log, tp->t_ticket);
-		tp->t_ticket = NULL;
-		tp->t_log_res = 0;
-		tp->t_flags &= ~XFS_TRANS_PERM_LOG_RES;
-	}
-
+	xfs_log_ticket_ungrant(mp->m_log, tp->t_ticket);
+	tp->t_ticket = NULL;
+	tp->t_log_res = 0;
+	tp->t_flags &= ~XFS_TRANS_PERM_LOG_RES;
 undo_blocks:
 	if (blocks > 0) {
 		xfs_add_fdblocks(mp, blocks);
@@ -1030,51 +999,57 @@ xfs_trans_cancel(
 }
 
 /*
- * Roll from one trans in the sequence of PERMANENT transactions to
- * the next: permanent transactions are only flushed out when
- * committed with xfs_trans_commit(), but we still want as soon
- * as possible to let chunks of it go to the log. So we commit the
- * chunk we've been working on and get a new transaction to continue.
+ * Roll from one trans in the sequence of PERMANENT transactions to the next:
+ * permanent transactions are only flushed out when committed with
+ * xfs_trans_commit(), but we still want as soon as possible to let chunks of it
+ * go to the log.  So we commit the chunk we've been working on and get a new
+ * transaction to continue.
  */
 int
 xfs_trans_roll(
 	struct xfs_trans	**tpp)
 {
-	struct xfs_trans	*trans = *tpp;
-	struct xfs_trans_res	tres;
+	struct xfs_trans	*tp = *tpp;
+	unsigned int		log_res = tp->t_log_res;
+	unsigned int		log_count = tp->t_log_count;
 	int			error;
 
-	trace_xfs_trans_roll(trans, _RET_IP_);
+	trace_xfs_trans_roll(tp, _RET_IP_);
+
+	ASSERT(log_res > 0);
 
 	/*
 	 * Copy the critical parameters from one trans to the next.
 	 */
-	tres.tr_logres = trans->t_log_res;
-	tres.tr_logcount = trans->t_log_count;
-
-	*tpp = xfs_trans_dup(trans);
+	*tpp = xfs_trans_dup(tp);
 
 	/*
 	 * Commit the current transaction.
-	 * If this commit failed, then it'd just unlock those items that
-	 * are not marked ihold. That also means that a filesystem shutdown
-	 * is in progress. The caller takes the responsibility to cancel
-	 * the duplicate transaction that gets returned.
+	 *
+	 * If this commit failed, then it'd just unlock those items that are not
+	 * marked ihold. That also means that a filesystem shutdown is in
+	 * progress.  The caller takes the responsibility to cancel the
+	 * duplicate transaction that gets returned.
 	 */
-	error = __xfs_trans_commit(trans, true);
+	error = __xfs_trans_commit(tp, true);
 	if (error)
 		return error;
 
 	/*
 	 * Reserve space in the log for the next transaction.
-	 * This also pushes items in the "AIL", the list of logged items,
-	 * out to disk if they are taking up space at the tail of the log
-	 * that we want to use.  This requires that either nothing be locked
-	 * across this call, or that anything that is locked be logged in
-	 * the prior and the next transactions.
+	 *
+	 * This also pushes items in the AIL out to disk if they are taking up
+	 * space at the tail of the log that we want to use.  This requires that
+	 * either nothing be locked across this call, or that anything that is
+	 * locked be logged in the prior and the next transactions.
 	 */
-	tres.tr_logflags = XFS_TRANS_PERM_LOG_RES;
-	return xfs_trans_reserve(*tpp, &tres, 0, 0);
+	tp = *tpp;
+	error = xfs_log_regrant(tp->t_mountp, tp->t_ticket);
+	if (error)
+		return error;
+	tp->t_log_res = log_res;
+	tp->t_log_count = log_count;
+	return 0;
 }
 
 /*
-- 
2.47.2

[PATCH 5/8] xfs: return the allocated transaction from xfs_trans_alloc_empty

[Christoph Hellwig]

xfs_trans_alloc_empty can't return errors, so return the allocated
transaction directly instead of an output double pointer argument.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/libxfs/xfs_refcount.c |  4 +---
 fs/xfs/scrub/common.c        |  3 ++-
 fs/xfs/scrub/repair.c        | 12 ++----------
 fs/xfs/scrub/scrub.c         |  5 +----
 fs/xfs/xfs_attr_item.c       |  5 +----
 fs/xfs/xfs_discard.c         | 12 +++---------
 fs/xfs/xfs_fsmap.c           |  4 +---
 fs/xfs/xfs_icache.c          |  5 +----
 fs/xfs/xfs_inode.c           |  7 ++-----
 fs/xfs/xfs_itable.c          | 18 +++---------------
 fs/xfs/xfs_iwalk.c           | 11 +++--------
 fs/xfs/xfs_notify_failure.c  |  5 +----
 fs/xfs/xfs_qm.c              | 10 ++--------
 fs/xfs/xfs_rtalloc.c         | 13 +++----------
 fs/xfs/xfs_trans.c           |  8 +++-----
 fs/xfs/xfs_trans.h           |  3 +--
 fs/xfs/xfs_zone_gc.c         |  5 +----
 17 files changed, 31 insertions(+), 99 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_refcount.c b/fs/xfs/libxfs/xfs_refcount.c
index cebe83f7842a..897784037483 100644
--- a/fs/xfs/libxfs/xfs_refcount.c
+++ b/fs/xfs/libxfs/xfs_refcount.c
@@ -2099,9 +2099,7 @@ xfs_refcount_recover_cow_leftovers(
 	 * recording the CoW debris we cancel the (empty) transaction
 	 * and everything goes away cleanly.
 	 */
-	error = xfs_trans_alloc_empty(mp, &tp);
-	if (error)
-		return error;
+	tp = xfs_trans_alloc_empty(mp);
 
 	if (isrt) {
 		xfs_rtgroup_lock(to_rtg(xg), XFS_RTGLOCK_REFCOUNT);
diff --git a/fs/xfs/scrub/common.c b/fs/xfs/scrub/common.c
index 28ad341df8ee..d080f4e6e9d8 100644
--- a/fs/xfs/scrub/common.c
+++ b/fs/xfs/scrub/common.c
@@ -870,7 +870,8 @@ int
 xchk_trans_alloc_empty(
 	struct xfs_scrub	*sc)
 {
-	return xfs_trans_alloc_empty(sc->mp, &sc->tp);
+	sc->tp = xfs_trans_alloc_empty(sc->mp);
+	return 0;
 }
 
 /*
diff --git a/fs/xfs/scrub/repair.c b/fs/xfs/scrub/repair.c
index f8f9ed30f56b..f7f80ff32afc 100644
--- a/fs/xfs/scrub/repair.c
+++ b/fs/xfs/scrub/repair.c
@@ -1279,18 +1279,10 @@ xrep_trans_alloc_hook_dummy(
 	void			**cookiep,
 	struct xfs_trans	**tpp)
 {
-	int			error;
-
 	*cookiep = current->journal_info;
 	current->journal_info = NULL;
-
-	error = xfs_trans_alloc_empty(mp, tpp);
-	if (!error)
-		return 0;
-
-	current->journal_info = *cookiep;
-	*cookiep = NULL;
-	return error;
+	*tpp = xfs_trans_alloc_empty(mp);
+	return 0;
 }
 
 /* Cancel a dummy transaction used by a live update hook function. */
diff --git a/fs/xfs/scrub/scrub.c b/fs/xfs/scrub/scrub.c
index 76e24032e99a..3c3b0d25006f 100644
--- a/fs/xfs/scrub/scrub.c
+++ b/fs/xfs/scrub/scrub.c
@@ -876,10 +876,7 @@ xchk_scrubv_open_by_handle(
 	struct xfs_inode		*ip;
 	int				error;
 
-	error = xfs_trans_alloc_empty(mp, &tp);
-	if (error)
-		return NULL;
-
+	tp = xfs_trans_alloc_empty(mp);
 	error = xfs_iget(mp, tp, head->svh_ino, XCHK_IGET_FLAGS, 0, &ip);
 	xfs_trans_cancel(tp);
 	if (error)
diff --git a/fs/xfs/xfs_attr_item.c b/fs/xfs/xfs_attr_item.c
index f683b7a9323f..da1e11f38eb0 100644
--- a/fs/xfs/xfs_attr_item.c
+++ b/fs/xfs/xfs_attr_item.c
@@ -616,10 +616,7 @@ xfs_attri_iread_extents(
 	struct xfs_trans		*tp;
 	int				error;
 
-	error = xfs_trans_alloc_empty(ip->i_mount, &tp);
-	if (error)
-		return error;
-
+	tp = xfs_trans_alloc_empty(ip->i_mount);
 	xfs_ilock(ip, XFS_ILOCK_EXCL);
 	error = xfs_iread_extents(tp, ip, XFS_ATTR_FORK);
 	xfs_iunlock(ip, XFS_ILOCK_EXCL);
diff --git a/fs/xfs/xfs_discard.c b/fs/xfs/xfs_discard.c
index 603d51365645..ee49f20875af 100644
--- a/fs/xfs/xfs_discard.c
+++ b/fs/xfs/xfs_discard.c
@@ -189,9 +189,7 @@ xfs_trim_gather_extents(
 	 */
 	xfs_log_force(mp, XFS_LOG_SYNC);
 
-	error = xfs_trans_alloc_empty(mp, &tp);
-	if (error)
-		return error;
+	tp = xfs_trans_alloc_empty(mp);
 
 	error = xfs_alloc_read_agf(pag, tp, 0, &agbp);
 	if (error)
@@ -583,9 +581,7 @@ xfs_trim_rtextents(
 	struct xfs_trans	*tp;
 	int			error;
 
-	error = xfs_trans_alloc_empty(mp, &tp);
-	if (error)
-		return error;
+	tp = xfs_trans_alloc_empty(mp);
 
 	/*
 	 * Walk the free ranges between low and high.  The query_range function
@@ -701,9 +697,7 @@ xfs_trim_rtgroup_extents(
 	struct xfs_trans	*tp;
 	int			error;
 
-	error = xfs_trans_alloc_empty(mp, &tp);
-	if (error)
-		return error;
+	tp = xfs_trans_alloc_empty(mp);
 
 	/*
 	 * Walk the free ranges between low and high.  The query_range function
diff --git a/fs/xfs/xfs_fsmap.c b/fs/xfs/xfs_fsmap.c
index 414b27a86458..af68c7de8ee8 100644
--- a/fs/xfs/xfs_fsmap.c
+++ b/fs/xfs/xfs_fsmap.c
@@ -1270,9 +1270,7 @@ xfs_getfsmap(
 		 * buffer locking abilities to detect cycles in the rmapbt
 		 * without deadlocking.
 		 */
-		error = xfs_trans_alloc_empty(mp, &tp);
-		if (error)
-			break;
+		tp = xfs_trans_alloc_empty(mp);
 
 		info.dev = handlers[i].dev;
 		info.last = false;
diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
index bbc2f2973dcc..4cf7abe50143 100644
--- a/fs/xfs/xfs_icache.c
+++ b/fs/xfs/xfs_icache.c
@@ -893,10 +893,7 @@ xfs_metafile_iget(
 	struct xfs_trans	*tp;
 	int			error;
 
-	error = xfs_trans_alloc_empty(mp, &tp);
-	if (error)
-		return error;
-
+	tp = xfs_trans_alloc_empty(mp);
 	error = xfs_trans_metafile_iget(tp, ino, metafile_type, ipp);
 	xfs_trans_cancel(tp);
 	return error;
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 761a996a857c..9c39251961a3 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -2932,12 +2932,9 @@ xfs_inode_reload_unlinked(
 	struct xfs_inode	*ip)
 {
 	struct xfs_trans	*tp;
-	int			error;
-
-	error = xfs_trans_alloc_empty(ip->i_mount, &tp);
-	if (error)
-		return error;
+	int			error = 0;
 
+	tp = xfs_trans_alloc_empty(ip->i_mount);
 	xfs_ilock(ip, XFS_ILOCK_SHARED);
 	if (xfs_inode_unlinked_incomplete(ip))
 		error = xfs_inode_reload_unlinked_bucket(tp, ip);
diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
index 1fa1c0564b0c..c8c9b8d8309f 100644
--- a/fs/xfs/xfs_itable.c
+++ b/fs/xfs/xfs_itable.c
@@ -239,14 +239,10 @@ xfs_bulkstat_one(
 	 * Grab an empty transaction so that we can use its recursive buffer
 	 * locking abilities to detect cycles in the inobt without deadlocking.
 	 */
-	error = xfs_trans_alloc_empty(breq->mp, &tp);
-	if (error)
-		goto out;
-
+	tp = xfs_trans_alloc_empty(breq->mp);
 	error = xfs_bulkstat_one_int(breq->mp, breq->idmap, tp,
 			breq->startino, &bc);
 	xfs_trans_cancel(tp);
-out:
 	kfree(bc.buf);
 
 	/*
@@ -331,17 +327,13 @@ xfs_bulkstat(
 	 * Grab an empty transaction so that we can use its recursive buffer
 	 * locking abilities to detect cycles in the inobt without deadlocking.
 	 */
-	error = xfs_trans_alloc_empty(breq->mp, &tp);
-	if (error)
-		goto out;
-
+	tp = xfs_trans_alloc_empty(breq->mp);
 	if (breq->flags & XFS_IBULK_SAME_AG)
 		iwalk_flags |= XFS_IWALK_SAME_AG;
 
 	error = xfs_iwalk(breq->mp, tp, breq->startino, iwalk_flags,
 			xfs_bulkstat_iwalk, breq->icount, &bc);
 	xfs_trans_cancel(tp);
-out:
 	kfree(bc.buf);
 
 	/*
@@ -464,14 +456,10 @@ xfs_inumbers(
 	 * Grab an empty transaction so that we can use its recursive buffer
 	 * locking abilities to detect cycles in the inobt without deadlocking.
 	 */
-	error = xfs_trans_alloc_empty(breq->mp, &tp);
-	if (error)
-		goto out;
-
+	tp = xfs_trans_alloc_empty(breq->mp);
 	error = xfs_inobt_walk(breq->mp, tp, breq->startino, breq->flags,
 			xfs_inumbers_walk, breq->icount, &ic);
 	xfs_trans_cancel(tp);
-out:
 
 	/*
 	 * We found some inode groups, so clear the error status and return
diff --git a/fs/xfs/xfs_iwalk.c b/fs/xfs/xfs_iwalk.c
index 7db3ece370b1..c1c31d1a8e21 100644
--- a/fs/xfs/xfs_iwalk.c
+++ b/fs/xfs/xfs_iwalk.c
@@ -377,11 +377,8 @@ xfs_iwalk_run_callbacks(
 	if (!has_more)
 		return 0;
 
-	if (iwag->drop_trans) {
-		error = xfs_trans_alloc_empty(mp, &iwag->tp);
-		if (error)
-			return error;
-	}
+	if (iwag->drop_trans)
+		iwag->tp = xfs_trans_alloc_empty(mp);
 
 	/* ...and recreate the cursor just past where we left off. */
 	error = xfs_ialloc_read_agi(iwag->pag, iwag->tp, 0, agi_bpp);
@@ -617,9 +614,7 @@ xfs_iwalk_ag_work(
 	 * Grab an empty transaction so that we can use its recursive buffer
 	 * locking abilities to detect cycles in the inobt without deadlocking.
 	 */
-	error = xfs_trans_alloc_empty(mp, &iwag->tp);
-	if (error)
-		goto out;
+	iwag->tp = xfs_trans_alloc_empty(mp);
 	iwag->drop_trans = 1;
 
 	error = xfs_iwalk_ag(iwag);
diff --git a/fs/xfs/xfs_notify_failure.c b/fs/xfs/xfs_notify_failure.c
index 42e9c72b85c0..fbd521f89874 100644
--- a/fs/xfs/xfs_notify_failure.c
+++ b/fs/xfs/xfs_notify_failure.c
@@ -279,10 +279,7 @@ xfs_dax_notify_dev_failure(
 		kernel_frozen = xfs_dax_notify_failure_freeze(mp) == 0;
 	}
 
-	error = xfs_trans_alloc_empty(mp, &tp);
-	if (error)
-		goto out;
-
+	tp = xfs_trans_alloc_empty(mp);
 	start_gno = xfs_fsb_to_gno(mp, start_bno, type);
 	end_gno = xfs_fsb_to_gno(mp, end_bno, type);
 	while ((xg = xfs_group_next_range(mp, xg, start_gno, end_gno, type))) {
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index fa135ac26471..23ba84ec919a 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -660,10 +660,7 @@ xfs_qm_load_metadir_qinos(
 	struct xfs_trans	*tp;
 	int			error;
 
-	error = xfs_trans_alloc_empty(mp, &tp);
-	if (error)
-		return error;
-
+	tp = xfs_trans_alloc_empty(mp);
 	error = xfs_dqinode_load_parent(tp, &qi->qi_dirip);
 	if (error == -ENOENT) {
 		/* no quota dir directory, but we'll create one later */
@@ -1755,10 +1752,7 @@ xfs_qm_qino_load(
 	struct xfs_inode	*dp = NULL;
 	int			error;
 
-	error = xfs_trans_alloc_empty(mp, &tp);
-	if (error)
-		return error;
-
+	tp = xfs_trans_alloc_empty(mp);
 	if (xfs_has_metadir(mp)) {
 		error = xfs_dqinode_load_parent(tp, &dp);
 		if (error)
diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
index 736eb0924573..6907e871fa15 100644
--- a/fs/xfs/xfs_rtalloc.c
+++ b/fs/xfs/xfs_rtalloc.c
@@ -729,9 +729,7 @@ xfs_rtginode_ensure(
 	if (rtg->rtg_inodes[type])
 		return 0;
 
-	error = xfs_trans_alloc_empty(rtg_mount(rtg), &tp);
-	if (error)
-		return error;
+	tp = xfs_trans_alloc_empty(rtg_mount(rtg));
 	error = xfs_rtginode_load(rtg, type, tp);
 	xfs_trans_cancel(tp);
 
@@ -1305,9 +1303,7 @@ xfs_growfs_rt_prep_groups(
 	if (!mp->m_rtdirip) {
 		struct xfs_trans	*tp;
 
-		error = xfs_trans_alloc_empty(mp, &tp);
-		if (error)
-			return error;
+		tp = xfs_trans_alloc_empty(mp);
 		error = xfs_rtginode_load_parent(tp);
 		xfs_trans_cancel(tp);
 
@@ -1674,10 +1670,7 @@ xfs_rtmount_inodes(
 	struct xfs_rtgroup	*rtg = NULL;
 	int			error;
 
-	error = xfs_trans_alloc_empty(mp, &tp);
-	if (error)
-		return error;
-
+	tp = xfs_trans_alloc_empty(mp);
 	if (xfs_has_rtgroups(mp) && mp->m_sb.sb_rgcount > 0) {
 		error = xfs_rtginode_load_parent(tp);
 		if (error)
diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
index 8d0aee747f72..ece374d622b3 100644
--- a/fs/xfs/xfs_trans.c
+++ b/fs/xfs/xfs_trans.c
@@ -298,13 +298,11 @@ xfs_trans_alloc(
  * where we can be grabbing buffers at the same time that freeze is trying to
  * drain the buffer LRU list.
  */
-int
+struct xfs_trans *
 xfs_trans_alloc_empty(
-	struct xfs_mount		*mp,
-	struct xfs_trans		**tpp)
+	struct xfs_mount		*mp)
 {
-	*tpp = __xfs_trans_alloc(mp, XFS_TRANS_NO_WRITECOUNT);
-	return 0;
+	return __xfs_trans_alloc(mp, XFS_TRANS_NO_WRITECOUNT);
 }
 
 /*
diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h
index 2b366851e9a4..a6b10aaeb1f1 100644
--- a/fs/xfs/xfs_trans.h
+++ b/fs/xfs/xfs_trans.h
@@ -168,8 +168,7 @@ int		xfs_trans_alloc(struct xfs_mount *mp, struct xfs_trans_res *resp,
 			struct xfs_trans **tpp);
 int		xfs_trans_reserve_more(struct xfs_trans *tp,
 			unsigned int blocks, unsigned int rtextents);
-int		xfs_trans_alloc_empty(struct xfs_mount *mp,
-			struct xfs_trans **tpp);
+struct xfs_trans *xfs_trans_alloc_empty(struct xfs_mount *mp);
 void		xfs_trans_mod_sb(xfs_trans_t *, uint, int64_t);
 
 int xfs_trans_get_buf_map(struct xfs_trans *tp, struct xfs_buftarg *target,
diff --git a/fs/xfs/xfs_zone_gc.c b/fs/xfs/xfs_zone_gc.c
index 9c00fc5baa30..e1954b0e6021 100644
--- a/fs/xfs/xfs_zone_gc.c
+++ b/fs/xfs/xfs_zone_gc.c
@@ -328,10 +328,7 @@ xfs_zone_gc_query(
 	iter->rec_idx = 0;
 	iter->rec_count = 0;
 
-	error = xfs_trans_alloc_empty(mp, &tp);
-	if (error)
-		return error;
-
+	tp = xfs_trans_alloc_empty(mp);
 	xfs_rtgroup_lock(rtg, XFS_RTGLOCK_RMAP);
 	cur = xfs_rtrmapbt_init_cursor(tp, rtg);
 	error = xfs_rmap_query_range(cur, &ri_low, &ri_high,
-- 
2.47.2

[PATCH 6/8] xfs: return the allocated transaction from xchk_trans_alloc_empty

[Christoph Hellwig]

xchk_trans_alloc_empty can't return errors, so return the allocated
transaction directly instead of an output double pointer argument.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/scrub/common.c        |  6 +++---
 fs/xfs/scrub/common.h        |  2 +-
 fs/xfs/scrub/dir_repair.c    |  8 ++------
 fs/xfs/scrub/fscounters.c    |  3 ++-
 fs/xfs/scrub/metapath.c      |  4 +---
 fs/xfs/scrub/nlinks.c        |  8 ++------
 fs/xfs/scrub/nlinks_repair.c |  4 +---
 fs/xfs/scrub/parent_repair.c | 12 +++---------
 fs/xfs/scrub/quotacheck.c    |  4 +---
 fs/xfs/scrub/rmap_repair.c   |  4 +---
 fs/xfs/scrub/rtrmap_repair.c |  4 +---
 11 files changed, 18 insertions(+), 41 deletions(-)

diff --git a/fs/xfs/scrub/common.c b/fs/xfs/scrub/common.c
index d080f4e6e9d8..2ef7742be7d3 100644
--- a/fs/xfs/scrub/common.c
+++ b/fs/xfs/scrub/common.c
@@ -866,12 +866,11 @@ xchk_trans_cancel(
 	sc->tp = NULL;
 }
 
-int
+void
 xchk_trans_alloc_empty(
 	struct xfs_scrub	*sc)
 {
 	sc->tp = xfs_trans_alloc_empty(sc->mp);
-	return 0;
 }
 
 /*
@@ -893,7 +892,8 @@ xchk_trans_alloc(
 		return xfs_trans_alloc(sc->mp, &M_RES(sc->mp)->tr_itruncate,
 				resblks, 0, 0, &sc->tp);
 
-	return xchk_trans_alloc_empty(sc);
+	xchk_trans_alloc_empty(sc);
+	return 0;
 }
 
 /* Set us up with a transaction and an empty context. */
diff --git a/fs/xfs/scrub/common.h b/fs/xfs/scrub/common.h
index 19877d99f255..ddbc065c798c 100644
--- a/fs/xfs/scrub/common.h
+++ b/fs/xfs/scrub/common.h
@@ -7,7 +7,7 @@
 #define __XFS_SCRUB_COMMON_H__
 
 int xchk_trans_alloc(struct xfs_scrub *sc, uint resblks);
-int xchk_trans_alloc_empty(struct xfs_scrub *sc);
+void xchk_trans_alloc_empty(struct xfs_scrub *sc);
 void xchk_trans_cancel(struct xfs_scrub *sc);
 
 bool xchk_process_error(struct xfs_scrub *sc, xfs_agnumber_t agno,
diff --git a/fs/xfs/scrub/dir_repair.c b/fs/xfs/scrub/dir_repair.c
index 249313882108..8d3b550990b5 100644
--- a/fs/xfs/scrub/dir_repair.c
+++ b/fs/xfs/scrub/dir_repair.c
@@ -1289,9 +1289,7 @@ xrep_dir_scan_dirtree(
 	if (sc->ilock_flags & (XFS_ILOCK_SHARED | XFS_ILOCK_EXCL))
 		xchk_iunlock(sc, sc->ilock_flags & (XFS_ILOCK_SHARED |
 						    XFS_ILOCK_EXCL));
-	error = xchk_trans_alloc_empty(sc);
-	if (error)
-		return error;
+	xchk_trans_alloc_empty(sc);
 
 	while ((error = xchk_iscan_iter(&rd->pscan.iscan, &ip)) == 1) {
 		bool		flush;
@@ -1317,9 +1315,7 @@ xrep_dir_scan_dirtree(
 			if (error)
 				break;
 
-			error = xchk_trans_alloc_empty(sc);
-			if (error)
-				break;
+			xchk_trans_alloc_empty(sc);
 		}
 
 		if (xchk_should_terminate(sc, &error))
diff --git a/fs/xfs/scrub/fscounters.c b/fs/xfs/scrub/fscounters.c
index 9b598c5790ad..cebd0d526926 100644
--- a/fs/xfs/scrub/fscounters.c
+++ b/fs/xfs/scrub/fscounters.c
@@ -237,7 +237,8 @@ xchk_setup_fscounters(
 			return error;
 	}
 
-	return xchk_trans_alloc_empty(sc);
+	xchk_trans_alloc_empty(sc);
+	return 0;
 }
 
 /*
diff --git a/fs/xfs/scrub/metapath.c b/fs/xfs/scrub/metapath.c
index e21c16fbd15d..14939d7de349 100644
--- a/fs/xfs/scrub/metapath.c
+++ b/fs/xfs/scrub/metapath.c
@@ -318,9 +318,7 @@ xchk_metapath(
 		return 0;
 	}
 
-	error = xchk_trans_alloc_empty(sc);
-	if (error)
-		return error;
+	xchk_trans_alloc_empty(sc);
 
 	error = xchk_metapath_ilock_both(mpath);
 	if (error)
diff --git a/fs/xfs/scrub/nlinks.c b/fs/xfs/scrub/nlinks.c
index 4a47d0aabf73..26721fab5cab 100644
--- a/fs/xfs/scrub/nlinks.c
+++ b/fs/xfs/scrub/nlinks.c
@@ -555,9 +555,7 @@ xchk_nlinks_collect(
 	 * do not take sb_internal.
 	 */
 	xchk_trans_cancel(sc);
-	error = xchk_trans_alloc_empty(sc);
-	if (error)
-		return error;
+	xchk_trans_alloc_empty(sc);
 
 	while ((error = xchk_iscan_iter(&xnc->collect_iscan, &ip)) == 1) {
 		if (S_ISDIR(VFS_I(ip)->i_mode))
@@ -880,9 +878,7 @@ xchk_nlinks_compare(
 	 * inactivation workqueue.
 	 */
 	xchk_trans_cancel(sc);
-	error = xchk_trans_alloc_empty(sc);
-	if (error)
-		return error;
+	xchk_trans_alloc_empty(sc);
 
 	/*
 	 * Use the inobt to walk all allocated inodes to compare the link
diff --git a/fs/xfs/scrub/nlinks_repair.c b/fs/xfs/scrub/nlinks_repair.c
index 4ebdee095428..6ef2ee9c3814 100644
--- a/fs/xfs/scrub/nlinks_repair.c
+++ b/fs/xfs/scrub/nlinks_repair.c
@@ -340,9 +340,7 @@ xrep_nlinks(
 		 * We can only push the inactivation workqueues with an empty
 		 * transaction.
 		 */
-		error = xchk_trans_alloc_empty(sc);
-		if (error)
-			break;
+		xchk_trans_alloc_empty(sc);
 	}
 	xchk_iscan_iter_finish(&xnc->compare_iscan);
 	xchk_iscan_teardown(&xnc->compare_iscan);
diff --git a/fs/xfs/scrub/parent_repair.c b/fs/xfs/scrub/parent_repair.c
index 31bfe10be22a..2949feda6271 100644
--- a/fs/xfs/scrub/parent_repair.c
+++ b/fs/xfs/scrub/parent_repair.c
@@ -569,9 +569,7 @@ xrep_parent_scan_dirtree(
 	if (sc->ilock_flags & (XFS_ILOCK_SHARED | XFS_ILOCK_EXCL))
 		xchk_iunlock(sc, sc->ilock_flags & (XFS_ILOCK_SHARED |
 						    XFS_ILOCK_EXCL));
-	error = xchk_trans_alloc_empty(sc);
-	if (error)
-		return error;
+	xchk_trans_alloc_empty(sc);
 
 	while ((error = xchk_iscan_iter(&rp->pscan.iscan, &ip)) == 1) {
 		bool		flush;
@@ -597,9 +595,7 @@ xrep_parent_scan_dirtree(
 			if (error)
 				break;
 
-			error = xchk_trans_alloc_empty(sc);
-			if (error)
-				break;
+			xchk_trans_alloc_empty(sc);
 		}
 
 		if (xchk_should_terminate(sc, &error))
@@ -1099,9 +1095,7 @@ xrep_parent_flush_xattrs(
 	xrep_tempfile_iounlock(rp->sc);
 
 	/* Recreate the empty transaction and relock the inode. */
-	error = xchk_trans_alloc_empty(rp->sc);
-	if (error)
-		return error;
+	xchk_trans_alloc_empty(rp->sc);
 	xchk_ilock(rp->sc, XFS_ILOCK_EXCL);
 	return 0;
 }
diff --git a/fs/xfs/scrub/quotacheck.c b/fs/xfs/scrub/quotacheck.c
index dc4033b91e44..e4105aaafe84 100644
--- a/fs/xfs/scrub/quotacheck.c
+++ b/fs/xfs/scrub/quotacheck.c
@@ -505,9 +505,7 @@ xqcheck_collect_counts(
 	 * transactions do not take sb_internal.
 	 */
 	xchk_trans_cancel(sc);
-	error = xchk_trans_alloc_empty(sc);
-	if (error)
-		return error;
+	xchk_trans_alloc_empty(sc);
 
 	while ((error = xchk_iscan_iter(&xqc->iscan, &ip)) == 1) {
 		error = xqcheck_collect_inode(xqc, ip);
diff --git a/fs/xfs/scrub/rmap_repair.c b/fs/xfs/scrub/rmap_repair.c
index f5f73078ffe2..bf1e632b449a 100644
--- a/fs/xfs/scrub/rmap_repair.c
+++ b/fs/xfs/scrub/rmap_repair.c
@@ -951,9 +951,7 @@ xrep_rmap_find_rmaps(
 	sa->agf_bp = NULL;
 	sa->agi_bp = NULL;
 	xchk_trans_cancel(sc);
-	error = xchk_trans_alloc_empty(sc);
-	if (error)
-		return error;
+	xchk_trans_alloc_empty(sc);
 
 	/* Iterate all AGs for inodes rmaps. */
 	while ((error = xchk_iscan_iter(&rr->iscan, &ip)) == 1) {
diff --git a/fs/xfs/scrub/rtrmap_repair.c b/fs/xfs/scrub/rtrmap_repair.c
index fc2592c53af5..4a56726d9952 100644
--- a/fs/xfs/scrub/rtrmap_repair.c
+++ b/fs/xfs/scrub/rtrmap_repair.c
@@ -580,9 +580,7 @@ xrep_rtrmap_find_rmaps(
 	 */
 	xchk_trans_cancel(sc);
 	xchk_rtgroup_unlock(&sc->sr);
-	error = xchk_trans_alloc_empty(sc);
-	if (error)
-		return error;
+	xchk_trans_alloc_empty(sc);
 
 	while ((error = xchk_iscan_iter(&rr->iscan, &ip)) == 1) {
 		error = xrep_rtrmap_scan_inode(rr, ip);
-- 
2.47.2

[PATCH 7/8] xfs: remove xrep_trans_{alloc,cancel}_hook_dummy

[Christoph Hellwig]

XFS stopped using current->journal_info in commit f2e812c1522d ("xfs:
don't use current->journal_info"), so there is no point in saving and
restoring it.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/scrub/repair.c        | 28 ----------------------------
 fs/xfs/scrub/repair.h        |  4 ----
 fs/xfs/scrub/rmap_repair.c   | 10 +++-------
 fs/xfs/scrub/rtrmap_repair.c | 10 +++-------
 4 files changed, 6 insertions(+), 46 deletions(-)

diff --git a/fs/xfs/scrub/repair.c b/fs/xfs/scrub/repair.c
index f7f80ff32afc..d00c18954a26 100644
--- a/fs/xfs/scrub/repair.c
+++ b/fs/xfs/scrub/repair.c
@@ -1268,34 +1268,6 @@ xrep_setup_xfbtree(
 	return xmbuf_alloc(sc->mp, descr, &sc->xmbtp);
 }
 
-/*
- * Create a dummy transaction for use in a live update hook function.  This
- * function MUST NOT be called from regular repair code because the current
- * process' transaction is saved via the cookie.
- */
-int
-xrep_trans_alloc_hook_dummy(
-	struct xfs_mount	*mp,
-	void			**cookiep,
-	struct xfs_trans	**tpp)
-{
-	*cookiep = current->journal_info;
-	current->journal_info = NULL;
-	*tpp = xfs_trans_alloc_empty(mp);
-	return 0;
-}
-
-/* Cancel a dummy transaction used by a live update hook function. */
-void
-xrep_trans_cancel_hook_dummy(
-	void			**cookiep,
-	struct xfs_trans	*tp)
-{
-	xfs_trans_cancel(tp);
-	current->journal_info = *cookiep;
-	*cookiep = NULL;
-}
-
 /*
  * See if this buffer can pass the given ->verify_struct() function.
  *
diff --git a/fs/xfs/scrub/repair.h b/fs/xfs/scrub/repair.h
index af0a3a9e5ed9..9c04295742c8 100644
--- a/fs/xfs/scrub/repair.h
+++ b/fs/xfs/scrub/repair.h
@@ -180,10 +180,6 @@ int xrep_quotacheck(struct xfs_scrub *sc);
 int xrep_reinit_pagf(struct xfs_scrub *sc);
 int xrep_reinit_pagi(struct xfs_scrub *sc);
 
-int xrep_trans_alloc_hook_dummy(struct xfs_mount *mp, void **cookiep,
-		struct xfs_trans **tpp);
-void xrep_trans_cancel_hook_dummy(void **cookiep, struct xfs_trans *tp);
-
 bool xrep_buf_verify_struct(struct xfs_buf *bp, const struct xfs_buf_ops *ops);
 void xrep_inode_set_nblocks(struct xfs_scrub *sc, int64_t new_blocks);
 int xrep_reset_metafile_resv(struct xfs_scrub *sc);
diff --git a/fs/xfs/scrub/rmap_repair.c b/fs/xfs/scrub/rmap_repair.c
index bf1e632b449a..17d4a38d735c 100644
--- a/fs/xfs/scrub/rmap_repair.c
+++ b/fs/xfs/scrub/rmap_repair.c
@@ -1610,7 +1610,6 @@ xrep_rmapbt_live_update(
 	struct xfs_mount		*mp;
 	struct xfs_btree_cur		*mcur;
 	struct xfs_trans		*tp;
-	void				*txcookie;
 	int				error;
 
 	rr = container_of(nb, struct xrep_rmap, rhook.rmap_hook.nb);
@@ -1621,9 +1620,7 @@ xrep_rmapbt_live_update(
 
 	trace_xrep_rmap_live_update(pag_group(rr->sc->sa.pag), action, p);
 
-	error = xrep_trans_alloc_hook_dummy(mp, &txcookie, &tp);
-	if (error)
-		goto out_abort;
+	tp = xfs_trans_alloc_empty(mp);
 
 	mutex_lock(&rr->lock);
 	mcur = xfs_rmapbt_mem_cursor(rr->sc->sa.pag, tp, &rr->rmap_btree);
@@ -1637,14 +1634,13 @@ xrep_rmapbt_live_update(
 	if (error)
 		goto out_cancel;
 
-	xrep_trans_cancel_hook_dummy(&txcookie, tp);
+	xfs_trans_cancel(tp);
 	mutex_unlock(&rr->lock);
 	return NOTIFY_DONE;
 
 out_cancel:
 	xfbtree_trans_cancel(&rr->rmap_btree, tp);
-	xrep_trans_cancel_hook_dummy(&txcookie, tp);
-out_abort:
+	xfs_trans_cancel(tp);
 	mutex_unlock(&rr->lock);
 	xchk_iscan_abort(&rr->iscan);
 out_unlock:
diff --git a/fs/xfs/scrub/rtrmap_repair.c b/fs/xfs/scrub/rtrmap_repair.c
index 4a56726d9952..7561941a337a 100644
--- a/fs/xfs/scrub/rtrmap_repair.c
+++ b/fs/xfs/scrub/rtrmap_repair.c
@@ -844,7 +844,6 @@ xrep_rtrmapbt_live_update(
 	struct xfs_mount		*mp;
 	struct xfs_btree_cur		*mcur;
 	struct xfs_trans		*tp;
-	void				*txcookie;
 	int				error;
 
 	rr = container_of(nb, struct xrep_rtrmap, rhook.rmap_hook.nb);
@@ -855,9 +854,7 @@ xrep_rtrmapbt_live_update(
 
 	trace_xrep_rmap_live_update(rtg_group(rr->sc->sr.rtg), action, p);
 
-	error = xrep_trans_alloc_hook_dummy(mp, &txcookie, &tp);
-	if (error)
-		goto out_abort;
+	tp = xfs_trans_alloc_empty(mp);
 
 	mutex_lock(&rr->lock);
 	mcur = xfs_rtrmapbt_mem_cursor(rr->sc->sr.rtg, tp, &rr->rtrmap_btree);
@@ -871,14 +868,13 @@ xrep_rtrmapbt_live_update(
 	if (error)
 		goto out_cancel;
 
-	xrep_trans_cancel_hook_dummy(&txcookie, tp);
+	xfs_trans_cancel(tp);
 	mutex_unlock(&rr->lock);
 	return NOTIFY_DONE;
 
 out_cancel:
 	xfbtree_trans_cancel(&rr->rtrmap_btree, tp);
-	xrep_trans_cancel_hook_dummy(&txcookie, tp);
-out_abort:
+	xfs_trans_cancel(tp);
 	xchk_iscan_abort(&rr->iscan);
 	mutex_unlock(&rr->lock);
 out_unlock:
-- 
2.47.2

[PATCH 8/8] xfs: remove the xlog_ticket_t typedef

[Christoph Hellwig]

Almost no users of the typedef left, kill it and switch the remaining
users to use the underlying struct.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/xfs_log.c      | 6 +++---
 fs/xfs/xfs_log_priv.h | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 793468b4d30d..bc3297da2143 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -3092,16 +3092,16 @@ xfs_log_force_seq(
  */
 void
 xfs_log_ticket_put(
-	xlog_ticket_t	*ticket)
+	struct xlog_ticket	*ticket)
 {
 	ASSERT(atomic_read(&ticket->t_ref) > 0);
 	if (atomic_dec_and_test(&ticket->t_ref))
 		kmem_cache_free(xfs_log_ticket_cache, ticket);
 }
 
-xlog_ticket_t *
+struct xlog_ticket *
 xfs_log_ticket_get(
-	xlog_ticket_t	*ticket)
+	struct xlog_ticket	*ticket)
 {
 	ASSERT(atomic_read(&ticket->t_ref) > 0);
 	atomic_inc(&ticket->t_ref);
diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h
index 39a102cc1b43..a9a7a271c15b 100644
--- a/fs/xfs/xfs_log_priv.h
+++ b/fs/xfs/xfs_log_priv.h
@@ -144,7 +144,7 @@ enum xlog_iclog_state {
 
 #define XLOG_COVER_OPS		5
 
-typedef struct xlog_ticket {
+struct xlog_ticket {
 	struct list_head	t_queue;	/* reserve/write queue */
 	struct task_struct	*t_task;	/* task that owns this ticket */
 	xlog_tid_t		t_tid;		/* transaction identifier */
@@ -155,7 +155,7 @@ typedef struct xlog_ticket {
 	char			t_cnt;		/* current unit count */
 	uint8_t			t_flags;	/* properties of reservation */
 	int			t_iclog_hdrs;	/* iclog hdrs in t_curr_res */
-} xlog_ticket_t;
+};
 
 /*
  * - A log record header is 512 bytes.  There is plenty of room to grow the
-- 
2.47.2

Re: [PATCH 2/8] xfs: don't use xfs_trans_reserve in xfs_trans_reserve_more

[Darrick J. Wong]

On Wed, Jul 16, 2025 at 02:43:12PM +0200, Christoph Hellwig wrote:
> xfs_trans_reserve_more just tries to allocate additional blocks and/or
> rtextents and is otherwise unrelated to the transaction reservation
> logic.  Open code the block and rtextent reservation in
> xfs_trans_reserve_more to prepare for simplifying xfs_trans_reserve.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/xfs/xfs_trans.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
> index 8b15bfe68774..2213cb2278d2 100644
> --- a/fs/xfs/xfs_trans.c
> +++ b/fs/xfs/xfs_trans.c
> @@ -1146,9 +1146,18 @@ xfs_trans_reserve_more(
>  	unsigned int		blocks,
>  	unsigned int		rtextents)
>  {
> -	struct xfs_trans_res	resv = { };
> -
> -	return xfs_trans_reserve(tp, &resv, blocks, rtextents);
> +	bool			rsvd = tp->t_flags & XFS_TRANS_RESERVE;
> +
> +	if (blocks && xfs_dec_fdblocks(tp->t_mountp, blocks, rsvd))
> +		return -ENOSPC;
> +	if (rtextents && xfs_dec_frextents(tp->t_mountp, rtextents)) {
> +		if (blocks)
> +			xfs_add_fdblocks(tp->t_mountp, blocks);
> +		return -ENOSPC;
> +	}
> +	tp->t_blk_res += blocks;
> +	tp->t_rtx_res += rtextents;
> +	return 0;
>  }
>  
>  /*
> -- 
> 2.47.2
> 
> 

Re: [PATCH 4/8] xfs: don't use xfs_trans_reserve in xfs_trans_roll

[Darrick J. Wong]

On Wed, Jul 16, 2025 at 02:43:14PM +0200, Christoph Hellwig wrote:
> xfs_trans_roll uses xfs_trans_reserve to basically just call into
> xfs_log_regrant while bypassing the reset of xfs_trans_reserve.
> 
> Open code the call to xfs_log_regrant in xfs_trans_roll and simplify
> xfs_trans_reserve now that it never regrants and always asks for a log
> reservation.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good now, and we don't even have the dummy stack resv anymore. :)
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/xfs/xfs_trans.c | 133 ++++++++++++++++++---------------------------
>  1 file changed, 54 insertions(+), 79 deletions(-)
> 
> diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
> index e5edb89909ed..8d0aee747f72 100644
> --- a/fs/xfs/xfs_trans.c
> +++ b/fs/xfs/xfs_trans.c
> @@ -134,18 +134,14 @@ xfs_trans_dup(
>  }
>  
>  /*
> - * 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 is called to reserve free disk blocks and log space for the given
> + * transaction 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.
> + * This does not do quota reservations. That typically is done by the caller
> + * afterwards.
>   */
>  static int
>  xfs_trans_reserve(
> @@ -158,10 +154,12 @@ xfs_trans_reserve(
>  	int			error = 0;
>  	bool			rsvd = (tp->t_flags & XFS_TRANS_RESERVE) != 0;
>  
> +	ASSERT(resp->tr_logres > 0);
> +
>  	/*
> -	 * Attempt to reserve the needed disk blocks by decrementing
> -	 * the number needed from the number available.  This will
> -	 * fail if the count would go below zero.
> +	 * Attempt to reserve the needed disk blocks by decrementing the number
> +	 * needed from the number available.  This will fail if the count would
> +	 * go below zero.
>  	 */
>  	if (blocks > 0) {
>  		error = xfs_dec_fdblocks(mp, blocks, rsvd);
> @@ -173,42 +171,20 @@ xfs_trans_reserve(
>  	/*
>  	 * Reserve the log space needed for this transaction.
>  	 */
> -	if (resp->tr_logres > 0) {
> -		bool	permanent = false;
> -
> -		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;
> -			permanent = true;
> -		} else {
> -			ASSERT(tp->t_ticket == NULL);
> -			ASSERT(!(tp->t_flags & XFS_TRANS_PERM_LOG_RES));
> -		}
> -
> -		if (tp->t_ticket != NULL) {
> -			ASSERT(resp->tr_logflags & XFS_TRANS_PERM_LOG_RES);
> -			error = xfs_log_regrant(mp, tp->t_ticket);
> -		} else {
> -			error = xfs_log_reserve(mp, resp->tr_logres,
> -						resp->tr_logcount,
> -						&tp->t_ticket, permanent);
> -		}
> -
> -		if (error)
> -			goto undo_blocks;
> +	if (resp->tr_logflags & XFS_TRANS_PERM_LOG_RES)
> +		tp->t_flags |= XFS_TRANS_PERM_LOG_RES;
> +	error = xfs_log_reserve(mp, resp->tr_logres, resp->tr_logcount,
> +			&tp->t_ticket, (tp->t_flags & XFS_TRANS_PERM_LOG_RES));
> +	if (error)
> +		goto undo_blocks;
>  
> -		tp->t_log_res = resp->tr_logres;
> -		tp->t_log_count = resp->tr_logcount;
> -	}
> +	tp->t_log_res = resp->tr_logres;
> +	tp->t_log_count = resp->tr_logcount;
>  
>  	/*
> -	 * 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.
> +	 * 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) {
>  		error = xfs_dec_frextents(mp, rtextents);
> @@ -221,18 +197,11 @@ xfs_trans_reserve(
>  
>  	return 0;
>  
> -	/*
> -	 * Error cases jump to one of these labels to undo any
> -	 * reservations which have already been performed.
> -	 */
>  undo_log:
> -	if (resp->tr_logres > 0) {
> -		xfs_log_ticket_ungrant(mp->m_log, tp->t_ticket);
> -		tp->t_ticket = NULL;
> -		tp->t_log_res = 0;
> -		tp->t_flags &= ~XFS_TRANS_PERM_LOG_RES;
> -	}
> -
> +	xfs_log_ticket_ungrant(mp->m_log, tp->t_ticket);
> +	tp->t_ticket = NULL;
> +	tp->t_log_res = 0;
> +	tp->t_flags &= ~XFS_TRANS_PERM_LOG_RES;
>  undo_blocks:
>  	if (blocks > 0) {
>  		xfs_add_fdblocks(mp, blocks);
> @@ -1030,51 +999,57 @@ xfs_trans_cancel(
>  }
>  
>  /*
> - * Roll from one trans in the sequence of PERMANENT transactions to
> - * the next: permanent transactions are only flushed out when
> - * committed with xfs_trans_commit(), but we still want as soon
> - * as possible to let chunks of it go to the log. So we commit the
> - * chunk we've been working on and get a new transaction to continue.
> + * Roll from one trans in the sequence of PERMANENT transactions to the next:
> + * permanent transactions are only flushed out when committed with
> + * xfs_trans_commit(), but we still want as soon as possible to let chunks of it
> + * go to the log.  So we commit the chunk we've been working on and get a new
> + * transaction to continue.
>   */
>  int
>  xfs_trans_roll(
>  	struct xfs_trans	**tpp)
>  {
> -	struct xfs_trans	*trans = *tpp;
> -	struct xfs_trans_res	tres;
> +	struct xfs_trans	*tp = *tpp;
> +	unsigned int		log_res = tp->t_log_res;
> +	unsigned int		log_count = tp->t_log_count;
>  	int			error;
>  
> -	trace_xfs_trans_roll(trans, _RET_IP_);
> +	trace_xfs_trans_roll(tp, _RET_IP_);
> +
> +	ASSERT(log_res > 0);
>  
>  	/*
>  	 * Copy the critical parameters from one trans to the next.
>  	 */
> -	tres.tr_logres = trans->t_log_res;
> -	tres.tr_logcount = trans->t_log_count;
> -
> -	*tpp = xfs_trans_dup(trans);
> +	*tpp = xfs_trans_dup(tp);
>  
>  	/*
>  	 * Commit the current transaction.
> -	 * If this commit failed, then it'd just unlock those items that
> -	 * are not marked ihold. That also means that a filesystem shutdown
> -	 * is in progress. The caller takes the responsibility to cancel
> -	 * the duplicate transaction that gets returned.
> +	 *
> +	 * If this commit failed, then it'd just unlock those items that are not
> +	 * marked ihold. That also means that a filesystem shutdown is in
> +	 * progress.  The caller takes the responsibility to cancel the
> +	 * duplicate transaction that gets returned.
>  	 */
> -	error = __xfs_trans_commit(trans, true);
> +	error = __xfs_trans_commit(tp, true);
>  	if (error)
>  		return error;
>  
>  	/*
>  	 * Reserve space in the log for the next transaction.
> -	 * This also pushes items in the "AIL", the list of logged items,
> -	 * out to disk if they are taking up space at the tail of the log
> -	 * that we want to use.  This requires that either nothing be locked
> -	 * across this call, or that anything that is locked be logged in
> -	 * the prior and the next transactions.
> +	 *
> +	 * This also pushes items in the AIL out to disk if they are taking up
> +	 * space at the tail of the log that we want to use.  This requires that
> +	 * either nothing be locked across this call, or that anything that is
> +	 * locked be logged in the prior and the next transactions.
>  	 */
> -	tres.tr_logflags = XFS_TRANS_PERM_LOG_RES;
> -	return xfs_trans_reserve(*tpp, &tres, 0, 0);
> +	tp = *tpp;
> +	error = xfs_log_regrant(tp->t_mountp, tp->t_ticket);
> +	if (error)
> +		return error;
> +	tp->t_log_res = log_res;
> +	tp->t_log_count = log_count;
> +	return 0;
>  }
>  
>  /*
> -- 
> 2.47.2
> 
> 

Re: [PATCH 7/8] xfs: remove xrep_trans_{alloc,cancel}_hook_dummy

[Darrick J. Wong]

On Wed, Jul 16, 2025 at 02:43:17PM +0200, Christoph Hellwig wrote:
> XFS stopped using current->journal_info in commit f2e812c1522d ("xfs:
> don't use current->journal_info"), so there is no point in saving and
> restoring it.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Excellent <tents fingers>
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/xfs/scrub/repair.c        | 28 ----------------------------
>  fs/xfs/scrub/repair.h        |  4 ----
>  fs/xfs/scrub/rmap_repair.c   | 10 +++-------
>  fs/xfs/scrub/rtrmap_repair.c | 10 +++-------
>  4 files changed, 6 insertions(+), 46 deletions(-)
> 
> diff --git a/fs/xfs/scrub/repair.c b/fs/xfs/scrub/repair.c
> index f7f80ff32afc..d00c18954a26 100644
> --- a/fs/xfs/scrub/repair.c
> +++ b/fs/xfs/scrub/repair.c
> @@ -1268,34 +1268,6 @@ xrep_setup_xfbtree(
>  	return xmbuf_alloc(sc->mp, descr, &sc->xmbtp);
>  }
>  
> -/*
> - * Create a dummy transaction for use in a live update hook function.  This
> - * function MUST NOT be called from regular repair code because the current
> - * process' transaction is saved via the cookie.
> - */
> -int
> -xrep_trans_alloc_hook_dummy(
> -	struct xfs_mount	*mp,
> -	void			**cookiep,
> -	struct xfs_trans	**tpp)
> -{
> -	*cookiep = current->journal_info;
> -	current->journal_info = NULL;
> -	*tpp = xfs_trans_alloc_empty(mp);
> -	return 0;
> -}
> -
> -/* Cancel a dummy transaction used by a live update hook function. */
> -void
> -xrep_trans_cancel_hook_dummy(
> -	void			**cookiep,
> -	struct xfs_trans	*tp)
> -{
> -	xfs_trans_cancel(tp);
> -	current->journal_info = *cookiep;
> -	*cookiep = NULL;
> -}
> -
>  /*
>   * See if this buffer can pass the given ->verify_struct() function.
>   *
> diff --git a/fs/xfs/scrub/repair.h b/fs/xfs/scrub/repair.h
> index af0a3a9e5ed9..9c04295742c8 100644
> --- a/fs/xfs/scrub/repair.h
> +++ b/fs/xfs/scrub/repair.h
> @@ -180,10 +180,6 @@ int xrep_quotacheck(struct xfs_scrub *sc);
>  int xrep_reinit_pagf(struct xfs_scrub *sc);
>  int xrep_reinit_pagi(struct xfs_scrub *sc);
>  
> -int xrep_trans_alloc_hook_dummy(struct xfs_mount *mp, void **cookiep,
> -		struct xfs_trans **tpp);
> -void xrep_trans_cancel_hook_dummy(void **cookiep, struct xfs_trans *tp);
> -
>  bool xrep_buf_verify_struct(struct xfs_buf *bp, const struct xfs_buf_ops *ops);
>  void xrep_inode_set_nblocks(struct xfs_scrub *sc, int64_t new_blocks);
>  int xrep_reset_metafile_resv(struct xfs_scrub *sc);
> diff --git a/fs/xfs/scrub/rmap_repair.c b/fs/xfs/scrub/rmap_repair.c
> index bf1e632b449a..17d4a38d735c 100644
> --- a/fs/xfs/scrub/rmap_repair.c
> +++ b/fs/xfs/scrub/rmap_repair.c
> @@ -1610,7 +1610,6 @@ xrep_rmapbt_live_update(
>  	struct xfs_mount		*mp;
>  	struct xfs_btree_cur		*mcur;
>  	struct xfs_trans		*tp;
> -	void				*txcookie;
>  	int				error;
>  
>  	rr = container_of(nb, struct xrep_rmap, rhook.rmap_hook.nb);
> @@ -1621,9 +1620,7 @@ xrep_rmapbt_live_update(
>  
>  	trace_xrep_rmap_live_update(pag_group(rr->sc->sa.pag), action, p);
>  
> -	error = xrep_trans_alloc_hook_dummy(mp, &txcookie, &tp);
> -	if (error)
> -		goto out_abort;
> +	tp = xfs_trans_alloc_empty(mp);
>  
>  	mutex_lock(&rr->lock);
>  	mcur = xfs_rmapbt_mem_cursor(rr->sc->sa.pag, tp, &rr->rmap_btree);
> @@ -1637,14 +1634,13 @@ xrep_rmapbt_live_update(
>  	if (error)
>  		goto out_cancel;
>  
> -	xrep_trans_cancel_hook_dummy(&txcookie, tp);
> +	xfs_trans_cancel(tp);
>  	mutex_unlock(&rr->lock);
>  	return NOTIFY_DONE;
>  
>  out_cancel:
>  	xfbtree_trans_cancel(&rr->rmap_btree, tp);
> -	xrep_trans_cancel_hook_dummy(&txcookie, tp);
> -out_abort:
> +	xfs_trans_cancel(tp);
>  	mutex_unlock(&rr->lock);
>  	xchk_iscan_abort(&rr->iscan);
>  out_unlock:
> diff --git a/fs/xfs/scrub/rtrmap_repair.c b/fs/xfs/scrub/rtrmap_repair.c
> index 4a56726d9952..7561941a337a 100644
> --- a/fs/xfs/scrub/rtrmap_repair.c
> +++ b/fs/xfs/scrub/rtrmap_repair.c
> @@ -844,7 +844,6 @@ xrep_rtrmapbt_live_update(
>  	struct xfs_mount		*mp;
>  	struct xfs_btree_cur		*mcur;
>  	struct xfs_trans		*tp;
> -	void				*txcookie;
>  	int				error;
>  
>  	rr = container_of(nb, struct xrep_rtrmap, rhook.rmap_hook.nb);
> @@ -855,9 +854,7 @@ xrep_rtrmapbt_live_update(
>  
>  	trace_xrep_rmap_live_update(rtg_group(rr->sc->sr.rtg), action, p);
>  
> -	error = xrep_trans_alloc_hook_dummy(mp, &txcookie, &tp);
> -	if (error)
> -		goto out_abort;
> +	tp = xfs_trans_alloc_empty(mp);
>  
>  	mutex_lock(&rr->lock);
>  	mcur = xfs_rtrmapbt_mem_cursor(rr->sc->sr.rtg, tp, &rr->rtrmap_btree);
> @@ -871,14 +868,13 @@ xrep_rtrmapbt_live_update(
>  	if (error)
>  		goto out_cancel;
>  
> -	xrep_trans_cancel_hook_dummy(&txcookie, tp);
> +	xfs_trans_cancel(tp);
>  	mutex_unlock(&rr->lock);
>  	return NOTIFY_DONE;
>  
>  out_cancel:
>  	xfbtree_trans_cancel(&rr->rtrmap_btree, tp);
> -	xrep_trans_cancel_hook_dummy(&txcookie, tp);
> -out_abort:
> +	xfs_trans_cancel(tp);
>  	xchk_iscan_abort(&rr->iscan);
>  	mutex_unlock(&rr->lock);
>  out_unlock:
> -- 
> 2.47.2
> 
> 

Re: cleanup transaction allocation v2

[Carlos Maiolino]

On Wed, 16 Jul 2025 14:43:10 +0200, Christoph Hellwig wrote:
> this series cleans up the xfs_trans_alloc* and xfs_trans_reserve*
> interfaces by keeping different code paths more separate and thus
> removing redundant arguments and error returns.
> 
> A git tree is also available here:
> 
>     git://git.infradead.org/users/hch/xfs.git xfs-trans-cleanups
> 
> [...]

Applied to for-next, thanks!

[1/8] xfs: use xfs_trans_reserve_more in xfs_trans_reserve_more_inode
      commit: 8f89c581c0da32ea6a8a1250e7479aa20c4d2824
[2/8] xfs: don't use xfs_trans_reserve in xfs_trans_reserve_more
      commit: e13f9ce5bec1273639e6baf2562f01a464e2735a
[3/8] xfs: decouple xfs_trans_alloc_empty from xfs_trans_alloc
      commit: ddf9708277ebe62f84f48f08e62392ff9b462501
[4/8] xfs: don't use xfs_trans_reserve in xfs_trans_roll
      commit: b4e174c374f677132bd5d16e27dffa4a22d6eed3
[5/8] xfs: return the allocated transaction from xfs_trans_alloc_empty
      commit: e967dc40d5017671e5e278aea858516440aa068a
[6/8] xfs: return the allocated transaction from xchk_trans_alloc_empty
      commit: 149b5cf8c7d235ffaa5eef011615f826080e3ca0
[7/8] xfs: remove xrep_trans_{alloc,cancel}_hook_dummy
      commit: a766ae6fe12014ca072b4e8ba194b39c93f45d92
[8/8] xfs: remove the xlog_ticket_t typedef
      commit: d9dbddd143db281a3c6981de5faba7f051382ceb

Best regards,
-- 
Carlos Maiolino <cem@kernel.org>