[PATCH 0/3] fstests: fixes for 64k pages and dax

[PATCH 0/3] fstests: fixes for 64k pages and dax

[Jeff Moyer]

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.  Right now we
only test for dax incompatibility in the dm flakey target.  This means
that tests that use dm-thin or the snapshot target will still try to
run.  Moving the check to _require_dm_target fixes that problem.

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.

Thanks!
Jeff

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

[PATCH 1/3] dax/dm: disable testing on devices that don't support dax

[Jeff Moyer]

Move the hack out of dmflakey and put it into _require_dm_target.  This
fixes up a lot of missed tests that are failing due to the lack of dax
support (such as tests on dm-thin, snapshot, etc).

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

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/rc b/common/rc
index eeac1355..785f34c6 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|error)
+			;;
+		*)
+			_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

[PATCH 2/3] t_mmap_collision: fix hard-coded page size

[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

[PATCH 3/3] xfs/300: modify test to work on any fs block size

[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

Re: [PATCH 0/3] fstests: fixes for 64k pages and dax

[Dave Chinner]

[cc fstests@vger.kernel.org]

On Wed, Feb 05, 2020 at 05:48:15PM -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.  Right now we
> only test for dax incompatibility in the dm flakey target.  This means
> that tests that use dm-thin or the snapshot target will still try to
> run.  Moving the check to _require_dm_target fixes that problem.
> 
> 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.
> 
> Thanks!
> Jeff
> 
> [PATCH 1/3] dax/dm: disable testing on devices that don't support dax
> [PATCH 2/3] t_mmap_collision: fix hard-coded page size
> [PATCH 3/3] xfs/300: modify test to work on any fs block size

Hi Jeff,

You probably should be sending fstests patches to
fstests@vger.kernel.org, otherwise they probably won't get noticed
by the fstests maintainer...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 1/3] dax/dm: disable testing on devices that don't support dax

[Zorro Lang]

On Wed, Feb 05, 2020 at 05:48:16PM -0500, Jeff Moyer wrote:
> Move the hack out of dmflakey and put it into _require_dm_target.  This
> fixes up a lot of missed tests that are failing due to the lack of dax
> support (such as tests on dm-thin, snapshot, etc).
> 
> Signed-off-by: Jeff Moyer <jmoyer@redhat.com>
> ---
>  common/dmflakey |  5 -----
>  common/rc       | 11 +++++++++++
>  2 files changed, 11 insertions(+), 5 deletions(-)
> 
> 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

If we need to remove this for common/dmflakey, why not do the same thing
in common/dmthin and common/dmdelay etc ?

> -
>  _init_flakey()
>  {
>  	local BLK_DEV_SIZE=`blockdev --getsz $SCRATCH_DEV`
> diff --git a/common/rc b/common/rc
> index eeac1355..785f34c6 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|error)
> +			;;
> +		*)
> +			_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
> 

Re: [PATCH 1/3] dax/dm: disable testing on devices that don't support dax

[Jeff Moyer]

Zorro Lang <zlang@redhat.com> writes:

> On Wed, Feb 05, 2020 at 05:48:16PM -0500, Jeff Moyer wrote:
>> Move the hack out of dmflakey and put it into _require_dm_target.  This
>> fixes up a lot of missed tests that are failing due to the lack of dax
>> support (such as tests on dm-thin, snapshot, etc).
>> 
>> Signed-off-by: Jeff Moyer <jmoyer@redhat.com>
>> ---
>>  common/dmflakey |  5 -----
>>  common/rc       | 11 +++++++++++
>>  2 files changed, 11 insertions(+), 5 deletions(-)
>> 
>> 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
>
> If we need to remove this for common/dmflakey, why not do the same thing
> in common/dmthin and common/dmdelay etc ?

I didn't realize they had this same code.  I'll make that change,
thanks!

-Jeff

>
>> -
>>  _init_flakey()
>>  {
>>  	local BLK_DEV_SIZE=`blockdev --getsz $SCRATCH_DEV`
>> diff --git a/common/rc b/common/rc
>> index eeac1355..785f34c6 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|error)
>> +			;;
>> +		*)
>> +			_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
>> 

Re: [PATCH 0/3] fstests: fixes for 64k pages and dax

[Jeff Moyer]

Dave Chinner <david@fromorbit.com> writes:

> [cc fstests@vger.kernel.org]
>
> On Wed, Feb 05, 2020 at 05:48:15PM -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.  Right now we
>> only test for dax incompatibility in the dm flakey target.  This means
>> that tests that use dm-thin or the snapshot target will still try to
>> run.  Moving the check to _require_dm_target fixes that problem.
>> 
>> 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.
>> 
>> Thanks!
>> Jeff
>> 
>> [PATCH 1/3] dax/dm: disable testing on devices that don't support dax
>> [PATCH 2/3] t_mmap_collision: fix hard-coded page size
>> [PATCH 3/3] xfs/300: modify test to work on any fs block size
>
> Hi Jeff,
>
> You probably should be sending fstests patches to
> fstests@vger.kernel.org, otherwise they probably won't get noticed
> by the fstests maintainer...

Hm, somehow I didn't know about that list.  I'll send v2 there, thanks!

-Jeff