* [PATCH] fstests: _require_dm_target should not always skip DAX capable devices
@ 2021-10-26 2:36 Dave Chinner
2021-10-26 15:42 ` Darrick J. Wong
0 siblings, 1 reply; 7+ messages in thread
From: Dave Chinner @ 2021-10-26 2:36 UTC (permalink / raw)
To: fstests
From: Dave Chinner <dchinner@redhat.com>
Recent changes have turned off all dm-error, dm-thin and dm-flakey
tests on pmem devices even when we are not explicitly testing DAX.
This is a regression resulting in a large number of log recovery
tests no longer running on my pmem-based test VMs. I added the "-o
dax=never" mount options to these test configs, only to find it
still would not run the dm tests even though the filesystem will
never use DAX.
Fix this so that the dm target DAX test explicitly ignores the
the block device DAX capability when the filesystem is mounted with
dax=never and hence we can use all the dm targets when the tests are
being run with FSDAX disabled.
Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
common/rc | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/common/rc b/common/rc
index 7f693d39..0cbe8a7d 100644
--- a/common/rc
+++ b/common/rc
@@ -1965,12 +1965,19 @@ _require_sane_bdev_flush()
}
# Decide if the scratch filesystem is likely to be mounted in fsdax mode.
-# If there's a dax clause in the mount options we assume the test runner
-# wants us to test DAX; or if the scratch device itself advertises dax mode
-# in sysfs.
-__detect_scratch_fsdax()
+# It goes 3 ways based on mount options::
+# 1. "dax" or "dax=always" means always test using DAX
+# 2. "dax=never" means we'll never use DAX
+# 3. "dax=inode" or nothing means "use scratch dev capability" to
+# determine whether DAX is going to be used.
+#
+# Returns 0 if DAX will be used, 1 if DAX is not going to be used.
+__scratch_uses_fsdax()
{
- _normalize_mount_options | egrep -q "dax(=always| |$)" && return 0
+ local ops=$(_normalize_mount_options)
+
+ echo $ops | egrep -q "dax(=always| |$)" && return 0
+ echo $ops | grep -q "dax=never" && return 1
local sysfs="/sys/block/$(_short_dev $SCRATCH_DEV)"
test -e "${sysfs}/dax" && return 0
@@ -1982,6 +1989,8 @@ __detect_scratch_fsdax()
_require_dm_target()
{
local target=$1
+ local fsdax
+ local bdevdax
# require SCRATCH_DEV to be a valid block device with sane BLKFLSBUF
# behaviour
@@ -1989,7 +1998,7 @@ _require_dm_target()
_require_sane_bdev_flush $SCRATCH_DEV
_require_command "$DMSETUP_PROG" dmsetup
- if __detect_scratch_fsdax; then
+ if __scratch_uses_fsdax; then
case $target in
stripe|linear|log-writes)
;;
--
2.33.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] fstests: _require_dm_target should not always skip DAX capable devices
2021-10-26 2:36 [PATCH] fstests: _require_dm_target should not always skip DAX capable devices Dave Chinner
@ 2021-10-26 15:42 ` Darrick J. Wong
2021-10-26 23:50 ` Dave Chinner
0 siblings, 1 reply; 7+ messages in thread
From: Darrick J. Wong @ 2021-10-26 15:42 UTC (permalink / raw)
To: Dave Chinner; +Cc: fstests
On Tue, Oct 26, 2021 at 01:36:22PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> Recent changes have turned off all dm-error, dm-thin and dm-flakey
> tests on pmem devices even when we are not explicitly testing DAX.
> This is a regression resulting in a large number of log recovery
> tests no longer running on my pmem-based test VMs. I added the "-o
> dax=never" mount options to these test configs, only to find it
> still would not run the dm tests even though the filesystem will
> never use DAX.
>
> Fix this so that the dm target DAX test explicitly ignores the
> the block device DAX capability when the filesystem is mounted with
> dax=never and hence we can use all the dm targets when the tests are
> being run with FSDAX disabled.
>
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
> common/rc | 21 +++++++++++++++------
> 1 file changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/common/rc b/common/rc
> index 7f693d39..0cbe8a7d 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -1965,12 +1965,19 @@ _require_sane_bdev_flush()
> }
>
> # Decide if the scratch filesystem is likely to be mounted in fsdax mode.
> -# If there's a dax clause in the mount options we assume the test runner
> -# wants us to test DAX; or if the scratch device itself advertises dax mode
> -# in sysfs.
> -__detect_scratch_fsdax()
> +# It goes 3 ways based on mount options::
> +# 1. "dax" or "dax=always" means always test using DAX
> +# 2. "dax=never" means we'll never use DAX
> +# 3. "dax=inode" or nothing means "use scratch dev capability" to
> +# determine whether DAX is going to be used.
> +#
> +# Returns 0 if DAX will be used, 1 if DAX is not going to be used.
> +__scratch_uses_fsdax()
> {
> - _normalize_mount_options | egrep -q "dax(=always| |$)" && return 0
> + local ops=$(_normalize_mount_options)
> +
> + echo $ops | egrep -q "dax(=always| |$)" && return 0
> + echo $ops | grep -q "dax=never" && return 1
Question: Do you want grep -w here so that grep won't match on anything
that isn't a whole "word"?
e.g. MOUNT_OPTIONS="-o ihatedax=alwaysp*****gmeoff" shouldn't be a
match, right?
--D
>
> local sysfs="/sys/block/$(_short_dev $SCRATCH_DEV)"
> test -e "${sysfs}/dax" && return 0
> @@ -1982,6 +1989,8 @@ __detect_scratch_fsdax()
> _require_dm_target()
> {
> local target=$1
> + local fsdax
> + local bdevdax
>
> # require SCRATCH_DEV to be a valid block device with sane BLKFLSBUF
> # behaviour
> @@ -1989,7 +1998,7 @@ _require_dm_target()
> _require_sane_bdev_flush $SCRATCH_DEV
> _require_command "$DMSETUP_PROG" dmsetup
>
> - if __detect_scratch_fsdax; then
> + if __scratch_uses_fsdax; then
> case $target in
> stripe|linear|log-writes)
> ;;
> --
> 2.33.0
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] fstests: _require_dm_target should not always skip DAX capable devices
2021-10-26 15:42 ` Darrick J. Wong
@ 2021-10-26 23:50 ` Dave Chinner
2021-10-27 0:26 ` Darrick J. Wong
0 siblings, 1 reply; 7+ messages in thread
From: Dave Chinner @ 2021-10-26 23:50 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: fstests
On Tue, Oct 26, 2021 at 08:42:40AM -0700, Darrick J. Wong wrote:
> On Tue, Oct 26, 2021 at 01:36:22PM +1100, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> >
> > Recent changes have turned off all dm-error, dm-thin and dm-flakey
> > tests on pmem devices even when we are not explicitly testing DAX.
> > This is a regression resulting in a large number of log recovery
> > tests no longer running on my pmem-based test VMs. I added the "-o
> > dax=never" mount options to these test configs, only to find it
> > still would not run the dm tests even though the filesystem will
> > never use DAX.
> >
> > Fix this so that the dm target DAX test explicitly ignores the
> > the block device DAX capability when the filesystem is mounted with
> > dax=never and hence we can use all the dm targets when the tests are
> > being run with FSDAX disabled.
> >
> > Signed-off-by: Dave Chinner <dchinner@redhat.com>
> > ---
> > common/rc | 21 +++++++++++++++------
> > 1 file changed, 15 insertions(+), 6 deletions(-)
> >
> > diff --git a/common/rc b/common/rc
> > index 7f693d39..0cbe8a7d 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -1965,12 +1965,19 @@ _require_sane_bdev_flush()
> > }
> >
> > # Decide if the scratch filesystem is likely to be mounted in fsdax mode.
> > -# If there's a dax clause in the mount options we assume the test runner
> > -# wants us to test DAX; or if the scratch device itself advertises dax mode
> > -# in sysfs.
> > -__detect_scratch_fsdax()
> > +# It goes 3 ways based on mount options::
> > +# 1. "dax" or "dax=always" means always test using DAX
> > +# 2. "dax=never" means we'll never use DAX
> > +# 3. "dax=inode" or nothing means "use scratch dev capability" to
> > +# determine whether DAX is going to be used.
> > +#
> > +# Returns 0 if DAX will be used, 1 if DAX is not going to be used.
> > +__scratch_uses_fsdax()
> > {
> > - _normalize_mount_options | egrep -q "dax(=always| |$)" && return 0
> > + local ops=$(_normalize_mount_options)
> > +
> > + echo $ops | egrep -q "dax(=always| |$)" && return 0
> > + echo $ops | grep -q "dax=never" && return 1
>
> Question: Do you want grep -w here so that grep won't match on anything
> that isn't a whole "word"?
>
> e.g. MOUNT_OPTIONS="-o ihatedax=alwaysp*****gmeoff" shouldn't be a
> match, right?
At which point fstests will be useless because the scratch device
won't mount.... :/
But, sure, if we care about people fuzzing fstests mount options
that much we can make it "grep -qw"....
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] fstests: _require_dm_target should not always skip DAX capable devices
2021-10-26 23:50 ` Dave Chinner
@ 2021-10-27 0:26 ` Darrick J. Wong
2021-10-27 1:38 ` [PATCH V2] " Dave Chinner
0 siblings, 1 reply; 7+ messages in thread
From: Darrick J. Wong @ 2021-10-27 0:26 UTC (permalink / raw)
To: Dave Chinner; +Cc: fstests
On Wed, Oct 27, 2021 at 10:50:58AM +1100, Dave Chinner wrote:
> On Tue, Oct 26, 2021 at 08:42:40AM -0700, Darrick J. Wong wrote:
> > On Tue, Oct 26, 2021 at 01:36:22PM +1100, Dave Chinner wrote:
> > > From: Dave Chinner <dchinner@redhat.com>
> > >
> > > Recent changes have turned off all dm-error, dm-thin and dm-flakey
> > > tests on pmem devices even when we are not explicitly testing DAX.
> > > This is a regression resulting in a large number of log recovery
> > > tests no longer running on my pmem-based test VMs. I added the "-o
> > > dax=never" mount options to these test configs, only to find it
> > > still would not run the dm tests even though the filesystem will
> > > never use DAX.
> > >
> > > Fix this so that the dm target DAX test explicitly ignores the
> > > the block device DAX capability when the filesystem is mounted with
> > > dax=never and hence we can use all the dm targets when the tests are
> > > being run with FSDAX disabled.
> > >
> > > Signed-off-by: Dave Chinner <dchinner@redhat.com>
> > > ---
> > > common/rc | 21 +++++++++++++++------
> > > 1 file changed, 15 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/common/rc b/common/rc
> > > index 7f693d39..0cbe8a7d 100644
> > > --- a/common/rc
> > > +++ b/common/rc
> > > @@ -1965,12 +1965,19 @@ _require_sane_bdev_flush()
> > > }
> > >
> > > # Decide if the scratch filesystem is likely to be mounted in fsdax mode.
> > > -# If there's a dax clause in the mount options we assume the test runner
> > > -# wants us to test DAX; or if the scratch device itself advertises dax mode
> > > -# in sysfs.
> > > -__detect_scratch_fsdax()
> > > +# It goes 3 ways based on mount options::
> > > +# 1. "dax" or "dax=always" means always test using DAX
> > > +# 2. "dax=never" means we'll never use DAX
> > > +# 3. "dax=inode" or nothing means "use scratch dev capability" to
> > > +# determine whether DAX is going to be used.
> > > +#
> > > +# Returns 0 if DAX will be used, 1 if DAX is not going to be used.
> > > +__scratch_uses_fsdax()
> > > {
> > > - _normalize_mount_options | egrep -q "dax(=always| |$)" && return 0
> > > + local ops=$(_normalize_mount_options)
> > > +
> > > + echo $ops | egrep -q "dax(=always| |$)" && return 0
> > > + echo $ops | grep -q "dax=never" && return 1
> >
> > Question: Do you want grep -w here so that grep won't match on anything
> > that isn't a whole "word"?
> >
> > e.g. MOUNT_OPTIONS="-o ihatedax=alwaysp*****gmeoff" shouldn't be a
> > match, right?
>
> At which point fstests will be useless because the scratch device
> won't mount.... :/
>
> But, sure, if we care about people fuzzing fstests mount options
> that much we can make it "grep -qw"....
I actually meant someone creating a legitimate mount option for another
filesystem that happens to match 'dax=always', not someone feeding bad
options. Maybe I should have given as an example:
mount -t deepspace9 -o everydax=alwaystrill /dev/sda /mnt
or something...
--D
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@fromorbit.com
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH V2] fstests: _require_dm_target should not always skip DAX capable devices
2021-10-27 0:26 ` Darrick J. Wong
@ 2021-10-27 1:38 ` Dave Chinner
2021-10-27 3:17 ` Darrick J. Wong
2021-10-27 12:04 ` Zorro Lang
0 siblings, 2 replies; 7+ messages in thread
From: Dave Chinner @ 2021-10-27 1:38 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: fstests
From: Dave Chinner <dchinner@redhat.com>
Recent changes have turned off all dm-error, dm-thin and dm-flakey
tests on pmem devices even when we are not explicitly testing DAX.
This is a regression resulting in a large number of log recovery
tests no longer running on my pmem-based test VMs. I added the "-o
dax=never" mount options to these test configs, only to find it
still would not run the dm tests even though the filesystem will
never use DAX.
Fix this so that the dm target DAX test explicitly ignores the
the block device DAX capability when the filesystem is mounted with
dax=never and hence we can use all the dm targets when the tests are
being run with FSDAX disabled.
Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
Version 2:
-use -w for grep command to do exact word matching for options.
common/rc | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/common/rc b/common/rc
index 7f693d39..75197453 100644
--- a/common/rc
+++ b/common/rc
@@ -1965,12 +1965,19 @@ _require_sane_bdev_flush()
}
# Decide if the scratch filesystem is likely to be mounted in fsdax mode.
-# If there's a dax clause in the mount options we assume the test runner
-# wants us to test DAX; or if the scratch device itself advertises dax mode
-# in sysfs.
-__detect_scratch_fsdax()
+# It goes 3 ways based on mount options::
+# 1. "dax" or "dax=always" means always test using DAX
+# 2. "dax=never" means we'll never use DAX
+# 3. "dax=inode" or nothing means "use scratch dev capability" to
+# determine whether DAX is going to be used.
+#
+# Returns 0 if DAX will be used, 1 if DAX is not going to be used.
+__scratch_uses_fsdax()
{
- _normalize_mount_options | egrep -q "dax(=always| |$)" && return 0
+ local ops=$(_normalize_mount_options)
+
+ echo $ops | egrep -qw "dax(=always| |$)" && return 0
+ echo $ops | grep -qw "dax=never" && return 1
local sysfs="/sys/block/$(_short_dev $SCRATCH_DEV)"
test -e "${sysfs}/dax" && return 0
@@ -1982,6 +1989,8 @@ __detect_scratch_fsdax()
_require_dm_target()
{
local target=$1
+ local fsdax
+ local bdevdax
# require SCRATCH_DEV to be a valid block device with sane BLKFLSBUF
# behaviour
@@ -1989,7 +1998,7 @@ _require_dm_target()
_require_sane_bdev_flush $SCRATCH_DEV
_require_command "$DMSETUP_PROG" dmsetup
- if __detect_scratch_fsdax; then
+ if __scratch_uses_fsdax; then
case $target in
stripe|linear|log-writes)
;;
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH V2] fstests: _require_dm_target should not always skip DAX capable devices
2021-10-27 1:38 ` [PATCH V2] " Dave Chinner
@ 2021-10-27 3:17 ` Darrick J. Wong
2021-10-27 12:04 ` Zorro Lang
1 sibling, 0 replies; 7+ messages in thread
From: Darrick J. Wong @ 2021-10-27 3:17 UTC (permalink / raw)
To: Dave Chinner; +Cc: fstests
On Wed, Oct 27, 2021 at 12:38:42PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> Recent changes have turned off all dm-error, dm-thin and dm-flakey
> tests on pmem devices even when we are not explicitly testing DAX.
> This is a regression resulting in a large number of log recovery
> tests no longer running on my pmem-based test VMs. I added the "-o
> dax=never" mount options to these test configs, only to find it
> still would not run the dm tests even though the filesystem will
> never use DAX.
>
> Fix this so that the dm target DAX test explicitly ignores the
> the block device DAX capability when the filesystem is mounted with
> dax=never and hence we can use all the dm targets when the tests are
> being run with FSDAX disabled.
>
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
Looks good to me,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
--D
> ---
> Version 2:
> -use -w for grep command to do exact word matching for options.
>
> common/rc | 21 +++++++++++++++------
> 1 file changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/common/rc b/common/rc
> index 7f693d39..75197453 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -1965,12 +1965,19 @@ _require_sane_bdev_flush()
> }
>
> # Decide if the scratch filesystem is likely to be mounted in fsdax mode.
> -# If there's a dax clause in the mount options we assume the test runner
> -# wants us to test DAX; or if the scratch device itself advertises dax mode
> -# in sysfs.
> -__detect_scratch_fsdax()
> +# It goes 3 ways based on mount options::
> +# 1. "dax" or "dax=always" means always test using DAX
> +# 2. "dax=never" means we'll never use DAX
> +# 3. "dax=inode" or nothing means "use scratch dev capability" to
> +# determine whether DAX is going to be used.
> +#
> +# Returns 0 if DAX will be used, 1 if DAX is not going to be used.
> +__scratch_uses_fsdax()
> {
> - _normalize_mount_options | egrep -q "dax(=always| |$)" && return 0
> + local ops=$(_normalize_mount_options)
> +
> + echo $ops | egrep -qw "dax(=always| |$)" && return 0
> + echo $ops | grep -qw "dax=never" && return 1
>
> local sysfs="/sys/block/$(_short_dev $SCRATCH_DEV)"
> test -e "${sysfs}/dax" && return 0
> @@ -1982,6 +1989,8 @@ __detect_scratch_fsdax()
> _require_dm_target()
> {
> local target=$1
> + local fsdax
> + local bdevdax
>
> # require SCRATCH_DEV to be a valid block device with sane BLKFLSBUF
> # behaviour
> @@ -1989,7 +1998,7 @@ _require_dm_target()
> _require_sane_bdev_flush $SCRATCH_DEV
> _require_command "$DMSETUP_PROG" dmsetup
>
> - if __detect_scratch_fsdax; then
> + if __scratch_uses_fsdax; then
> case $target in
> stripe|linear|log-writes)
> ;;
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH V2] fstests: _require_dm_target should not always skip DAX capable devices
2021-10-27 1:38 ` [PATCH V2] " Dave Chinner
2021-10-27 3:17 ` Darrick J. Wong
@ 2021-10-27 12:04 ` Zorro Lang
1 sibling, 0 replies; 7+ messages in thread
From: Zorro Lang @ 2021-10-27 12:04 UTC (permalink / raw)
To: fstests
On Wed, Oct 27, 2021 at 12:38:42PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> Recent changes have turned off all dm-error, dm-thin and dm-flakey
> tests on pmem devices even when we are not explicitly testing DAX.
> This is a regression resulting in a large number of log recovery
> tests no longer running on my pmem-based test VMs. I added the "-o
> dax=never" mount options to these test configs, only to find it
> still would not run the dm tests even though the filesystem will
> never use DAX.
>
> Fix this so that the dm target DAX test explicitly ignores the
> the block device DAX capability when the filesystem is mounted with
> dax=never and hence we can use all the dm targets when the tests are
> being run with FSDAX disabled.
>
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
Make sense.
Reviewed-by: Zorro Lang <zlang@redhat.com>
> Version 2:
> -use -w for grep command to do exact word matching for options.
>
> common/rc | 21 +++++++++++++++------
> 1 file changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/common/rc b/common/rc
> index 7f693d39..75197453 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -1965,12 +1965,19 @@ _require_sane_bdev_flush()
> }
>
> # Decide if the scratch filesystem is likely to be mounted in fsdax mode.
> -# If there's a dax clause in the mount options we assume the test runner
> -# wants us to test DAX; or if the scratch device itself advertises dax mode
> -# in sysfs.
> -__detect_scratch_fsdax()
> +# It goes 3 ways based on mount options::
> +# 1. "dax" or "dax=always" means always test using DAX
> +# 2. "dax=never" means we'll never use DAX
> +# 3. "dax=inode" or nothing means "use scratch dev capability" to
> +# determine whether DAX is going to be used.
> +#
> +# Returns 0 if DAX will be used, 1 if DAX is not going to be used.
> +__scratch_uses_fsdax()
> {
> - _normalize_mount_options | egrep -q "dax(=always| |$)" && return 0
> + local ops=$(_normalize_mount_options)
> +
> + echo $ops | egrep -qw "dax(=always| |$)" && return 0
> + echo $ops | grep -qw "dax=never" && return 1
>
> local sysfs="/sys/block/$(_short_dev $SCRATCH_DEV)"
> test -e "${sysfs}/dax" && return 0
> @@ -1982,6 +1989,8 @@ __detect_scratch_fsdax()
> _require_dm_target()
> {
> local target=$1
> + local fsdax
> + local bdevdax
>
> # require SCRATCH_DEV to be a valid block device with sane BLKFLSBUF
> # behaviour
> @@ -1989,7 +1998,7 @@ _require_dm_target()
> _require_sane_bdev_flush $SCRATCH_DEV
> _require_command "$DMSETUP_PROG" dmsetup
>
> - if __detect_scratch_fsdax; then
> + if __scratch_uses_fsdax; then
> case $target in
> stripe|linear|log-writes)
> ;;
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-10-27 11:46 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-26 2:36 [PATCH] fstests: _require_dm_target should not always skip DAX capable devices Dave Chinner
2021-10-26 15:42 ` Darrick J. Wong
2021-10-26 23:50 ` Dave Chinner
2021-10-27 0:26 ` Darrick J. Wong
2021-10-27 1:38 ` [PATCH V2] " Dave Chinner
2021-10-27 3:17 ` Darrick J. Wong
2021-10-27 12:04 ` Zorro Lang
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.