| Message ID | 20231122025753.1209527-1-alison.schofield@intel.com |
|---|---|
| State | Accepted |
| Commit | 8c1f501d8f572400da97ef0fa80839a1b3df94b8 |
| Headers | show |
| Series | [ndctl] cxl/test: replace a bad root decoder usage in cxl-xor-region.sh | expand |
On Tue, Nov 21, 2023 at 06:57:53PM -0800, alison.schofield@intel.com wrote: > From: Alison Schofield <alison.schofield@intel.com> > > The 4-way XOR region as defined in this test uses a root decoder that > is created using an improperly defined CFMWS. The problem with the > CFMWS is that Host Bridges repeat in the target list like this: > { 0, 1, 0, 1 }. > > Stop using that root decoder and create a 4-way XOR region using an > x2 root decoder that supports XOR arithmetic. > > The test passes prior to this patch but there is an interleave check [1] > introduced in the CXL region driver that will expose the bad interleave > this test creates via dev_dbg() messages like this: > > [ ] cxl_core:cxl_region_attach:1808: cxl decoder17.0: Test cxl_calc_interleave_pos(): fail test_pos:4 cxled->pos:2 > [ ] cxl_core:cxl_region_attach:1808: cxl decoder18.0: Test cxl_calc_interleave_pos(): fail test_pos:5 cxled->pos:3 > > Note that the CFMWS's are defined in the kernel cxl-test module, so a > kernel patch removing the bad CFMWS will also need to be merged, but > that cleanup can follow this patch. Also note that the bad CFMWS is not > used in the default cxl-test environment. It is only visible when the > cxl-test module is loaded using the param interleave_arithmetic=1. It is > a special config that provides the XOR math CFMWS's for this test. > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a3e00c964fb943934af916f48f0dd43b5110c866 > Fixes: 05486f8bf154 ("cxl/test: add cxl_xor_region test") > Signed-off-by: Alison Schofield <alison.schofield@intel.com> > --- > > Vishal - I'm hoping you will merge this in ndctl v79 even though the > exposure with the kernel doesn't happen until kernel 6.7. This way > users of cxl-test are not learning to ignore the interleave calc > warnings. > > Also, hopefully I have not introduced any new shell issues, but > I know this unit test has existing warnings. Can we do a shell > cleanup in a follow-on patchset across the CXL tests? > (and not last minute for your ndctl release) > > > test/cxl-xor-region.sh | 14 +++++--------- > 1 file changed, 5 insertions(+), 9 deletions(-) > snip >
On Tue, 2023-11-21 at 18:57 -0800, alison.schofield@intel.com wrote: > From: Alison Schofield <alison.schofield@intel.com> > > The 4-way XOR region as defined in this test uses a root decoder that > is created using an improperly defined CFMWS. The problem with the > CFMWS is that Host Bridges repeat in the target list like this: > { 0, 1, 0, 1 }. > > Stop using that root decoder and create a 4-way XOR region using an > x2 root decoder that supports XOR arithmetic. > > The test passes prior to this patch but there is an interleave check [1] > introduced in the CXL region driver that will expose the bad interleave > this test creates via dev_dbg() messages like this: > > [ ] cxl_core:cxl_region_attach:1808: cxl decoder17.0: Test cxl_calc_interleave_pos(): fail test_pos:4 cxled->pos:2 > [ ] cxl_core:cxl_region_attach:1808: cxl decoder18.0: Test cxl_calc_interleave_pos(): fail test_pos:5 cxled->pos:3 > > Note that the CFMWS's are defined in the kernel cxl-test module, so a > kernel patch removing the bad CFMWS will also need to be merged, but > that cleanup can follow this patch. Also note that the bad CFMWS is not > used in the default cxl-test environment. It is only visible when the > cxl-test module is loaded using the param interleave_arithmetic=1. It is > a special config that provides the XOR math CFMWS's for this test. > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a3e00c964fb943934af916f48f0dd43b5110c866 > > Signed-off-by: Alison Schofield <alison.schofield@intel.com> > --- > > Vishal - I'm hoping you will merge this in ndctl v79 even though the > exposure with the kernel doesn't happen until kernel 6.7. This way > users of cxl-test are not learning to ignore the interleave calc > warnings. Yep makes sense - I'm also thinking that since we're at -rc2, v79 can just be the v6.7 equivalent release (and I'll include the sanitize patches too since that's the only other pending v6.7 specific thing. > > Also, hopefully I have not introduced any new shell issues, but > I know this unit test has existing warnings. Can we do a shell > cleanup in a follow-on patchset across the CXL tests? > (and not last minute for your ndctl release) Agreed, there are a few shellcheck complaints that have snuck in over time, fixing these as a separate cleanup is definitely the way to go. > > test/cxl-xor-region.sh | 14 +++++--------- > 1 file changed, 5 insertions(+), 9 deletions(-) >
diff --git a/test/cxl-xor-region.sh b/test/cxl-xor-region.sh index 1962327bd00a..117e7a4bba61 100644 --- a/test/cxl-xor-region.sh +++ b/test/cxl-xor-region.sh @@ -68,10 +68,10 @@ setup_x2() setup_x4() { - # find x4 decoder + # find an x2 decoder decoder=$($CXL list -b cxl_test -D -d root | jq -r ".[] | select(.pmem_capable == true) | - select(.nr_targets == 4) | + select(.nr_targets == 2) | .decoder") # Find a memdev for each host-bridge interleave position @@ -79,14 +79,10 @@ setup_x4() .targets | .[] | select(.position == 0) | .target") port_dev1=$($CXL list -T -d $decoder | jq -r ".[] | .targets | .[] | select(.position == 1) | .target") - port_dev2=$($CXL list -T -d $decoder | jq -r ".[] | - .targets | .[] | select(.position == 2) | .target") - port_dev3=$($CXL list -T -d $decoder | jq -r ".[] | - .targets | .[] | select(.position == 3) | .target") mem0=$($CXL list -M -p $port_dev0 | jq -r ".[0].memdev") - mem1=$($CXL list -M -p $port_dev1 | jq -r ".[1].memdev") - mem2=$($CXL list -M -p $port_dev2 | jq -r ".[2].memdev") - mem3=$($CXL list -M -p $port_dev3 | jq -r ".[3].memdev") + mem1=$($CXL list -M -p $port_dev1 | jq -r ".[0].memdev") + mem2=$($CXL list -M -p $port_dev0 | jq -r ".[1].memdev") + mem3=$($CXL list -M -p $port_dev1 | jq -r ".[1].memdev") memdevs="$mem0 $mem1 $mem2 $mem3" }