[ndctl PATCH 0/5] ndctl: Add missing test dependencies and other fixups

[ndctl PATCH 0/5] ndctl: Add missing test dependencies and other fixups

[Dan Williams]

Recently I stood up a new test system from scratch and discovered some
gaps in the documentation and the tests.

Fix those up to get all tests passing with v6.16-rc2 and latest version
of all test dependencies currently available in Fedora Rawhide.

Dan Williams (5):
  build: Fix meson feature deprecation warnings
  test: Fix 'ndctl' dependency in test/sub-section.sh
  test: Fix dax.sh expectations
  test: Update documentation with required packages to install
  test: Fixup fwctl dependency

 README.md           | 5 +++++
 contrib/meson.build | 2 +-
 meson.build         | 5 +++--
 test/dax.sh         | 3 ++-
 test/meson.build    | 1 +
 test/sub-section.sh | 2 +-
 6 files changed, 13 insertions(+), 5 deletions(-)


base-commit: 74b9e411bf13e87df39a517d10143fafa7e2ea92
-- 
2.49.0

[ndctl PATCH 1/5] build: Fix meson feature deprecation warnings

[Dan Williams]

There are a few instances of the warning:

"meson.build: WARNING: Project does not target a minimum version but
uses feature deprecated since '0.56.0': dependency.get_pkgconfig_variable.
use dependency.get_variable(pkgconfig : ...) instead"

Move to the new style and mark the project as needing at least that minimum
version.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 contrib/meson.build | 2 +-
 meson.build         | 5 +++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/contrib/meson.build b/contrib/meson.build
index 48aa7c071f92..33a409a2d7d0 100644
--- a/contrib/meson.build
+++ b/contrib/meson.build
@@ -2,7 +2,7 @@ bashcompletiondir = get_option('bashcompletiondir')
 if bashcompletiondir == ''
   bash_completion = dependency('bash-completion', required : false)
   if bash_completion.found()
-      bashcompletiondir = bash_completion.get_pkgconfig_variable('completionsdir')
+      bashcompletiondir = bash_completion.get_variable(pkgconfig : 'completionsdir')
   else
     bashcompletiondir = datadir / 'bash-completion/completions'
   endif
diff --git a/meson.build b/meson.build
index 19808bb21db8..300eddb99235 100644
--- a/meson.build
+++ b/meson.build
@@ -1,5 +1,6 @@
 project('ndctl', 'c',
   version : '82',
+  meson_version: '>= 0.56.0',
   license : [
     'GPL-2.0',
     'LGPL-2.1',
@@ -159,9 +160,9 @@ endif
 
 if get_option('systemd').enabled()
   systemd = dependency('systemd', required : true)
-  systemdunitdir = systemd.get_pkgconfig_variable('systemdsystemunitdir')
+  systemdunitdir = systemd.get_variable(pkgconfig : 'systemdsystemunitdir')
   udev = dependency('udev', required : true)
-  udevdir = udev.get_pkgconfig_variable('udevdir')
+  udevdir = udev.get_variable(pkgconfig : 'udevdir')
   udevrulesdir = udevdir / 'rules.d'
 endif
 
-- 
2.49.0

Re: [ndctl PATCH 1/5] build: Fix meson feature deprecation warnings

[Dave Jiang]

On 6/18/25 3:21 PM, Dan Williams wrote:
> There are a few instances of the warning:
> 
> "meson.build: WARNING: Project does not target a minimum version but
> uses feature deprecated since '0.56.0': dependency.get_pkgconfig_variable.
> use dependency.get_variable(pkgconfig : ...) instead"
> 
> Move to the new style and mark the project as needing at least that minimum
> version.
> 
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  contrib/meson.build | 2 +-
>  meson.build         | 5 +++--
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/contrib/meson.build b/contrib/meson.build
> index 48aa7c071f92..33a409a2d7d0 100644
> --- a/contrib/meson.build
> +++ b/contrib/meson.build
> @@ -2,7 +2,7 @@ bashcompletiondir = get_option('bashcompletiondir')
>  if bashcompletiondir == ''
>    bash_completion = dependency('bash-completion', required : false)
>    if bash_completion.found()
> -      bashcompletiondir = bash_completion.get_pkgconfig_variable('completionsdir')
> +      bashcompletiondir = bash_completion.get_variable(pkgconfig : 'completionsdir')
>    else
>      bashcompletiondir = datadir / 'bash-completion/completions'
>    endif
> diff --git a/meson.build b/meson.build
> index 19808bb21db8..300eddb99235 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1,5 +1,6 @@
>  project('ndctl', 'c',
>    version : '82',
> +  meson_version: '>= 0.56.0',
>    license : [
>      'GPL-2.0',
>      'LGPL-2.1',
> @@ -159,9 +160,9 @@ endif
>  
>  if get_option('systemd').enabled()
>    systemd = dependency('systemd', required : true)
> -  systemdunitdir = systemd.get_pkgconfig_variable('systemdsystemunitdir')
> +  systemdunitdir = systemd.get_variable(pkgconfig : 'systemdsystemunitdir')
>    udev = dependency('udev', required : true)
> -  udevdir = udev.get_pkgconfig_variable('udevdir')
> +  udevdir = udev.get_variable(pkgconfig : 'udevdir')
>    udevrulesdir = udevdir / 'rules.d'
>  endif
>  

[ndctl PATCH 2/5] test: Fix 'ndctl' dependency in test/sub-section.sh

[Dan Williams]

The test fails with:

"sub-section.sh: line 23: ndctl: command not found"

...it should be using the built ndctl program, not the one installed in the
filesystem.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 test/sub-section.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/sub-section.sh b/test/sub-section.sh
index 77b963355c8f..5acc25eefe87 100755
--- a/test/sub-section.sh
+++ b/test/sub-section.sh
@@ -20,7 +20,7 @@ MIN_AVAIL=$((TEST_SIZE*4))
 MAX_NS=10
 NAME="subsection-test"
 
-ndctl list -N | jq -r ".[] | select(.name==\"subsection-test\") | .dev"
+$NDCTL list -N | jq -r ".[] | select(.name==\"subsection-test\") | .dev"
 
 rc=$FAIL
 cleanup() {
-- 
2.49.0

Re: [ndctl PATCH 2/5] test: Fix 'ndctl' dependency in test/sub-section.sh

[Dave Jiang]

On 6/18/25 3:21 PM, Dan Williams wrote:
> The test fails with:
> 
> "sub-section.sh: line 23: ndctl: command not found"
> 
> ...it should be using the built ndctl program, not the one installed in the
> filesystem.
> 
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
>  test/sub-section.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/test/sub-section.sh b/test/sub-section.sh
> index 77b963355c8f..5acc25eefe87 100755
> --- a/test/sub-section.sh
> +++ b/test/sub-section.sh
> @@ -20,7 +20,7 @@ MIN_AVAIL=$((TEST_SIZE*4))
>  MAX_NS=10
>  NAME="subsection-test"
>  
> -ndctl list -N | jq -r ".[] | select(.name==\"subsection-test\") | .dev"
> +$NDCTL list -N | jq -r ".[] | select(.name==\"subsection-test\") | .dev"
>  
>  rc=$FAIL
>  cleanup() {

[ndctl PATCH 3/5] test: Fix dax.sh expectations

[Dan Williams]

With current kernel+tracecmd combinations stdout is no longer purely trace
records and column "21" is no longer the vmfault_t result.

Drop, if present, the diagnostic print of how many CPUs are in the trace
and use the more universally compatible assumption that the fault result is
the last column rather than a specific column.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 test/dax.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/test/dax.sh b/test/dax.sh
index 3ffbc8079eba..98faaf0eb9b2 100755
--- a/test/dax.sh
+++ b/test/dax.sh
@@ -37,13 +37,14 @@ run_test() {
 	rc=1
 	while read -r p; do
 		[[ $p ]] || continue
+		[[ $p == cpus=* ]] && continue
 		if [ "$count" -lt 10 ]; then
 			if [ "$p" != "0x100" ] && [ "$p" != "NOPAGE" ]; then
 				cleanup "$1"
 			fi
 		fi
 		count=$((count + 1))
-	done < <(trace-cmd report | awk '{ print $21 }')
+	done < <(trace-cmd report | awk '{ print $NF }')
 
 	if [ $count -lt 10 ]; then
 		cleanup "$1"
-- 
2.49.0

Re: [ndctl PATCH 3/5] test: Fix dax.sh expectations

[Dave Jiang]

On 6/18/25 3:21 PM, Dan Williams wrote:
> With current kernel+tracecmd combinations stdout is no longer purely trace
> records and column "21" is no longer the vmfault_t result.
> 
> Drop, if present, the diagnostic print of how many CPUs are in the trace
> and use the more universally compatible assumption that the fault result is
> the last column rather than a specific column.
> 
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

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

> ---
>  test/dax.sh | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/test/dax.sh b/test/dax.sh
> index 3ffbc8079eba..98faaf0eb9b2 100755
> --- a/test/dax.sh
> +++ b/test/dax.sh
> @@ -37,13 +37,14 @@ run_test() {
>  	rc=1
>  	while read -r p; do
>  		[[ $p ]] || continue
> +		[[ $p == cpus=* ]] && continue
>  		if [ "$count" -lt 10 ]; then
>  			if [ "$p" != "0x100" ] && [ "$p" != "NOPAGE" ]; then
>  				cleanup "$1"
>  			fi
>  		fi
>  		count=$((count + 1))
> -	done < <(trace-cmd report | awk '{ print $21 }')
> +	done < <(trace-cmd report | awk '{ print $NF }')
>  
>  	if [ $count -lt 10 ]; then
>  		cleanup "$1"

Re: [ndctl PATCH 3/5] test: Fix dax.sh expectations

[Alison Schofield]

On Wed, Jun 18, 2025 at 03:21:28PM -0700, Dan Williams wrote:
> With current kernel+tracecmd combinations stdout is no longer purely trace
> records and column "21" is no longer the vmfault_t result.
> 
> Drop, if present, the diagnostic print of how many CPUs are in the trace
> and use the more universally compatible assumption that the fault result is
> the last column rather than a specific column.
> 
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  test/dax.sh | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/test/dax.sh b/test/dax.sh
> index 3ffbc8079eba..98faaf0eb9b2 100755
> --- a/test/dax.sh
> +++ b/test/dax.sh
> @@ -37,13 +37,14 @@ run_test() {
>  	rc=1
>  	while read -r p; do
>  		[[ $p ]] || continue
> +		[[ $p == cpus=* ]] && continue
remove above line
>  		if [ "$count" -lt 10 ]; then
>  			if [ "$p" != "0x100" ] && [ "$p" != "NOPAGE" ]; then
>  				cleanup "$1"
>  			fi
>  		fi
>  		count=$((count + 1))
> -	done < <(trace-cmd report | awk '{ print $21 }')
> +	done < <(trace-cmd report | awk '{ print $NF }')
replace above line w
	done < <(trace-cmd report | grep dax_pmd_fault_done | awk '{ print $NF }')


Thanks for all of these Dan!

For this one, I ran into more metadata in the trace file, other than
'cpu=' causing the test to fail. I've tested what I'm showing above,
which makes it immune to other things in the trace file.

Tell me you are OK w this and I'll apply this set to pending.

FYI the more metadata was:
version = 7
CPU 0 is empty
cpus=8

>  
>  	if [ $count -lt 10 ]; then
>  		cleanup "$1"
> -- 
> 2.49.0
> 

Re: [ndctl PATCH 3/5] test: Fix dax.sh expectations

[Dan Williams]

Alison Schofield wrote:
> On Wed, Jun 18, 2025 at 03:21:28PM -0700, Dan Williams wrote:
> > With current kernel+tracecmd combinations stdout is no longer purely trace
> > records and column "21" is no longer the vmfault_t result.
> > 
> > Drop, if present, the diagnostic print of how many CPUs are in the trace
> > and use the more universally compatible assumption that the fault result is
> > the last column rather than a specific column.
> > 
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > ---
> >  test/dax.sh | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/test/dax.sh b/test/dax.sh
> > index 3ffbc8079eba..98faaf0eb9b2 100755
> > --- a/test/dax.sh
> > +++ b/test/dax.sh
> > @@ -37,13 +37,14 @@ run_test() {
> >  	rc=1
> >  	while read -r p; do
> >  		[[ $p ]] || continue
> > +		[[ $p == cpus=* ]] && continue
> remove above line
> >  		if [ "$count" -lt 10 ]; then
> >  			if [ "$p" != "0x100" ] && [ "$p" != "NOPAGE" ]; then
> >  				cleanup "$1"
> >  			fi
> >  		fi
> >  		count=$((count + 1))
> > -	done < <(trace-cmd report | awk '{ print $21 }')
> > +	done < <(trace-cmd report | awk '{ print $NF }')
> replace above line w
> 	done < <(trace-cmd report | grep dax_pmd_fault_done | awk '{ print $NF }')
> 
> 
> Thanks for all of these Dan!
> 
> For this one, I ran into more metadata in the trace file, other than
> 'cpu=' causing the test to fail. I've tested what I'm showing above,
> which makes it immune to other things in the trace file.
> 
> Tell me you are OK w this and I'll apply this set to pending.

Yup, looks good to me and is even more robust. Thanks for the fixups!

Re: [ndctl PATCH 3/5] test: Fix dax.sh expectations

[Verma, Vishal L]

On Wed, 2025-06-18 at 18:06 -0700, Alison Schofield wrote:
> On Wed, Jun 18, 2025 at 03:21:28PM -0700, Dan Williams wrote:
> > With current kernel+tracecmd combinations stdout is no longer purely trace
> > records and column "21" is no longer the vmfault_t result.
> > 
> > Drop, if present, the diagnostic print of how many CPUs are in the trace
> > and use the more universally compatible assumption that the fault result is
> > the last column rather than a specific column.
> > 
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > ---
> >  test/dax.sh | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/test/dax.sh b/test/dax.sh
> > index 3ffbc8079eba..98faaf0eb9b2 100755
> > --- a/test/dax.sh
> > +++ b/test/dax.sh
> > @@ -37,13 +37,14 @@ run_test() {
> >  	rc=1
> >  	while read -r p; do
> >  		[[ $p ]] || continue
> > +		[[ $p == cpus=* ]] && continue
> remove above line
> >  		if [ "$count" -lt 10 ]; then
> >  			if [ "$p" != "0x100" ] && [ "$p" != "NOPAGE" ]; then
> >  				cleanup "$1"
> >  			fi
> >  		fi
> >  		count=$((count + 1))
> > -	done < <(trace-cmd report | awk '{ print $21 }')
> > +	done < <(trace-cmd report | awk '{ print $NF }')
> replace above line w
> 	done < <(trace-cmd report | grep dax_pmd_fault_done | awk '{ print $NF }')

Very minor nit, but since you're already using awk, no need to grep
first, instead you can use awk's 'first part' to do the filtering - 

  done < <(trace-cmd report | awk '/dax_pmd_fault_done/{ print $NF }')

You can stick any regex between the /../ and it will only act on lines
matching that.

Re: [ndctl PATCH 3/5] test: Fix dax.sh expectations

[Alison Schofield]

On Fri, Jun 20, 2025 at 01:11:00PM -0700, Vishal Verma wrote:
> On Wed, 2025-06-18 at 18:06 -0700, Alison Schofield wrote:
> > On Wed, Jun 18, 2025 at 03:21:28PM -0700, Dan Williams wrote:
> > > With current kernel+tracecmd combinations stdout is no longer purely trace
> > > records and column "21" is no longer the vmfault_t result.
> > > 
> > > Drop, if present, the diagnostic print of how many CPUs are in the trace
> > > and use the more universally compatible assumption that the fault result is
> > > the last column rather than a specific column.
> > > 
> > > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > > ---
> > >  test/dax.sh | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/test/dax.sh b/test/dax.sh
> > > index 3ffbc8079eba..98faaf0eb9b2 100755
> > > --- a/test/dax.sh
> > > +++ b/test/dax.sh
> > > @@ -37,13 +37,14 @@ run_test() {
> > >  	rc=1
> > >  	while read -r p; do
> > >  		[[ $p ]] || continue
> > > +		[[ $p == cpus=* ]] && continue
> > remove above line
> > >  		if [ "$count" -lt 10 ]; then
> > >  			if [ "$p" != "0x100" ] && [ "$p" != "NOPAGE" ]; then
> > >  				cleanup "$1"
> > >  			fi
> > >  		fi
> > >  		count=$((count + 1))
> > > -	done < <(trace-cmd report | awk '{ print $21 }')
> > > +	done < <(trace-cmd report | awk '{ print $NF }')
> > replace above line w
> > 	done < <(trace-cmd report | grep dax_pmd_fault_done | awk '{ print $NF }')
> 
> Very minor nit, but since you're already using awk, no need to grep
> first, instead you can use awk's 'first part' to do the filtering - 
> 
>   done < <(trace-cmd report | awk '/dax_pmd_fault_done/{ print $NF }')
> 
> You can stick any regex between the /../ and it will only act on lines
> matching that.

Thanks Vishal! I hope to remember that for 'next time' I see grep and awk used
together. Not rev'ing this one.

[ndctl PATCH 4/5] test: Update documentation with required packages to install

[Dan Williams]

After recently needing to manually rebuild a test environment I discovered
the dependencies that need to be installed. Add a section to the README for
package dependencies.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 README.md | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/README.md b/README.md
index ac24eeb28b6b..1943fd66d432 100644
--- a/README.md
+++ b/README.md
@@ -85,6 +85,11 @@ loaded.  To build and install nfit_test.ko:
    CONFIG_TRANSPARENT_HUGEPAGE=y
    ```
 
+1. Install the following packages, (Fedora instructions):
+   ```
+   dnf install e2fsprogs xfsprogs parted jq trace-cmd hostname fio fio-engine-dev-dax
+   ```
+
 1. Build and install the unit test enabled libnvdimm modules in the
    following order.  The unit test modules need to be in place prior to
    the `depmod` that runs during the final `modules_install`  
-- 
2.49.0

Re: [ndctl PATCH 4/5] test: Update documentation with required packages to install

[Dave Jiang]

On 6/18/25 3:21 PM, Dan Williams wrote:
> After recently needing to manually rebuild a test environment I discovered
> the dependencies that need to be installed. Add a section to the README for
> package dependencies.
> 
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

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

> ---
>  README.md | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/README.md b/README.md
> index ac24eeb28b6b..1943fd66d432 100644
> --- a/README.md
> +++ b/README.md
> @@ -85,6 +85,11 @@ loaded.  To build and install nfit_test.ko:
>     CONFIG_TRANSPARENT_HUGEPAGE=y
>     ```
>  
> +1. Install the following packages, (Fedora instructions):
> +   ```
> +   dnf install e2fsprogs xfsprogs parted jq trace-cmd hostname fio fio-engine-dev-dax
> +   ```
> +
>  1. Build and install the unit test enabled libnvdimm modules in the
>     following order.  The unit test modules need to be in place prior to
>     the `depmod` that runs during the final `modules_install`  

[ndctl PATCH 5/5] test: Fixup fwctl dependency

[Dan Williams]

Ensure the 'fwctl' test binary is always built for test runs.

Fixes: e461c7e2da63 ("test/cxl-features.sh: add test for CXL features device")
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 test/meson.build | 1 +
 1 file changed, 1 insertion(+)

diff --git a/test/meson.build b/test/meson.build
index 91eb6c2b1363..775542c1b787 100644
--- a/test/meson.build
+++ b/test/meson.build
@@ -275,6 +275,7 @@ foreach t : tests
       dax_errors,
       daxdev_errors,
       dax_dev,
+      fwctl,
       mmap,
     ],
     suite: t[2],
-- 
2.49.0

Re: [ndctl PATCH 5/5] test: Fixup fwctl dependency

[Dave Jiang]

On 6/18/25 3:21 PM, Dan Williams wrote:
> Ensure the 'fwctl' test binary is always built for test runs.
> 
> Fixes: e461c7e2da63 ("test/cxl-features.sh: add test for CXL features device")
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

thanks for the fix

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

> ---
>  test/meson.build | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/test/meson.build b/test/meson.build
> index 91eb6c2b1363..775542c1b787 100644
> --- a/test/meson.build
> +++ b/test/meson.build
> @@ -275,6 +275,7 @@ foreach t : tests
>        dax_errors,
>        daxdev_errors,
>        dax_dev,
> +      fwctl,
>        mmap,
>      ],
>      suite: t[2],

Re: [ndctl PATCH 0/5] ndctl: Add missing test dependencies and other fixups

[Alison Schofield]

On Wed, Jun 18, 2025 at 03:21:25PM -0700, Dan Williams wrote:
> Recently I stood up a new test system from scratch and discovered some
> gaps in the documentation and the tests.
> 
> Fix those up to get all tests passing with v6.16-rc2 and latest version
> of all test dependencies currently available in Fedora Rawhide.

Thanks and applied to pending at: https://github.com/pmem/ndctl/

[ commit msg updates and the mentioned dax.sh awk change ]

> 
> Dan Williams (5):
>   build: Fix meson feature deprecation warnings
>   test: Fix 'ndctl' dependency in test/sub-section.sh
>   test: Fix dax.sh expectations
>   test: Update documentation with required packages to install
>   test: Fixup fwctl dependency
> 
>  README.md           | 5 +++++
>  contrib/meson.build | 2 +-
>  meson.build         | 5 +++--
>  test/dax.sh         | 3 ++-
>  test/meson.build    | 1 +
>  test/sub-section.sh | 2 +-
>  6 files changed, 13 insertions(+), 5 deletions(-)
> 
> 
> base-commit: 74b9e411bf13e87df39a517d10143fafa7e2ea92
> -- 
> 2.49.0
>