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

Dan Williams
2017-04-14T14:49:49Z

On Fri, Apr 14, 2017 at 3:01 AM, Xiong Zhou <xzhou@redhat.com> wrote:
> 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.

I don't understand this comment. How is the test determining if it is
on an memmap defined namespace without querying sysfs?