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@xxxxxx>
That makes sense, especially because all the _empty callsites become so
much cleaner later on...
Reviewed-by: "Darrick J. Wong" <djwong@xxxxxxxxxx>
--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
>
>
