| Message ID | 20240327184642.2181254-1-alison.schofield@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ndctl] cxl/test: use max_available_extent in cxl-destroy-region | expand |
On Wed, 2024-03-27 at 11:46 -0700, alison.schofield@intel.com wrote: > From: Alison Schofield <alison.schofield@intel.com> > > Using .size in decoder selection can lead to a set_size failure with > these error messages: > > cxl region: create_region: region8: set_size failed: Numerical result out of range > > [] cxl_core:alloc_hpa:555: cxl region8: HPA allocation error (-34) for size:0x0000000020000000 in CXL Window 0 [mem 0xf010000000-0xf04fffffff flags 0x200] > > Use max_available_extent for decoder selection instead. > > The test overlooked the region creation failure because it used a > not 'null' comparison which always succeeds. Use the ! comparator > after create-region and for the ramsize check so that the test fails > or continues as expected. > > Signed-off-by: Alison Schofield <alison.schofield@intel.com> > --- > test/cxl-destroy-region.sh | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/test/cxl-destroy-region.sh b/test/cxl-destroy-region.sh > index cf0a46d6ba58..167fcc4a7ff9 100644 > --- a/test/cxl-destroy-region.sh > +++ b/test/cxl-destroy-region.sh > @@ -22,7 +22,7 @@ check_destroy_ram() > decoder=$2 > > region="$("$CXL" create-region -d "$decoder" -m "$mem" | jq -r ".region")" > - if [ "$region" == "null" ]; then > + if [[ ! $region ]]; then > err "$LINENO" > fi > "$CXL" enable-region "$region" > @@ -38,7 +38,7 @@ check_destroy_devdax() > decoder=$2 > > region="$("$CXL" create-region -d "$decoder" -m "$mem" | jq -r ".region")" > - if [ "$region" == "null" ]; then > + if [[ ! $region ]]; then While these ! $region changes are correct (because cxl create-region) doesn't output any json if creation fails).. > err "$LINENO" > fi > "$CXL" enable-region "$region" > @@ -55,14 +55,14 @@ check_destroy_devdax() > readarray -t mems < <("$CXL" list -b "$CXL_TEST_BUS" -M | jq -r '.[].memdev') > for mem in "${mems[@]}"; do > ramsize="$("$CXL" list -m "$mem" | jq -r '.[].ram_size')" > - if [[ $ramsize == "null" ]]; then > + if [[ ! $ramsize ]]; then .. I think this check needs to check for both empty and "null" - a memdev that doesn't have ram_size but otherwise emits valid json will result in "null" here. e.g.: $ echo "" | jq -r ".region" $ echo "{ }" | jq -r ".region" null So this probably wants to be: if [[ $ramsize == "null" || ! $ram_size ]]; then ... > continue > fi > decoder="$("$CXL" list -b "$CXL_TEST_BUS" -D -d root -m "$mem" | > jq -r ".[] | > select(.volatile_capable == true) | > select(.nr_targets == 1) | > - select(.size >= ${ramsize}) | > + select(.max_available_extent >= ${ramsize}) | > .decoder")" > if [[ $decoder ]]; then > check_destroy_ram "$mem" "$decoder" > > base-commit: e0d0680bd3e554bd5f211e989480c5a13a023b2d
On Mon, Apr 22, 2024 at 10:07:58AM -0700, Vishal Verma wrote: > On Wed, 2024-03-27 at 11:46 -0700, alison.schofield@intel.com wrote: > > From: Alison Schofield <alison.schofield@intel.com> > > > > Using .size in decoder selection can lead to a set_size failure with > > these error messages: > > > > cxl region: create_region: region8: set_size failed: Numerical result out of range > > > > [] cxl_core:alloc_hpa:555: cxl region8: HPA allocation error (-34) for size:0x0000000020000000 in CXL Window 0 [mem 0xf010000000-0xf04fffffff flags 0x200] > > > > Use max_available_extent for decoder selection instead. > > > > The test overlooked the region creation failure because it used a > > not 'null' comparison which always succeeds. Use the ! comparator > > after create-region and for the ramsize check so that the test fails > > or continues as expected. > > > > Signed-off-by: Alison Schofield <alison.schofield@intel.com> > > --- > > test/cxl-destroy-region.sh | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/test/cxl-destroy-region.sh b/test/cxl-destroy-region.sh > > index cf0a46d6ba58..167fcc4a7ff9 100644 > > --- a/test/cxl-destroy-region.sh > > +++ b/test/cxl-destroy-region.sh > > @@ -22,7 +22,7 @@ check_destroy_ram() > > decoder=$2 > > > > region="$("$CXL" create-region -d "$decoder" -m "$mem" | jq -r ".region")" > > - if [ "$region" == "null" ]; then > > + if [[ ! $region ]]; then > > err "$LINENO" > > fi > > "$CXL" enable-region "$region" > > @@ -38,7 +38,7 @@ check_destroy_devdax() > > decoder=$2 > > > > region="$("$CXL" create-region -d "$decoder" -m "$mem" | jq -r ".region")" > > - if [ "$region" == "null" ]; then > > + if [[ ! $region ]]; then > > While these ! $region changes are correct (because cxl create-region) > doesn't output any json if creation fails).. > > > err "$LINENO" > > fi > > "$CXL" enable-region "$region" > > @@ -55,14 +55,14 @@ check_destroy_devdax() > > readarray -t mems < <("$CXL" list -b "$CXL_TEST_BUS" -M | jq -r '.[].memdev') > > for mem in "${mems[@]}"; do > > ramsize="$("$CXL" list -m "$mem" | jq -r '.[].ram_size')" > > - if [[ $ramsize == "null" ]]; then > > + if [[ ! $ramsize ]]; then > > .. I think this check needs to check for both empty and "null" - a > memdev that doesn't have ram_size but otherwise emits valid json will > result in "null" here. e.g.: > > $ echo "" | jq -r ".region" > > $ echo "{ }" | jq -r ".region" > null > > So this probably wants to be: > > if [[ $ramsize == "null" || ! $ram_size ]]; then > ... > I see now! Thanks Vishal. -- Alison
diff --git a/test/cxl-destroy-region.sh b/test/cxl-destroy-region.sh index cf0a46d6ba58..167fcc4a7ff9 100644 --- a/test/cxl-destroy-region.sh +++ b/test/cxl-destroy-region.sh @@ -22,7 +22,7 @@ check_destroy_ram() decoder=$2 region="$("$CXL" create-region -d "$decoder" -m "$mem" | jq -r ".region")" - if [ "$region" == "null" ]; then + if [[ ! $region ]]; then err "$LINENO" fi "$CXL" enable-region "$region" @@ -38,7 +38,7 @@ check_destroy_devdax() decoder=$2 region="$("$CXL" create-region -d "$decoder" -m "$mem" | jq -r ".region")" - if [ "$region" == "null" ]; then + if [[ ! $region ]]; then err "$LINENO" fi "$CXL" enable-region "$region" @@ -55,14 +55,14 @@ check_destroy_devdax() readarray -t mems < <("$CXL" list -b "$CXL_TEST_BUS" -M | jq -r '.[].memdev') for mem in "${mems[@]}"; do ramsize="$("$CXL" list -m "$mem" | jq -r '.[].ram_size')" - if [[ $ramsize == "null" ]]; then + if [[ ! $ramsize ]]; then continue fi decoder="$("$CXL" list -b "$CXL_TEST_BUS" -D -d root -m "$mem" | jq -r ".[] | select(.volatile_capable == true) | select(.nr_targets == 1) | - select(.size >= ${ramsize}) | + select(.max_available_extent >= ${ramsize}) | .decoder")" if [[ $decoder ]]; then check_destroy_ram "$mem" "$decoder"