* [ndctl PATCH 0/3] cxl/test: CXL unit test helpers
@ 2023-11-28 4:11 alison.schofield
2023-11-28 4:11 ` [ndctl PATCH 1/3] cxl/test: add and use cxl_common_[start|stop] helpers alison.schofield
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: alison.schofield @ 2023-11-28 4:11 UTC (permalink / raw
To: Vishal Verma; +Cc: Alison Schofield, nvdimm, linux-cxl
From: Alison Schofield <alison.schofield@intel.com>
This set started with the intent of expanding the check_dmesg()
to look for the 'calc interleave' failure message that is added
to kernel 6.7. It ended up being a bit more than that including a
small reorg of some commonly used setup and shutdown commands
plus a fixup to a timing issue with the dmesg log searches.
This does not depend upon 6.7 as it is only checking for the existence
of the failed message. In fact, it's value at the moment, is probably
in the hands of folks developing and testing on 6.7rc* and onward.
This is built upon ndctl origin/pending branch. (base commit below)
Alison Schofield (3):
cxl/test: add and use cxl_common_[start|stop] helpers
cxl/test: add a cxl_ derivative of check_dmesg()
cxl/test: use an explicit --since time in journalctl
test/common | 37 +++++++++++++++++++++++++++++++++++++
test/cxl-create-region.sh | 16 ++--------------
test/cxl-events.sh | 18 +++---------------
test/cxl-labels.sh | 16 ++--------------
test/cxl-poison.sh | 17 ++---------------
test/cxl-region-sysfs.sh | 16 ++--------------
test/cxl-topology.sh | 16 ++--------------
test/cxl-update-firmware.sh | 17 ++---------------
test/cxl-xor-region.sh | 15 ++-------------
9 files changed, 54 insertions(+), 114 deletions(-)
base-commit: cbf049039482a56c2b66ede3e10d5e9c652890b7
--
2.37.3
^ permalink raw reply [flat|nested] 10+ messages in thread
* [ndctl PATCH 1/3] cxl/test: add and use cxl_common_[start|stop] helpers
2023-11-28 4:11 [ndctl PATCH 0/3] cxl/test: CXL unit test helpers alison.schofield
@ 2023-11-28 4:11 ` alison.schofield
2023-11-28 18:14 ` Dave Jiang
2023-11-28 21:50 ` Verma, Vishal L
2023-11-28 4:11 ` [ndctl PATCH 2/3] cxl/test: add a cxl_ derivative of check_dmesg() alison.schofield
2023-11-28 4:11 ` [ndctl PATCH 3/3] cxl/test: use an explicit --since time in journalctl alison.schofield
2 siblings, 2 replies; 10+ messages in thread
From: alison.schofield @ 2023-11-28 4:11 UTC (permalink / raw
To: Vishal Verma; +Cc: Alison Schofield, nvdimm, linux-cxl
From: Alison Schofield <alison.schofield@intel.com>
CXL unit tests use a mostly common set of commands to setup and tear down
their test environments. Standardize on a common set and make all unit
tests that run as part of the CXL suite use the helpers.
This assures that each test is following the best known practice of
set up and tear down, and that each is using the existing common
helper - check_dmesg(). It also allows for expansion of the common
helpers without the need to touch every unit test.
Note that this makes all tests have the same execution prerequisites,
so all tests will skip if a prerequisite for any test is not present.
At the moment, the extra prereqs are sha256sum and dd, both used by
cxl-update-firmware.sh. The broad requirement is a good thing, in that
it enforces correct setup and complete runs of the entire CXL suite.
cxl-security.sh was excluded from this migration as its setup has more
in common with the nfit_test and legacy security test than with the
other CXL unit tests.
Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
test/common | 23 +++++++++++++++++++++++
test/cxl-create-region.sh | 16 ++--------------
test/cxl-events.sh | 18 +++---------------
test/cxl-labels.sh | 16 ++--------------
test/cxl-poison.sh | 17 ++---------------
test/cxl-region-sysfs.sh | 16 ++--------------
test/cxl-topology.sh | 16 ++--------------
test/cxl-update-firmware.sh | 17 ++---------------
test/cxl-xor-region.sh | 15 ++-------------
9 files changed, 40 insertions(+), 114 deletions(-)
diff --git a/test/common b/test/common
index f1023ef20f7e..7a4711593624 100644
--- a/test/common
+++ b/test/common
@@ -150,3 +150,26 @@ check_dmesg()
grep -q "Call Trace" <<< $log && err $1
true
}
+
+# cxl_common_start
+# $1: optional module parameter(s) for cxl-test
+cxl_common_start()
+{
+ rc=77
+ set -ex
+ trap 'err $LINENO' ERR
+ check_prereq "jq"
+ check_prereq "dd"
+ check_prereq "sha256sum"
+ modprobe -r cxl_test
+ modprobe cxl_test "$1"
+ rc=1
+}
+
+# cxl_common_end
+# $1: line number where this is called
+cxl_common_stop()
+{
+ check_dmesg "$1"
+ modprobe -r cxl_test
+}
diff --git a/test/cxl-create-region.sh b/test/cxl-create-region.sh
index 658b9b8ff58a..aa586b1471f6 100644
--- a/test/cxl-create-region.sh
+++ b/test/cxl-create-region.sh
@@ -4,17 +4,7 @@
. $(dirname $0)/common
-rc=77
-
-set -ex
-
-trap 'err $LINENO' ERR
-
-check_prereq "jq"
-
-modprobe -r cxl_test
-modprobe cxl_test
-rc=1
+cxl_common_start
destroy_regions()
{
@@ -149,6 +139,4 @@ for mem in ${mems[@]}; do
create_subregions "$mem"
done
-check_dmesg "$LINENO"
-
-modprobe -r cxl_test
+cxl_common_stop "$LINENO"
diff --git a/test/cxl-events.sh b/test/cxl-events.sh
index fe702bf98ad4..b181646d0fcb 100644
--- a/test/cxl-events.sh
+++ b/test/cxl-events.sh
@@ -4,24 +4,14 @@
. "$(dirname "$0")/common"
+cxl_common_start
+
# Results expected
num_overflow_expected=1
num_fatal_expected=2
num_failure_expected=16
num_info_expected=3
-rc=77
-
-set -ex
-
-trap 'err $LINENO' ERR
-
-check_prereq "jq"
-
-modprobe -r cxl_test
-modprobe cxl_test
-rc=1
-
dev_path="/sys/bus/platform/devices"
test_cxl_events()
@@ -74,6 +64,4 @@ if [ "$num_info" -ne $num_info_expected ]; then
err "$LINENO"
fi
-check_dmesg "$LINENO"
-
-modprobe -r cxl_test
+cxl_common_stop "$LINENO"
diff --git a/test/cxl-labels.sh b/test/cxl-labels.sh
index 36b0341c8039..c911816696c5 100644
--- a/test/cxl-labels.sh
+++ b/test/cxl-labels.sh
@@ -4,17 +4,7 @@
. $(dirname $0)/common
-rc=77
-
-set -ex
-
-trap 'err $LINENO' ERR
-
-check_prereq "jq"
-
-modprobe -r cxl_test
-modprobe cxl_test
-rc=1
+cxl_common_start
test_label_ops()
{
@@ -66,6 +56,4 @@ for nmem in ${nmems[@]}; do
test_label_ops "$nmem"
done
-check_dmesg "$LINENO"
-
-modprobe -r cxl_test
+cxl_common_stop "$LINENO"
diff --git a/test/cxl-poison.sh b/test/cxl-poison.sh
index 8747ffe8cff7..2f16dc11884c 100644
--- a/test/cxl-poison.sh
+++ b/test/cxl-poison.sh
@@ -4,18 +4,7 @@
. "$(dirname "$0")"/common
-rc=77
-
-set -ex
-
-trap 'err $LINENO' ERR
-
-check_prereq "jq"
-
-modprobe -r cxl_test
-modprobe cxl_test
-
-rc=1
+cxl_common_start
# THEORY OF OPERATION: Exercise cxl-cli and cxl driver ability to
# inject, clear, and get the poison list. Do it by memdev and by region.
@@ -153,6 +142,4 @@ echo 1 > /sys/kernel/tracing/events/cxl/cxl_poison/enable
test_poison_by_memdev
test_poison_by_region
-check_dmesg "$LINENO"
-
-modprobe -r cxl-test
+cxl_common_stop "$LINENO"
diff --git a/test/cxl-region-sysfs.sh b/test/cxl-region-sysfs.sh
index 863639271afa..2c81d8f0b006 100644
--- a/test/cxl-region-sysfs.sh
+++ b/test/cxl-region-sysfs.sh
@@ -4,17 +4,7 @@
. $(dirname $0)/common
-rc=77
-
-set -ex
-
-trap 'err $LINENO' ERR
-
-check_prereq "jq"
-
-modprobe -r cxl_test
-modprobe cxl_test
-rc=1
+cxl_common_start
# THEORY OF OPERATION: Create a x8 interleave across the pmem capacity
# of the 8 endpoints defined by cxl_test, commit the decoders (which
@@ -163,6 +153,4 @@ readarray -t endpoint < <($CXL free-dpa -t pmem ${mem[*]} |
jq -r ".[] | .decoder.decoder")
echo "$region released ${#endpoint[@]} targets: ${endpoint[@]}"
-check_dmesg "$LINENO"
-
-modprobe -r cxl_test
+cxl_common_stop "$LINENO"
diff --git a/test/cxl-topology.sh b/test/cxl-topology.sh
index e8b9f56543b5..7822abada7dc 100644
--- a/test/cxl-topology.sh
+++ b/test/cxl-topology.sh
@@ -4,17 +4,7 @@
. $(dirname $0)/common
-rc=77
-
-set -ex
-
-trap 'err $LINENO' ERR
-
-check_prereq "jq"
-
-modprobe -r cxl_test
-modprobe cxl_test
-rc=1
+cxl_common_start
# THEORY OF OPERATION: Validate the hard coded assumptions of the
# cxl_test.ko module that defines its topology in
@@ -187,6 +177,4 @@ done
# validate that the bus can be disabled without issue
$CXL disable-bus $root -f
-check_dmesg "$LINENO"
-
-modprobe -r cxl_test
+cxl_common_stop "$LINENO"
diff --git a/test/cxl-update-firmware.sh b/test/cxl-update-firmware.sh
index f326868977a9..cf080150ccbc 100755
--- a/test/cxl-update-firmware.sh
+++ b/test/cxl-update-firmware.sh
@@ -4,19 +4,7 @@
. $(dirname $0)/common
-rc=77
-
-set -ex
-
-trap 'err $LINENO' ERR
-
-check_prereq "jq"
-check_prereq "dd"
-check_prereq "sha256sum"
-
-modprobe -r cxl_test
-modprobe cxl_test
-rc=1
+cxl_common_start
mk_fw_file()
{
@@ -192,5 +180,4 @@ test_nonblocking_update
test_multiple_memdev
test_cancel
-check_dmesg "$LINENO"
-modprobe -r cxl_test
+cxl_common_stop "$LINENO"
diff --git a/test/cxl-xor-region.sh b/test/cxl-xor-region.sh
index 117e7a4bba61..6d74af8c98cd 100644
--- a/test/cxl-xor-region.sh
+++ b/test/cxl-xor-region.sh
@@ -4,18 +4,7 @@
. $(dirname $0)/common
-rc=77
-
-set -ex
-
-trap 'err $LINENO' ERR
-
-check_prereq "jq"
-
-modprobe -r cxl_test
-modprobe cxl_test interleave_arithmetic=1
-udevadm settle
-rc=1
+cxl_common_start "interleave_arithmetic=1"
# THEORY OF OPERATION: Create x1,2,3,4 regions to exercise the XOR math
# option of the CXL driver. As with other cxl_test tests, changes to the
@@ -93,4 +82,4 @@ create_and_destroy_region
setup_x4
create_and_destroy_region
-modprobe -r cxl_test
+cxl_common_stop "$LINENO"
--
2.37.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [ndctl PATCH 2/3] cxl/test: add a cxl_ derivative of check_dmesg()
2023-11-28 4:11 [ndctl PATCH 0/3] cxl/test: CXL unit test helpers alison.schofield
2023-11-28 4:11 ` [ndctl PATCH 1/3] cxl/test: add and use cxl_common_[start|stop] helpers alison.schofield
@ 2023-11-28 4:11 ` alison.schofield
2023-11-28 18:18 ` Dave Jiang
2023-11-28 22:11 ` Verma, Vishal L
2023-11-28 4:11 ` [ndctl PATCH 3/3] cxl/test: use an explicit --since time in journalctl alison.schofield
2 siblings, 2 replies; 10+ messages in thread
From: alison.schofield @ 2023-11-28 4:11 UTC (permalink / raw
To: Vishal Verma; +Cc: Alison Schofield, nvdimm, linux-cxl
From: Alison Schofield <alison.schofield@intel.com>
check_dmesg() is used by CXL unit tests as well as by a few
DAX unit tests. Add a cxl_check_dmesg() version that can be
expanded for CXL special checks like this:
Add a check for an interleave calculation failure. This is
a dev_dbg() message that spews (success or failure) whenever
a user creates a region. It is useful as a regression check
across the entire CXL suite.
Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
test/common | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/test/common b/test/common
index 7a4711593624..c20b7e48c2b6 100644
--- a/test/common
+++ b/test/common
@@ -151,6 +151,19 @@ check_dmesg()
true
}
+# cxl_check_dmesg
+# $1: line number where this is called
+cxl_check_dmesg()
+{
+ sleep 1
+ log=$(journalctl -r -k --since "-$((SECONDS+1))s")
+ # validate no WARN or lockdep report during the run
+ grep -q "Call Trace" <<< "$log" && err "$1"
+ # validate no failures of the interleave calc dev_dbg() check
+ grep -q "Test cxl_calc_interleave_pos(): fail" <<< "$log" && err "$1"
+ true
+}
+
# cxl_common_start
# $1: optional module parameter(s) for cxl-test
cxl_common_start()
@@ -170,6 +183,6 @@ cxl_common_start()
# $1: line number where this is called
cxl_common_stop()
{
- check_dmesg "$1"
+ cxl_check_dmesg "$1"
modprobe -r cxl_test
}
--
2.37.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [ndctl PATCH 3/3] cxl/test: use an explicit --since time in journalctl
2023-11-28 4:11 [ndctl PATCH 0/3] cxl/test: CXL unit test helpers alison.schofield
2023-11-28 4:11 ` [ndctl PATCH 1/3] cxl/test: add and use cxl_common_[start|stop] helpers alison.schofield
2023-11-28 4:11 ` [ndctl PATCH 2/3] cxl/test: add a cxl_ derivative of check_dmesg() alison.schofield
@ 2023-11-28 4:11 ` alison.schofield
2023-11-28 18:19 ` Dave Jiang
2023-11-28 22:18 ` Verma, Vishal L
2 siblings, 2 replies; 10+ messages in thread
From: alison.schofield @ 2023-11-28 4:11 UTC (permalink / raw
To: Vishal Verma; +Cc: Alison Schofield, nvdimm, linux-cxl
From: Alison Schofield <alison.schofield@intel.com>
Using the bash variable 'SECONDS' plus 1 for searching the
dmesg log sometimes led to one test picking up error messages
from the previous test when run as a suite. SECONDS alone may
miss some logs, but SECONDS + 1 is just as often too great.
Since unit tests in the CXL suite are using common helpers to
start and stop work, initialize and use a "starttime" variable
with millisecond granularity for journalctl.
Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
test/common | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/test/common b/test/common
index c20b7e48c2b6..93a280c7c150 100644
--- a/test/common
+++ b/test/common
@@ -156,7 +156,7 @@ check_dmesg()
cxl_check_dmesg()
{
sleep 1
- log=$(journalctl -r -k --since "-$((SECONDS+1))s")
+ log=$(journalctl -r -k --since "$starttime")
# validate no WARN or lockdep report during the run
grep -q "Call Trace" <<< "$log" && err "$1"
# validate no failures of the interleave calc dev_dbg() check
@@ -175,6 +175,7 @@ cxl_common_start()
check_prereq "dd"
check_prereq "sha256sum"
modprobe -r cxl_test
+ starttime=$(date +"%T.%3N")
modprobe cxl_test "$1"
rc=1
}
--
2.37.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [ndctl PATCH 1/3] cxl/test: add and use cxl_common_[start|stop] helpers
2023-11-28 4:11 ` [ndctl PATCH 1/3] cxl/test: add and use cxl_common_[start|stop] helpers alison.schofield
@ 2023-11-28 18:14 ` Dave Jiang
2023-11-28 21:50 ` Verma, Vishal L
1 sibling, 0 replies; 10+ messages in thread
From: Dave Jiang @ 2023-11-28 18:14 UTC (permalink / raw
To: alison.schofield, Vishal Verma; +Cc: nvdimm, linux-cxl
On 11/27/23 21:11, alison.schofield@intel.com wrote:
> From: Alison Schofield <alison.schofield@intel.com>
>
> CXL unit tests use a mostly common set of commands to setup and tear down
> their test environments. Standardize on a common set and make all unit
> tests that run as part of the CXL suite use the helpers.
>
> This assures that each test is following the best known practice of
> set up and tear down, and that each is using the existing common
> helper - check_dmesg(). It also allows for expansion of the common
> helpers without the need to touch every unit test.
>
> Note that this makes all tests have the same execution prerequisites,
> so all tests will skip if a prerequisite for any test is not present.
> At the moment, the extra prereqs are sha256sum and dd, both used by
> cxl-update-firmware.sh. The broad requirement is a good thing, in that
> it enforces correct setup and complete runs of the entire CXL suite.
>
> cxl-security.sh was excluded from this migration as its setup has more
> in common with the nfit_test and legacy security test than with the
> other CXL unit tests.
>
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
> test/common | 23 +++++++++++++++++++++++
> test/cxl-create-region.sh | 16 ++--------------
> test/cxl-events.sh | 18 +++---------------
> test/cxl-labels.sh | 16 ++--------------
> test/cxl-poison.sh | 17 ++---------------
> test/cxl-region-sysfs.sh | 16 ++--------------
> test/cxl-topology.sh | 16 ++--------------
> test/cxl-update-firmware.sh | 17 ++---------------
> test/cxl-xor-region.sh | 15 ++-------------
> 9 files changed, 40 insertions(+), 114 deletions(-)
>
> diff --git a/test/common b/test/common
> index f1023ef20f7e..7a4711593624 100644
> --- a/test/common
> +++ b/test/common
> @@ -150,3 +150,26 @@ check_dmesg()
> grep -q "Call Trace" <<< $log && err $1
> true
> }
> +
> +# cxl_common_start
> +# $1: optional module parameter(s) for cxl-test
> +cxl_common_start()
> +{
> + rc=77
> + set -ex
> + trap 'err $LINENO' ERR
> + check_prereq "jq"
> + check_prereq "dd"
> + check_prereq "sha256sum"
> + modprobe -r cxl_test
> + modprobe cxl_test "$1"
> + rc=1
> +}
> +
> +# cxl_common_end
> +# $1: line number where this is called
> +cxl_common_stop()
> +{
> + check_dmesg "$1"
> + modprobe -r cxl_test
> +}
> diff --git a/test/cxl-create-region.sh b/test/cxl-create-region.sh
> index 658b9b8ff58a..aa586b1471f6 100644
> --- a/test/cxl-create-region.sh
> +++ b/test/cxl-create-region.sh
> @@ -4,17 +4,7 @@
>
> . $(dirname $0)/common
>
> -rc=77
> -
> -set -ex
> -
> -trap 'err $LINENO' ERR
> -
> -check_prereq "jq"
> -
> -modprobe -r cxl_test
> -modprobe cxl_test
> -rc=1
> +cxl_common_start
>
> destroy_regions()
> {
> @@ -149,6 +139,4 @@ for mem in ${mems[@]}; do
> create_subregions "$mem"
> done
>
> -check_dmesg "$LINENO"
> -
> -modprobe -r cxl_test
> +cxl_common_stop "$LINENO"
> diff --git a/test/cxl-events.sh b/test/cxl-events.sh
> index fe702bf98ad4..b181646d0fcb 100644
> --- a/test/cxl-events.sh
> +++ b/test/cxl-events.sh
> @@ -4,24 +4,14 @@
>
> . "$(dirname "$0")/common"
>
> +cxl_common_start
> +
> # Results expected
> num_overflow_expected=1
> num_fatal_expected=2
> num_failure_expected=16
> num_info_expected=3
>
> -rc=77
> -
> -set -ex
> -
> -trap 'err $LINENO' ERR
> -
> -check_prereq "jq"
> -
> -modprobe -r cxl_test
> -modprobe cxl_test
> -rc=1
> -
> dev_path="/sys/bus/platform/devices"
>
> test_cxl_events()
> @@ -74,6 +64,4 @@ if [ "$num_info" -ne $num_info_expected ]; then
> err "$LINENO"
> fi
>
> -check_dmesg "$LINENO"
> -
> -modprobe -r cxl_test
> +cxl_common_stop "$LINENO"
> diff --git a/test/cxl-labels.sh b/test/cxl-labels.sh
> index 36b0341c8039..c911816696c5 100644
> --- a/test/cxl-labels.sh
> +++ b/test/cxl-labels.sh
> @@ -4,17 +4,7 @@
>
> . $(dirname $0)/common
>
> -rc=77
> -
> -set -ex
> -
> -trap 'err $LINENO' ERR
> -
> -check_prereq "jq"
> -
> -modprobe -r cxl_test
> -modprobe cxl_test
> -rc=1
> +cxl_common_start
>
> test_label_ops()
> {
> @@ -66,6 +56,4 @@ for nmem in ${nmems[@]}; do
> test_label_ops "$nmem"
> done
>
> -check_dmesg "$LINENO"
> -
> -modprobe -r cxl_test
> +cxl_common_stop "$LINENO"
> diff --git a/test/cxl-poison.sh b/test/cxl-poison.sh
> index 8747ffe8cff7..2f16dc11884c 100644
> --- a/test/cxl-poison.sh
> +++ b/test/cxl-poison.sh
> @@ -4,18 +4,7 @@
>
> . "$(dirname "$0")"/common
>
> -rc=77
> -
> -set -ex
> -
> -trap 'err $LINENO' ERR
> -
> -check_prereq "jq"
> -
> -modprobe -r cxl_test
> -modprobe cxl_test
> -
> -rc=1
> +cxl_common_start
>
> # THEORY OF OPERATION: Exercise cxl-cli and cxl driver ability to
> # inject, clear, and get the poison list. Do it by memdev and by region.
> @@ -153,6 +142,4 @@ echo 1 > /sys/kernel/tracing/events/cxl/cxl_poison/enable
> test_poison_by_memdev
> test_poison_by_region
>
> -check_dmesg "$LINENO"
> -
> -modprobe -r cxl-test
> +cxl_common_stop "$LINENO"
> diff --git a/test/cxl-region-sysfs.sh b/test/cxl-region-sysfs.sh
> index 863639271afa..2c81d8f0b006 100644
> --- a/test/cxl-region-sysfs.sh
> +++ b/test/cxl-region-sysfs.sh
> @@ -4,17 +4,7 @@
>
> . $(dirname $0)/common
>
> -rc=77
> -
> -set -ex
> -
> -trap 'err $LINENO' ERR
> -
> -check_prereq "jq"
> -
> -modprobe -r cxl_test
> -modprobe cxl_test
> -rc=1
> +cxl_common_start
>
> # THEORY OF OPERATION: Create a x8 interleave across the pmem capacity
> # of the 8 endpoints defined by cxl_test, commit the decoders (which
> @@ -163,6 +153,4 @@ readarray -t endpoint < <($CXL free-dpa -t pmem ${mem[*]} |
> jq -r ".[] | .decoder.decoder")
> echo "$region released ${#endpoint[@]} targets: ${endpoint[@]}"
>
> -check_dmesg "$LINENO"
> -
> -modprobe -r cxl_test
> +cxl_common_stop "$LINENO"
> diff --git a/test/cxl-topology.sh b/test/cxl-topology.sh
> index e8b9f56543b5..7822abada7dc 100644
> --- a/test/cxl-topology.sh
> +++ b/test/cxl-topology.sh
> @@ -4,17 +4,7 @@
>
> . $(dirname $0)/common
>
> -rc=77
> -
> -set -ex
> -
> -trap 'err $LINENO' ERR
> -
> -check_prereq "jq"
> -
> -modprobe -r cxl_test
> -modprobe cxl_test
> -rc=1
> +cxl_common_start
>
> # THEORY OF OPERATION: Validate the hard coded assumptions of the
> # cxl_test.ko module that defines its topology in
> @@ -187,6 +177,4 @@ done
> # validate that the bus can be disabled without issue
> $CXL disable-bus $root -f
>
> -check_dmesg "$LINENO"
> -
> -modprobe -r cxl_test
> +cxl_common_stop "$LINENO"
> diff --git a/test/cxl-update-firmware.sh b/test/cxl-update-firmware.sh
> index f326868977a9..cf080150ccbc 100755
> --- a/test/cxl-update-firmware.sh
> +++ b/test/cxl-update-firmware.sh
> @@ -4,19 +4,7 @@
>
> . $(dirname $0)/common
>
> -rc=77
> -
> -set -ex
> -
> -trap 'err $LINENO' ERR
> -
> -check_prereq "jq"
> -check_prereq "dd"
> -check_prereq "sha256sum"
> -
> -modprobe -r cxl_test
> -modprobe cxl_test
> -rc=1
> +cxl_common_start
>
> mk_fw_file()
> {
> @@ -192,5 +180,4 @@ test_nonblocking_update
> test_multiple_memdev
> test_cancel
>
> -check_dmesg "$LINENO"
> -modprobe -r cxl_test
> +cxl_common_stop "$LINENO"
> diff --git a/test/cxl-xor-region.sh b/test/cxl-xor-region.sh
> index 117e7a4bba61..6d74af8c98cd 100644
> --- a/test/cxl-xor-region.sh
> +++ b/test/cxl-xor-region.sh
> @@ -4,18 +4,7 @@
>
> . $(dirname $0)/common
>
> -rc=77
> -
> -set -ex
> -
> -trap 'err $LINENO' ERR
> -
> -check_prereq "jq"
> -
> -modprobe -r cxl_test
> -modprobe cxl_test interleave_arithmetic=1
> -udevadm settle
> -rc=1
> +cxl_common_start "interleave_arithmetic=1"
>
> # THEORY OF OPERATION: Create x1,2,3,4 regions to exercise the XOR math
> # option of the CXL driver. As with other cxl_test tests, changes to the
> @@ -93,4 +82,4 @@ create_and_destroy_region
> setup_x4
> create_and_destroy_region
>
> -modprobe -r cxl_test
> +cxl_common_stop "$LINENO"
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [ndctl PATCH 2/3] cxl/test: add a cxl_ derivative of check_dmesg()
2023-11-28 4:11 ` [ndctl PATCH 2/3] cxl/test: add a cxl_ derivative of check_dmesg() alison.schofield
@ 2023-11-28 18:18 ` Dave Jiang
2023-11-28 22:11 ` Verma, Vishal L
1 sibling, 0 replies; 10+ messages in thread
From: Dave Jiang @ 2023-11-28 18:18 UTC (permalink / raw
To: alison.schofield, Vishal Verma; +Cc: nvdimm, linux-cxl
On 11/27/23 21:11, alison.schofield@intel.com wrote:
> From: Alison Schofield <alison.schofield@intel.com>
>
> check_dmesg() is used by CXL unit tests as well as by a few
> DAX unit tests. Add a cxl_check_dmesg() version that can be
> expanded for CXL special checks like this:
>
> Add a check for an interleave calculation failure. This is
> a dev_dbg() message that spews (success or failure) whenever
> a user creates a region. It is useful as a regression check
> across the entire CXL suite.
>
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
> test/common | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/test/common b/test/common
> index 7a4711593624..c20b7e48c2b6 100644
> --- a/test/common
> +++ b/test/common
> @@ -151,6 +151,19 @@ check_dmesg()
> true
> }
>
> +# cxl_check_dmesg
> +# $1: line number where this is called
> +cxl_check_dmesg()
> +{
> + sleep 1
> + log=$(journalctl -r -k --since "-$((SECONDS+1))s")
> + # validate no WARN or lockdep report during the run
> + grep -q "Call Trace" <<< "$log" && err "$1"
> + # validate no failures of the interleave calc dev_dbg() check
> + grep -q "Test cxl_calc_interleave_pos(): fail" <<< "$log" && err "$1"
> + true
> +}
> +
> # cxl_common_start
> # $1: optional module parameter(s) for cxl-test
> cxl_common_start()
> @@ -170,6 +183,6 @@ cxl_common_start()
> # $1: line number where this is called
> cxl_common_stop()
> {
> - check_dmesg "$1"
> + cxl_check_dmesg "$1"
> modprobe -r cxl_test
> }
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [ndctl PATCH 3/3] cxl/test: use an explicit --since time in journalctl
2023-11-28 4:11 ` [ndctl PATCH 3/3] cxl/test: use an explicit --since time in journalctl alison.schofield
@ 2023-11-28 18:19 ` Dave Jiang
2023-11-28 22:18 ` Verma, Vishal L
1 sibling, 0 replies; 10+ messages in thread
From: Dave Jiang @ 2023-11-28 18:19 UTC (permalink / raw
To: alison.schofield, Vishal Verma; +Cc: nvdimm, linux-cxl
On 11/27/23 21:11, alison.schofield@intel.com wrote:
> From: Alison Schofield <alison.schofield@intel.com>
>
> Using the bash variable 'SECONDS' plus 1 for searching the
> dmesg log sometimes led to one test picking up error messages
> from the previous test when run as a suite. SECONDS alone may
> miss some logs, but SECONDS + 1 is just as often too great.
>
> Since unit tests in the CXL suite are using common helpers to
> start and stop work, initialize and use a "starttime" variable
> with millisecond granularity for journalctl.
>
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
> test/common | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/test/common b/test/common
> index c20b7e48c2b6..93a280c7c150 100644
> --- a/test/common
> +++ b/test/common
> @@ -156,7 +156,7 @@ check_dmesg()
> cxl_check_dmesg()
> {
> sleep 1
> - log=$(journalctl -r -k --since "-$((SECONDS+1))s")
> + log=$(journalctl -r -k --since "$starttime")
> # validate no WARN or lockdep report during the run
> grep -q "Call Trace" <<< "$log" && err "$1"
> # validate no failures of the interleave calc dev_dbg() check
> @@ -175,6 +175,7 @@ cxl_common_start()
> check_prereq "dd"
> check_prereq "sha256sum"
> modprobe -r cxl_test
> + starttime=$(date +"%T.%3N")
> modprobe cxl_test "$1"
> rc=1
> }
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [ndctl PATCH 1/3] cxl/test: add and use cxl_common_[start|stop] helpers
2023-11-28 4:11 ` [ndctl PATCH 1/3] cxl/test: add and use cxl_common_[start|stop] helpers alison.schofield
2023-11-28 18:14 ` Dave Jiang
@ 2023-11-28 21:50 ` Verma, Vishal L
1 sibling, 0 replies; 10+ messages in thread
From: Verma, Vishal L @ 2023-11-28 21:50 UTC (permalink / raw
To: Schofield, Alison; +Cc: linux-cxl@vger.kernel.org, nvdimm@lists.linux.dev
On Mon, 2023-11-27 at 20:11 -0800, alison.schofield@intel.com wrote:
> From: Alison Schofield <alison.schofield@intel.com>
>
> CXL unit tests use a mostly common set of commands to setup and tear down
> their test environments. Standardize on a common set and make all unit
> tests that run as part of the CXL suite use the helpers.
>
> This assures that each test is following the best known practice of
> set up and tear down, and that each is using the existing common
> helper - check_dmesg(). It also allows for expansion of the common
> helpers without the need to touch every unit test.
>
> Note that this makes all tests have the same execution prerequisites,
> so all tests will skip if a prerequisite for any test is not present.
> At the moment, the extra prereqs are sha256sum and dd, both used by
> cxl-update-firmware.sh. The broad requirement is a good thing, in that
> it enforces correct setup and complete runs of the entire CXL suite.
>
> cxl-security.sh was excluded from this migration as its setup has more
> in common with the nfit_test and legacy security test than with the
> other CXL unit tests.
>
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> ---
> test/common | 23 +++++++++++++++++++++++
> test/cxl-create-region.sh | 16 ++--------------
> test/cxl-events.sh | 18 +++---------------
> test/cxl-labels.sh | 16 ++--------------
> test/cxl-poison.sh | 17 ++---------------
> test/cxl-region-sysfs.sh | 16 ++--------------
> test/cxl-topology.sh | 16 ++--------------
> test/cxl-update-firmware.sh | 17 ++---------------
> test/cxl-xor-region.sh | 15 ++-------------
> 9 files changed, 40 insertions(+), 114 deletions(-)
>
> diff --git a/test/common b/test/common
> index f1023ef20f7e..7a4711593624 100644
> --- a/test/common
> +++ b/test/common
> @@ -150,3 +150,26 @@ check_dmesg()
> grep -q "Call Trace" <<< $log && err $1
> true
> }
> +
> +# cxl_common_start
> +# $1: optional module parameter(s) for cxl-test
> +cxl_common_start()
> +{
> + rc=77
> + set -ex
> + trap 'err $LINENO' ERR
> + check_prereq "jq"
> + check_prereq "dd"
> + check_prereq "sha256sum"
> + modprobe -r cxl_test
> + modprobe cxl_test "$1"
This should use "$@". $1 will break if multiple parameters need to be
passed (currently we only ever pass one, so it happens to work, but if
a second param ever gets added this will be surprising).
Rest of this looks good, thanks for doing this cleanup!
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [ndctl PATCH 2/3] cxl/test: add a cxl_ derivative of check_dmesg()
2023-11-28 4:11 ` [ndctl PATCH 2/3] cxl/test: add a cxl_ derivative of check_dmesg() alison.schofield
2023-11-28 18:18 ` Dave Jiang
@ 2023-11-28 22:11 ` Verma, Vishal L
1 sibling, 0 replies; 10+ messages in thread
From: Verma, Vishal L @ 2023-11-28 22:11 UTC (permalink / raw
To: Schofield, Alison; +Cc: linux-cxl@vger.kernel.org, nvdimm@lists.linux.dev
On Mon, 2023-11-27 at 20:11 -0800, alison.schofield@intel.com wrote:
> From: Alison Schofield <alison.schofield@intel.com>
>
> check_dmesg() is used by CXL unit tests as well as by a few
> DAX unit tests. Add a cxl_check_dmesg() version that can be
> expanded for CXL special checks like this:
>
> Add a check for an interleave calculation failure. This is
> a dev_dbg() message that spews (success or failure) whenever
> a user creates a region. It is useful as a regression check
> across the entire CXL suite.
>
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> ---
> test/common | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/test/common b/test/common
> index 7a4711593624..c20b7e48c2b6 100644
> --- a/test/common
> +++ b/test/common
> @@ -151,6 +151,19 @@ check_dmesg()
> true
> }
>
> +# cxl_check_dmesg
> +# $1: line number where this is called
> +cxl_check_dmesg()
> +{
> + sleep 1
> + log=$(journalctl -r -k --since "-$((SECONDS+1))s")
> + # validate no WARN or lockdep report during the run
> + grep -q "Call Trace" <<< "$log" && err "$1"
> + # validate no failures of the interleave calc dev_dbg() check
> + grep -q "Test cxl_calc_interleave_pos(): fail" <<< "$log" && err "$1"
> + true
> +}
I like the idea of adding new checks - how about a generic helper that
greps on a list of strings passed to it, and wrappers on top of it can
have their own custom set of strings.
Something like this (untested):
# __check_dmesg
# $1: line number where this is called
# $2.. : strings to check for
__check_dmesg()
{
line="$1"
shift
strings=( "$@" )
sleep 1
log=$(journalctl -r -k --since "-$((SECONDS+1))s")
for string in "${strings[@]}"; do
if grep -q "$string" <<< $log; then
err "$line"
fi
done
true
}
check_dmesg()
{
line="$1"
shift
strings=( "$@" )
__check_dmesg "$line" "Call Trace" "${strings[@]}"
}
cxl_check_dmesg()
{
line="$1"
shift
strings=( "$@" )
check_dmesg "$line" \
"Test cxl_calc_interleave_pos(): fail" \
"${strings[@]}"
}
This lets tests opt in to any 'level' of checks, and lets them add any
of their own test-specific strings to be checked at any stage as well.
> +
> # cxl_common_start
> # $1: optional module parameter(s) for cxl-test
> cxl_common_start()
> @@ -170,6 +183,6 @@ cxl_common_start()
> # $1: line number where this is called
> cxl_common_stop()
> {
> - check_dmesg "$1"
> + cxl_check_dmesg "$1"
> modprobe -r cxl_test
> }
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [ndctl PATCH 3/3] cxl/test: use an explicit --since time in journalctl
2023-11-28 4:11 ` [ndctl PATCH 3/3] cxl/test: use an explicit --since time in journalctl alison.schofield
2023-11-28 18:19 ` Dave Jiang
@ 2023-11-28 22:18 ` Verma, Vishal L
1 sibling, 0 replies; 10+ messages in thread
From: Verma, Vishal L @ 2023-11-28 22:18 UTC (permalink / raw
To: Schofield, Alison; +Cc: linux-cxl@vger.kernel.org, nvdimm@lists.linux.dev
On Mon, 2023-11-27 at 20:11 -0800, alison.schofield@intel.com wrote:
> From: Alison Schofield <alison.schofield@intel.com>
>
> Using the bash variable 'SECONDS' plus 1 for searching the
> dmesg log sometimes led to one test picking up error messages
> from the previous test when run as a suite. SECONDS alone may
> miss some logs, but SECONDS + 1 is just as often too great.
>
> Since unit tests in the CXL suite are using common helpers to
> start and stop work, initialize and use a "starttime" variable
> with millisecond granularity for journalctl.
>
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> ---
> test/common | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/test/common b/test/common
> index c20b7e48c2b6..93a280c7c150 100644
> --- a/test/common
> +++ b/test/common
> @@ -156,7 +156,7 @@ check_dmesg()
> cxl_check_dmesg()
> {
> sleep 1
> - log=$(journalctl -r -k --since "-$((SECONDS+1))s")
> + log=$(journalctl -r -k --since "$starttime")
Once this is moved to the geenric helper, the other check_dmesg() will
get this benefit too, not just the cxl version :)
It might be worth adding a check to see if $starttime has been set, and
erroring out if not, in case a future test tries to use this, but
doesn't realize that they should've also used the common start helper.
> # validate no WARN or lockdep report during the run
> grep -q "Call Trace" <<< "$log" && err "$1"
> # validate no failures of the interleave calc dev_dbg() check
> @@ -175,6 +175,7 @@ cxl_common_start()
> check_prereq "dd"
> check_prereq "sha256sum"
> modprobe -r cxl_test
> + starttime=$(date +"%T.%3N")
> modprobe cxl_test "$1"
> rc=1
> }
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-11-28 22:18 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-28 4:11 [ndctl PATCH 0/3] cxl/test: CXL unit test helpers alison.schofield
2023-11-28 4:11 ` [ndctl PATCH 1/3] cxl/test: add and use cxl_common_[start|stop] helpers alison.schofield
2023-11-28 18:14 ` Dave Jiang
2023-11-28 21:50 ` Verma, Vishal L
2023-11-28 4:11 ` [ndctl PATCH 2/3] cxl/test: add a cxl_ derivative of check_dmesg() alison.schofield
2023-11-28 18:18 ` Dave Jiang
2023-11-28 22:11 ` Verma, Vishal L
2023-11-28 4:11 ` [ndctl PATCH 3/3] cxl/test: use an explicit --since time in journalctl alison.schofield
2023-11-28 18:19 ` Dave Jiang
2023-11-28 22:18 ` Verma, Vishal L
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).