Re: [PATCH ndctl RESEND 1/2] ndctl/test: Add destroy region test

From: Ira Weiny <ira.weiny@intel.com>
To: Alison Schofield <alison.schofield@intel.com>,
	Ira Weiny <ira.weiny@intel.com>
Cc: Vishal Verma <vishal.l.verma@intel.com>,
	Dave Jiang <dave.jiang@intel.com>,
	Dan Williams <dan.j.williams@intel.com>, <nvdimm@lists.linux.dev>,
	<linux-cxl@vger.kernel.org>
Subject: Re: [PATCH ndctl RESEND 1/2] ndctl/test: Add destroy region test
Date: Mon, 4 Dec 2023 10:05:12 -0800	[thread overview]
Message-ID: <656e14d8285f7_16287c29422@iweiny-mobl.notmuch> (raw)
In-Reply-To: <ZWo2f2eWVtsJrYD9@aschofie-mobl2>

Alison Schofield wrote:
> On Thu, Nov 30, 2023 at 08:06:13PM -0800, Ira Weiny wrote:

[snip]

> > +
> > +check_destroy_devdax()
> > +{
> > +	mem=$1
> > +	decoder=$2
> > +
> > +	region=$($CXL create-region -d "$decoder" -m "$mem" | jq -r ".region")
> > +	if [ "$region" == "null" ]; then
> > +		err "$LINENO"
> > +	fi
> > +	$CXL enable-region "$region"
> > +
> > +	dax=$($CXL list -X -r "$region" | jq -r ".[].daxregion.devices" | jq -r '.[].chardev')
> > +
> > +	$DAXCTL reconfigure-device -m devdax "$dax"
> > +
> > +	$CXL disable-region $region
> > +	$CXL destroy-region $region
> > +}
> > +
> > +# Find a memory device to create regions on to test the destroy
> > +readarray -t mems < <("$CXL" list -b cxl_test -M | jq -r '.[].memdev')
> > +for mem in ${mems[@]}; do
> > +        ramsize=$($CXL list -m $mem | jq -r '.[].ram_size')
> > +        if [ "$ramsize" == "null" ]; then
> > +                continue
> > +        fi
> > +        decoder=$($CXL list -b cxl_test -D -d root -m "$mem" |
> > +                  jq -r ".[] |
> > +                  select(.volatile_capable == true) |
> > +                  select(.nr_targets == 1) |
> > +                  select(.size >= ${ramsize}) |
> > +                  .decoder")
> > +        if [[ $decoder ]]; then
> > +		check_destroy_ram $mem $decoder
> > +		check_destroy_devdax $mem $decoder
> > +                break
> > +        fi
> > +done
> 
> Does this need to check results of the region disable & destroy?
> 
> Did the regression this is after leave a trace in the dmesg log,
> so checking that is all that's needed?
> 

The regression causes

	check_destroy_devdax()
		$CXL disable-region $region

to fail.  That command failure will exit with an error which causes the
test script to exit with that error as well.

At least that is what happened when I used this to test the fix.  I'll
defer to Vishal if there is a more explicit or better way to check for
that cxl-cli command to fail.

Ira