| Message ID | 20200929025606.322543-1-yangx.jy@cn.fujitsu.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | controllers/cpuset_base_ops_testset.sh: Accept either 0 or -EINVAL when passing '0-' | expand |
Hi! > When we write '0-' to cpuset.cpus/cpuset.mems, new bitmap_parselist() > in kernel(e.g. newer than v4.2) treats it as an invalid value and old > one treats it as a valid '0': > ------------------------------------------- > on v5.8.0: > # echo 0- > cpuset.cpus > -bash: echo: write error: Invalid argument > # echo 0- > cpuset.mems > -bash: echo: write error: Invalid argument > > on v4.0.0: > # echo '0-' >cpuset.cpus > # cat cpuset.cpus > 0 > # echo '0-' >cpuset.mems > # cat cpuset.cpus > 0 > ------------------------------------------- > Note: commit d9282cb66353b changes the behavior. > > Drop the check of kernel version and accept either 0 or -EINVAL > because the change of behavior can be backported into old kernel. Why can't we just simply adjust the kernel check, it looks like the commit you mentioned was added to 4.3 so it should be fixed by changing the line to: if tst_kvcmp -lt "4.3 RHEL6:2.6.32"; then We want to at least keep the check for kernels newer than 4.3 just to make sure that kernel keeps rejecting the '0-' invalid value.
On 2020/9/30 18:47, Cyril Hrubis wrote: > Hi! >> When we write '0-' to cpuset.cpus/cpuset.mems, new bitmap_parselist() >> in kernel(e.g. newer than v4.2) treats it as an invalid value and old >> one treats it as a valid '0': >> ------------------------------------------- >> on v5.8.0: >> # echo 0-> cpuset.cpus >> -bash: echo: write error: Invalid argument >> # echo 0-> cpuset.mems >> -bash: echo: write error: Invalid argument >> >> on v4.0.0: >> # echo '0-'>cpuset.cpus >> # cat cpuset.cpus >> 0 >> # echo '0-'>cpuset.mems >> # cat cpuset.cpus >> 0 >> ------------------------------------------- >> Note: commit d9282cb66353b changes the behavior. >> >> Drop the check of kernel version and accept either 0 or -EINVAL >> because the change of behavior can be backported into old kernel. > Why can't we just simply adjust the kernel check, it looks like the > commit you mentioned was added to 4.3 so it should be fixed by changing > the line to: > > if tst_kvcmp -lt "4.3 RHEL6:2.6.32"; then > > We want to at least keep the check for kernels newer than 4.3 just to > make sure that kernel keeps rejecting the '0-' invalid value. Hi Cyril, Thanks for your reply. :-) 1) I still got '0' value instead of -EINVAL on Centos 6.10(2.6.32-754) so not sure why we have the wrong kernel check before. Perhaps, is there anothe older kernel commit to change the behavior as well? 2) I don't think that kernel check is enough because the change of behavior may be backported into old kernel. How about removing the combination directly as Richard suggested on #695. Best Regards, Xiao Yang
Hi! > Thanks for your reply. :-) > 1) I still got '0' value instead of -EINVAL on Centos 6.10(2.6.32-754) > so not sure why we have the wrong kernel check before. > Perhaps, is there anothe older kernel commit to change the behavior > as well? > 2) I don't think that kernel check is enough because the change of > behavior may be backported into old kernel. > > How about removing the combination directly as Richard suggested on #695. So what about disabling the test on older kernels completely and expect EINVAL on 4.3 and newer? That will still catch regression in mailine kernel but will not fail on older ones.
On 2020/9/30 20:40, Cyril Hrubis wrote: > Hi! >> Thanks for your reply. :-) >> 1) I still got '0' value instead of -EINVAL on Centos 6.10(2.6.32-754) >> so not sure why we have the wrong kernel check before. >> Perhaps, is there anothe older kernel commit to change the behavior >> as well? >> 2) I don't think that kernel check is enough because the change of >> behavior may be backported into old kernel. >> >> How about removing the combination directly as Richard suggested on #695. > So what about disabling the test on older kernels completely and expect > EINVAL on 4.3 and newer? > > That will still catch regression in mailine kernel but will not fail on > older ones. Hi Cyril, It is reasonable for me to foucs on new kernel(4.3 and newer). I will send v2 patch soon. :-) BTW: Sorry, I misread the meaning of 'tst_kvcmp -lt "3.0 RHEL6:2.6.32"'. It means that writing '0-' also get -EINVAL on very old kernel(3.0/RHEL6:2.6.32 and older), so I think there is another old commit to change the behavior as well. Thanks, Xiao Yang
diff --git a/testcases/kernel/controllers/cpuset/cpuset_base_ops_test/cpuset_base_ops_testset.sh b/testcases/kernel/controllers/cpuset/cpuset_base_ops_test/cpuset_base_ops_testset.sh index 67f3611d7..1fa40d2d9 100755 --- a/testcases/kernel/controllers/cpuset/cpuset_base_ops_test/cpuset_base_ops_testset.sh +++ b/testcases/kernel/controllers/cpuset/cpuset_base_ops_test/cpuset_base_ops_testset.sh @@ -45,7 +45,7 @@ base_op_write_and_test() { local write_file="$1" local write_string="$2" - local expect_string="$3" + local expect_strings="$3" local write_result= local ret=0 @@ -59,24 +59,27 @@ base_op_write_and_test() ret=$? write_result="$(cat "$write_file")" - case "$expect_string" in - EMPTY) - test -z "$write_result" -a $ret = 0 - ret=$? - ;; - WRITE_ERROR) - ret=$((!$ret)) - ;; - *) - test "$expect_string" = "$write_result" -a $ret = 0 - ret=$? - ;; - esac + for expect_string in $(echo $expect_strings | sed 's/|/ /'); do + case "$expect_string" in + EMPTY) + test -z "$write_result" -a $ret = 0 + ret=$? + ;; + WRITE_ERROR) + ret=$((!$ret)) + ;; + *) + test "$expect_string" = "$write_result" -a $ret = 0 + ret=$? + ;; + esac + [ $ret -eq 0 ] && break + done if [ $ret -eq 0 ]; then tst_resm TPASS "$cfile_name: Get the expected string" else - tst_resm TFAIL "$cfile_name: Test result - $write_result Expected string - \"$expect_string\"" + tst_resm TFAIL "$cfile_name: Test result - $write_result Expected string - \"$expect_strings\"" fi return $ret } @@ -114,6 +117,7 @@ test_cpus() ${cpus_all}$nr_cpus WRITE_ERROR 0,0 0 0-0 0 + 0- 0|WRITE_ERROR 0-$((nr_cpus-1)) 0-$((nr_cpus-1)) -1 WRITE_ERROR 0-$nr_cpus WRITE_ERROR @@ -127,12 +131,6 @@ test_cpus() base_op_test "$CPUSET/1/cpuset.cpus" "0,1-$((nr_cpus-2)),$((nr_cpus-1))" "0-$((nr_cpus-1))" base_op_test "$CPUSET/1/cpuset.cpus" "0,1-$((nr_cpus-2))," "0-$((nr_cpus-2))" fi - - if tst_kvcmp -lt "3.0 RHEL6:2.6.32"; then - base_op_test "$CPUSET/1/cpuset.cpus" "0-" "WRITE_ERROR" - else - base_op_test "$CPUSET/1/cpuset.cpus" "0-" "0" - fi } test_mems() @@ -149,6 +147,7 @@ test_mems() ${mems_all}$nr_mems WRITE_ERROR 0,0 0 0-0 0 + 0- 0|WRITE_ERROR 0-$((nr_mems-1)) 0-$((nr_mems-1)) -1 WRITE_ERROR 0-$nr_mems WRITE_ERROR @@ -162,12 +161,6 @@ test_mems() base_op_test "$CPUSET/1/cpuset.mems" "0,1-$((nr_mems-2)),$((nr_mems-1))" "0-$((nr_mems-1))" base_op_test "$CPUSET/1/cpuset.mems" "0,1-$((nr_mems-2))," "0-$((nr_mems-2))" fi - - if tst_kvcmp -lt "3.0 RHEL6:2.6.32"; then - base_op_test "$CPUSET/1/cpuset.mems" "0-" "WRITE_ERROR" - else - base_op_test "$CPUSET/1/cpuset.mems" "0-" "0" - fi } test_flags()
When we write '0-' to cpuset.cpus/cpuset.mems, new bitmap_parselist() in kernel(e.g. newer than v4.2) treats it as an invalid value and old one treats it as a valid '0': ------------------------------------------- on v5.8.0: # echo 0- > cpuset.cpus -bash: echo: write error: Invalid argument # echo 0- > cpuset.mems -bash: echo: write error: Invalid argument on v4.0.0: # echo '0-' >cpuset.cpus # cat cpuset.cpus 0 # echo '0-' >cpuset.mems # cat cpuset.cpus 0 ------------------------------------------- Note: commit d9282cb66353b changes the behavior. Drop the check of kernel version and accept either 0 or -EINVAL because the change of behavior can be backported into old kernel. Fixes: #695 Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com> --- Hi Cyril, Petr, Jan, Li Do you think it is better to remove '0-' subtest directly? .../cpuset_base_ops_testset.sh | 47 ++++++++----------- 1 file changed, 20 insertions(+), 27 deletions(-)