* [PATCH 0/2] Fix test failures caused by storage devcies with 4k sectors
@ 2023-10-22 21:55 Theodore Ts'o
2023-10-22 21:55 ` [PATCH 1/2] common: check if the scratch device can support 1024 block sizes Theodore Ts'o
2023-10-22 21:55 ` [PATCH 2/2] generic/563: create the loop dev with the same block size as the scratch dev Theodore Ts'o
0 siblings, 2 replies; 6+ messages in thread
From: Theodore Ts'o @ 2023-10-22 21:55 UTC (permalink / raw)
To: fstests; +Cc: Theodore Ts'o
When testing with an SSD with a 4k logical sector size, I ran into a
number of test failures that were caused by assumption that the
storage device can support 1k block sizes.
Fix this by skipping those tests, or in the case of generic/563, make
sure the loop device has the same block size as the backing scratch
device. (Arguably losetup should do that, but at least today, it
doesn't.)
This test series was tested using:
gce-xfstests --local-ssd-nvme -c ext4,xfs,btrfs -g auto
and comparing it against the results of running the same set of tests
without the --local-ssd-nvme option, which introduces the use of a 4k
sector storage device.
With these patches applied, there is one remaining failure with
xfs/157 but I'm not sure how to deal with it, since Google searches
regarding the failure message just simply say "Don't try to use a
regular file as a logdev", which isn't particularly helpful here.
Maybe the right thing is to just hard code a _notrun if "blockdev
--getss $SCRATCH_DEV" is not 512? It's not clear to me, so I've left
it.
I also noted two new failures with btrfs, generic/175 and generic/251.
The cause of why these tests are failing with a 4k sector device is
not obvious to me. But certainly things are much better with this
patch, and perhaps the the btrfs and xfs developers can address these
last new test failures if they care about this particular test
scenario.
Theodore Ts'o (2):
common: check if the scratch device can support 1024 block sizes
generic/563: create the loop dev with the same block size as the
scratch dev
common/rc | 22 ++++++++++++++++++++--
tests/ext4/055 | 1 +
tests/generic/563 | 2 +-
tests/xfs/205 | 1 +
tests/xfs/432 | 1 +
tests/xfs/516 | 1 +
6 files changed, 25 insertions(+), 3 deletions(-)
--
2.31.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] common: check if the scratch device can support 1024 block sizes
2023-10-22 21:55 [PATCH 0/2] Fix test failures caused by storage devcies with 4k sectors Theodore Ts'o
@ 2023-10-22 21:55 ` Theodore Ts'o
2023-10-23 15:45 ` Darrick J. Wong
2023-10-22 21:55 ` [PATCH 2/2] generic/563: create the loop dev with the same block size as the scratch dev Theodore Ts'o
1 sibling, 1 reply; 6+ messages in thread
From: Theodore Ts'o @ 2023-10-22 21:55 UTC (permalink / raw)
To: fstests; +Cc: Theodore Ts'o
If the scratch device has as logical blocksize larger than 512 --- for
example, some SSD or HDD's may have a 4k logical blocksize, and so
will not support a file system with a 1k block size.
Add a new function, _require_scratch_support_blocksize so we can skip
tests that use _scratch_mkfs_blocksized with a size less than the
scratch device's logical block size.
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
common/rc | 12 ++++++++++++
tests/ext4/055 | 1 +
tests/xfs/205 | 1 +
tests/xfs/432 | 1 +
tests/xfs/516 | 1 +
5 files changed, 16 insertions(+)
diff --git a/common/rc b/common/rc
index 7f5a9527c..8d7179567 100644
--- a/common/rc
+++ b/common/rc
@@ -1124,6 +1124,9 @@ _scratch_mkfs_blocksized()
if [ $blocksize -lt $(_get_page_size) ]; then
_exclude_scratch_mount_option dax
fi
+ if [ $blocksize -lt $(blockdev --getss $SCRATCH_DEV) ]; then
+ _require_scratch_support_blocksize "$blocksize"
+ fi
case $FSTYP in
btrfs)
@@ -4452,6 +4455,15 @@ _get_device_size()
echo $(($(blockdev --getsz $1) >> 1))
}
+_require_scratch_support_blocksize()
+{
+ local blocksize=$1
+
+ if [ $blocksize -lt $(blockdev --getss $SCRATCH_DEV) ]; then
+ _notrun "$SCRATCH_DEV does not support a block size of $blocksize."
+ fi
+}
+
# Make sure we actually have dmesg checking set up.
_require_check_dmesg()
{
diff --git a/tests/ext4/055 b/tests/ext4/055
index aa15cfe98..7025f6283 100755
--- a/tests/ext4/055
+++ b/tests/ext4/055
@@ -27,6 +27,7 @@ echo "Silence is golden"
# The 1K blocksize is designed for debugfs.
_exclude_scratch_mount_option dax
+_require_scratch_support_blocksize 1024
_scratch_mkfs "-F -O quota -b 1024" > $seqres.full 2>&1
# Start from 0, fill block 1 with 6，replace the original 2.
diff --git a/tests/xfs/205 b/tests/xfs/205
index 104f1f45a..84c099208 100755
--- a/tests/xfs/205
+++ b/tests/xfs/205
@@ -23,6 +23,7 @@ _require_scratch_nocheck
unset SCRATCH_RTDEV
fsblksz=1024
+_require_scratch_support_blocksize $fsblksz
_scratch_mkfs_xfs -d size=$((32768*fsblksz)) -b size=$fsblksz >> $seqres.full 2>&1
_scratch_mount
diff --git a/tests/xfs/432 b/tests/xfs/432
index 66315b039..2efa6230b 100755
--- a/tests/xfs/432
+++ b/tests/xfs/432
@@ -50,6 +50,7 @@ echo "Format and mount"
# block. 8187 hashes/dablk / 248 dirents/dirblock = ~33 dirblocks per
# dablock. 33 dirblocks * 64k mean that we can expand a directory by
# 2112k before we have to allocate another da btree block.
+_require_scratch_support_blocksize 1024
_scratch_mkfs -b size=1k -n size=64k > "$seqres.full" 2>&1
_scratch_mount >> "$seqres.full" 2>&1
diff --git a/tests/xfs/516 b/tests/xfs/516
index 9e1b99317..65fc635dd 100755
--- a/tests/xfs/516
+++ b/tests/xfs/516
@@ -23,6 +23,7 @@ _cleanup()
# real QA test starts here
_supported_fs xfs
_require_scratch_nocheck
+_require_scratch_support_blocksize 1024
# Assume that if we can run scrub on the test dev we can run it on the scratch
# fs too.
--
2.31.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] generic/563: create the loop dev with the same block size as the scratch dev
2023-10-22 21:55 [PATCH 0/2] Fix test failures caused by storage devcies with 4k sectors Theodore Ts'o
2023-10-22 21:55 ` [PATCH 1/2] common: check if the scratch device can support 1024 block sizes Theodore Ts'o
@ 2023-10-22 21:55 ` Theodore Ts'o
2023-10-23 15:54 ` Darrick J. Wong
1 sibling, 1 reply; 6+ messages in thread
From: Theodore Ts'o @ 2023-10-22 21:55 UTC (permalink / raw)
To: fstests; +Cc: Theodore Ts'o
The generic/563 test creates the loop device using $SCRATCH_DEV
directly. We need to create the loop device with the same logical
block size. Otherwise, the loop device will always be created with
the default logical block size of 512, and if its underlying backing
store has a different logical block size, then mkfs may create a file
system in the loop device that will fail to mount.
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
common/rc | 10 ++++++++--
tests/generic/563 | 2 +-
2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/common/rc b/common/rc
index 8d7179567..01f065a9f 100644
--- a/common/rc
+++ b/common/rc
@@ -4246,9 +4246,15 @@ _require_userns()
_create_loop_device()
{
- local file=$1 dev
- dev=`losetup -f --show $file` || _fail "Cannot assign $file to a loop device"
+ local file=$1
+ local blocksize=$2
+ local dev
+
+ if [ -n "$blocksize" ]; then
+ blocksize="-b $blocksize"
+ fi
+ dev=`losetup -f $blocksize --show $file` || _fail "Cannot assign $file to a loop device"
# Try to enable asynchronous directio mode on the loopback device so
# that writeback started by a filesystem mounted on the loop device
# won't be throttled by buffered writes to the lower filesystem. This
diff --git a/tests/generic/563 b/tests/generic/563
index f98c6e42b..7e6bab49e 100755
--- a/tests/generic/563
+++ b/tests/generic/563
@@ -89,7 +89,7 @@ reset()
# cgroup I/O accounting doesn't work on partitions. Use a loop device to rule
# that out.
-LOOP_DEV=$(_create_loop_device $SCRATCH_DEV)
+LOOP_DEV=$(_create_loop_device $SCRATCH_DEV $(blockdev --getss $SCRATCH_DEV))
smajor=$((0x`stat -L -c %t $LOOP_DEV`))
sminor=$((0x`stat -L -c %T $LOOP_DEV`))
--
2.31.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] common: check if the scratch device can support 1024 block sizes
2023-10-22 21:55 ` [PATCH 1/2] common: check if the scratch device can support 1024 block sizes Theodore Ts'o
@ 2023-10-23 15:45 ` Darrick J. Wong
2023-10-23 19:48 ` Theodore Ts'o
0 siblings, 1 reply; 6+ messages in thread
From: Darrick J. Wong @ 2023-10-23 15:45 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: fstests
On Sun, Oct 22, 2023 at 05:55:28PM -0400, Theodore Ts'o wrote:
> If the scratch device has as logical blocksize larger than 512 --- for
> example, some SSD or HDD's may have a 4k logical blocksize, and so
> will not support a file system with a 1k block size.
>
> Add a new function, _require_scratch_support_blocksize so we can skip
> tests that use _scratch_mkfs_blocksized with a size less than the
> scratch device's logical block size.
>
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> ---
> common/rc | 12 ++++++++++++
> tests/ext4/055 | 1 +
> tests/xfs/205 | 1 +
> tests/xfs/432 | 1 +
> tests/xfs/516 | 1 +
> 5 files changed, 16 insertions(+)
>
> diff --git a/common/rc b/common/rc
> index 7f5a9527c..8d7179567 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -1124,6 +1124,9 @@ _scratch_mkfs_blocksized()
> if [ $blocksize -lt $(_get_page_size) ]; then
> _exclude_scratch_mount_option dax
> fi
> + if [ $blocksize -lt $(blockdev --getss $SCRATCH_DEV) ]; then
> + _require_scratch_support_blocksize "$blocksize"
> + fi
This duplicates the logic in _require_scratch_support_blocksize, so I
think you can drop it.
>
> case $FSTYP in
> btrfs)
> @@ -4452,6 +4455,15 @@ _get_device_size()
> echo $(($(blockdev --getsz $1) >> 1))
> }
>
> +_require_scratch_support_blocksize()
> +{
> + local blocksize=$1
> +
> + if [ $blocksize -lt $(blockdev --getss $SCRATCH_DEV) ]; then
> + _notrun "$SCRATCH_DEV does not support a block size of $blocksize."
"block" is a bit vague in this context -- you mean the LBA size, not the
internal physical block size, right?
May I suggest "...does not support an LBA size of $blocksize." ?
--D
> + fi
> +}
> +
> # Make sure we actually have dmesg checking set up.
> _require_check_dmesg()
> {
> diff --git a/tests/ext4/055 b/tests/ext4/055
> index aa15cfe98..7025f6283 100755
> --- a/tests/ext4/055
> +++ b/tests/ext4/055
> @@ -27,6 +27,7 @@ echo "Silence is golden"
>
> # The 1K blocksize is designed for debugfs.
> _exclude_scratch_mount_option dax
> +_require_scratch_support_blocksize 1024
> _scratch_mkfs "-F -O quota -b 1024" > $seqres.full 2>&1
>
> # Start from 0, fill block 1 with 6，replace the original 2.
> diff --git a/tests/xfs/205 b/tests/xfs/205
> index 104f1f45a..84c099208 100755
> --- a/tests/xfs/205
> +++ b/tests/xfs/205
> @@ -23,6 +23,7 @@ _require_scratch_nocheck
> unset SCRATCH_RTDEV
>
> fsblksz=1024
> +_require_scratch_support_blocksize $fsblksz
> _scratch_mkfs_xfs -d size=$((32768*fsblksz)) -b size=$fsblksz >> $seqres.full 2>&1
> _scratch_mount
>
> diff --git a/tests/xfs/432 b/tests/xfs/432
> index 66315b039..2efa6230b 100755
> --- a/tests/xfs/432
> +++ b/tests/xfs/432
> @@ -50,6 +50,7 @@ echo "Format and mount"
> # block. 8187 hashes/dablk / 248 dirents/dirblock = ~33 dirblocks per
> # dablock. 33 dirblocks * 64k mean that we can expand a directory by
> # 2112k before we have to allocate another da btree block.
> +_require_scratch_support_blocksize 1024
> _scratch_mkfs -b size=1k -n size=64k > "$seqres.full" 2>&1
> _scratch_mount >> "$seqres.full" 2>&1
>
> diff --git a/tests/xfs/516 b/tests/xfs/516
> index 9e1b99317..65fc635dd 100755
> --- a/tests/xfs/516
> +++ b/tests/xfs/516
> @@ -23,6 +23,7 @@ _cleanup()
> # real QA test starts here
> _supported_fs xfs
> _require_scratch_nocheck
> +_require_scratch_support_blocksize 1024
>
> # Assume that if we can run scrub on the test dev we can run it on the scratch
> # fs too.
> --
> 2.31.0
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] generic/563: create the loop dev with the same block size as the scratch dev
2023-10-22 21:55 ` [PATCH 2/2] generic/563: create the loop dev with the same block size as the scratch dev Theodore Ts'o
@ 2023-10-23 15:54 ` Darrick J. Wong
0 siblings, 0 replies; 6+ messages in thread
From: Darrick J. Wong @ 2023-10-23 15:54 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: fstests
On Sun, Oct 22, 2023 at 05:55:29PM -0400, Theodore Ts'o wrote:
> The generic/563 test creates the loop device using $SCRATCH_DEV
> directly. We need to create the loop device with the same logical
> block size. Otherwise, the loop device will always be created with
> the default logical block size of 512, and if its underlying backing
> store has a different logical block size, then mkfs may create a file
> system in the loop device that will fail to mount.
>
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Looks fine to me,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
--D
> ---
> common/rc | 10 ++++++++--
> tests/generic/563 | 2 +-
> 2 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/common/rc b/common/rc
> index 8d7179567..01f065a9f 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -4246,9 +4246,15 @@ _require_userns()
>
> _create_loop_device()
> {
> - local file=$1 dev
> - dev=`losetup -f --show $file` || _fail "Cannot assign $file to a loop device"
> + local file=$1
> + local blocksize=$2
> + local dev
> +
> + if [ -n "$blocksize" ]; then
> + blocksize="-b $blocksize"
> + fi
>
> + dev=`losetup -f $blocksize --show $file` || _fail "Cannot assign $file to a loop device"
> # Try to enable asynchronous directio mode on the loopback device so
> # that writeback started by a filesystem mounted on the loop device
> # won't be throttled by buffered writes to the lower filesystem. This
> diff --git a/tests/generic/563 b/tests/generic/563
> index f98c6e42b..7e6bab49e 100755
> --- a/tests/generic/563
> +++ b/tests/generic/563
> @@ -89,7 +89,7 @@ reset()
>
> # cgroup I/O accounting doesn't work on partitions. Use a loop device to rule
> # that out.
> -LOOP_DEV=$(_create_loop_device $SCRATCH_DEV)
> +LOOP_DEV=$(_create_loop_device $SCRATCH_DEV $(blockdev --getss $SCRATCH_DEV))
> smajor=$((0x`stat -L -c %t $LOOP_DEV`))
> sminor=$((0x`stat -L -c %T $LOOP_DEV`))
>
> --
> 2.31.0
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] common: check if the scratch device can support 1024 block sizes
2023-10-23 15:45 ` Darrick J. Wong
@ 2023-10-23 19:48 ` Theodore Ts'o
0 siblings, 0 replies; 6+ messages in thread
From: Theodore Ts'o @ 2023-10-23 19:48 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: fstests
On Mon, Oct 23, 2023 at 08:45:13AM -0700, Darrick J. Wong wrote:
> > + if [ $blocksize -lt $(blockdev --getss $SCRATCH_DEV) ]; then
> > + _require_scratch_support_blocksize "$blocksize"
> > + fi
>
> This duplicates the logic in _require_scratch_support_blocksize, so I
> think you can drop it.
Yes, oops. I'll drop the if statement.
> > +_require_scratch_support_blocksize()
> > +{
> > + local blocksize=$1
> > +
> > + if [ $blocksize -lt $(blockdev --getss $SCRATCH_DEV) ]; then
> > + _notrun "$SCRATCH_DEV does not support a block size of $blocksize."
>
> "block" is a bit vague in this context -- you mean the LBA size, not the
> internal physical block size, right?
>
> May I suggest "...does not support an LBA size of $blocksize." ?
How about "does not support a file system block size of $blocksize"?
LBA size refers to the granularity of "logical block address" for a
particular block device, which would be confusing and not quite right
here.
- Ted
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-10-23 19:48 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-22 21:55 [PATCH 0/2] Fix test failures caused by storage devcies with 4k sectors Theodore Ts'o
2023-10-22 21:55 ` [PATCH 1/2] common: check if the scratch device can support 1024 block sizes Theodore Ts'o
2023-10-23 15:45 ` Darrick J. Wong
2023-10-23 19:48 ` Theodore Ts'o
2023-10-22 21:55 ` [PATCH 2/2] generic/563: create the loop dev with the same block size as the scratch dev Theodore Ts'o
2023-10-23 15:54 ` Darrick J. Wong
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox