Re: [PATCH 2/8] xfs: don't commit the first deferred transaction without intents

Dave Chinner
2022-04-27T04:52:37Z

On Tue, Apr 26, 2022 at 08:03:30PM -0700, Darrick J. Wong wrote:
> On Wed, Apr 27, 2022 at 12:22:53PM +1000, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> >
> > 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@redhat.com>
> > ---
> > 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;

Oh, sorry, I must have missed that. My fault! Fixed.

> and not returning the log intent item itself.
>
> Otherwise looks ok, so with that fixed,
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>

Thanks!

-Dave.
--
Dave Chinner
david@fromorbit.com