On Mon, 2023-12-04 at 10:05 -0800, Ira Weiny wrote:
> 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.
>
Correct, the set -e will cause the script to abort with an error exit
code whenever a command fails.
I do wonder if we need this new test - with Dave's patch here[1],
destroy-region and disable-region both use the same helper that
performs the libdaxctl checks.
cxl-create-region.sh already has flows that create a region and then
destroy it. Those should now cover this case as well yeah?
[1]:
https://lore.kernel.org/all/170112921107.2687457.2741231995154639197.stgit@djiang5-mobl3/
