Re: [PATCH] fstests: _require_dm_target should not always skip DAX capable devices

["Darrick J. Wong" <djwong@kernel.org>]

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