On Mon, Jul 15, 2024 at 08:24:53AM +0200, Christoph Hellwig wrote:
> New kernels where reservation grant track the actual reservation space
> consumed in bytes instead of LSNs in cycle/block tuples export different
> sysfs files for this information.
>
> Adapt the test to detect which version is exported, and simply check
> for a near-zero reservation space consumption for the byte based version.
>
> Based on work from Dave Chinner.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
Seems fine to me
Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx>
--D
> ---
>
> Changes since v1:
> - rebased to latests xfstests for-next
> - improve a comment based on a mail from Dave Chinner
>
> tests/xfs/011 | 77 ++++++++++++++++++++++++++++++++++++---------------
> 1 file changed, 54 insertions(+), 23 deletions(-)
>
> diff --git a/tests/xfs/011 b/tests/xfs/011
> index f9303d594..df967f098 100755
> --- a/tests/xfs/011
> +++ b/tests/xfs/011
> @@ -11,7 +11,15 @@
> . ./common/preamble
> _begin_fstest auto freeze log metadata quick
>
> -# Import common functions.
> +_require_scratch
> +_require_freeze
> +_require_xfs_sysfs $(_short_dev $TEST_DEV)/log
> +_require_command "$KILLALL_PROG" killall
> +
> +. ./common/filter
> +
> +devname=`_short_dev $SCRATCH_DEV`
> +attrprefix="/sys/fs/xfs/$devname/log"
>
> # Override the default cleanup function.
> _cleanup()
> @@ -24,27 +32,40 @@ _cleanup()
> rm -f $tmp.*
> }
>
> -# Use the information exported by XFS to sysfs to determine whether the log has
> -# active reservations after a filesystem freeze.
> -_check_scratch_log_state()
> +#
> +# The grant heads record reservations in bytes.
> +#
> +# The value is not exactly zero for complex reason. In short: we must always
> +# have space for at least one minimum sized log write between ailp->ail_head_lsn
> +# and log->l_tail_lsn, and that is what is showing up in the grant head
> +# reservation values. We don't need to explicitly reserve it for the first
> +# iclog write after mount, but we always end up with it being present after the
> +# first checkpoint commits and the AIL returns the checkpoint's unused space
> +# back to the grant head.
> +#
> +# Hence just check the value is between 0 and the maximum iclog size (256kB).
> +#
> +_check_scratch_log_state_new()
> {
> - devname=`_short_dev $SCRATCH_DEV`
> - attrpath="/sys/fs/xfs/$devname/log"
> -
> - # freeze the fs to ensure data is synced and the log is flushed. this
> - # means no outstanding transactions, and thus no outstanding log
> - # reservations, should exist
> - xfs_freeze -f $SCRATCH_MNT
> + for attr in "reserve_grant_head_bytes" "write_grant_head_bytes"; do
> + space=`cat $attrprefix/$attr`
> + _within_tolerance $space 1024 0 $((256 * 1024))
> + done
> +}
>
> - # the log head is exported in basic blocks and the log grant heads in
> - # bytes. convert the log head to bytes for precise comparison
> - log_head_cycle=`awk -F : '{ print $1 }' $attrpath/log_head_lsn`
> - log_head_bytes=`awk -F : '{ print $2 }' $attrpath/log_head_lsn`
> +#
> +# The log head is exported in basic blocks and the log grant heads in bytes.
> +# Convert the log head to bytes for precise comparison.
> +#
> +_check_scratch_log_state_old()
> +{
> + log_head_cycle=`awk -F : '{ print $1 }' $attrprefix/log_head_lsn`
> + log_head_bytes=`awk -F : '{ print $2 }' $attrprefix/log_head_lsn`
> log_head_bytes=$((log_head_bytes * 512))
>
> for attr in "reserve_grant_head" "write_grant_head"; do
> - cycle=`cat $attrpath/$attr | awk -F : '{ print $1 }'`
> - bytes=`cat $attrpath/$attr | awk -F : '{ print $2 }'`
> + cycle=`cat $attrprefix/$attr | awk -F : '{ print $1 }'`
> + bytes=`cat $attrprefix/$attr | awk -F : '{ print $2 }'`
>
> if [ $cycle != $log_head_cycle ] ||
> [ $bytes != $log_head_bytes ]
> @@ -54,15 +75,25 @@ _check_scratch_log_state()
> "possible leak detected."
> fi
> done
> -
> - xfs_freeze -u $SCRATCH_MNT
> }
>
> +# Use the information exported by XFS to sysfs to determine whether the log has
> +# active reservations after a filesystem freeze.
> +_check_scratch_log_state()
> +{
> + # freeze the fs to ensure data is synced and the log is flushed. this
> + # means no outstanding transactions, and thus no outstanding log
> + # reservations, should exist
> + xfs_freeze -f $SCRATCH_MNT
> +
> + if [ -f "${attrprefix}/reserve_grant_head_bytes" ]; then
> + _check_scratch_log_state_new
> + else
> + _check_scratch_log_state_old
> + fi
>
> -_require_scratch
> -_require_freeze
> -_require_xfs_sysfs $(_short_dev $TEST_DEV)/log
> -_require_command "$KILLALL_PROG" killall
> + xfs_freeze -u $SCRATCH_MNT
> +}
>
> echo "Silence is golden."
>
> --
> 2.43.0
>
>
