Re: [PATCH 10/11] xfs: grant heads track byte counts, not LSNs

[Dave Chinner <david@fromorbit.com>]

On Thu, Jun 20, 2024 at 09:21:27AM +0200, Christoph Hellwig wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> The grant heads in the log track the space reserved in the log for
> running transactions. They do this by tracking how far ahead of the
> tail that the reservation has reached, and the units for doing this
> are {cycle,bytes} for the reserve head rather than {cycle,blocks}
> which are normal used by LSNs.
> 
> This is annoyingly complex because we have to split, crack and
> combined these tuples for any calculation we do to determine log
> space and targets. This is computationally expensive as well as
> difficult to do atomically and locklessly, as well as limiting the
> size of the log to 2^32 bytes.
> 
> Really, though, all the grant heads are tracking is how much space
> is currently available for use in the log. We can track this as a
> simply byte count - we just don't care what the actual physical
> location in the log the head and tail are at, just how much space we
> have remaining before the head and tail overlap.
> 
> So, convert the grant heads to track the byte reservations that are
> active rather than the current (cycle, offset) tuples. This means an
> empty log has zero bytes consumed, and a full log is when the
> reservations reach the size of the log minus the space consumed by
> the AIL.
> 
> This greatly simplifies the accounting and checks for whether there
> is space available. We no longer need to crack or combine LSNs to
> determine how much space the log has left, nor do we need to look at
> the head or tail of the log to determine how close to full we are.
> 
> There is, however, a complexity that needs to be handled. We know
> how much space is being tracked in the AIL now via log->l_tail_space
> and the log tickets track active reservations and return the unused
> portions to the grant heads when ungranted.  Unfortunately, we don't
> track the used portion of the grant, so when we transfer log items
> from the CIL to the AIL, the space accounted to the grant heads is
> transferred to the log tail space.  Hence when we move the AIL head
> forwards on item insert, we have to remove that space from the grant
> heads.
> 
> We also remove the xlog_verify_grant_tail() debug function as it is
> no longer useful. The check it performs has been racy since delayed
> logging was introduced, but now it is clearly only detecting false
> positives so remove it.
> 
> The result of this substantially simpler accounting algorithm is an
> increase in sustained transaction rate from ~1.3 million
> transactions/s to ~1.9 million transactions/s with no increase in
> CPU usage. We also remove the 32 bit space limitation on the grant
> heads, which will allow us to increase the journal size beyond 2GB
> in future.
> 
> Note that this renames the sysfs files exposing the log grant space
> now that the values are exported in bytes.  This allows xfstests
> to auto-detect the old or new ABI.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> [hch: move xlog_grant_sub_space out of line,
>       update the xlog_grant_{add,sub}_space prototypes,
>       rename the sysfs files to allow auto-detection in xfstests]
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Updates look fine.

Reviewed-by: Dave Chinner <dchinner@redhat.com>
-- 
Dave Chinner
david@fromorbit.com