| Message ID | 20240715062522.593299-1-hch@lst.de (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | [v2] xfs/011: support byte-based grant heads are stored in bytes now | expand |
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@lst.de> Seems fine to me Reviewed-by: Darrick J. Wong <djwong@kernel.org> --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 > >
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."
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@lst.de> --- 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(-)