Re: [RFC v5 PATCH 2/9] xfs: introduce ->tr_relog transaction

[Brian Foster <bfoster@redhat.com>]

On Thu, Feb 27, 2020 at 03:31:53PM -0800, Darrick J. Wong wrote:
> On Thu, Feb 27, 2020 at 08:43:14AM -0500, Brian Foster wrote:
> > Create a transaction reservation specifically for relog
> > transactions. For now it only supports the quotaoff intent, so use
> > the associated reservation.
> > 
> > Signed-off-by: Brian Foster <bfoster@redhat.com>
> > ---
> >  fs/xfs/libxfs/xfs_trans_resv.c | 15 +++++++++++++++
> >  fs/xfs/libxfs/xfs_trans_resv.h |  1 +
> >  2 files changed, 16 insertions(+)
> > 
> > diff --git a/fs/xfs/libxfs/xfs_trans_resv.c b/fs/xfs/libxfs/xfs_trans_resv.c
> > index 7a9c04920505..1f5c9e6e1afc 100644
> > --- a/fs/xfs/libxfs/xfs_trans_resv.c
> > +++ b/fs/xfs/libxfs/xfs_trans_resv.c
> > @@ -832,6 +832,17 @@ xfs_calc_sb_reservation(
> >  	return xfs_calc_buf_res(1, mp->m_sb.sb_sectsize);
> >  }
> >  
> > +/*
> > + * Internal relog transaction.
> > + *   quotaoff intent
> > + */
> > +STATIC uint
> > +xfs_calc_relog_reservation(
> > +	struct xfs_mount	*mp)
> > +{
> > +	return xfs_calc_qm_quotaoff_reservation(mp);
> 
> So when we add the next reloggable intent item, this will turn this
> into an n-way max(sizeof(type0), sizeof(type1), ...sizeof(typeN)); ?
> 

Possibly. I'm trying to keep things simple for now. So if we suppose the
near term use cases are the quotaoff intent, the scrub EFI intent and
perhaps the writeback stale data exposure zeroing intent, then I'd
probably just leave it as a max of those. We could also multiply that by
some constant factor for a simple form of batching, since the log
reservation is still likely to be on the smaller size.

If longer term we end up with relog support for a variety of item times
and the potential for a lot of concurrent relog activity, I'd be more
inclined to consider a specific calculation or to pick off the current
max transaction size or something and require batching implement some
form of reservation use tracking (i.e., consider an
xfs_trans_add_item_try(...) interface that performed a magical size
check and failed when the transaction is full).

As it is, I don't see enough use case right now to cross that complexity
threshold from the first model to the second right away..

> > +}
> > +
> >  void
> >  xfs_trans_resv_calc(
> >  	struct xfs_mount	*mp,
> > @@ -946,4 +957,8 @@ xfs_trans_resv_calc(
> >  	resp->tr_clearagi.tr_logres = xfs_calc_clear_agi_bucket_reservation(mp);
> >  	resp->tr_growrtzero.tr_logres = xfs_calc_growrtzero_reservation(mp);
> >  	resp->tr_growrtfree.tr_logres = xfs_calc_growrtfree_reservation(mp);
> > +
> > +	resp->tr_relog.tr_logres = xfs_calc_relog_reservation(mp);
> > +	resp->tr_relog.tr_logcount = XFS_DEFAULT_PERM_LOG_COUNT;
> 
> Relog operations can roll?  I would have figured that you'd simply log
> the old item(s) in a new transaction and commit it, along with some
> magic to let the log tail move forward.  I guess I'll see what happens
> in the next 7 patches. :)
> 

The current scheme is that the relog transaction rolls one item at a
time. This is again, simplified for the purpose of a POC. For a
production iteration, I'd probably just turn that into a fixed count to
be able to batch 5 or 10 items at a time or something along those lines
(probably more depends on what the transaction size looks like and the
pressure put on by the scrub use case).

Brian

> --D
> 
> > +	resp->tr_relog.tr_logflags |= XFS_TRANS_PERM_LOG_RES;
> >  }
> > diff --git a/fs/xfs/libxfs/xfs_trans_resv.h b/fs/xfs/libxfs/xfs_trans_resv.h
> > index 7241ab28cf84..b723979cad09 100644
> > --- a/fs/xfs/libxfs/xfs_trans_resv.h
> > +++ b/fs/xfs/libxfs/xfs_trans_resv.h
> > @@ -50,6 +50,7 @@ struct xfs_trans_resv {
> >  	struct xfs_trans_res	tr_qm_equotaoff;/* end of turn quota off */
> >  	struct xfs_trans_res	tr_sb;		/* modify superblock */
> >  	struct xfs_trans_res	tr_fsyncts;	/* update timestamps on fsync */
> > +	struct xfs_trans_res	tr_relog;	/* internal relog transaction */
> >  };
> >  
> >  /* shorthand way of accessing reservation structure */
> > -- 
> > 2.21.1
> > 
>