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

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:

quoted

From: Dave Chinner <redacted>

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