| Message ID | 20211026023622.914273-1-david@fromorbit.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fstests: _require_dm_target should not always skip DAX capable devices | expand |
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 >
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.
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
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) ;;