[PATCH] xfs/011: support byte-based grant heads are stored in bytes now

[PATCH] xfs/011: support byte-based grant heads are stored in bytes now

[Christoph Hellwig]

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>
---
 tests/xfs/011 | 67 +++++++++++++++++++++++++++++++++------------------
 1 file changed, 44 insertions(+), 23 deletions(-)

diff --git a/tests/xfs/011 b/tests/xfs/011
index ed44d074b..ef2366adf 100755
--- a/tests/xfs/011
+++ b/tests/xfs/011
@@ -11,7 +11,18 @@
 . ./common/preamble
 _begin_fstest auto freeze log metadata quick
 
-# Import common functions.
+# real QA test starts here
+_supported_fs xfs
+
+_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 +35,29 @@ _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()
+_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
+	# The grant heads record reservations in bytes.  For complex reasons
+	# beyond the scope fo this test, these aren't going to be exactly zero
+	# for frozen filesystems. Hence just check the value is between 0 and
+	# the maximum iclog size (256kB).
+	for attr in "reserve_grant_head_bytes" "write_grant_head_bytes"; do
+		space=`cat $attrprefix/$attr`
+		_within_tolerance $space 1024 0 $((256 * 1024))
+	done
+}
 
+_check_scratch_log_state_old()
+{
 	# 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`
+	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,17 +67,25 @@ _check_scratch_log_state()
 				"possible leak detected."
 		fi
 	done
-
-	xfs_freeze -u $SCRATCH_MNT
 }
 
-# real QA test starts here
-_supported_fs xfs
+# 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
 
-_require_scratch
-_require_freeze
-_require_xfs_sysfs $(_short_dev $TEST_DEV)/log
-_require_command "$KILLALL_PROG" killall
+	if [ -f "${attrprefix}/reserve_grant_head_bytes" ]; then
+	    _check_scratch_log_state_new
+	else
+	    _check_scratch_log_state_old
+	fi
+
+	xfs_freeze -u $SCRATCH_MNT
+}
 
 echo "Silence is golden."
 
-- 
2.43.0

Re: [PATCH] xfs/011: support byte-based grant heads are stored in bytes now

[Darrick J. Wong]

On Thu, Jun 20, 2024 at 09:23:09AM +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>
> ---
>  tests/xfs/011 | 67 +++++++++++++++++++++++++++++++++------------------
>  1 file changed, 44 insertions(+), 23 deletions(-)
> 
> diff --git a/tests/xfs/011 b/tests/xfs/011
> index ed44d074b..ef2366adf 100755
> --- a/tests/xfs/011
> +++ b/tests/xfs/011
> @@ -11,7 +11,18 @@
>  . ./common/preamble
>  _begin_fstest auto freeze log metadata quick
>  
> -# Import common functions.
> +# real QA test starts here
> +_supported_fs xfs
> +
> +_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 +35,29 @@ _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()
> +_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
> +	# The grant heads record reservations in bytes.  For complex reasons
> +	# beyond the scope fo this test, these aren't going to be exactly zero

                           of

Why aren't they going to be exactly zero?

--D

> +	# for frozen filesystems. Hence just check the value is between 0 and
> +	# the maximum iclog size (256kB).
> +	for attr in "reserve_grant_head_bytes" "write_grant_head_bytes"; do
> +		space=`cat $attrprefix/$attr`
> +		_within_tolerance $space 1024 0 $((256 * 1024))
> +	done
> +}
>  
> +_check_scratch_log_state_old()
> +{
>  	# 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`
> +	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,17 +67,25 @@ _check_scratch_log_state()
>  				"possible leak detected."
>  		fi
>  	done
> -
> -	xfs_freeze -u $SCRATCH_MNT
>  }
>  
> -# real QA test starts here
> -_supported_fs xfs
> +# 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
>  
> -_require_scratch
> -_require_freeze
> -_require_xfs_sysfs $(_short_dev $TEST_DEV)/log
> -_require_command "$KILLALL_PROG" killall
> +	if [ -f "${attrprefix}/reserve_grant_head_bytes" ]; then
> +	    _check_scratch_log_state_new
> +	else
> +	    _check_scratch_log_state_old
> +	fi
> +
> +	xfs_freeze -u $SCRATCH_MNT
> +}
>  
>  echo "Silence is golden."
>  
> -- 
> 2.43.0
> 
> 

Re: [PATCH] xfs/011: support byte-based grant heads are stored in bytes now

[Christoph Hellwig]

On Thu, Jun 20, 2024 at 12:56:06PM -0700, Darrick J. Wong wrote:
> > +	# The grant heads record reservations in bytes.  For complex reasons
> > +	# beyond the scope fo this test, these aren't going to be exactly zero
> 
>                            of
> 
> Why aren't they going to be exactly zero?

Given that frozen file systems always have a dirty log to force
recovery after a crash we also have space granted to it.

Re: [PATCH] xfs/011: support byte-based grant heads are stored in bytes now

[Darrick J. Wong]

On Fri, Jun 21, 2024 at 08:29:03AM +0200, Christoph Hellwig wrote:
> On Thu, Jun 20, 2024 at 12:56:06PM -0700, Darrick J. Wong wrote:
> > > +	# The grant heads record reservations in bytes.  For complex reasons
> > > +	# beyond the scope fo this test, these aren't going to be exactly zero
> > 
> >                            of
> > 
> > Why aren't they going to be exactly zero?
> 
> Given that frozen file systems always have a dirty log to force
> recovery after a crash we also have space granted to it.

Might as well say that in the comment, then.

"The grant heads record log reservations in bytes.  Frozen filesystems
always have a dirty log to force recovery, so the grant heads won't be
exactly zero..."

With something like that
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D