On Thu, Oct 01, 2020 at 12:02:56PM -0400, Brian Foster wrote:
> On Tue, Sep 29, 2020 at 10:44:35AM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> >
> > Now that we have the ability to ask the log how far the tail needs to be
> > pushed to maintain its free space targets, augment the decision to relog
> > an intent item so that we only do it if the log has hit the 75% full
> > threshold. There's no point in relogging an intent into the same
> > checkpoint, and there's no need to relog if there's plenty of free space
> > in the log.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> > ---
> > fs/xfs/libxfs/xfs_defer.c | 16 ++++++++++++++++
> > 1 file changed, 16 insertions(+)
> >
> >
> > diff --git a/fs/xfs/libxfs/xfs_defer.c b/fs/xfs/libxfs/xfs_defer.c
> > index 554777d1069c..2ba02f2e59a1 100644
> > --- a/fs/xfs/libxfs/xfs_defer.c
> > +++ b/fs/xfs/libxfs/xfs_defer.c
> > @@ -356,7 +356,10 @@ xfs_defer_relog(
> > struct xfs_trans **tpp,
> > struct list_head *dfops)
> > {
> > + struct xlog *log = (*tpp)->t_mountp->m_log;
> > struct xfs_defer_pending *dfp;
> > + xfs_lsn_t threshold_lsn = NULLCOMMITLSN;
> > +
> >
> > ASSERT((*tpp)->t_flags & XFS_TRANS_PERM_LOG_RES);
> >
> > @@ -372,6 +375,19 @@ xfs_defer_relog(
> > xfs_log_item_in_current_chkpt(dfp->dfp_intent))
> > continue;
> >
> > + /*
> > + * Figure out where we need the tail to be in order to maintain
> > + * the minimum required free space in the log. Only sample
> > + * the log threshold once per call.
> > + */
> > + if (threshold_lsn == NULLCOMMITLSN) {
> > + threshold_lsn = xlog_grant_push_threshold(log, 0);
> > + if (threshold_lsn == NULLCOMMITLSN)
> > + break;
> > + }
>
> FWIW, this looks slightly different from the referenced repo again. :P
> It might be good practice to create a -v# branch for patches sent to the
> list and to keep that one stable for the associated review cycle.
I did, as we talked about on irc. For anyone following along at home:
https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfs-linux.git/tag/?h=defer-ops-stalls-5.10_2020-09-29
> That aside, I'm not quite clear where we stand with this patch. My
> preference was to keep it unless there was some fundamental correctness
> issue that I'm not aware of. I think your and Dave's preference was to
> drop it. So either way, for posterity:
As the author who keeps spraying this patch onto the list, I'm still of
a mind to include it. :) Since we don't really need to relog the
intents unless we need to move the log forward.
That said, even my attempts to force worst case scenarios (make a few
hundred of the big-extent-partially-shared files, truncate them all at
the same time while repeatedly fsyncing (or freezing the fs) still shows
that the relog counts are a tiny percentage of the total transactions
run. I guess I'll try a higher AG count fs next...
--D
>
> Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>
>
> > + if (XFS_LSN_CMP(dfp->dfp_intent->li_lsn, threshold_lsn) >= 0)
> > + continue;
> > +
> > trace_xfs_defer_relog_intent((*tpp)->t_mountp, dfp);
> > XFS_STATS_INC((*tpp)->t_mountp, defer_relog);
> > dfp->dfp_intent = xfs_trans_item_relog(dfp->dfp_intent, *tpp);
> >
>
