Re: [PATCH v7] generic/413: skip dax to nondax dio test if needed

Xiong Zhou
2017-09-30T02:30:38Z

On Fri, Sep 29, 2017 at 10:24:46AM -0600, Ross Zwisler wrote:
> On Fri, Sep 29, 2017 at 10:16:11AM +0800, Xiong Zhou wrote:
> > This test requires there is struct page backend for the
> > testing dax device. But not all devices which support dax
> > have that. So we give it a try, if it fails with EFAULT,
> > which is the same errno with the wrong device situation,
> > we skip this subtest.
> >
> > This is not perfect, but it's efficient. Many devices
> > support dax, and there are more coming. It's nearly
> > impossible to maintain an uniq way to detect struct page
> > present for all kinds of devices modes.
> >
> > From testing perspectice, a testrun could cover this
> > code path as a sanity check and avoid more unnecessary
> > failires. If the device is compatible with the test, one
> > more testrun will not hurt much.
> >
> > Signed-off-by: Xiong Zhou <xzhou@redhat.com>
> > ---
> >
> > v7:
> > split this patch from that unrelated series.
> > minor comment fix.
> >
> > src/t_mmap_dio.c | 2 +-
> > tests/generic/413 | 12 ++++++++++++
> > 2 files changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/t_mmap_dio.c b/src/t_mmap_dio.c
> > index 6c8ca1a..73e7648 100644
> > --- a/src/t_mmap_dio.c
> > +++ b/src/t_mmap_dio.c
> > @@ -27,7 +27,7 @@ void err_exit(char *op, unsigned long len, char *s)
> > {
> > fprintf(stderr, "%s(%s) len %lu %s\n",
> > op, strerror(errno), len, s);
> > - exit(1);
> > + exit(errno);
> > }
> >
> > int main(int argc, char **argv)
> > diff --git a/tests/generic/413 b/tests/generic/413
> > index a1cc514..311bdc2 100755
> > --- a/tests/generic/413
> > +++ b/tests/generic/413
> > @@ -88,6 +88,18 @@ t_nondax_to_dax()
> > t_dax_to_nondax()
> > {
> > prep_files
> > +
> > + # dax to nondax dio needs struct page backend, which is
> > + # not always available among various devices. Skip this
> > + # subtest if EFAULT(14 Bad address) returned, which means
> > + # probably the device is not compatible with this test.
> > + #
> > + src/t_mmap_dio $SCRATCH_MNT/tf_s $TEST_DIR/tf_d \
> > + $1 "test" > /dev/null 2>&1
> > + if [ $? -eq 14 ] ; then
> > + return
> > + fi
> > +
> > src/t_mmap_dio $SCRATCH_MNT/tf_s \
> > $TEST_DIR/tf_d $1 "dio dax to nondax"
> >
> > --
> > 1.8.3.1
>
> I think you may have missed this feedback from Jeff & Eryu? (I can't find an
> online archive of fstests, so just copy-n-pasting here:
>
> Eryu Guan <eguan@redhat.com> writes:
>
> > On Mon, Sep 25, 2017 at 04:40:46PM +0800, Xiong Zhou wrote:
> >> Since not all devices support dax has struct page backend,
> >> which will not support this test.
> >>
> >> Signed-off-by: Xiong Zhou <xzhou@redhat.com>
> >> ---
> >> tests/generic/413 | 8 ++++++++
> >> 1 file changed, 8 insertions(+)
> >>
> >> diff --git a/tests/generic/413 b/tests/generic/413
> >> index a1cc514..b86c10f 100755
> >> --- a/tests/generic/413
> >> +++ b/tests/generic/413
> >> @@ -88,6 +88,14 @@ t_nondax_to_dax()
> >> t_dax_to_nondax()
> >> {
> >> prep_files
> >> + # dax to nondax dio needs struct page backend, which is
> >> + # not always avaiable among various devices. Skip this
> >> + # subtest if not compatible.
> >> + if ! src/t_mmap_dio $SCRATCH_MNT/tf_s \
> >> + $TEST_DIR/tf_d $1 "test" > /dev/null 2>&1 ; then
> >> + return
> >> + fi
> >> +
> >
> > Then we will never get a failure from this case, even if it's a real
> > bug.. We need better way to tell if there's struct page present :)

I was adding errno based verdict in v7 per Eryu's advice.

>
> ndctl list will tell you the mode of the namespace. If it's 'raw', then
> it doesn't have struct page backing. If it's 'memory', it should work
> fine.
>
> -Jeff

My original solution was also ndctl. However ndctl can't do all the
job. brd ramdisk and tmpfs support dax but they have nothing to do
with NFIT.

Quoting HCH:

On Tue, Apr 18, 2017 at 03:12:26AM -0700, Christoph Hellwig wrote:
> On Mon, Apr 17, 2017 at 03:14:13PM +0800, Xiong Zhou wrote:
> > Mount TEST_DEV as non-DAX, SCRATCH_DEV as DAX, then
> > do mmap DIO from DAX to non-DAX.
> >
> > This test is split from generic/413. Since DIO from DAX
> > to non-DAX is only supported/doable when device underneath
> > has memory(struct page) backend.
> >
> > By ndctl looking at SCRATCH_DEV, skip this test if it is
> > not in "memory mode".
>
> DAX devices don't need to be something using NFIT, so I don't think this
> method is correct.

Thanks,
Xiong