* [ndctl PATCH 1/3] test/cxl-region-sysfs.sh: covert size and resource to hex before test
@ 2023-11-23 2:30 Li Zhijian
2023-11-23 2:30 ` [ndctl PATCH 2/3] test/cxl-region-sysfs.sh: use operator '!=' to compare hexadecimal value Li Zhijian
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Li Zhijian @ 2023-11-23 2:30 UTC (permalink / raw
To: nvdimm; +Cc: linux-cxl, Li Zhijian
size and resource are both decimal
Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
---
test/cxl-region-sysfs.sh | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/test/cxl-region-sysfs.sh b/test/cxl-region-sysfs.sh
index 8636392..ded7aa1 100644
--- a/test/cxl-region-sysfs.sh
+++ b/test/cxl-region-sysfs.sh
@@ -123,6 +123,11 @@ readarray -t switch_decoders < <(echo $json | jq -r ".[].decoder")
[ ${#switch_decoders[@]} -ne $nr_switch_decoders ] && err \
"$LINENO: expected $nr_switch_decoders got ${#switch_decoders[@]} switch decoders"
+decimal_to_hex()
+{
+ printf "0x%x" $1
+}
+
for i in ${switch_decoders[@]}
do
decoder=$(echo $json | jq -r ".[] | select(.decoder == \"$i\")")
@@ -136,8 +141,8 @@ do
[ $ig -ne $((r_ig << depth)) ] && err \
"$LINENO: decoder: $i ig: $ig switch_ig: $((r_ig << depth))"
- res=$(echo $decoder | jq -r ".resource")
- sz=$(echo $decoder | jq -r ".size")
+ res=$(decimal_to_hex $(echo $decoder | jq -r ".resource"))
+ sz=$(decimal_to_hex $(echo $decoder | jq -r ".size"))
[ $sz -ne $region_size ] && err \
"$LINENO: decoder: $i sz: $sz region_size: $region_size"
[ $res -ne $region_base ] && err \
--
2.41.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* [ndctl PATCH 2/3] test/cxl-region-sysfs.sh: use operator '!=' to compare hexadecimal value 2023-11-23 2:30 [ndctl PATCH 1/3] test/cxl-region-sysfs.sh: covert size and resource to hex before test Li Zhijian @ 2023-11-23 2:30 ` Li Zhijian 2023-12-06 21:35 ` Dan Williams 2023-11-23 2:30 ` [ndctl PATCH 3/3] test/cxl-region-sysfs.sh: Fix cxl-region-sysfs.sh: line 107: [: missing `]' Li Zhijian 2023-12-06 21:31 ` [ndctl PATCH 1/3] test/cxl-region-sysfs.sh: covert size and resource to hex before test Dan Williams 2 siblings, 1 reply; 8+ messages in thread From: Li Zhijian @ 2023-11-23 2:30 UTC (permalink / raw To: nvdimm; +Cc: linux-cxl, Li Zhijian Fix errors: line 111: [: 0x80000000: integer expression expected line 112: [: 0x3ff110000000: integer expression expected line 141: [: 0x80000000: integer expression expected line 143: [: 0x3ff110000000: integer expression expected Signed-off-by: Li Zhijian <lizhijian@fujitsu.com> --- test/cxl-region-sysfs.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/cxl-region-sysfs.sh b/test/cxl-region-sysfs.sh index ded7aa1..89f21a3 100644 --- a/test/cxl-region-sysfs.sh +++ b/test/cxl-region-sysfs.sh @@ -108,8 +108,8 @@ do sz=$(cat /sys/bus/cxl/devices/$i/size) res=$(cat /sys/bus/cxl/devices/$i/start) - [ $sz -ne $region_size ] && err "$LINENO: decoder: $i sz: $sz region_size: $region_size" - [ $res -ne $region_base ] && err "$LINENO: decoder: $i base: $res region_base: $region_base" + [ "$sz" != "$region_size" ] && err "$LINENO: decoder: $i sz: $sz region_size: $region_size" + [ "$res" != "$region_base" ] && err "$LINENO: decoder: $i base: $res region_base: $region_base" done # validate all switch decoders have the correct settings @@ -143,9 +143,9 @@ do res=$(decimal_to_hex $(echo $decoder | jq -r ".resource")) sz=$(decimal_to_hex $(echo $decoder | jq -r ".size")) - [ $sz -ne $region_size ] && err \ + [ "$sz" != "$region_size" ] && err \ "$LINENO: decoder: $i sz: $sz region_size: $region_size" - [ $res -ne $region_base ] && err \ + [ "$res" != "$region_base" ] && err \ "$LINENO: decoder: $i base: $res region_base: $region_base" done -- 2.41.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* RE: [ndctl PATCH 2/3] test/cxl-region-sysfs.sh: use operator '!=' to compare hexadecimal value 2023-11-23 2:30 ` [ndctl PATCH 2/3] test/cxl-region-sysfs.sh: use operator '!=' to compare hexadecimal value Li Zhijian @ 2023-12-06 21:35 ` Dan Williams 2023-12-07 9:00 ` Zhijian Li (Fujitsu) 0 siblings, 1 reply; 8+ messages in thread From: Dan Williams @ 2023-12-06 21:35 UTC (permalink / raw To: Li Zhijian, nvdimm; +Cc: linux-cxl, Li Zhijian Li Zhijian wrote: > Fix errors: > line 111: [: 0x80000000: integer expression expected > line 112: [: 0x3ff110000000: integer expression expected > line 141: [: 0x80000000: integer expression expected > line 143: [: 0x3ff110000000: integer expression expected Similar commentary on how found and how someone knows that they need this patch, and maybe a note about which version of bash is upset about this given this problem has been present for a long time without issue. > > Signed-off-by: Li Zhijian <lizhijian@fujitsu.com> > --- > test/cxl-region-sysfs.sh | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/test/cxl-region-sysfs.sh b/test/cxl-region-sysfs.sh > index ded7aa1..89f21a3 100644 > --- a/test/cxl-region-sysfs.sh > +++ b/test/cxl-region-sysfs.sh > @@ -108,8 +108,8 @@ do > > sz=$(cat /sys/bus/cxl/devices/$i/size) > res=$(cat /sys/bus/cxl/devices/$i/start) > - [ $sz -ne $region_size ] && err "$LINENO: decoder: $i sz: $sz region_size: $region_size" > - [ $res -ne $region_base ] && err "$LINENO: decoder: $i base: $res region_base: $region_base" > + [ "$sz" != "$region_size" ] && err "$LINENO: decoder: $i sz: $sz region_size: $region_size" > + [ "$res" != "$region_base" ] && err "$LINENO: decoder: $i base: $res region_base: $region_base" maybe use ((sz != region_size)) to make it explicit that this is an arithmetic comparison? I would defer to Vishal's preference here. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [ndctl PATCH 2/3] test/cxl-region-sysfs.sh: use operator '!=' to compare hexadecimal value 2023-12-06 21:35 ` Dan Williams @ 2023-12-07 9:00 ` Zhijian Li (Fujitsu) 0 siblings, 0 replies; 8+ messages in thread From: Zhijian Li (Fujitsu) @ 2023-12-07 9:00 UTC (permalink / raw To: Dan Williams, nvdimm@lists.linux.dev; +Cc: linux-cxl@vger.kernel.org On 07/12/2023 05:35, Dan Williams wrote: > Li Zhijian wrote: >> Fix errors: >> line 111: [: 0x80000000: integer expression expected >> line 112: [: 0x3ff110000000: integer expression expected >> line 141: [: 0x80000000: integer expression expected >> line 143: [: 0x3ff110000000: integer expression expected > > Similar commentary on how found and how someone knows that they need > this patch, and maybe a note about which version of bash is upset about > this given this problem has been present for a long time without issue. I think it impacted all version of BASH (tested on bash 4.1, 4.2, 5.0, 5.1) > >> >> Signed-off-by: Li Zhijian <lizhijian@fujitsu.com> >> --- >> test/cxl-region-sysfs.sh | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/test/cxl-region-sysfs.sh b/test/cxl-region-sysfs.sh >> index ded7aa1..89f21a3 100644 >> --- a/test/cxl-region-sysfs.sh >> +++ b/test/cxl-region-sysfs.sh >> @@ -108,8 +108,8 @@ do >> >> sz=$(cat /sys/bus/cxl/devices/$i/size) >> res=$(cat /sys/bus/cxl/devices/$i/start) >> - [ $sz -ne $region_size ] && err "$LINENO: decoder: $i sz: $sz region_size: $region_size" >> - [ $res -ne $region_base ] && err "$LINENO: decoder: $i base: $res region_base: $region_base" >> + [ "$sz" != "$region_size" ] && err "$LINENO: decoder: $i sz: $sz region_size: $region_size" >> + [ "$res" != "$region_base" ] && err "$LINENO: decoder: $i base: $res region_base: $region_base" > > maybe use ((sz != region_size)) to make it explicit that this is an > arithmetic comparison? I would defer to Vishal's preference here. Per bash man page, we can also use [[ ]] instead of [ ], so that we are able to get rid of patch1 arg1 OP arg2 OP is one of -eq, -ne, -lt, -le, -gt, or -ge. These arithmetic binary operators return true if arg1 is equal to, not equal to, less than, less than or equal to, greater than, or greater than or equal to arg2, respectively. Arg1 and arg2 may be positive or negative integers. When used with the [[ command, Arg1 and Arg2 are evaluated as arithmetic expressions (see ARITHMETIC EVALUATION above). ^ permalink raw reply [flat|nested] 8+ messages in thread
* [ndctl PATCH 3/3] test/cxl-region-sysfs.sh: Fix cxl-region-sysfs.sh: line 107: [: missing `]' 2023-11-23 2:30 [ndctl PATCH 1/3] test/cxl-region-sysfs.sh: covert size and resource to hex before test Li Zhijian 2023-11-23 2:30 ` [ndctl PATCH 2/3] test/cxl-region-sysfs.sh: use operator '!=' to compare hexadecimal value Li Zhijian @ 2023-11-23 2:30 ` Li Zhijian 2023-12-06 21:37 ` Dan Williams 2023-12-06 21:31 ` [ndctl PATCH 1/3] test/cxl-region-sysfs.sh: covert size and resource to hex before test Dan Williams 2 siblings, 1 reply; 8+ messages in thread From: Li Zhijian @ 2023-11-23 2:30 UTC (permalink / raw To: nvdimm; +Cc: linux-cxl, Li Zhijian Signed-off-by: Li Zhijian <lizhijian@fujitsu.com> --- test/cxl-region-sysfs.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/cxl-region-sysfs.sh b/test/cxl-region-sysfs.sh index 89f21a3..3878351 100644 --- a/test/cxl-region-sysfs.sh +++ b/test/cxl-region-sysfs.sh @@ -104,7 +104,7 @@ do iw=$(cat /sys/bus/cxl/devices/$i/interleave_ways) ig=$(cat /sys/bus/cxl/devices/$i/interleave_granularity) [ $iw -ne $nr_targets ] && err "$LINENO: decoder: $i iw: $iw targets: $nr_targets" - [ $ig -ne $r_ig] && err "$LINENO: decoder: $i ig: $ig root ig: $r_ig" + [ $ig -ne $r_ig ] && err "$LINENO: decoder: $i ig: $ig root ig: $r_ig" sz=$(cat /sys/bus/cxl/devices/$i/size) res=$(cat /sys/bus/cxl/devices/$i/start) -- 2.41.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* RE: [ndctl PATCH 3/3] test/cxl-region-sysfs.sh: Fix cxl-region-sysfs.sh: line 107: [: missing `]' 2023-11-23 2:30 ` [ndctl PATCH 3/3] test/cxl-region-sysfs.sh: Fix cxl-region-sysfs.sh: line 107: [: missing `]' Li Zhijian @ 2023-12-06 21:37 ` Dan Williams 2023-12-07 9:07 ` Zhijian Li (Fujitsu) 0 siblings, 1 reply; 8+ messages in thread From: Dan Williams @ 2023-12-06 21:37 UTC (permalink / raw To: Li Zhijian, nvdimm; +Cc: linux-cxl, Li Zhijian Li Zhijian wrote: > Signed-off-by: Li Zhijian <lizhijian@fujitsu.com> Please no patches with empty changelogs. Commentary on the impact of the change is always welcome. Otherwise change looks good to me, and I wonder why this error is only triggering now? Acked-by: Dan Williams <dan.j.williams@intel.com> > --- > test/cxl-region-sysfs.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/test/cxl-region-sysfs.sh b/test/cxl-region-sysfs.sh > index 89f21a3..3878351 100644 > --- a/test/cxl-region-sysfs.sh > +++ b/test/cxl-region-sysfs.sh > @@ -104,7 +104,7 @@ do > iw=$(cat /sys/bus/cxl/devices/$i/interleave_ways) > ig=$(cat /sys/bus/cxl/devices/$i/interleave_granularity) > [ $iw -ne $nr_targets ] && err "$LINENO: decoder: $i iw: $iw targets: $nr_targets" > - [ $ig -ne $r_ig] && err "$LINENO: decoder: $i ig: $ig root ig: $r_ig" > + [ $ig -ne $r_ig ] && err "$LINENO: decoder: $i ig: $ig root ig: $r_ig" > > sz=$(cat /sys/bus/cxl/devices/$i/size) > res=$(cat /sys/bus/cxl/devices/$i/start) > -- > 2.41.0 > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [ndctl PATCH 3/3] test/cxl-region-sysfs.sh: Fix cxl-region-sysfs.sh: line 107: [: missing `]' 2023-12-06 21:37 ` Dan Williams @ 2023-12-07 9:07 ` Zhijian Li (Fujitsu) 0 siblings, 0 replies; 8+ messages in thread From: Zhijian Li (Fujitsu) @ 2023-12-07 9:07 UTC (permalink / raw To: Dan Williams, nvdimm@lists.linux.dev; +Cc: linux-cxl@vger.kernel.org On 07/12/2023 05:37, Dan Williams wrote: > Li Zhijian wrote: >> Signed-off-by: Li Zhijian <lizhijian@fujitsu.com> > > Please no patches with empty changelogs. Commentary on the impact of the > change is always welcome. > > Otherwise change looks good to me, and I wonder why this error is only > triggering now? I have to say current condition checking 1) easily hides *BUG* 1) [ a -ne b ] && echo NG Instead, 2) as below are more reliable. 2) [ a -eq b ] || echo NG > > Acked-by: Dan Williams <dan.j.williams@intel.com> > >> --- >> test/cxl-region-sysfs.sh | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/test/cxl-region-sysfs.sh b/test/cxl-region-sysfs.sh >> index 89f21a3..3878351 100644 >> --- a/test/cxl-region-sysfs.sh >> +++ b/test/cxl-region-sysfs.sh >> @@ -104,7 +104,7 @@ do >> iw=$(cat /sys/bus/cxl/devices/$i/interleave_ways) >> ig=$(cat /sys/bus/cxl/devices/$i/interleave_granularity) >> [ $iw -ne $nr_targets ] && err "$LINENO: decoder: $i iw: $iw targets: $nr_targets" >> - [ $ig -ne $r_ig] && err "$LINENO: decoder: $i ig: $ig root ig: $r_ig" >> + [ $ig -ne $r_ig ] && err "$LINENO: decoder: $i ig: $ig root ig: $r_ig" >> >> sz=$(cat /sys/bus/cxl/devices/$i/size) >> res=$(cat /sys/bus/cxl/devices/$i/start) >> -- >> 2.41.0 >> >> > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [ndctl PATCH 1/3] test/cxl-region-sysfs.sh: covert size and resource to hex before test 2023-11-23 2:30 [ndctl PATCH 1/3] test/cxl-region-sysfs.sh: covert size and resource to hex before test Li Zhijian 2023-11-23 2:30 ` [ndctl PATCH 2/3] test/cxl-region-sysfs.sh: use operator '!=' to compare hexadecimal value Li Zhijian 2023-11-23 2:30 ` [ndctl PATCH 3/3] test/cxl-region-sysfs.sh: Fix cxl-region-sysfs.sh: line 107: [: missing `]' Li Zhijian @ 2023-12-06 21:31 ` Dan Williams 2 siblings, 0 replies; 8+ messages in thread From: Dan Williams @ 2023-12-06 21:31 UTC (permalink / raw To: Li Zhijian, nvdimm; +Cc: linux-cxl, Li Zhijian I would summarize this as "Fix hex vs decimal confusion" Li Zhijian wrote: > size and resource are both decimal Please always describe the "why" in the changelog including the motivation for the change and the effect of not applying the patch. I.e. how would someone know that they are hitting the problem that this patch fixes. Sample output usually helps here. > Signed-off-by: Li Zhijian <lizhijian@fujitsu.com> > --- > test/cxl-region-sysfs.sh | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/test/cxl-region-sysfs.sh b/test/cxl-region-sysfs.sh > index 8636392..ded7aa1 100644 > --- a/test/cxl-region-sysfs.sh > +++ b/test/cxl-region-sysfs.sh > @@ -123,6 +123,11 @@ readarray -t switch_decoders < <(echo $json | jq -r ".[].decoder") > [ ${#switch_decoders[@]} -ne $nr_switch_decoders ] && err \ > "$LINENO: expected $nr_switch_decoders got ${#switch_decoders[@]} switch decoders" > > +decimal_to_hex() > +{ > + printf "0x%x" $1 > +} > + > for i in ${switch_decoders[@]} > do > decoder=$(echo $json | jq -r ".[] | select(.decoder == \"$i\")") > @@ -136,8 +141,8 @@ do > [ $ig -ne $((r_ig << depth)) ] && err \ > "$LINENO: decoder: $i ig: $ig switch_ig: $((r_ig << depth))" > > - res=$(echo $decoder | jq -r ".resource") > - sz=$(echo $decoder | jq -r ".size") > + res=$(decimal_to_hex $(echo $decoder | jq -r ".resource")) > + sz=$(decimal_to_hex $(echo $decoder | jq -r ".size")) This feels like overkill, I think bash arithmentic operations can handle hex conversion and decimal comparison... > [ $sz -ne $region_size ] && err \ > "$LINENO: decoder: $i sz: $sz region_size: $region_size" > [ $res -ne $region_base ] && err \ ...i.e. does this solve the issue? @@ -138,9 +138,9 @@ do res=$(echo $decoder | jq -r ".resource") sz=$(echo $decoder | jq -r ".size") - [ $sz -ne $region_size ] && err \ + (( sz != $region_size )) && err \ "$LINENO: decoder: $i sz: $sz region_size: $region_size" - [ $res -ne $region_base ] && err \ + (( $res != $region_base )) && err \ "$LINENO: decoder: $i base: $res region_base: $region_base" done ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-12-07 9:08 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-11-23 2:30 [ndctl PATCH 1/3] test/cxl-region-sysfs.sh: covert size and resource to hex before test Li Zhijian 2023-11-23 2:30 ` [ndctl PATCH 2/3] test/cxl-region-sysfs.sh: use operator '!=' to compare hexadecimal value Li Zhijian 2023-12-06 21:35 ` Dan Williams 2023-12-07 9:00 ` Zhijian Li (Fujitsu) 2023-11-23 2:30 ` [ndctl PATCH 3/3] test/cxl-region-sysfs.sh: Fix cxl-region-sysfs.sh: line 107: [: missing `]' Li Zhijian 2023-12-06 21:37 ` Dan Williams 2023-12-07 9:07 ` Zhijian Li (Fujitsu) 2023-12-06 21:31 ` [ndctl PATCH 1/3] test/cxl-region-sysfs.sh: covert size and resource to hex before test Dan Williams
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).