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 <[email protected]>
> > > > Signed-off-by: Vishal Verma <[email protected]>
> > > > ---
> > > > 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.

Reply via email to