Re: [PATCH 43/45] xfs: avoid cil push lock if possible

Dave Chinner
2021-03-12T02:37:58Z

On Wed, Mar 10, 2021 at 05:47:09PM -0800, Darrick J. Wong wrote:
> On Fri, Mar 05, 2021 at 04:11:41PM +1100, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> >
> > Because now it hurts when the CIL fills up.
> >
> > - 37.20% __xfs_trans_commit
> > - 35.84% xfs_log_commit_cil
> > - 19.34% _raw_spin_lock
> > - do_raw_spin_lock
> > 19.01% __pv_queued_spin_lock_slowpath
> > - 4.20% xfs_log_ticket_ungrant
> > 0.90% xfs_log_space_wake
> >
> >
> > Signed-off-by: Dave Chinner <dchinner@redhat.com>
> > ---
> > fs/xfs/xfs_log_cil.c | 14 +++++++++++---
> > 1 file changed, 11 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c
> > index 6dcc23829bef..d60c72ad391a 100644
> > --- a/fs/xfs/xfs_log_cil.c
> > +++ b/fs/xfs/xfs_log_cil.c
> > @@ -1115,10 +1115,18 @@ xlog_cil_push_background(
> > ASSERT(!test_bit(XLOG_CIL_EMPTY, &cil->xc_flags));
> >
> > /*
> > - * Don't do a background push if we haven't used up all the
> > - * space available yet.
> > + * We are done if:
> > + * - we haven't used up all the space available yet; or
> > + * - we've already queued up a push; and
> > + * - we're not over the hard limit; and
> > + * - nothing has been over the hard limit.
>
> Er... do these last three bullet points correspond to the last three
> lines of the if test? I'm not sure how !waitqueue_active() determines
> that nothing has been over the hard limit?

If a commit has made space used go over the hard limit, it will be
throttled and put to sleep on the push wait queue. Another commit
can then return space (inode fork gets smaller) and bring us back
under the hard limit. Hence just checking against the space used does
not tell us if we've hit the hard limit or not, but checking if
there is a throttled process on the wait queue does...

> Or for that matter how
> comparing push_seq against current_seq tells us if we've queued a
> push?

We only set push_seq == current_seq when we queue up a push in
xlog_cil_push_now() or xlog_cil_push_background(). Hence if no push
has been queued, then push_seq will be less than current_seq.

Cheers,

Dave.
--
Dave Chinner
david@fromorbit.com