On Wed, Apr 27, 2022 at 12:22:53PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> If the first operation in a string of defer ops has no intents,
> then there is no reason to commit it before running the first call
> to xfs_defer_finish_one(). This allows the defer ops to be used
> effectively for non-intent based operations without requiring an
> unnecessary extra transaction commit when first called.
>
> This fixes a regression in per-attribute modification transaction
> count when delayed attributes are not being used.
>
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> ---
> fs/xfs/libxfs/xfs_defer.c | 29 +++++++++++++++++------------
> 1 file changed, 17 insertions(+), 12 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_defer.c b/fs/xfs/libxfs/xfs_defer.c
> index 0805ade2d300..66b4555bda8e 100644
> --- a/fs/xfs/libxfs/xfs_defer.c
> +++ b/fs/xfs/libxfs/xfs_defer.c
> @@ -186,7 +186,7 @@ static const struct xfs_defer_op_type *defer_op_types[] = {
> [XFS_DEFER_OPS_TYPE_AGFL_FREE] = &xfs_agfl_free_defer_type,
> };
>
> -static void
> +static bool
> xfs_defer_create_intent(
> struct xfs_trans *tp,
> struct xfs_defer_pending *dfp,
> @@ -197,6 +197,7 @@ xfs_defer_create_intent(
> if (!dfp->dfp_intent)
> dfp->dfp_intent = ops->create_intent(tp, &dfp->dfp_work,
> dfp->dfp_count, sort);
> + return dfp->dfp_intent;
Same comment as last time -- please make it more obvious that we're
returning whether or not ->create_intent actually added a log item:
return dfp->dfp_intent != NULL;
and not returning the log intent item itself.
Otherwise looks ok, so with that fixed,
Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx>
--D
> }
>
> /*
> @@ -204,16 +205,18 @@ xfs_defer_create_intent(
> * associated extents, then add the entire intake list to the end of
> * the pending list.
> */
> -STATIC void
> +static bool
> xfs_defer_create_intents(
> struct xfs_trans *tp)
> {
> struct xfs_defer_pending *dfp;
> + bool ret = false;
>
> list_for_each_entry(dfp, &tp->t_dfops, dfp_list) {
> trace_xfs_defer_create_intent(tp->t_mountp, dfp);
> - xfs_defer_create_intent(tp, dfp, true);
> + ret |= xfs_defer_create_intent(tp, dfp, true);
> }
> + return ret;
> }
>
> /* Abort all the intents that were committed. */
> @@ -487,7 +490,7 @@ int
> xfs_defer_finish_noroll(
> struct xfs_trans **tp)
> {
> - struct xfs_defer_pending *dfp;
> + struct xfs_defer_pending *dfp = NULL;
> int error = 0;
> LIST_HEAD(dop_pending);
>
> @@ -506,17 +509,19 @@ xfs_defer_finish_noroll(
> * of time that any one intent item can stick around in memory,
> * pinning the log tail.
> */
> - xfs_defer_create_intents(*tp);
> + bool has_intents = xfs_defer_create_intents(*tp);
> list_splice_init(&(*tp)->t_dfops, &dop_pending);
>
> - error = xfs_defer_trans_roll(tp);
> - if (error)
> - goto out_shutdown;
> + if (has_intents || dfp) {
> + error = xfs_defer_trans_roll(tp);
> + if (error)
> + goto out_shutdown;
>
> - /* Possibly relog intent items to keep the log moving. */
> - error = xfs_defer_relog(tp, &dop_pending);
> - if (error)
> - goto out_shutdown;
> + /* Possibly relog intent items to keep the log moving. */
> + error = xfs_defer_relog(tp, &dop_pending);
> + if (error)
> + goto out_shutdown;
> + }
>
> dfp = list_first_entry(&dop_pending, struct xfs_defer_pending,
> dfp_list);
> --
> 2.35.1
>
