[PATCH ndctl RESEND 0/2] ndctl: Fix ups for region destroy

[PATCH ndctl RESEND 0/2] ndctl: Fix ups for region destroy

[Ira Weiny]

The patch to force device tear down on region disable caused a
regression for regions without system ram.

Add a test for such a case and a fix to the patch.

Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
[djiang: RESEND with proper mailing lists.]

---
Ira Weiny (2):
      ndctl/test: Add destroy region test
      cxl/region: Fix memory device teardown in disable-region

 cxl/region.c               |  3 ++
 daxctl/lib/libdaxctl.c     |  4 +--
 daxctl/lib/libdaxctl.sym   |  5 +++
 daxctl/libdaxctl.h         |  1 +
 test/cxl-destroy-region.sh | 76 ++++++++++++++++++++++++++++++++++++++++++++++
 test/meson.build           |  2 ++
 6 files changed, 89 insertions(+), 2 deletions(-)
---
base-commit: cbf049039482a56c2b66ede3e10d5e9c652890b7
change-id: 20231130-fix-region-destroy-a85ad1fde87b

Best regards,
-- 
Ira Weiny <ira.weiny@intel.com>

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

[Ira Weiny]

Commit 9399aa667ab0 ("cxl/region: Add -f option for disable-region")
introduced a regression when destroying a region.

Add a tests for destroying a region.

Cc: Dave Jiang <dave.jiang@intel.com>
Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
 test/cxl-destroy-region.sh | 76 ++++++++++++++++++++++++++++++++++++++++++++++
 test/meson.build           |  2 ++
 2 files changed, 78 insertions(+)

diff --git a/test/cxl-destroy-region.sh b/test/cxl-destroy-region.sh
new file mode 100644
index 000000000000..251720a98688
--- /dev/null
+++ b/test/cxl-destroy-region.sh
@@ -0,0 +1,76 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (C) 2023 Intel Corporation. All rights reserved.
+
+. $(dirname $0)/common
+
+rc=77
+
+set -ex
+
+trap 'err $LINENO' ERR
+
+check_prereq "jq"
+
+modprobe -r cxl_test
+modprobe cxl_test
+rc=1
+
+check_destroy_ram()
+{
+	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"
+
+	# default is memory is system-ram offline
+	$CXL disable-region $region
+	$CXL destroy-region $region
+}
+
+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
+
+check_dmesg "$LINENO"
+
+modprobe -r cxl_test
diff --git a/test/meson.build b/test/meson.build
index 2706fa5d633c..126d663dfce2 100644
--- a/test/meson.build
+++ b/test/meson.build
@@ -158,6 +158,7 @@ cxl_xor_region = find_program('cxl-xor-region.sh')
 cxl_update_firmware = find_program('cxl-update-firmware.sh')
 cxl_events = find_program('cxl-events.sh')
 cxl_poison = find_program('cxl-poison.sh')
+cxl_destroy_region = find_program('cxl-destroy-region.sh')
 
 tests = [
   [ 'libndctl',               libndctl,		  'ndctl' ],
@@ -188,6 +189,7 @@ tests = [
   [ 'cxl-xor-region.sh',      cxl_xor_region,     'cxl'   ],
   [ 'cxl-events.sh',          cxl_events,         'cxl'   ],
   [ 'cxl-poison.sh',          cxl_poison,         'cxl'   ],
+  [ 'cxl-destroy-region.sh',  cxl_destroy_region, 'cxl'   ],
 ]
 
 if get_option('destructive').enabled()

-- 
2.42.0

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

[Dave Jiang]

On 11/30/23 21:06, Ira Weiny wrote:
> Commit 9399aa667ab0 ("cxl/region: Add -f option for disable-region")
> introduced a regression when destroying a region.
> 
> Add a tests for destroying a region.
> 
> Cc: Dave Jiang <dave.jiang@intel.com>
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  test/cxl-destroy-region.sh | 76 ++++++++++++++++++++++++++++++++++++++++++++++
>  test/meson.build           |  2 ++
>  2 files changed, 78 insertions(+)
> 
> diff --git a/test/cxl-destroy-region.sh b/test/cxl-destroy-region.sh
> new file mode 100644
> index 000000000000..251720a98688
> --- /dev/null
> +++ b/test/cxl-destroy-region.sh
> @@ -0,0 +1,76 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2023 Intel Corporation. All rights reserved.
> +
> +. $(dirname $0)/common
> +
> +rc=77
> +
> +set -ex
> +
> +trap 'err $LINENO' ERR
> +
> +check_prereq "jq"
> +
> +modprobe -r cxl_test
> +modprobe cxl_test
> +rc=1
> +
> +check_destroy_ram()
> +{
> +	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"
> +
> +	# default is memory is system-ram offline
> +	$CXL disable-region $region
> +	$CXL destroy-region $region
> +}
> +
> +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
> +
> +check_dmesg "$LINENO"
> +
> +modprobe -r cxl_test
> diff --git a/test/meson.build b/test/meson.build
> index 2706fa5d633c..126d663dfce2 100644
> --- a/test/meson.build
> +++ b/test/meson.build
> @@ -158,6 +158,7 @@ cxl_xor_region = find_program('cxl-xor-region.sh')
>  cxl_update_firmware = find_program('cxl-update-firmware.sh')
>  cxl_events = find_program('cxl-events.sh')
>  cxl_poison = find_program('cxl-poison.sh')
> +cxl_destroy_region = find_program('cxl-destroy-region.sh')
>  
>  tests = [
>    [ 'libndctl',               libndctl,		  'ndctl' ],
> @@ -188,6 +189,7 @@ tests = [
>    [ 'cxl-xor-region.sh',      cxl_xor_region,     'cxl'   ],
>    [ 'cxl-events.sh',          cxl_events,         'cxl'   ],
>    [ 'cxl-poison.sh',          cxl_poison,         'cxl'   ],
> +  [ 'cxl-destroy-region.sh',  cxl_destroy_region, 'cxl'   ],
>  ]
>  
>  if get_option('destructive').enabled()
> 

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

[Alison Schofield]

On Thu, Nov 30, 2023 at 08:06:13PM -0800, Ira Weiny wrote:
> Commit 9399aa667ab0 ("cxl/region: Add -f option for disable-region")
> introduced a regression when destroying a region.
> 
> Add a tests for destroying a region.
> 
> Cc: Dave Jiang <dave.jiang@intel.com>
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> ---
>  test/cxl-destroy-region.sh | 76 ++++++++++++++++++++++++++++++++++++++++++++++
>  test/meson.build           |  2 ++
>  2 files changed, 78 insertions(+)
> 
> diff --git a/test/cxl-destroy-region.sh b/test/cxl-destroy-region.sh
> new file mode 100644
> index 000000000000..251720a98688
> --- /dev/null
> +++ b/test/cxl-destroy-region.sh
> @@ -0,0 +1,76 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2023 Intel Corporation. All rights reserved.
> +
> +. $(dirname $0)/common
> +
> +rc=77
> +
> +set -ex
> +
> +trap 'err $LINENO' ERR
> +
> +check_prereq "jq"
> +
> +modprobe -r cxl_test
> +modprobe cxl_test
> +rc=1
> +
> +check_destroy_ram()
> +{
> +	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"
> +
> +	# default is memory is system-ram offline
> +	$CXL disable-region $region
> +	$CXL destroy-region $region
> +}
> +
> +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?


> +
> +check_dmesg "$LINENO"
> +
> +modprobe -r cxl_test
> diff --git a/test/meson.build b/test/meson.build
> index 2706fa5d633c..126d663dfce2 100644
> --- a/test/meson.build
> +++ b/test/meson.build
> @@ -158,6 +158,7 @@ cxl_xor_region = find_program('cxl-xor-region.sh')
>  cxl_update_firmware = find_program('cxl-update-firmware.sh')
>  cxl_events = find_program('cxl-events.sh')
>  cxl_poison = find_program('cxl-poison.sh')
> +cxl_destroy_region = find_program('cxl-destroy-region.sh')
>  
>  tests = [
>    [ 'libndctl',               libndctl,		  'ndctl' ],
> @@ -188,6 +189,7 @@ tests = [
>    [ 'cxl-xor-region.sh',      cxl_xor_region,     'cxl'   ],
>    [ 'cxl-events.sh',          cxl_events,         'cxl'   ],
>    [ 'cxl-poison.sh',          cxl_poison,         'cxl'   ],
> +  [ 'cxl-destroy-region.sh',  cxl_destroy_region, 'cxl'   ],
>  ]
>  
>  if get_option('destructive').enabled()
> 
> -- 
> 2.42.0
> 
> 

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

[Ira Weiny]

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

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

[Verma, Vishal L]

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/

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

[Ira Weiny]

Verma, Vishal L wrote:

[snip]

> > > > +# 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],

I'm not sure.

> 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?

I thought it would have but I don't think it covers the case where the dax
device is not system ram (the default when creating a region).

Ira

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

[Verma, Vishal L]

On Tue, 2023-12-05 at 13:22 -0800, Ira Weiny wrote:
> Verma, Vishal L wrote:
> 
> [snip]
> 
> > > > 
> > > 
> > 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],
> 
> I'm not sure.
> 
> > 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?
> 
> I thought it would have but I don't think it covers the case where the dax
> device is not system ram (the default when creating a region).

Oh, you're right, the devdax case isn't covered by the other test. I'll
keep this then, thanks!

[PATCH ndctl RESEND 2/2] cxl/region: Fix memory device teardown in disable-region

[Ira Weiny]

When a region is requested to be disabled, memory devices are normally
automatically torn down.  Commit 9399aa667ab0 prevents tear down if
memory is online without a force flag.  However, daxctl_dev_get_memory()
may return NULL if the memory device in question is not system-ram
capable as is the case for a region with only devdax devices.  Such
devices do not need to be off-lined explicitly.

Skip non-system-ram devices rather than error the operation.

Fixes: 9399aa667ab0 ("cxl/region: Add -f option for disable-region")
Cc: Dave Jiang <dave.jiang@intel.com>
Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
 cxl/region.c             | 3 +++
 daxctl/lib/libdaxctl.c   | 4 ++--
 daxctl/lib/libdaxctl.sym | 5 +++++
 daxctl/libdaxctl.h       | 1 +
 4 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/cxl/region.c b/cxl/region.c
index 5cbbf2749e2d..44ac76b001e9 100644
--- a/cxl/region.c
+++ b/cxl/region.c
@@ -805,6 +805,9 @@ static int disable_region(struct cxl_region *region)
 		goto out;
 
 	daxctl_dev_foreach(dax_region, dev) {
+		if (!daxctl_dev_is_system_ram_capable(dev))
+			continue;
+
 		mem = daxctl_dev_get_memory(dev);
 		if (!mem)
 			return -ENXIO;
diff --git a/daxctl/lib/libdaxctl.c b/daxctl/lib/libdaxctl.c
index 4f9aba0b09f2..9fbefe2e8329 100644
--- a/daxctl/lib/libdaxctl.c
+++ b/daxctl/lib/libdaxctl.c
@@ -385,7 +385,7 @@ static bool device_model_is_dax_bus(struct daxctl_dev *dev)
 	return false;
 }
 
-static int dev_is_system_ram_capable(struct daxctl_dev *dev)
+DAXCTL_EXPORT int daxctl_dev_is_system_ram_capable(struct daxctl_dev *dev)
 {
 	const char *devname = daxctl_dev_get_devname(dev);
 	struct daxctl_ctx *ctx = daxctl_dev_get_ctx(dev);
@@ -432,7 +432,7 @@ static struct daxctl_memory *daxctl_dev_alloc_mem(struct daxctl_dev *dev)
 	char buf[SYSFS_ATTR_SIZE];
 	int node_num;
 
-	if (!dev_is_system_ram_capable(dev))
+	if (!daxctl_dev_is_system_ram_capable(dev))
 		return NULL;
 
 	mem = calloc(1, sizeof(*mem));
diff --git a/daxctl/lib/libdaxctl.sym b/daxctl/lib/libdaxctl.sym
index fe68fd0a9cde..309881196c86 100644
--- a/daxctl/lib/libdaxctl.sym
+++ b/daxctl/lib/libdaxctl.sym
@@ -99,3 +99,8 @@ global:
 	daxctl_set_config_path;
 	daxctl_get_config_path;
 } LIBDAXCTL_8;
+
+LIBDAXCTL_10 {
+global:
+	daxctl_dev_is_system_ram_capable;
+} LIBDAXCTL_9;
diff --git a/daxctl/libdaxctl.h b/daxctl/libdaxctl.h
index 6876037a9427..53c6bbdae5c3 100644
--- a/daxctl/libdaxctl.h
+++ b/daxctl/libdaxctl.h
@@ -77,6 +77,7 @@ int daxctl_dev_will_auto_online_memory(struct daxctl_dev *dev);
 int daxctl_dev_has_online_memory(struct daxctl_dev *dev);
 
 struct daxctl_memory;
+int daxctl_dev_is_system_ram_capable(struct daxctl_dev *dev);
 struct daxctl_memory *daxctl_dev_get_memory(struct daxctl_dev *dev);
 struct daxctl_dev *daxctl_memory_get_dev(struct daxctl_memory *mem);
 const char *daxctl_memory_get_node_path(struct daxctl_memory *mem);

-- 
2.42.0

Re: [PATCH ndctl RESEND 2/2] cxl/region: Fix memory device teardown in disable-region

[Dave Jiang]

On 11/30/23 21:06, Ira Weiny wrote:
> When a region is requested to be disabled, memory devices are normally
> automatically torn down.  Commit 9399aa667ab0 prevents tear down if
> memory is online without a force flag.  However, daxctl_dev_get_memory()
> may return NULL if the memory device in question is not system-ram
> capable as is the case for a region with only devdax devices.  Such
> devices do not need to be off-lined explicitly.
> 
> Skip non-system-ram devices rather than error the operation.
> 
> Fixes: 9399aa667ab0 ("cxl/region: Add -f option for disable-region")
> Cc: Dave Jiang <dave.jiang@intel.com>
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
>  cxl/region.c             | 3 +++
>  daxctl/lib/libdaxctl.c   | 4 ++--
>  daxctl/lib/libdaxctl.sym | 5 +++++
>  daxctl/libdaxctl.h       | 1 +
>  4 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/cxl/region.c b/cxl/region.c
> index 5cbbf2749e2d..44ac76b001e9 100644
> --- a/cxl/region.c
> +++ b/cxl/region.c
> @@ -805,6 +805,9 @@ static int disable_region(struct cxl_region *region)
>  		goto out;
>  
>  	daxctl_dev_foreach(dax_region, dev) {
> +		if (!daxctl_dev_is_system_ram_capable(dev))
> +			continue;
> +
>  		mem = daxctl_dev_get_memory(dev);
>  		if (!mem)
>  			return -ENXIO;
> diff --git a/daxctl/lib/libdaxctl.c b/daxctl/lib/libdaxctl.c
> index 4f9aba0b09f2..9fbefe2e8329 100644
> --- a/daxctl/lib/libdaxctl.c
> +++ b/daxctl/lib/libdaxctl.c
> @@ -385,7 +385,7 @@ static bool device_model_is_dax_bus(struct daxctl_dev *dev)
>  	return false;
>  }
>  
> -static int dev_is_system_ram_capable(struct daxctl_dev *dev)
> +DAXCTL_EXPORT int daxctl_dev_is_system_ram_capable(struct daxctl_dev *dev)
>  {
>  	const char *devname = daxctl_dev_get_devname(dev);
>  	struct daxctl_ctx *ctx = daxctl_dev_get_ctx(dev);
> @@ -432,7 +432,7 @@ static struct daxctl_memory *daxctl_dev_alloc_mem(struct daxctl_dev *dev)
>  	char buf[SYSFS_ATTR_SIZE];
>  	int node_num;
>  
> -	if (!dev_is_system_ram_capable(dev))
> +	if (!daxctl_dev_is_system_ram_capable(dev))
>  		return NULL;
>  
>  	mem = calloc(1, sizeof(*mem));
> diff --git a/daxctl/lib/libdaxctl.sym b/daxctl/lib/libdaxctl.sym
> index fe68fd0a9cde..309881196c86 100644
> --- a/daxctl/lib/libdaxctl.sym
> +++ b/daxctl/lib/libdaxctl.sym
> @@ -99,3 +99,8 @@ global:
>  	daxctl_set_config_path;
>  	daxctl_get_config_path;
>  } LIBDAXCTL_8;
> +
> +LIBDAXCTL_10 {
> +global:
> +	daxctl_dev_is_system_ram_capable;
> +} LIBDAXCTL_9;
> diff --git a/daxctl/libdaxctl.h b/daxctl/libdaxctl.h
> index 6876037a9427..53c6bbdae5c3 100644
> --- a/daxctl/libdaxctl.h
> +++ b/daxctl/libdaxctl.h
> @@ -77,6 +77,7 @@ int daxctl_dev_will_auto_online_memory(struct daxctl_dev *dev);
>  int daxctl_dev_has_online_memory(struct daxctl_dev *dev);
>  
>  struct daxctl_memory;
> +int daxctl_dev_is_system_ram_capable(struct daxctl_dev *dev);
>  struct daxctl_memory *daxctl_dev_get_memory(struct daxctl_dev *dev);
>  struct daxctl_dev *daxctl_memory_get_dev(struct daxctl_memory *mem);
>  const char *daxctl_memory_get_node_path(struct daxctl_memory *mem);
>