[LTP] [PATCH] controllers/cpuset_base_ops_testset.sh: Accept either 0 or -EINVAL when passing '0-'

[LTP] [PATCH] controllers/cpuset_base_ops_testset.sh: Accept either 0 or -EINVAL when passing '0-'

[Xiao Yang]

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(-)

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()
-- 
2.25.1

[LTP] [PATCH] controllers/cpuset_base_ops_testset.sh: Accept either 0 or -EINVAL when passing '0-'

[Cyril Hrubis]

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.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH] controllers/cpuset_base_ops_testset.sh: Accept either 0 or -EINVAL when passing '0-'

[Xiao Yang]

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


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20200930/d1be3b57/attachment-0001.htm>

[LTP] [PATCH] controllers/cpuset_base_ops_testset.sh: Accept either 0 or -EINVAL when passing '0-'

[Cyril Hrubis]

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.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH] controllers/cpuset_base_ops_testset.sh: Accept either 0 or -EINVAL when passing '0-'

[Xiao Yang]

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