Re: [PATCH 44/45] xfs: xlog_sync() manually adjusts grant head space

Dave Chinner
2021-03-16T03:05:00Z

On Wed, Mar 10, 2021 at 06:00:45PM -0800, Darrick J. Wong wrote:
> On Fri, Mar 05, 2021 at 04:11:42PM +1100, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> >
> > When xlog_sync() rounds off the tail the iclog that is being
> > flushed, it manually subtracts that space from the grant heads. This
> > space is actually reserved by the transaction ticket that covers
> > the xlog_sync() call from xlog_write(), but we don't plumb the
> > ticket down far enough for it to account for the space consumed in
> > the current log ticket.
> >
> > The grant heads are hot, so we really should be accounting this to
> > the ticket is we can, rather than adding thousands of extra grant
> > head updates every CIL commit.
> >
> > Interestingly, this actually indicates a potential log space overrun
> > can occur when we force the log. By the time that xfs_log_force()
> > pushes out an active iclog and consumes the roundoff space, the
>
> Ok I was wondering about that when I was trying to figure out what all
> this ticket space stealing code was doing.
>
> So in addition to fixing the theoretical overrun, I guess the
> performance fix here is that every time we write an iclog we might have
> to move the grant heads forward so that we always write a full log
> sector / log stripe unit? And since a CIL context might write a lot of
> iclogs, it's cheaper to make those grant adjustments to the CIL ticket
> (which already asked for enough space to handle the roundoffs) since the
> ticket only jumps in the hot path once when the ticket is ungranted?
>
> If I got that right,
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>

You got it right. :)

Thanks!

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