| Message ID | 20220713075157.411479-1-vishal.l.verma@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ndctl] cxl/test: add a test to {read,write,zero}-labels | expand |
On Wed, 13 Jul 2022, Vishal Verma wrote: >Add a unit test to test writing, reading, and zeroing LSA aread for >cxl_test based memdevs using ndctl commands. > >Cc: Dan Williams <dan.j.williams@intel.com> >Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> >--- > test/cxl-labels.sh | 53 ++++++++++++++++++++++++++++++++++++++++++++++ > test/meson.build | 2 ++ > 2 files changed, 55 insertions(+) > create mode 100644 test/cxl-labels.sh > >diff --git a/test/cxl-labels.sh b/test/cxl-labels.sh >new file mode 100644 >index 0000000..ce73963 >--- /dev/null >+++ b/test/cxl-labels.sh >@@ -0,0 +1,53 @@ >+#!/bin/bash >+# SPDX-License-Identifier: GPL-2.0 >+# Copyright (C) 2022 Intel Corporation. All rights reserved. >+ >+. $(dirname $0)/common >+ >+rc=1 >+ >+set -ex >+ >+trap 'err $LINENO' ERR >+ >+check_prereq "jq" >+ >+modprobe -r cxl_test >+modprobe cxl_test >+udevadm settle >+ >+test_label_ops() >+{ >+ nmem="$1" >+ lsa=$(mktemp /tmp/lsa-$nmem.XXXX) >+ lsa_read=$(mktemp /tmp/lsa-read-$nmem.XXXX) >+ >+ # determine LSA size >+ "$NDCTL" read-labels -o "$lsa_read" "$nmem" >+ lsa_size=$(stat -c %s "$lsa_read") >+ >+ dd "if=/dev/urandom" "of=$lsa" "bs=$lsa_size" "count=1" >+ "$NDCTL" write-labels -i "$lsa" "$nmem" >+ "$NDCTL" read-labels -o "$lsa_read" "$nmem" >+ >+ # compare what was written vs read >+ diff "$lsa" "$lsa_read" >+ >+ # zero the LSA and test >+ "$NDCTL" zero-labels "$nmem" >+ dd "if=/dev/zero" "of=$lsa" "bs=$lsa_size" "count=1" >+ "$NDCTL" read-labels -o "$lsa_read" "$nmem" >+ diff "$lsa" "$lsa_read" >+ >+ # cleanup >+ rm "$lsa" "$lsa_read" >+} >+ >+# find nmem devices corresponding to cxl memdevs >+readarray -t nmems < <("$NDCTL" list -b cxl_test -Di | jq -r '.[].dev') s/$NDCTL/$CXL Beyond sharing a repo, I would really avoid mixing and matching ndctl and cxl tooling and thereby keep them self sufficient. I understand that there are cases where pmem specific operations can can be done reusing relevant pmem/nvdimm/ndctl machinery and interfaces, but I don't see this as the case for something like lsa unit testing. Thanks, Davidlohr >+ >+for nmem in ${nmems[@]}; do >+ test_label_ops "$nmem" >+done >+ >+modprobe -r cxl_test >diff --git a/test/meson.build b/test/meson.build >index fbcfc08..687a71f 100644 >--- a/test/meson.build >+++ b/test/meson.build >@@ -152,6 +152,7 @@ pfn_meta_errors = find_program('pfn-meta-errors.sh') > track_uuid = find_program('track-uuid.sh') > cxl_topo = find_program('cxl-topology.sh') > cxl_region = find_program('cxl-region-create.sh') >+cxl_labels = find_program('cxl-labels.sh') > > tests = [ > [ 'libndctl', libndctl, 'ndctl' ], >@@ -178,6 +179,7 @@ tests = [ > [ 'track-uuid.sh', track_uuid, 'ndctl' ], > [ 'cxl-topology.sh', cxl_topo, 'cxl' ], > [ 'cxl-region-create.sh', cxl_region, 'cxl' ], >+ [ 'cxl-labels.sh', cxl_labels, 'cxl' ], > ] > > if get_option('destructive').enabled() >-- >2.36.1 >
Vishal Verma wrote: > Add a unit test to test writing, reading, and zeroing LSA aread for > cxl_test based memdevs using ndctl commands. > > Cc: Dan Williams <dan.j.williams@intel.com> > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> Looks good to me: Reviewed-by: Dan Williams <dan.j.williams@intel.com>
On Wed, 2022-07-13 at 11:50 -0700, Davidlohr Bueso wrote: > On Wed, 13 Jul 2022, Vishal Verma wrote: > > > Add a unit test to test writing, reading, and zeroing LSA aread for > > cxl_test based memdevs using ndctl commands. > > > > Cc: Dan Williams <dan.j.williams@intel.com> > > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> > > --- > > test/cxl-labels.sh | 53 ++++++++++++++++++++++++++++++++++++++++++++++ > > test/meson.build | 2 ++ > > 2 files changed, 55 insertions(+) > > create mode 100644 test/cxl-labels.sh > > > > diff --git a/test/cxl-labels.sh b/test/cxl-labels.sh > > new file mode 100644 > > index 0000000..ce73963 > > --- /dev/null > > +++ b/test/cxl-labels.sh > > @@ -0,0 +1,53 @@ > > +#!/bin/bash > > +# SPDX-License-Identifier: GPL-2.0 > > +# Copyright (C) 2022 Intel Corporation. All rights reserved. > > + > > +. $(dirname $0)/common > > + > > +rc=1 > > + > > +set -ex > > + > > +trap 'err $LINENO' ERR > > + > > +check_prereq "jq" > > + > > +modprobe -r cxl_test > > +modprobe cxl_test > > +udevadm settle > > + > > +test_label_ops() > > +{ > > + nmem="$1" > > + lsa=$(mktemp /tmp/lsa-$nmem.XXXX) > > + lsa_read=$(mktemp /tmp/lsa-read-$nmem.XXXX) > > + > > + # determine LSA size > > + "$NDCTL" read-labels -o "$lsa_read" "$nmem" > > + lsa_size=$(stat -c %s "$lsa_read") > > + > > + dd "if=/dev/urandom" "of=$lsa" "bs=$lsa_size" "count=1" > > + "$NDCTL" write-labels -i "$lsa" "$nmem" > > + "$NDCTL" read-labels -o "$lsa_read" "$nmem" > > + > > + # compare what was written vs read > > + diff "$lsa" "$lsa_read" > > + > > + # zero the LSA and test > > + "$NDCTL" zero-labels "$nmem" > > + dd "if=/dev/zero" "of=$lsa" "bs=$lsa_size" "count=1" > > + "$NDCTL" read-labels -o "$lsa_read" "$nmem" > > + diff "$lsa" "$lsa_read" > > + > > + # cleanup > > + rm "$lsa" "$lsa_read" > > +} > > + > > +# find nmem devices corresponding to cxl memdevs > > +readarray -t nmems < <("$NDCTL" list -b cxl_test -Di | jq -r '.[].dev') > > s/$NDCTL/$CXL > > Beyond sharing a repo, I would really avoid mixing and matching ndctl and cxl > tooling and thereby keep them self sufficient. I understand that there are cases > where pmem specific operations can can be done reusing relevant pmem/nvdimm/ndctl > machinery and interfaces, but I don't see this as the case for something like lsa > unit testing. > > Thanks, > Davidlohr > Hi David, Thanks for the review - however this was intentional. cxl-cli may block LSA write access because libnvdimm could 'own' the label space. cxl memdev: action_write: mem0: has active nvdimm bridge, abort label write So the test has to use ndctl for label writes. Reads could be done with 'cxl', but for now there isn't a good/predictable mapping between ndctl 'nmemX' and cxl 'memX'. Once that is solved, either by a shared id allocator, or by listings showing the nmem->mem mapping, I will at least add another cxl read-labels here which would validate the same LSA data through cxl-cli. >
Verma, Vishal L wrote: > On Wed, 2022-07-13 at 11:50 -0700, Davidlohr Bueso wrote: > > On Wed, 13 Jul 2022, Vishal Verma wrote: > > > > > Add a unit test to test writing, reading, and zeroing LSA aread for > > > cxl_test based memdevs using ndctl commands. > > > > > > Cc: Dan Williams <dan.j.williams@intel.com> > > > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> > > > --- > > > test/cxl-labels.sh | 53 ++++++++++++++++++++++++++++++++++++++++++++++ > > > test/meson.build | 2 ++ > > > 2 files changed, 55 insertions(+) > > > create mode 100644 test/cxl-labels.sh > > > > > > diff --git a/test/cxl-labels.sh b/test/cxl-labels.sh > > > new file mode 100644 > > > index 0000000..ce73963 > > > --- /dev/null > > > +++ b/test/cxl-labels.sh > > > @@ -0,0 +1,53 @@ > > > +#!/bin/bash > > > +# SPDX-License-Identifier: GPL-2.0 > > > +# Copyright (C) 2022 Intel Corporation. All rights reserved. > > > + > > > +. $(dirname $0)/common > > > + > > > +rc=1 > > > + > > > +set -ex > > > + > > > +trap 'err $LINENO' ERR > > > + > > > +check_prereq "jq" > > > + > > > +modprobe -r cxl_test > > > +modprobe cxl_test > > > +udevadm settle > > > + > > > +test_label_ops() > > > +{ > > > + nmem="$1" > > > + lsa=$(mktemp /tmp/lsa-$nmem.XXXX) > > > + lsa_read=$(mktemp /tmp/lsa-read-$nmem.XXXX) > > > + > > > + # determine LSA size > > > + "$NDCTL" read-labels -o "$lsa_read" "$nmem" > > > + lsa_size=$(stat -c %s "$lsa_read") > > > + > > > + dd "if=/dev/urandom" "of=$lsa" "bs=$lsa_size" "count=1" > > > + "$NDCTL" write-labels -i "$lsa" "$nmem" > > > + "$NDCTL" read-labels -o "$lsa_read" "$nmem" > > > + > > > + # compare what was written vs read > > > + diff "$lsa" "$lsa_read" > > > + > > > + # zero the LSA and test > > > + "$NDCTL" zero-labels "$nmem" > > > + dd "if=/dev/zero" "of=$lsa" "bs=$lsa_size" "count=1" > > > + "$NDCTL" read-labels -o "$lsa_read" "$nmem" > > > + diff "$lsa" "$lsa_read" > > > + > > > + # cleanup > > > + rm "$lsa" "$lsa_read" > > > +} > > > + > > > +# find nmem devices corresponding to cxl memdevs > > > +readarray -t nmems < <("$NDCTL" list -b cxl_test -Di | jq -r '.[].dev') > > > > s/$NDCTL/$CXL > > > > Beyond sharing a repo, I would really avoid mixing and matching ndctl and cxl > > tooling and thereby keep them self sufficient. I understand that there are cases > > where pmem specific operations can can be done reusing relevant pmem/nvdimm/ndctl > > machinery and interfaces, but I don't see this as the case for something like lsa > > unit testing. > > > > Thanks, > > Davidlohr > > > Hi David, > > Thanks for the review - however this was intentional. cxl-cli may block > LSA write access because libnvdimm could 'own' the label space. > > cxl memdev: action_write: mem0: has active nvdimm bridge, abort label write > > So the test has to use ndctl for label writes. Reads could be done with > 'cxl', but for now there isn't a good/predictable mapping between ndctl > 'nmemX' and cxl 'memX'. Once that is solved, either by a shared id > allocator, or by listings showing the nmem->mem mapping, I will at > least add another cxl read-labels here which would validate the same > LSA data through cxl-cli. I do think this test should eventually do both to validate that the nvdimm passthrough and the native CXL operations are working. However, the native CXL case needs a bit more infrastructure to disconnect the memdev from nvdimm. Something like: cxl disable-pmem mem0 cxl write-labels mem0 ... You can add a "cxl read-labels" test though since that is not blocked while the memdev is attached to nvdimm, only label writes.
On Wed, 2022-07-13 at 12:47 -0700, Dan Williams wrote: > Verma, Vishal L wrote: > > On Wed, 2022-07-13 at 11:50 -0700, Davidlohr Bueso wrote: > > > On Wed, 13 Jul 2022, Vishal Verma wrote: > > > > > > > Add a unit test to test writing, reading, and zeroing LSA aread > > > > for > > > > cxl_test based memdevs using ndctl commands. > > > > > > > > Cc: Dan Williams <dan.j.williams@intel.com> > > > > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> > > > > --- > > > > test/cxl-labels.sh | 53 > > > > ++++++++++++++++++++++++++++++++++++++++++++++ > > > > test/meson.build | 2 ++ > > > > 2 files changed, 55 insertions(+) > > > > create mode 100644 test/cxl-labels.sh > > > > > > > > diff --git a/test/cxl-labels.sh b/test/cxl-labels.sh > > > > new file mode 100644 > > > > index 0000000..ce73963 > > > > --- /dev/null > > > > +++ b/test/cxl-labels.sh > > > > @@ -0,0 +1,53 @@ > > > > +#!/bin/bash > > > > +# SPDX-License-Identifier: GPL-2.0 > > > > +# Copyright (C) 2022 Intel Corporation. All rights reserved. > > > > + > > > > +. $(dirname $0)/common > > > > + > > > > +rc=1 > > > > + > > > > +set -ex > > > > + > > > > +trap 'err $LINENO' ERR > > > > + > > > > +check_prereq "jq" > > > > + > > > > +modprobe -r cxl_test > > > > +modprobe cxl_test > > > > +udevadm settle > > > > + > > > > +test_label_ops() > > > > +{ > > > > + nmem="$1" > > > > + lsa=$(mktemp /tmp/lsa-$nmem.XXXX) > > > > + lsa_read=$(mktemp /tmp/lsa-read-$nmem.XXXX) > > > > + > > > > + # determine LSA size > > > > + "$NDCTL" read-labels -o "$lsa_read" "$nmem" > > > > + lsa_size=$(stat -c %s "$lsa_read") > > > > + > > > > + dd "if=/dev/urandom" "of=$lsa" "bs=$lsa_size" "count=1" > > > > + "$NDCTL" write-labels -i "$lsa" "$nmem" > > > > + "$NDCTL" read-labels -o "$lsa_read" "$nmem" > > > > + > > > > + # compare what was written vs read > > > > + diff "$lsa" "$lsa_read" > > > > + > > > > + # zero the LSA and test > > > > + "$NDCTL" zero-labels "$nmem" > > > > + dd "if=/dev/zero" "of=$lsa" "bs=$lsa_size" "count=1" > > > > + "$NDCTL" read-labels -o "$lsa_read" "$nmem" > > > > + diff "$lsa" "$lsa_read" > > > > + > > > > + # cleanup > > > > + rm "$lsa" "$lsa_read" > > > > +} > > > > + > > > > +# find nmem devices corresponding to cxl memdevs > > > > +readarray -t nmems < <("$NDCTL" list -b cxl_test -Di | jq -r > > > > '.[].dev') > > > > > > s/$NDCTL/$CXL > > > > > > Beyond sharing a repo, I would really avoid mixing and matching > > > ndctl and cxl > > > tooling and thereby keep them self sufficient. I understand that > > > there are cases > > > where pmem specific operations can can be done reusing relevant > > > pmem/nvdimm/ndctl > > > machinery and interfaces, but I don't see this as the case for > > > something like lsa > > > unit testing. > > > > > > Thanks, > > > Davidlohr > > > > > Hi David, > > > > Thanks for the review - however this was intentional. cxl-cli may > > block > > LSA write access because libnvdimm could 'own' the label space. > > > > cxl memdev: action_write: mem0: has active nvdimm bridge, abort > > label write > > > > So the test has to use ndctl for label writes. Reads could be done > > with > > 'cxl', but for now there isn't a good/predictable mapping between > > ndctl > > 'nmemX' and cxl 'memX'. Once that is solved, either by a shared id > > allocator, or by listings showing the nmem->mem mapping, I will at > > least add another cxl read-labels here which would validate the > > same > > LSA data through cxl-cli. > > I do think this test should eventually do both to validate that the > nvdimm passthrough and the native CXL operations are working. > However, > the native CXL case needs a bit more infrastructure to disconnect the > memdev from nvdimm. Something like: > > cxl disable-pmem mem0 > cxl write-labels mem0 ... > > You can add a "cxl read-labels" test though since that is not blocked > while the memdev is attached to nvdimm, only label writes. Ah - yeah I think just going through all memdevs and reading the LSA is harmless enough, and it exercises the cxl-cli paths. I'll add it to v2.
diff --git a/test/cxl-labels.sh b/test/cxl-labels.sh new file mode 100644 index 0000000..ce73963 --- /dev/null +++ b/test/cxl-labels.sh @@ -0,0 +1,53 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (C) 2022 Intel Corporation. All rights reserved. + +. $(dirname $0)/common + +rc=1 + +set -ex + +trap 'err $LINENO' ERR + +check_prereq "jq" + +modprobe -r cxl_test +modprobe cxl_test +udevadm settle + +test_label_ops() +{ + nmem="$1" + lsa=$(mktemp /tmp/lsa-$nmem.XXXX) + lsa_read=$(mktemp /tmp/lsa-read-$nmem.XXXX) + + # determine LSA size + "$NDCTL" read-labels -o "$lsa_read" "$nmem" + lsa_size=$(stat -c %s "$lsa_read") + + dd "if=/dev/urandom" "of=$lsa" "bs=$lsa_size" "count=1" + "$NDCTL" write-labels -i "$lsa" "$nmem" + "$NDCTL" read-labels -o "$lsa_read" "$nmem" + + # compare what was written vs read + diff "$lsa" "$lsa_read" + + # zero the LSA and test + "$NDCTL" zero-labels "$nmem" + dd "if=/dev/zero" "of=$lsa" "bs=$lsa_size" "count=1" + "$NDCTL" read-labels -o "$lsa_read" "$nmem" + diff "$lsa" "$lsa_read" + + # cleanup + rm "$lsa" "$lsa_read" +} + +# find nmem devices corresponding to cxl memdevs +readarray -t nmems < <("$NDCTL" list -b cxl_test -Di | jq -r '.[].dev') + +for nmem in ${nmems[@]}; do + test_label_ops "$nmem" +done + +modprobe -r cxl_test diff --git a/test/meson.build b/test/meson.build index fbcfc08..687a71f 100644 --- a/test/meson.build +++ b/test/meson.build @@ -152,6 +152,7 @@ pfn_meta_errors = find_program('pfn-meta-errors.sh') track_uuid = find_program('track-uuid.sh') cxl_topo = find_program('cxl-topology.sh') cxl_region = find_program('cxl-region-create.sh') +cxl_labels = find_program('cxl-labels.sh') tests = [ [ 'libndctl', libndctl, 'ndctl' ], @@ -178,6 +179,7 @@ tests = [ [ 'track-uuid.sh', track_uuid, 'ndctl' ], [ 'cxl-topology.sh', cxl_topo, 'cxl' ], [ 'cxl-region-create.sh', cxl_region, 'cxl' ], + [ 'cxl-labels.sh', cxl_labels, 'cxl' ], ] if get_option('destructive').enabled()
Add a unit test to test writing, reading, and zeroing LSA aread for cxl_test based memdevs using ndctl commands. Cc: Dan Williams <dan.j.williams@intel.com> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> --- test/cxl-labels.sh | 53 ++++++++++++++++++++++++++++++++++++++++++++++ test/meson.build | 2 ++ 2 files changed, 55 insertions(+) create mode 100644 test/cxl-labels.sh