Re: [PATCH v3 2/4] generic: test mmap io fom DAX to non-DAX

[Xiong Zhou <xzhou@redhat.com>]

On Thu, Apr 13, 2017 at 06:36:34AM -0700, Dan Williams wrote:
> On Wed, Apr 12, 2017 at 9:11 PM, Xiong Zhou <xzhou@redhat.com> wrote:
> > On Wed, Apr 12, 2017 at 10:46:18PM +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".
> >>
> >> Adding helper to check pmem device status, which requires new
> >> PROGs ndctl to tweaking pmem devices and jq to parse ndctl's
> >> JSON format outputs.
> >>
> >> Signed-off-by: Xiong Zhou <xzhou@redhat.com>
> >> ---
> >>  common/config         |   2 +
> >>  common/rc             |  41 ++++++++++++++++++
> >>  tests/generic/423     | 113 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >>  tests/generic/423.out |   2 +
> >>  tests/generic/group   |   1 +
> >>  5 files changed, 159 insertions(+)
> >>  create mode 100755 tests/generic/423
> >>  create mode 100644 tests/generic/423.out
> >>
> >> diff --git a/common/config b/common/config
> >> index 59041a3..dfdcb8e 100644
> >> --- a/common/config
> >> +++ b/common/config
> >> @@ -212,6 +212,8 @@ export XZ_PROG="`set_prog_path xz`"
> >>  export FLOCK_PROG="`set_prog_path flock`"
> >>  export LDD_PROG="`set_prog_path ldd`"
> >>  export TIMEOUT_PROG="`set_prog_path timeout`"
> >> +export NDCTL_PROG="`set_prog_path ndctl`"
> >> +export JQ_PROG="`set_prog_path jq`"
> >>
> >>  # use 'udevadm settle' or 'udevsettle' to wait for lv to be settled.
> >>  # newer systems have udevadm command but older systems like RHEL5 don't.
> >> diff --git a/common/rc b/common/rc
> >> index 78a2101..35b8f6a 100644
> >> --- a/common/rc
> >> +++ b/common/rc
> >> @@ -3151,6 +3151,47 @@ _require_chattr()
> >>       rm -f $TEST_DIR/syscalltest.out
> >>  }
> >>
> >> +# Looking up pmem devices' arttributes in
> >> +# `ndctl list` output like this:
> >> +#  {
> >> +#    "dev":"namespace2.0",
> >> +#    "mode":"raw",
> >> +#    "size":8589934592,
> >> +#    "blockdev":"pmem2"
> >> +#  },
> >> +_ndctl_get_pmem_key_value()
> >> +{
> >> +     local dev=$1
> >> +     local key=$2
> >> +
> >> +     $NDCTL_PROG list | \
> >> +       $JQ_PROG -r ".[] | \
> >> +       select(.blockdev == \"${dev#/dev/}\") | .$key"
> >> +}
> >> +
> >> +# Require pmem device having specific arttibute key/value
> >> +# we need.
> >> +_require_pmem_key_value()
> >> +{
> >> +     local dev=$1
> >> +     local key=$2
> >> +     local value=$3
> >> +
> >> +     [[ ! "${dev#/dev/}" =~ pmem ]] && \
> >> +       _notrun "this test requires pmem device"
> >> +
> >> +     # if pmem devices are setup by memmap, just run
> >> +     grep -q memmap /proc/cmdline && return
> >> +
> >> +     # pmem devices from NFIT
> >> +     _require_command "$NDCTL_PROG" "ndctl"
> >> +     _require_command "$JQ_PROG" "jq"
> >> +     # get dev specific attr
> >> +     dev_value=$(_ndctl_get_pmem_key_value $dev $key)
> >> +     [ "$dev_value" != "$value" ] && \
> >> +       _notrun "this test requires $dev $value $key"
> >
> > This is too ugly. I'm looking for another way by searching sysfs for
> > required info of *_DEV, then we don't need these commands and ugly
> > device name matching.
> >
> > Thanks Eryu for the review!
> 
> Why not just let jq do this key matching validation for you? I don't

I agree with Ross that we don't even need ndctl and jq, if we run tests
on memmap setup.

> think the word NFIT should appear anywhere in xfstests. That's an
> internal detail of an nvdimm implementation and going forward makes no
> sense when we have NVDIMMs on powerpc for example.

Fair enough.