All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <djwong@kernel.org>
To: djwong@kernel.org, zlang@redhat.com
Cc: linux-xfs@vger.kernel.org, fstests@vger.kernel.org, guan@eryu.me
Subject: [PATCH 1/3] generic/{251,260}: compute maximum fitrim offset
Date: Tue, 28 Mar 2023 17:58:10 -0700	[thread overview]
Message-ID: <168005149047.4147931.2729971759269213680.stgit@frogsfrogsfrogs> (raw)
In-Reply-To: <168005148468.4147931.1986862498548445502.stgit@frogsfrogsfrogs>

From: Darrick J. Wong <djwong@kernel.org>

FITRIM is a bizarre ioctl.  Callers are allowed to pass in "start" and
"length" parameters, which are clearly some kind of range argument.  No
means is provided to discover the minimum or maximum range.  Although
regular userspace programs default to (start=0, length=-1ULL), this test
tries to exercise different parameters.

However, the test assumes that the "size" column returned by the df
command is the maximum value supported by the FITRIM command, and is
surprised if the number of bytes trimmed by (start=0, length=-1ULL) is
larger than this size quantity.

This is completely wrong on XFS with realtime volumes, because the
statfs output (which is what df reports) will reflect the realtime
volume if the directory argument is a realtime file or a directory
flagged with rtinherit.  This is trivially reproducible by configuring a
rt volume that is much larger than the data volume, setting rtinherit on
the root dir at mkfs time, and running either of these tests.

Refactor the open-coded df logic so that we can determine the value
programmatically for XFS.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 common/rc         |   15 +++++++++++++++
 common/xfs        |   11 +++++++++++
 tests/generic/251 |    2 +-
 tests/generic/260 |    2 +-
 4 files changed, 28 insertions(+), 2 deletions(-)


diff --git a/common/rc b/common/rc
index 90749343f3..228982fa4d 100644
--- a/common/rc
+++ b/common/rc
@@ -3927,6 +3927,21 @@ _require_batched_discard()
 	fi
 }
 
+# Given a mountpoint and the device associated with that mountpoint, return the
+# maximum start offset that the FITRIM command will accept, in units of 1024
+# byte blocks.
+_discard_max_offset_kb()
+{
+	case "$FSTYP" in
+	xfs)
+		_xfs_discard_max_offset_kb "$1"
+		;;
+	*)
+		$DF_PROG -k | grep "$1" | grep "$2" | awk '{print $3}'
+		;;
+	esac
+}
+
 _require_dumpe2fs()
 {
 	if [ -z "$DUMPE2FS_PROG" ]; then
diff --git a/common/xfs b/common/xfs
index e8e4832cea..a6c82fc6c7 100644
--- a/common/xfs
+++ b/common/xfs
@@ -1783,3 +1783,14 @@ _require_xfs_scratch_atomicswap()
 		_notrun "atomicswap dependencies not supported by scratch filesystem type: $FSTYP"
 	_scratch_unmount
 }
+
+# Return the maximum start offset that the FITRIM command will accept, in units
+# of 1024 byte blocks.
+_xfs_discard_max_offset_kb()
+{
+	local path="$1"
+	local blksz="$($XFS_IO_PROG -c 'statfs' "$path" | grep "geom.bsize" | cut -d ' ' -f 3)"
+	local dblks="$($XFS_IO_PROG -c 'statfs' "$path" | grep "geom.datablocks" | cut -d ' ' -f 3)"
+
+	echo "$((dblks * blksz / 1024))"
+}
diff --git a/tests/generic/251 b/tests/generic/251
index 2a271cd126..8ee74980cc 100755
--- a/tests/generic/251
+++ b/tests/generic/251
@@ -71,7 +71,7 @@ _guess_max_minlen()
 fstrim_loop()
 {
 	trap "_destroy_fstrim; exit \$status" 2 15
-	fsize=$($DF_PROG | grep $SCRATCH_MNT | grep $SCRATCH_DEV  | awk '{print $3}')
+	fsize=$(_discard_max_offset_kb "$SCRATCH_MNT" "$SCRATCH_DEV")
 	mmlen=$(_guess_max_minlen)
 
 	while true ; do
diff --git a/tests/generic/260 b/tests/generic/260
index 2f653b4af2..08fde46873 100755
--- a/tests/generic/260
+++ b/tests/generic/260
@@ -27,7 +27,7 @@ _scratch_mount
 
 _require_batched_discard $SCRATCH_MNT
 
-fssize=$($DF_PROG -k | grep "$SCRATCH_MNT" | grep "$SCRATCH_DEV"  | awk '{print $3}')
+fssize=$(_discard_max_offset_kb "$SCRATCH_MNT" "$SCRATCH_DEV")
 
 beyond_eofs=$(_math "$fssize*2048")
 max_64bit=$(_math "2^64 - 1")


  reply	other threads:[~2023-03-29  0:58 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-29  0:58 [PATCHSET 0/3] fstests: random fixes for v2023.03.26 Darrick J. Wong
2023-03-29  0:58 ` Darrick J. Wong [this message]
2023-03-29 10:39   ` [PATCH 1/3] generic/{251,260}: compute maximum fitrim offset David Disseldorp
2023-03-30  2:14     ` Darrick J. Wong
2023-03-30 16:46   ` [PATCH v1.1 " Darrick J. Wong
2023-03-29  0:58 ` [PATCH 2/3] xfs/242: fix _filter_bmap for xfs_io bmap that does rt file properly Darrick J. Wong
2023-03-29 11:02   ` David Disseldorp
2023-03-29  0:58 ` [PATCH 3/3] common/report: fix typo in FSSTRESS_AVOID Darrick J. Wong
2023-03-29 11:02   ` David Disseldorp

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=168005149047.4147931.2729971759269213680.stgit@frogsfrogsfrogs \
    --to=djwong@kernel.org \
    --cc=fstests@vger.kernel.org \
    --cc=guan@eryu.me \
    --cc=linux-xfs@vger.kernel.org \
    --cc=zlang@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.