FSTests Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH V2 0/3] fstests: fixes for 64k pages and dax
@ 2020-02-20 20:06 Jeff Moyer
  2020-02-20 20:06 ` [PATCH V2 1/3] dax/dm: disable testing on devices that don't support dax Jeff Moyer
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Jeff Moyer @ 2020-02-20 20:06 UTC (permalink / raw)
  To: fstests; +Cc: linux-fsdevel, linux-xfs

This set of patches fixes a few false positives I encountered when
testing DAX on ppc64le (which has a 64k page size).

Patch 1 is actually not specific to non-4k page sizes.  Currently,
each individual dm rc file checks for the presence of the DAX mount
option, and _notruns the test as part of the initializtion.  This
doesn't work for the snapshot target.  Moving the check into the
_require_dm_target fixes the problem, and keeps from the cut-n-paste
of boilerplate.

Patches 2 and 3 get rid of hard coded block/page sizes in the tests.
They run just fine on 64k pages and 64k block sizes.

Even after these patches, there are many more tests that fail in the
following configuration:

MKFS_OPTIONS="-b size=65536 -m reflink=0" MOUNT_OPTIONS="-o dax"

One class of failures is tests that create a really small file system
size.  Some of those tests seem to require the very small size, but
others seem like they could live with a slightly bigger size that
would then fit the log (the typical failure is a mkfs failure due to
not enough blocks for the log).  For the former case, I'm tempted to
send patches to _notrun those tests, and for the latter, I'd like to
bump the file system sizes up.  300MB seems to be large enough to
accommodate the log.  Would folks be opposed to those approaches?

Another class of failure is tests that either hard-code a block size
to trigger a specific error case, or that test a multitude of block
sizes.  I'd like to send a patch to _notrun those tests if there is
a user-specified block size.  That will require parsing the MKFS_OPTIONS
based on the fs type, of course.  Is that something that seems
reasonable?

I will follow up with a series of patches to implement those changes
if there is consensus on the approach.  These first three seemed
straight-forward to me, so that's where I'm starting.

Changes in v2:
- patch 2: remove the boilerplate from all dm rc files (Zorro Lang)
- cc fstests (thanks, Dave)

[PATCH V2 1/3] dax/dm: disable testing on devices that don't support
[PATCH V2 2/3] t_mmap_collision: fix hard-coded page size
[PATCH V2 3/3] xfs/300: modify test to work on any fs block size


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH V2 1/3] dax/dm: disable testing on devices that don't support dax
  2020-02-20 20:06 [PATCH V2 0/3] fstests: fixes for 64k pages and dax Jeff Moyer
@ 2020-02-20 20:06 ` Jeff Moyer
  2020-02-21  9:48   ` Zorro Lang
  2020-02-20 20:06 ` [PATCH V2 2/3] t_mmap_collision: fix hard-coded page size Jeff Moyer
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 13+ messages in thread
From: Jeff Moyer @ 2020-02-20 20:06 UTC (permalink / raw)
  To: fstests; +Cc: linux-fsdevel, linux-xfs, Jeff Moyer

Move the check for dax from the individual target scripts into
_require_dm_target.  This fixes up a couple of missed tests that are
failing due to the lack of dax support (such as tests requiring
dm-snapshot).

Signed-off-by: Jeff Moyer <jmoyer@redhat.com>
---
 common/dmdelay  |  5 -----
 common/dmerror  |  5 -----
 common/dmflakey |  5 -----
 common/dmthin   |  5 -----
 common/rc       | 11 +++++++++++
 5 files changed, 11 insertions(+), 20 deletions(-)

diff --git a/common/dmdelay b/common/dmdelay
index f1e725b9..66cac1a7 100644
--- a/common/dmdelay
+++ b/common/dmdelay
@@ -7,11 +7,6 @@
 DELAY_NONE=0
 DELAY_READ=1
 
-echo $MOUNT_OPTIONS | grep -q dax
-if [ $? -eq 0 ]; then
-	_notrun "Cannot run tests with DAX on dmdelay devices"
-fi
-
 _init_delay()
 {
 	local BLK_DEV_SIZE=`blockdev --getsz $SCRATCH_DEV`
diff --git a/common/dmerror b/common/dmerror
index 426f1e96..7d12e0a1 100644
--- a/common/dmerror
+++ b/common/dmerror
@@ -4,11 +4,6 @@
 #
 # common functions for setting up and tearing down a dmerror device
 
-echo $MOUNT_OPTIONS | grep -q dax
-if [ $? -eq 0 ]; then
-	_notrun "Cannot run tests with DAX on dmerror devices"
-fi
-
 _dmerror_setup()
 {
 	local dm_backing_dev=$SCRATCH_DEV
diff --git a/common/dmflakey b/common/dmflakey
index 2af3924d..b4e11ae9 100644
--- a/common/dmflakey
+++ b/common/dmflakey
@@ -8,11 +8,6 @@ FLAKEY_ALLOW_WRITES=0
 FLAKEY_DROP_WRITES=1
 FLAKEY_ERROR_WRITES=2
 
-echo $MOUNT_OPTIONS | grep -q dax
-if [ $? -eq 0 ]; then
-	_notrun "Cannot run tests with DAX on dmflakey devices"
-fi
-
 _init_flakey()
 {
 	local BLK_DEV_SIZE=`blockdev --getsz $SCRATCH_DEV`
diff --git a/common/dmthin b/common/dmthin
index 7946e9a7..61dd6f89 100644
--- a/common/dmthin
+++ b/common/dmthin
@@ -21,11 +21,6 @@ DMTHIN_POOL_DEV="/dev/mapper/$DMTHIN_POOL_NAME"
 DMTHIN_VOL_NAME="thin-vol"
 DMTHIN_VOL_DEV="/dev/mapper/$DMTHIN_VOL_NAME"
 
-echo $MOUNT_OPTIONS | grep -q dax
-if [ $? -eq 0 ]; then
-	_notrun "Cannot run tests with DAX on dmthin devices"
-fi
-
 _dmthin_cleanup()
 {
 	$UMOUNT_PROG $SCRATCH_MNT > /dev/null 2>&1
diff --git a/common/rc b/common/rc
index eeac1355..65cde32b 100644
--- a/common/rc
+++ b/common/rc
@@ -1874,6 +1874,17 @@ _require_dm_target()
 	_require_sane_bdev_flush $SCRATCH_DEV
 	_require_command "$DMSETUP_PROG" dmsetup
 
+	echo $MOUNT_OPTIONS | grep -q dax
+	if [ $? -eq 0 ]; then
+		case $target in
+		stripe|linear|log-writes)
+			;;
+		*)
+			_notrun "Cannot run tests with DAX on $target devices."
+			;;
+		esac
+	fi
+
 	modprobe dm-$target >/dev/null 2>&1
 
 	$DMSETUP_PROG targets 2>&1 | grep -q ^$target
-- 
2.19.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH V2 2/3] t_mmap_collision: fix hard-coded page size
  2020-02-20 20:06 [PATCH V2 0/3] fstests: fixes for 64k pages and dax Jeff Moyer
  2020-02-20 20:06 ` [PATCH V2 1/3] dax/dm: disable testing on devices that don't support dax Jeff Moyer
@ 2020-02-20 20:06 ` Jeff Moyer
  2020-02-21 13:53   ` Zorro Lang
  2020-02-20 20:06 ` [PATCH V2 3/3] xfs/300: modify test to work on any fs block size Jeff Moyer
  2020-02-20 21:21 ` [PATCH V2 0/3] fstests: fixes for 64k pages and dax Darrick J. Wong
  3 siblings, 1 reply; 13+ messages in thread
From: Jeff Moyer @ 2020-02-20 20:06 UTC (permalink / raw)
  To: fstests; +Cc: linux-fsdevel, linux-xfs, Jeff Moyer

Fix the test to run on non-4k page size systems.

Signed-off-by: Jeff Moyer <jmoyer@redhat.com>
---
 src/t_mmap_collision.c | 40 +++++++++++++++++++++-------------------
 1 file changed, 21 insertions(+), 19 deletions(-)

diff --git a/src/t_mmap_collision.c b/src/t_mmap_collision.c
index d547bc05..c872f4e2 100644
--- a/src/t_mmap_collision.c
+++ b/src/t_mmap_collision.c
@@ -25,13 +25,12 @@
 #include <sys/types.h>
 #include <unistd.h>
 
-#define PAGE(a) ((a)*0x1000)
-#define FILE_SIZE PAGE(4)
-
 void *dax_data;
 int nodax_fd;
 int dax_fd;
 bool done;
+static int pagesize;
+static int file_size;
 
 #define err_exit(op)                                                          \
 {                                                                             \
@@ -49,18 +48,18 @@ void punch_hole_fn(void *ptr)
 		read = 0;
 
 		do {
-			rc = pread(nodax_fd, dax_data + read, FILE_SIZE - read,
+			rc = pread(nodax_fd, dax_data + read, file_size - read,
 					read);
 			if (rc > 0)
 				read += rc;
 		} while (rc > 0);
 
-		if (read != FILE_SIZE || rc != 0)
+		if (read != file_size || rc != 0)
 			err_exit("pread");
 
 		rc = fallocate(dax_fd,
 				FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
-				0, FILE_SIZE);
+				0, file_size);
 		if (rc < 0)
 			err_exit("fallocate");
 
@@ -81,18 +80,18 @@ void zero_range_fn(void *ptr)
 		read = 0;
 
 		do {
-			rc = pread(nodax_fd, dax_data + read, FILE_SIZE - read,
+			rc = pread(nodax_fd, dax_data + read, file_size - read,
 					read);
 			if (rc > 0)
 				read += rc;
 		} while (rc > 0);
 
-		if (read != FILE_SIZE || rc != 0)
+		if (read != file_size || rc != 0)
 			err_exit("pread");
 
 		rc = fallocate(dax_fd,
 				FALLOC_FL_ZERO_RANGE | FALLOC_FL_KEEP_SIZE,
-				0, FILE_SIZE);
+				0, file_size);
 		if (rc < 0)
 			err_exit("fallocate");
 
@@ -113,11 +112,11 @@ void truncate_down_fn(void *ptr)
 
 		if (ftruncate(dax_fd, 0) < 0)
 			err_exit("ftruncate");
-		if (fallocate(dax_fd, 0, 0, FILE_SIZE) < 0)
+		if (fallocate(dax_fd, 0, 0, file_size) < 0)
 			err_exit("fallocate");
 
 		do {
-			rc = pread(nodax_fd, dax_data + read, FILE_SIZE - read,
+			rc = pread(nodax_fd, dax_data + read, file_size - read,
 					read);
 			if (rc > 0)
 				read += rc;
@@ -142,15 +141,15 @@ void collapse_range_fn(void *ptr)
 	while (!done) {
 		read = 0;
 
-		if (fallocate(dax_fd, 0, 0, FILE_SIZE) < 0)
+		if (fallocate(dax_fd, 0, 0, file_size) < 0)
 			err_exit("fallocate 1");
-		if (fallocate(dax_fd, FALLOC_FL_COLLAPSE_RANGE, 0, PAGE(1)) < 0)
+		if (fallocate(dax_fd, FALLOC_FL_COLLAPSE_RANGE, 0, pagesize) < 0)
 			err_exit("fallocate 2");
-		if (fallocate(dax_fd, 0, 0, FILE_SIZE) < 0)
+		if (fallocate(dax_fd, 0, 0, file_size) < 0)
 			err_exit("fallocate 3");
 
 		do {
-			rc = pread(nodax_fd, dax_data + read, FILE_SIZE - read,
+			rc = pread(nodax_fd, dax_data + read, file_size - read,
 					read);
 			if (rc > 0)
 				read += rc;
@@ -192,6 +191,9 @@ int main(int argc, char *argv[])
 		exit(0);
 	}
 
+	pagesize = getpagesize();
+	file_size = 4 * pagesize;
+
 	dax_fd = open(argv[1], O_RDWR|O_CREAT, S_IRUSR|S_IWUSR);
 	if (dax_fd < 0)
 		err_exit("dax_fd open");
@@ -202,15 +204,15 @@ int main(int argc, char *argv[])
 
 	if (ftruncate(dax_fd, 0) < 0)
 		err_exit("dax_fd ftruncate");
-	if (fallocate(dax_fd, 0, 0, FILE_SIZE) < 0)
+	if (fallocate(dax_fd, 0, 0, file_size) < 0)
 		err_exit("dax_fd fallocate");
 
 	if (ftruncate(nodax_fd, 0) < 0)
 		err_exit("nodax_fd ftruncate");
-	if (fallocate(nodax_fd, 0, 0, FILE_SIZE) < 0)
+	if (fallocate(nodax_fd, 0, 0, file_size) < 0)
 		err_exit("nodax_fd fallocate");
 
-	dax_data = mmap(NULL, FILE_SIZE, PROT_READ|PROT_WRITE, MAP_SHARED,
+	dax_data = mmap(NULL, file_size, PROT_READ|PROT_WRITE, MAP_SHARED,
 			dax_fd, 0);
 	if (dax_data == MAP_FAILED)
 		err_exit("mmap");
@@ -220,7 +222,7 @@ int main(int argc, char *argv[])
 	run_test(&truncate_down_fn);
 	run_test(&collapse_range_fn);
 
-	if (munmap(dax_data, FILE_SIZE) != 0)
+	if (munmap(dax_data, file_size) != 0)
 		err_exit("munmap");
 
 	err = close(dax_fd);
-- 
2.19.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH V2 3/3] xfs/300: modify test to work on any fs block size
  2020-02-20 20:06 [PATCH V2 0/3] fstests: fixes for 64k pages and dax Jeff Moyer
  2020-02-20 20:06 ` [PATCH V2 1/3] dax/dm: disable testing on devices that don't support dax Jeff Moyer
  2020-02-20 20:06 ` [PATCH V2 2/3] t_mmap_collision: fix hard-coded page size Jeff Moyer
@ 2020-02-20 20:06 ` Jeff Moyer
  2020-02-22  5:31   ` Zorro Lang
  2020-02-20 21:21 ` [PATCH V2 0/3] fstests: fixes for 64k pages and dax Darrick J. Wong
  3 siblings, 1 reply; 13+ messages in thread
From: Jeff Moyer @ 2020-02-20 20:06 UTC (permalink / raw)
  To: fstests; +Cc: linux-fsdevel, linux-xfs, Jeff Moyer

The test currently assumes a file system block size of 4k.  It will
work just fine on any user-specified block size, though.

Signed-off-by: Jeff Moyer <jmoyer@redhat.com>
---
 tests/xfs/300 | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/xfs/300 b/tests/xfs/300
index 28608b81..4f1c927a 100755
--- a/tests/xfs/300
+++ b/tests/xfs/300
@@ -50,8 +50,9 @@ $XFS_IO_PROG -f -c "pwrite -S 0x63 0 4096" $SCRATCH_MNT/attrvals >> $seqres.full
 cat $SCRATCH_MNT/attrvals | attr -s name $SCRATCH_MNT/$seq.test >> $seqres.full 2>&1
 
 # Fragment the file by writing backwards
+bs=$(_get_file_block_size $SCRATCH_MNT)
 for I in `seq 6 -1 0`; do
-	dd if=/dev/zero of=$SCRATCH_MNT/$seq.test seek=$I bs=4k \
+	dd if=/dev/zero of=$SCRATCH_MNT/$seq.test seek=$I bs=${bs} \
 	   oflag=direct count=1 conv=notrunc >> $seqres.full 2>&1
 done
 
-- 
2.19.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH V2 0/3] fstests: fixes for 64k pages and dax
  2020-02-20 20:06 [PATCH V2 0/3] fstests: fixes for 64k pages and dax Jeff Moyer
                   ` (2 preceding siblings ...)
  2020-02-20 20:06 ` [PATCH V2 3/3] xfs/300: modify test to work on any fs block size Jeff Moyer
@ 2020-02-20 21:21 ` Darrick J. Wong
  2020-02-21 20:11   ` Jeff Moyer
  3 siblings, 1 reply; 13+ messages in thread
From: Darrick J. Wong @ 2020-02-20 21:21 UTC (permalink / raw)
  To: Jeff Moyer; +Cc: fstests, linux-fsdevel, linux-xfs

On Thu, Feb 20, 2020 at 03:06:29PM -0500, Jeff Moyer wrote:
> This set of patches fixes a few false positives I encountered when
> testing DAX on ppc64le (which has a 64k page size).
> 
> Patch 1 is actually not specific to non-4k page sizes.  Currently,
> each individual dm rc file checks for the presence of the DAX mount
> option, and _notruns the test as part of the initializtion.  This
> doesn't work for the snapshot target.  Moving the check into the
> _require_dm_target fixes the problem, and keeps from the cut-n-paste
> of boilerplate.
> 
> Patches 2 and 3 get rid of hard coded block/page sizes in the tests.
> They run just fine on 64k pages and 64k block sizes.
> 
> Even after these patches, there are many more tests that fail in the
> following configuration:
> 
> MKFS_OPTIONS="-b size=65536 -m reflink=0" MOUNT_OPTIONS="-o dax"
> 
> One class of failures is tests that create a really small file system
> size.  Some of those tests seem to require the very small size, but
> others seem like they could live with a slightly bigger size that
> would then fit the log (the typical failure is a mkfs failure due to
> not enough blocks for the log).  For the former case, I'm tempted to
> send patches to _notrun those tests, and for the latter, I'd like to
> bump the file system sizes up.  300MB seems to be large enough to
> accommodate the log.  Would folks be opposed to those approaches?

Seems fine to me.  Do we have a helper function to compute (or maybe
just format) the minimum supported filesystem size for the given
MKFS_OPTIONS?

> Another class of failure is tests that either hard-code a block size
> to trigger a specific error case, or that test a multitude of block
> sizes.  I'd like to send a patch to _notrun those tests if there is
> a user-specified block size.  That will require parsing the MKFS_OPTIONS
> based on the fs type, of course.  Is that something that seems
> reasonable?

I think it's fine to _notrun a test that requires a specific blocksize
when when that blocksize is not supported by the system under test.

The ones that cycle through a range of block sizes, not so much--I guess
the question here is can we distinguish "test only this blocksize" vs
"default to this block size"?  And do we want to?

--D

> I will follow up with a series of patches to implement those changes
> if there is consensus on the approach.  These first three seemed
> straight-forward to me, so that's where I'm starting.
> 
> Changes in v2:
> - patch 2: remove the boilerplate from all dm rc files (Zorro Lang)
> - cc fstests (thanks, Dave)
> 
> [PATCH V2 1/3] dax/dm: disable testing on devices that don't support
> [PATCH V2 2/3] t_mmap_collision: fix hard-coded page size
> [PATCH V2 3/3] xfs/300: modify test to work on any fs block size
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH V2 1/3] dax/dm: disable testing on devices that don't support dax
  2020-02-20 20:06 ` [PATCH V2 1/3] dax/dm: disable testing on devices that don't support dax Jeff Moyer
@ 2020-02-21  9:48   ` Zorro Lang
  2020-02-23 15:07     ` Eryu Guan
  0 siblings, 1 reply; 13+ messages in thread
From: Zorro Lang @ 2020-02-21  9:48 UTC (permalink / raw)
  To: Jeff Moyer; +Cc: fstests, linux-fsdevel, linux-xfs

On Thu, Feb 20, 2020 at 03:06:30PM -0500, Jeff Moyer wrote:
> Move the check for dax from the individual target scripts into
> _require_dm_target.  This fixes up a couple of missed tests that are
> failing due to the lack of dax support (such as tests requiring
> dm-snapshot).
> 
> Signed-off-by: Jeff Moyer <jmoyer@redhat.com>
> ---
>  common/dmdelay  |  5 -----
>  common/dmerror  |  5 -----
>  common/dmflakey |  5 -----
>  common/dmthin   |  5 -----
>  common/rc       | 11 +++++++++++
>  5 files changed, 11 insertions(+), 20 deletions(-)
> 
> diff --git a/common/dmdelay b/common/dmdelay
> index f1e725b9..66cac1a7 100644
> --- a/common/dmdelay
> +++ b/common/dmdelay
> @@ -7,11 +7,6 @@
>  DELAY_NONE=0
>  DELAY_READ=1
>  
> -echo $MOUNT_OPTIONS | grep -q dax
> -if [ $? -eq 0 ]; then
> -	_notrun "Cannot run tests with DAX on dmdelay devices"
> -fi
> -
>  _init_delay()
>  {
>  	local BLK_DEV_SIZE=`blockdev --getsz $SCRATCH_DEV`
> diff --git a/common/dmerror b/common/dmerror
> index 426f1e96..7d12e0a1 100644
> --- a/common/dmerror
> +++ b/common/dmerror
> @@ -4,11 +4,6 @@
>  #
>  # common functions for setting up and tearing down a dmerror device
>  
> -echo $MOUNT_OPTIONS | grep -q dax
> -if [ $? -eq 0 ]; then
> -	_notrun "Cannot run tests with DAX on dmerror devices"
> -fi
> -
>  _dmerror_setup()
>  {
>  	local dm_backing_dev=$SCRATCH_DEV
> diff --git a/common/dmflakey b/common/dmflakey
> index 2af3924d..b4e11ae9 100644
> --- a/common/dmflakey
> +++ b/common/dmflakey
> @@ -8,11 +8,6 @@ FLAKEY_ALLOW_WRITES=0
>  FLAKEY_DROP_WRITES=1
>  FLAKEY_ERROR_WRITES=2
>  
> -echo $MOUNT_OPTIONS | grep -q dax
> -if [ $? -eq 0 ]; then
> -	_notrun "Cannot run tests with DAX on dmflakey devices"
> -fi
> -
>  _init_flakey()
>  {
>  	local BLK_DEV_SIZE=`blockdev --getsz $SCRATCH_DEV`
> diff --git a/common/dmthin b/common/dmthin
> index 7946e9a7..61dd6f89 100644
> --- a/common/dmthin
> +++ b/common/dmthin
> @@ -21,11 +21,6 @@ DMTHIN_POOL_DEV="/dev/mapper/$DMTHIN_POOL_NAME"
>  DMTHIN_VOL_NAME="thin-vol"
>  DMTHIN_VOL_DEV="/dev/mapper/$DMTHIN_VOL_NAME"
>  
> -echo $MOUNT_OPTIONS | grep -q dax
> -if [ $? -eq 0 ]; then
> -	_notrun "Cannot run tests with DAX on dmthin devices"
> -fi
> -
>  _dmthin_cleanup()
>  {
>  	$UMOUNT_PROG $SCRATCH_MNT > /dev/null 2>&1
> diff --git a/common/rc b/common/rc
> index eeac1355..65cde32b 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -1874,6 +1874,17 @@ _require_dm_target()
>  	_require_sane_bdev_flush $SCRATCH_DEV
>  	_require_command "$DMSETUP_PROG" dmsetup
>  
> +	echo $MOUNT_OPTIONS | grep -q dax
> +	if [ $? -eq 0 ]; then
> +		case $target in
> +		stripe|linear|log-writes)

I've checked all cases which import ./common/dm.* (without dmapi), they all
has _require_dm_target. So this patch is good to me.

And by checking current linux source code:

  0 dm-linear.c      226 .direct_access = linear_dax_direct_access,
  1 dm-log-writes.c 1016 .direct_access = log_writes_dax_direct_access,
  2 dm-stripe.c      486 .direct_access = stripe_dax_direct_access,
  3 dm-target.c      159 .direct_access = io_err_dax_direct_access,

Only linear, stripe and log-writes support direct_access.

Thanks,
Zorro

> +			;;
> +		*)
> +			_notrun "Cannot run tests with DAX on $target devices."
> +			;;
> +		esac
> +	fi
> +
>  	modprobe dm-$target >/dev/null 2>&1
>  
>  	$DMSETUP_PROG targets 2>&1 | grep -q ^$target
> -- 
> 2.19.1
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH V2 2/3] t_mmap_collision: fix hard-coded page size
  2020-02-20 20:06 ` [PATCH V2 2/3] t_mmap_collision: fix hard-coded page size Jeff Moyer
@ 2020-02-21 13:53   ` Zorro Lang
  0 siblings, 0 replies; 13+ messages in thread
From: Zorro Lang @ 2020-02-21 13:53 UTC (permalink / raw)
  To: Jeff Moyer; +Cc: fstests, linux-fsdevel, linux-xfs

On Thu, Feb 20, 2020 at 03:06:31PM -0500, Jeff Moyer wrote:
> Fix the test to run on non-4k page size systems.
> 
> Signed-off-by: Jeff Moyer <jmoyer@redhat.com>
> ---

This patch looks good to me, and it's really helpful.

Thanks,
Zorro

Before patched:

FSTYP         -- xfs (non-debug)
PLATFORM      -- Linux/aarch64
MKFS_OPTIONS  -- -f -b size=65536 -m crc=1,finobt=1,reflink=1,rmapbt=1 -i sparse=1 /dev/sda5
MOUNT_OPTIONS -- -o context=system_u:object_r:nfs_t:s0 /dev/sda5 /mnt/xfstests/mnt2

generic/503 14s ... - output mismatch (see /root/xfstests-dev/results//generic/503.out.bad)
    --- tests/generic/503.out   2020-02-21 05:41:37.992675071 -0500
    +++ /root/xfstests-dev/results//generic/503.out.bad 2020-02-21 08:20:19.736550319 -0500
    @@ -1,2 +1,4 @@
     QA output created by 503
    +collapse_range_fn fallocate 2: Invalid argument
    +collapse_range_fn fallocate 2: Invalid argument
...

After patched:

FSTYP         -- xfs (non-debug)
PLATFORM      -- Linux/aarch64
MKFS_OPTIONS  -- -f -b size=65536 -m crc=1,finobt=1,reflink=1,rmapbt=1 -i sparse=1 /dev/sda5
MOUNT_OPTIONS -- -o context=system_u:object_r:nfs_t:s0 /dev/sda5 /mnt/xfstests/mnt2

generic/503 16s ...  16s
Ran: generic/503
Passed all 1 tests

>  src/t_mmap_collision.c | 40 +++++++++++++++++++++-------------------
>  1 file changed, 21 insertions(+), 19 deletions(-)
> 
> diff --git a/src/t_mmap_collision.c b/src/t_mmap_collision.c
> index d547bc05..c872f4e2 100644
> --- a/src/t_mmap_collision.c
> +++ b/src/t_mmap_collision.c
> @@ -25,13 +25,12 @@
>  #include <sys/types.h>
>  #include <unistd.h>
>  
> -#define PAGE(a) ((a)*0x1000)
> -#define FILE_SIZE PAGE(4)
> -
>  void *dax_data;
>  int nodax_fd;
>  int dax_fd;
>  bool done;
> +static int pagesize;
> +static int file_size;
>  
>  #define err_exit(op)                                                          \
>  {                                                                             \
> @@ -49,18 +48,18 @@ void punch_hole_fn(void *ptr)
>  		read = 0;
>  
>  		do {
> -			rc = pread(nodax_fd, dax_data + read, FILE_SIZE - read,
> +			rc = pread(nodax_fd, dax_data + read, file_size - read,
>  					read);
>  			if (rc > 0)
>  				read += rc;
>  		} while (rc > 0);
>  
> -		if (read != FILE_SIZE || rc != 0)
> +		if (read != file_size || rc != 0)
>  			err_exit("pread");
>  
>  		rc = fallocate(dax_fd,
>  				FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
> -				0, FILE_SIZE);
> +				0, file_size);
>  		if (rc < 0)
>  			err_exit("fallocate");
>  
> @@ -81,18 +80,18 @@ void zero_range_fn(void *ptr)
>  		read = 0;
>  
>  		do {
> -			rc = pread(nodax_fd, dax_data + read, FILE_SIZE - read,
> +			rc = pread(nodax_fd, dax_data + read, file_size - read,
>  					read);
>  			if (rc > 0)
>  				read += rc;
>  		} while (rc > 0);
>  
> -		if (read != FILE_SIZE || rc != 0)
> +		if (read != file_size || rc != 0)
>  			err_exit("pread");
>  
>  		rc = fallocate(dax_fd,
>  				FALLOC_FL_ZERO_RANGE | FALLOC_FL_KEEP_SIZE,
> -				0, FILE_SIZE);
> +				0, file_size);
>  		if (rc < 0)
>  			err_exit("fallocate");
>  
> @@ -113,11 +112,11 @@ void truncate_down_fn(void *ptr)
>  
>  		if (ftruncate(dax_fd, 0) < 0)
>  			err_exit("ftruncate");
> -		if (fallocate(dax_fd, 0, 0, FILE_SIZE) < 0)
> +		if (fallocate(dax_fd, 0, 0, file_size) < 0)
>  			err_exit("fallocate");
>  
>  		do {
> -			rc = pread(nodax_fd, dax_data + read, FILE_SIZE - read,
> +			rc = pread(nodax_fd, dax_data + read, file_size - read,
>  					read);
>  			if (rc > 0)
>  				read += rc;
> @@ -142,15 +141,15 @@ void collapse_range_fn(void *ptr)
>  	while (!done) {
>  		read = 0;
>  
> -		if (fallocate(dax_fd, 0, 0, FILE_SIZE) < 0)
> +		if (fallocate(dax_fd, 0, 0, file_size) < 0)
>  			err_exit("fallocate 1");
> -		if (fallocate(dax_fd, FALLOC_FL_COLLAPSE_RANGE, 0, PAGE(1)) < 0)
> +		if (fallocate(dax_fd, FALLOC_FL_COLLAPSE_RANGE, 0, pagesize) < 0)
>  			err_exit("fallocate 2");
> -		if (fallocate(dax_fd, 0, 0, FILE_SIZE) < 0)
> +		if (fallocate(dax_fd, 0, 0, file_size) < 0)
>  			err_exit("fallocate 3");
>  
>  		do {
> -			rc = pread(nodax_fd, dax_data + read, FILE_SIZE - read,
> +			rc = pread(nodax_fd, dax_data + read, file_size - read,
>  					read);
>  			if (rc > 0)
>  				read += rc;
> @@ -192,6 +191,9 @@ int main(int argc, char *argv[])
>  		exit(0);
>  	}
>  
> +	pagesize = getpagesize();
> +	file_size = 4 * pagesize;
> +
>  	dax_fd = open(argv[1], O_RDWR|O_CREAT, S_IRUSR|S_IWUSR);
>  	if (dax_fd < 0)
>  		err_exit("dax_fd open");
> @@ -202,15 +204,15 @@ int main(int argc, char *argv[])
>  
>  	if (ftruncate(dax_fd, 0) < 0)
>  		err_exit("dax_fd ftruncate");
> -	if (fallocate(dax_fd, 0, 0, FILE_SIZE) < 0)
> +	if (fallocate(dax_fd, 0, 0, file_size) < 0)
>  		err_exit("dax_fd fallocate");
>  
>  	if (ftruncate(nodax_fd, 0) < 0)
>  		err_exit("nodax_fd ftruncate");
> -	if (fallocate(nodax_fd, 0, 0, FILE_SIZE) < 0)
> +	if (fallocate(nodax_fd, 0, 0, file_size) < 0)
>  		err_exit("nodax_fd fallocate");
>  
> -	dax_data = mmap(NULL, FILE_SIZE, PROT_READ|PROT_WRITE, MAP_SHARED,
> +	dax_data = mmap(NULL, file_size, PROT_READ|PROT_WRITE, MAP_SHARED,
>  			dax_fd, 0);
>  	if (dax_data == MAP_FAILED)
>  		err_exit("mmap");
> @@ -220,7 +222,7 @@ int main(int argc, char *argv[])
>  	run_test(&truncate_down_fn);
>  	run_test(&collapse_range_fn);
>  
> -	if (munmap(dax_data, FILE_SIZE) != 0)
> +	if (munmap(dax_data, file_size) != 0)
>  		err_exit("munmap");
>  
>  	err = close(dax_fd);
> -- 
> 2.19.1
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH V2 0/3] fstests: fixes for 64k pages and dax
  2020-02-20 21:21 ` [PATCH V2 0/3] fstests: fixes for 64k pages and dax Darrick J. Wong
@ 2020-02-21 20:11   ` Jeff Moyer
  0 siblings, 0 replies; 13+ messages in thread
From: Jeff Moyer @ 2020-02-21 20:11 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: fstests, linux-fsdevel, linux-xfs

Hi, Darrick,

"Darrick J. Wong" <darrick.wong@oracle.com> writes:

>> One class of failures is tests that create a really small file system
>> size.  Some of those tests seem to require the very small size, but
>> others seem like they could live with a slightly bigger size that
>> would then fit the log (the typical failure is a mkfs failure due to
>> not enough blocks for the log).  For the former case, I'm tempted to
>> send patches to _notrun those tests, and for the latter, I'd like to
>> bump the file system sizes up.  300MB seems to be large enough to
>> accommodate the log.  Would folks be opposed to those approaches?
>
> Seems fine to me.  Do we have a helper function to compute (or maybe
> just format) the minimum supported filesystem size for the given
> MKFS_OPTIONS?

Not that I could find.  I'm not sure how you'd do that, even.

>> Another class of failure is tests that either hard-code a block size
>> to trigger a specific error case, or that test a multitude of block
>> sizes.  I'd like to send a patch to _notrun those tests if there is
>> a user-specified block size.  That will require parsing the MKFS_OPTIONS
>> based on the fs type, of course.  Is that something that seems
>> reasonable?
>
> I think it's fine to _notrun a test that requires a specific blocksize
> when when that blocksize is not supported by the system under test.

OK.

> The ones that cycle through a range of block sizes, not so much--I guess
> the question here is can we distinguish "test only this blocksize" vs
> "default to this block size"?  And do we want to?

Well, I'd like to prevent false test failures.  In this instance, the
block size is required in order for the system to function.  I'm
guessing this is a new and special kind of suck.  If we treat the
MKFS_OPTIONS as advisory, then there will be false positive failures.
If we treat MKFS_OPTIONS as mandatory, then there will be less test
coverage for a given set of options.  I think that's the preferrable
solution, but I'm probably too focused on this one use case.

-Jeff


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH V2 3/3] xfs/300: modify test to work on any fs block size
  2020-02-20 20:06 ` [PATCH V2 3/3] xfs/300: modify test to work on any fs block size Jeff Moyer
@ 2020-02-22  5:31   ` Zorro Lang
  2020-02-24 13:46     ` Jeff Moyer
  0 siblings, 1 reply; 13+ messages in thread
From: Zorro Lang @ 2020-02-22  5:31 UTC (permalink / raw)
  To: Jeff Moyer; +Cc: fstests, linux-fsdevel, linux-xfs

On Thu, Feb 20, 2020 at 03:06:32PM -0500, Jeff Moyer wrote:
> The test currently assumes a file system block size of 4k.  It will
> work just fine on any user-specified block size, though.
> 
> Signed-off-by: Jeff Moyer <jmoyer@redhat.com>
> ---
>  tests/xfs/300 | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/xfs/300 b/tests/xfs/300
> index 28608b81..4f1c927a 100755
> --- a/tests/xfs/300
> +++ b/tests/xfs/300
> @@ -50,8 +50,9 @@ $XFS_IO_PROG -f -c "pwrite -S 0x63 0 4096" $SCRATCH_MNT/attrvals >> $seqres.full
>  cat $SCRATCH_MNT/attrvals | attr -s name $SCRATCH_MNT/$seq.test >> $seqres.full 2>&1
>  
>  # Fragment the file by writing backwards
> +bs=$(_get_file_block_size $SCRATCH_MNT)
>  for I in `seq 6 -1 0`; do
> -	dd if=/dev/zero of=$SCRATCH_MNT/$seq.test seek=$I bs=4k \
> +	dd if=/dev/zero of=$SCRATCH_MNT/$seq.test seek=$I bs=${bs} \

Although the original case won't fail on 64k test. But this change makes
more sense.

Thanks,
Zorro

>  	   oflag=direct count=1 conv=notrunc >> $seqres.full 2>&1
>  done
>  
> -- 
> 2.19.1
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH V2 1/3] dax/dm: disable testing on devices that don't support dax
  2020-02-21  9:48   ` Zorro Lang
@ 2020-02-23 15:07     ` Eryu Guan
  2020-02-24  6:15       ` Zorro Lang
  0 siblings, 1 reply; 13+ messages in thread
From: Eryu Guan @ 2020-02-23 15:07 UTC (permalink / raw)
  To: zlang; +Cc: Jeff Moyer, fstests, linux-fsdevel, linux-xfs

On Fri, Feb 21, 2020 at 05:48:01PM +0800, Zorro Lang wrote:
> On Thu, Feb 20, 2020 at 03:06:30PM -0500, Jeff Moyer wrote:
> > Move the check for dax from the individual target scripts into
> > _require_dm_target.  This fixes up a couple of missed tests that are
> > failing due to the lack of dax support (such as tests requiring
> > dm-snapshot).
> > 
> > Signed-off-by: Jeff Moyer <jmoyer@redhat.com>
> > ---
> >  common/dmdelay  |  5 -----
> >  common/dmerror  |  5 -----
> >  common/dmflakey |  5 -----
> >  common/dmthin   |  5 -----
> >  common/rc       | 11 +++++++++++
> >  5 files changed, 11 insertions(+), 20 deletions(-)
> > 
> > diff --git a/common/dmdelay b/common/dmdelay
> > index f1e725b9..66cac1a7 100644
> > --- a/common/dmdelay
> > +++ b/common/dmdelay
> > @@ -7,11 +7,6 @@
> >  DELAY_NONE=0
> >  DELAY_READ=1
> >  
> > -echo $MOUNT_OPTIONS | grep -q dax
> > -if [ $? -eq 0 ]; then
> > -	_notrun "Cannot run tests with DAX on dmdelay devices"
> > -fi
> > -
> >  _init_delay()
> >  {
> >  	local BLK_DEV_SIZE=`blockdev --getsz $SCRATCH_DEV`
> > diff --git a/common/dmerror b/common/dmerror
> > index 426f1e96..7d12e0a1 100644
> > --- a/common/dmerror
> > +++ b/common/dmerror
> > @@ -4,11 +4,6 @@
> >  #
> >  # common functions for setting up and tearing down a dmerror device
> >  
> > -echo $MOUNT_OPTIONS | grep -q dax
> > -if [ $? -eq 0 ]; then
> > -	_notrun "Cannot run tests with DAX on dmerror devices"
> > -fi
> > -
> >  _dmerror_setup()
> >  {
> >  	local dm_backing_dev=$SCRATCH_DEV
> > diff --git a/common/dmflakey b/common/dmflakey
> > index 2af3924d..b4e11ae9 100644
> > --- a/common/dmflakey
> > +++ b/common/dmflakey
> > @@ -8,11 +8,6 @@ FLAKEY_ALLOW_WRITES=0
> >  FLAKEY_DROP_WRITES=1
> >  FLAKEY_ERROR_WRITES=2
> >  
> > -echo $MOUNT_OPTIONS | grep -q dax
> > -if [ $? -eq 0 ]; then
> > -	_notrun "Cannot run tests with DAX on dmflakey devices"
> > -fi
> > -
> >  _init_flakey()
> >  {
> >  	local BLK_DEV_SIZE=`blockdev --getsz $SCRATCH_DEV`
> > diff --git a/common/dmthin b/common/dmthin
> > index 7946e9a7..61dd6f89 100644
> > --- a/common/dmthin
> > +++ b/common/dmthin
> > @@ -21,11 +21,6 @@ DMTHIN_POOL_DEV="/dev/mapper/$DMTHIN_POOL_NAME"
> >  DMTHIN_VOL_NAME="thin-vol"
> >  DMTHIN_VOL_DEV="/dev/mapper/$DMTHIN_VOL_NAME"
> >  
> > -echo $MOUNT_OPTIONS | grep -q dax
> > -if [ $? -eq 0 ]; then
> > -	_notrun "Cannot run tests with DAX on dmthin devices"
> > -fi
> > -
> >  _dmthin_cleanup()
> >  {
> >  	$UMOUNT_PROG $SCRATCH_MNT > /dev/null 2>&1
> > diff --git a/common/rc b/common/rc
> > index eeac1355..65cde32b 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -1874,6 +1874,17 @@ _require_dm_target()
> >  	_require_sane_bdev_flush $SCRATCH_DEV
> >  	_require_command "$DMSETUP_PROG" dmsetup
> >  
> > +	echo $MOUNT_OPTIONS | grep -q dax
> > +	if [ $? -eq 0 ]; then
> > +		case $target in
> > +		stripe|linear|log-writes)
> 
> I've checked all cases which import ./common/dm.* (without dmapi), they all
> has _require_dm_target. So this patch is good to me.

So can I add "Reviewed-by: Zorro Lang <zlang@redhat.com>" to all these
three patches? :)

Thanks for the review!

Eryu
> 
> And by checking current linux source code:
> 
>   0 dm-linear.c      226 .direct_access = linear_dax_direct_access,
>   1 dm-log-writes.c 1016 .direct_access = log_writes_dax_direct_access,
>   2 dm-stripe.c      486 .direct_access = stripe_dax_direct_access,
>   3 dm-target.c      159 .direct_access = io_err_dax_direct_access,
> 
> Only linear, stripe and log-writes support direct_access.
> 
> Thanks,
> Zorro
> 
> > +			;;
> > +		*)
> > +			_notrun "Cannot run tests with DAX on $target devices."
> > +			;;
> > +		esac
> > +	fi
> > +
> >  	modprobe dm-$target >/dev/null 2>&1
> >  
> >  	$DMSETUP_PROG targets 2>&1 | grep -q ^$target
> > -- 
> > 2.19.1
> > 
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH V2 1/3] dax/dm: disable testing on devices that don't support dax
  2020-02-23 15:07     ` Eryu Guan
@ 2020-02-24  6:15       ` Zorro Lang
  0 siblings, 0 replies; 13+ messages in thread
From: Zorro Lang @ 2020-02-24  6:15 UTC (permalink / raw)
  To: Eryu Guan; +Cc: fstests, linux-xfs

On Sun, Feb 23, 2020 at 11:07:15PM +0800, Eryu Guan wrote:
> On Fri, Feb 21, 2020 at 05:48:01PM +0800, Zorro Lang wrote:
> > On Thu, Feb 20, 2020 at 03:06:30PM -0500, Jeff Moyer wrote:
> > > Move the check for dax from the individual target scripts into
> > > _require_dm_target.  This fixes up a couple of missed tests that are
> > > failing due to the lack of dax support (such as tests requiring
> > > dm-snapshot).
> > > 
> > > Signed-off-by: Jeff Moyer <jmoyer@redhat.com>
> > > ---
> > >  common/dmdelay  |  5 -----
> > >  common/dmerror  |  5 -----
> > >  common/dmflakey |  5 -----
> > >  common/dmthin   |  5 -----
> > >  common/rc       | 11 +++++++++++
> > >  5 files changed, 11 insertions(+), 20 deletions(-)
> > > 
> > > diff --git a/common/dmdelay b/common/dmdelay
> > > index f1e725b9..66cac1a7 100644
> > > --- a/common/dmdelay
> > > +++ b/common/dmdelay
> > > @@ -7,11 +7,6 @@
> > >  DELAY_NONE=0
> > >  DELAY_READ=1
> > >  
> > > -echo $MOUNT_OPTIONS | grep -q dax
> > > -if [ $? -eq 0 ]; then
> > > -	_notrun "Cannot run tests with DAX on dmdelay devices"
> > > -fi
> > > -
> > >  _init_delay()
> > >  {
> > >  	local BLK_DEV_SIZE=`blockdev --getsz $SCRATCH_DEV`
> > > diff --git a/common/dmerror b/common/dmerror
> > > index 426f1e96..7d12e0a1 100644
> > > --- a/common/dmerror
> > > +++ b/common/dmerror
> > > @@ -4,11 +4,6 @@
> > >  #
> > >  # common functions for setting up and tearing down a dmerror device
> > >  
> > > -echo $MOUNT_OPTIONS | grep -q dax
> > > -if [ $? -eq 0 ]; then
> > > -	_notrun "Cannot run tests with DAX on dmerror devices"
> > > -fi
> > > -
> > >  _dmerror_setup()
> > >  {
> > >  	local dm_backing_dev=$SCRATCH_DEV
> > > diff --git a/common/dmflakey b/common/dmflakey
> > > index 2af3924d..b4e11ae9 100644
> > > --- a/common/dmflakey
> > > +++ b/common/dmflakey
> > > @@ -8,11 +8,6 @@ FLAKEY_ALLOW_WRITES=0
> > >  FLAKEY_DROP_WRITES=1
> > >  FLAKEY_ERROR_WRITES=2
> > >  
> > > -echo $MOUNT_OPTIONS | grep -q dax
> > > -if [ $? -eq 0 ]; then
> > > -	_notrun "Cannot run tests with DAX on dmflakey devices"
> > > -fi
> > > -
> > >  _init_flakey()
> > >  {
> > >  	local BLK_DEV_SIZE=`blockdev --getsz $SCRATCH_DEV`
> > > diff --git a/common/dmthin b/common/dmthin
> > > index 7946e9a7..61dd6f89 100644
> > > --- a/common/dmthin
> > > +++ b/common/dmthin
> > > @@ -21,11 +21,6 @@ DMTHIN_POOL_DEV="/dev/mapper/$DMTHIN_POOL_NAME"
> > >  DMTHIN_VOL_NAME="thin-vol"
> > >  DMTHIN_VOL_DEV="/dev/mapper/$DMTHIN_VOL_NAME"
> > >  
> > > -echo $MOUNT_OPTIONS | grep -q dax
> > > -if [ $? -eq 0 ]; then
> > > -	_notrun "Cannot run tests with DAX on dmthin devices"
> > > -fi
> > > -
> > >  _dmthin_cleanup()
> > >  {
> > >  	$UMOUNT_PROG $SCRATCH_MNT > /dev/null 2>&1
> > > diff --git a/common/rc b/common/rc
> > > index eeac1355..65cde32b 100644
> > > --- a/common/rc
> > > +++ b/common/rc
> > > @@ -1874,6 +1874,17 @@ _require_dm_target()
> > >  	_require_sane_bdev_flush $SCRATCH_DEV
> > >  	_require_command "$DMSETUP_PROG" dmsetup
> > >  
> > > +	echo $MOUNT_OPTIONS | grep -q dax
> > > +	if [ $? -eq 0 ]; then
> > > +		case $target in
> > > +		stripe|linear|log-writes)
> > 
> > I've checked all cases which import ./common/dm.* (without dmapi), they all
> > has _require_dm_target. So this patch is good to me.
> 
> So can I add "Reviewed-by: Zorro Lang <zlang@redhat.com>" to all these
> three patches? :)

Sure, my pleasure :)

> 
> Thanks for the review!
> 
> Eryu
> > 
> > And by checking current linux source code:
> > 
> >   0 dm-linear.c      226 .direct_access = linear_dax_direct_access,
> >   1 dm-log-writes.c 1016 .direct_access = log_writes_dax_direct_access,
> >   2 dm-stripe.c      486 .direct_access = stripe_dax_direct_access,
> >   3 dm-target.c      159 .direct_access = io_err_dax_direct_access,
> > 
> > Only linear, stripe and log-writes support direct_access.
> > 
> > Thanks,
> > Zorro
> > 
> > > +			;;
> > > +		*)
> > > +			_notrun "Cannot run tests with DAX on $target devices."
> > > +			;;
> > > +		esac
> > > +	fi
> > > +
> > >  	modprobe dm-$target >/dev/null 2>&1
> > >  
> > >  	$DMSETUP_PROG targets 2>&1 | grep -q ^$target
> > > -- 
> > > 2.19.1
> > > 
> > 
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH V2 3/3] xfs/300: modify test to work on any fs block size
  2020-02-22  5:31   ` Zorro Lang
@ 2020-02-24 13:46     ` Jeff Moyer
  2020-02-26  7:42       ` Zorro Lang
  0 siblings, 1 reply; 13+ messages in thread
From: Jeff Moyer @ 2020-02-24 13:46 UTC (permalink / raw)
  To: fstests; +Cc: linux-fsdevel, linux-xfs

Zorro Lang <zlang@redhat.com> writes:

> On Thu, Feb 20, 2020 at 03:06:32PM -0500, Jeff Moyer wrote:
>> The test currently assumes a file system block size of 4k.  It will
>> work just fine on any user-specified block size, though.
>> 
>> Signed-off-by: Jeff Moyer <jmoyer@redhat.com>
>> ---
>>  tests/xfs/300 | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>> 
>> diff --git a/tests/xfs/300 b/tests/xfs/300
>> index 28608b81..4f1c927a 100755
>> --- a/tests/xfs/300
>> +++ b/tests/xfs/300
>> @@ -50,8 +50,9 @@ $XFS_IO_PROG -f -c "pwrite -S 0x63 0 4096" $SCRATCH_MNT/attrvals >> $seqres.full
>>  cat $SCRATCH_MNT/attrvals | attr -s name $SCRATCH_MNT/$seq.test >> $seqres.full 2>&1
>>  
>>  # Fragment the file by writing backwards
>> +bs=$(_get_file_block_size $SCRATCH_MNT)
>>  for I in `seq 6 -1 0`; do
>> -	dd if=/dev/zero of=$SCRATCH_MNT/$seq.test seek=$I bs=4k \
>> +	dd if=/dev/zero of=$SCRATCH_MNT/$seq.test seek=$I bs=${bs} \
>
> Although the original case won't fail on 64k test. But this change makes
> more sense.

It will fail for the case mentioned in the cover letter.  That is:

MKFS_OPTIONS="-m reflink=0 -b size=65536" MOUNT_OPTIONS="-o dax"

on a system with >4k page size (xfs, in this case).

Thanks,
Jeff


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH V2 3/3] xfs/300: modify test to work on any fs block size
  2020-02-24 13:46     ` Jeff Moyer
@ 2020-02-26  7:42       ` Zorro Lang
  0 siblings, 0 replies; 13+ messages in thread
From: Zorro Lang @ 2020-02-26  7:42 UTC (permalink / raw)
  To: fstests

On Mon, Feb 24, 2020 at 08:46:40AM -0500, Jeff Moyer wrote:
> Zorro Lang <zlang@redhat.com> writes:
> 
> > On Thu, Feb 20, 2020 at 03:06:32PM -0500, Jeff Moyer wrote:
> >> The test currently assumes a file system block size of 4k.  It will
> >> work just fine on any user-specified block size, though.
> >> 
> >> Signed-off-by: Jeff Moyer <jmoyer@redhat.com>
> >> ---
> >>  tests/xfs/300 | 3 ++-
> >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/tests/xfs/300 b/tests/xfs/300
> >> index 28608b81..4f1c927a 100755
> >> --- a/tests/xfs/300
> >> +++ b/tests/xfs/300
> >> @@ -50,8 +50,9 @@ $XFS_IO_PROG -f -c "pwrite -S 0x63 0 4096" $SCRATCH_MNT/attrvals >> $seqres.full
> >>  cat $SCRATCH_MNT/attrvals | attr -s name $SCRATCH_MNT/$seq.test >> $seqres.full 2>&1
> >>  
> >>  # Fragment the file by writing backwards
> >> +bs=$(_get_file_block_size $SCRATCH_MNT)
> >>  for I in `seq 6 -1 0`; do
> >> -	dd if=/dev/zero of=$SCRATCH_MNT/$seq.test seek=$I bs=4k \
> >> +	dd if=/dev/zero of=$SCRATCH_MNT/$seq.test seek=$I bs=${bs} \
> >
> > Although the original case won't fail on 64k test. But this change makes
> > more sense.
> 
> It will fail for the case mentioned in the cover letter.  That is:
> 
> MKFS_OPTIONS="-m reflink=0 -b size=65536" MOUNT_OPTIONS="-o dax"
> 
> on a system with >4k page size (xfs, in this case).

Thanks, good to know that :)

> 
> Thanks,
> Jeff
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, back to index

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-20 20:06 [PATCH V2 0/3] fstests: fixes for 64k pages and dax Jeff Moyer
2020-02-20 20:06 ` [PATCH V2 1/3] dax/dm: disable testing on devices that don't support dax Jeff Moyer
2020-02-21  9:48   ` Zorro Lang
2020-02-23 15:07     ` Eryu Guan
2020-02-24  6:15       ` Zorro Lang
2020-02-20 20:06 ` [PATCH V2 2/3] t_mmap_collision: fix hard-coded page size Jeff Moyer
2020-02-21 13:53   ` Zorro Lang
2020-02-20 20:06 ` [PATCH V2 3/3] xfs/300: modify test to work on any fs block size Jeff Moyer
2020-02-22  5:31   ` Zorro Lang
2020-02-24 13:46     ` Jeff Moyer
2020-02-26  7:42       ` Zorro Lang
2020-02-20 21:21 ` [PATCH V2 0/3] fstests: fixes for 64k pages and dax Darrick J. Wong
2020-02-21 20:11   ` Jeff Moyer

FSTests Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/fstests/0 fstests/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 fstests fstests/ https://lore.kernel.org/fstests \
		fstests@vger.kernel.org
	public-inbox-index fstests

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.fstests


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git