[LTP] [PATCH] cpuset_inherit: Use the original mem value instead of N_NODES

[LTP] [PATCH] cpuset_inherit: Use the original mem value instead of N_NODES

[Yang Xu]

Since ltp commit cf33086a1ca, we add cgroup.clone_children switch for
cpuset.cpus and mems, we used the original memory value to set in cpuset_inherit case.

After ltp commit 6872ad15a, we improve the node number calculation for N_NODES,
so it can calculate for N_NODES obtained from the file contains only "0,8".

But it doesn't think about this patch will affect mem_string value, so this
cpuset_inherit case will fail on 4 numa nodes pc, as below:

cpuset_inherit 1 TPASS: cpus: Inherited information is right!
cpuset_inherit 3 TPASS: cpus: Inherited information is right!
cpuset_inherit 5 TPASS: cpus: Inherited information is right!
cpuset_inherit 7 TPASS: cpus: Inherited information is right!
cpuset_inherit 9 TPASS: cpus: Inherited information is right!
cpuset_inherit 11 TPASS: cpus: Inherited information is right!
cpuset_inherit 13 TPASS: mems: Inherited information is right!
cpuset_inherit 15 TPASS: mems: Inherited information is right!
cpuset_inherit 17 TPASS: mems: Inherited information is right!
cpuset_inherit 19 TPASS: mems: Inherited information is right!
cpuset_inherit 21 TPASS: mems: Inherited information is right!
cpuset_inherit 23 TFAIL: mems: Test result - 0-3 Expected string - "4"

Fix this by using original mem value.

Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
 testcases/kernel/controllers/cpuset/cpuset_funcs.sh        | 7 +++----
 .../cpuset/cpuset_inherit_test/cpuset_inherit_testset.sh   | 6 ++----
 2 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/testcases/kernel/controllers/cpuset/cpuset_funcs.sh b/testcases/kernel/controllers/cpuset/cpuset_funcs.sh
index f4365af2c..b469140ca 100755
--- a/testcases/kernel/controllers/cpuset/cpuset_funcs.sh
+++ b/testcases/kernel/controllers/cpuset/cpuset_funcs.sh
@@ -28,10 +28,11 @@
 
 NR_CPUS=`tst_ncpus`
 if [ -f "/sys/devices/system/node/has_high_memory" ]; then
-	N_NODES="`cat /sys/devices/system/node/has_high_memory | tr ',' ' '`"
+	mem_string="`cat /sys/devices/system/node/has_high_memory`"
 else
-	N_NODES="`cat /sys/devices/system/node/has_normal_memory | tr ',' ' '`"
+	mem_string="`cat /sys/devices/system/node/has_normal_memory`"
 fi
+N_NODES="`echo $mem_string | tr ',' ' '`"
 count=0
 for item in $N_NODES; do
 	delta=1
@@ -42,8 +43,6 @@ for item in $N_NODES; do
 done
 N_NODES=$count
 
-mem_string="$N_NODES"
-
 CPUSET="/dev/cpuset"
 CPUSET_TMP="/tmp/cpuset_tmp"
 CLONE_CHILDREN="/dev/cpuset/cgroup.clone_children"
diff --git a/testcases/kernel/controllers/cpuset/cpuset_inherit_test/cpuset_inherit_testset.sh b/testcases/kernel/controllers/cpuset/cpuset_inherit_test/cpuset_inherit_testset.sh
index 73eed2cb9..27ff19532 100755
--- a/testcases/kernel/controllers/cpuset/cpuset_inherit_test/cpuset_inherit_testset.sh
+++ b/testcases/kernel/controllers/cpuset/cpuset_inherit_test/cpuset_inherit_testset.sh
@@ -31,10 +31,8 @@ export TST_COUNT=1
 check 1 1
 
 nr_cpus=$NR_CPUS
-nr_mems=$N_NODES
 
 cpus_all="$(seq -s, 0 $((nr_cpus-1)))"
-mems_all="$(seq -s, 0 $((nr_mems-1)))"
 
 exit_status=0
 
@@ -134,10 +132,10 @@ test_mems()
 	done <<- EOF
 		0	NULL					EMPTY
 		0	0					EMPTY
-		0	$mems_all				EMPTY
+		0	$mem_string				EMPTY
 		1	NULL					EMPTY
 		1	0					0
-		1	$mems_all				$mem_string
+		1	$mems_string				$mem_string
 	EOF
 	# while read mems result
 }
-- 
2.23.0

[LTP] [PATCH] cpuset_inherit: Use the original mem value instead of N_NODES

[Cyril Hrubis]

Hi!
> Since ltp commit cf33086a1ca, we add cgroup.clone_children switch for
> cpuset.cpus and mems, we used the original memory value to set in cpuset_inherit case.
> 
> After ltp commit 6872ad15a, we improve the node number calculation for N_NODES,
> so it can calculate for N_NODES obtained from the file contains only "0,8".
> 
> But it doesn't think about this patch will affect mem_string value, so this
> cpuset_inherit case will fail on 4 numa nodes pc, as below:
> 
> cpuset_inherit 1 TPASS: cpus: Inherited information is right!
> cpuset_inherit 3 TPASS: cpus: Inherited information is right!
> cpuset_inherit 5 TPASS: cpus: Inherited information is right!
> cpuset_inherit 7 TPASS: cpus: Inherited information is right!
> cpuset_inherit 9 TPASS: cpus: Inherited information is right!
> cpuset_inherit 11 TPASS: cpus: Inherited information is right!
> cpuset_inherit 13 TPASS: mems: Inherited information is right!
> cpuset_inherit 15 TPASS: mems: Inherited information is right!
> cpuset_inherit 17 TPASS: mems: Inherited information is right!
> cpuset_inherit 19 TPASS: mems: Inherited information is right!
> cpuset_inherit 21 TPASS: mems: Inherited information is right!
> cpuset_inherit 23 TFAIL: mems: Test result - 0-3 Expected string - "4"
> 
> Fix this by using original mem value.
> 
> Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
> ---
>  testcases/kernel/controllers/cpuset/cpuset_funcs.sh        | 7 +++----
>  .../cpuset/cpuset_inherit_test/cpuset_inherit_testset.sh   | 6 ++----
>  2 files changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/testcases/kernel/controllers/cpuset/cpuset_funcs.sh b/testcases/kernel/controllers/cpuset/cpuset_funcs.sh
> index f4365af2c..b469140ca 100755
> --- a/testcases/kernel/controllers/cpuset/cpuset_funcs.sh
> +++ b/testcases/kernel/controllers/cpuset/cpuset_funcs.sh
> @@ -28,10 +28,11 @@
>  
>  NR_CPUS=`tst_ncpus`
>  if [ -f "/sys/devices/system/node/has_high_memory" ]; then
> -	N_NODES="`cat /sys/devices/system/node/has_high_memory | tr ',' ' '`"
> +	mem_string="`cat /sys/devices/system/node/has_high_memory`"
>  else
> -	N_NODES="`cat /sys/devices/system/node/has_normal_memory | tr ',' ' '`"
> +	mem_string="`cat /sys/devices/system/node/has_normal_memory`"
>  fi
> +N_NODES="`echo $mem_string | tr ',' ' '`"
>  count=0
>  for item in $N_NODES; do
>  	delta=1
> @@ -42,8 +43,6 @@ for item in $N_NODES; do
>  done
>  N_NODES=$count
>  
> -mem_string="$N_NODES"
> -
>  CPUSET="/dev/cpuset"
>  CPUSET_TMP="/tmp/cpuset_tmp"
>  CLONE_CHILDREN="/dev/cpuset/cgroup.clone_children"
>
> diff --git a/testcases/kernel/controllers/cpuset/cpuset_inherit_test/cpuset_inherit_testset.sh b/testcases/kernel/controllers/cpuset/cpuset_inherit_test/cpuset_inherit_testset.sh
> index 73eed2cb9..27ff19532 100755
> --- a/testcases/kernel/controllers/cpuset/cpuset_inherit_test/cpuset_inherit_testset.sh
> +++ b/testcases/kernel/controllers/cpuset/cpuset_inherit_test/cpuset_inherit_testset.sh
> @@ -31,10 +31,8 @@ export TST_COUNT=1
>  check 1 1
>  
>  nr_cpus=$NR_CPUS
> -nr_mems=$N_NODES
>  
>  cpus_all="$(seq -s, 0 $((nr_cpus-1)))"
> -mems_all="$(seq -s, 0 $((nr_mems-1)))"
>  
>  exit_status=0
>  
> @@ -134,10 +132,10 @@ test_mems()
>  	done <<- EOF
>  		0	NULL					EMPTY
>  		0	0					EMPTY
> -		0	$mems_all				EMPTY
> +		0	$mem_string				EMPTY
>  		1	NULL					EMPTY
>  		1	0					0
> -		1	$mems_all				$mem_string
> +		1	$mems_string				$mem_string
>  	EOF
>  	# while read mems result
>  }

I guess that it looks okay to me. I guess that we can commit this before
the release, so acked.

But don't we have the same problem for cpus_all? If we, for instance,
have a machine where cpus are not numbered continously we will fail as
well, right?

I guess that a proper fix would be to rewrite the tests to parse the
strings into a bitflag arrays and compare these arrays instead of the
string comparsion and hacks that keeps up pilling up.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH] cpuset_inherit: Use the original mem value instead of N_NODES

[Yang Xu]

Hi Cyril
> Hi!
>> Since ltp commit cf33086a1ca, we add cgroup.clone_children switch for
>> cpuset.cpus and mems, we used the original memory value to set in cpuset_inherit case.
>>
>> After ltp commit 6872ad15a, we improve the node number calculation for N_NODES,
>> so it can calculate for N_NODES obtained from the file contains only "0,8".
>>
>> But it doesn't think about this patch will affect mem_string value, so this
>> cpuset_inherit case will fail on 4 numa nodes pc, as below:
>>
>> cpuset_inherit 1 TPASS: cpus: Inherited information is right!
>> cpuset_inherit 3 TPASS: cpus: Inherited information is right!
>> cpuset_inherit 5 TPASS: cpus: Inherited information is right!
>> cpuset_inherit 7 TPASS: cpus: Inherited information is right!
>> cpuset_inherit 9 TPASS: cpus: Inherited information is right!
>> cpuset_inherit 11 TPASS: cpus: Inherited information is right!
>> cpuset_inherit 13 TPASS: mems: Inherited information is right!
>> cpuset_inherit 15 TPASS: mems: Inherited information is right!
>> cpuset_inherit 17 TPASS: mems: Inherited information is right!
>> cpuset_inherit 19 TPASS: mems: Inherited information is right!
>> cpuset_inherit 21 TPASS: mems: Inherited information is right!
>> cpuset_inherit 23 TFAIL: mems: Test result - 0-3 Expected string - "4"
>>
>> Fix this by using original mem value.
>>
>> Signed-off-by: Yang Xu<xuyang2018.jy@cn.fujitsu.com>
>> ---
>>   testcases/kernel/controllers/cpuset/cpuset_funcs.sh        | 7 +++----
>>   .../cpuset/cpuset_inherit_test/cpuset_inherit_testset.sh   | 6 ++----
>>   2 files changed, 5 insertions(+), 8 deletions(-)
>>
>> diff --git a/testcases/kernel/controllers/cpuset/cpuset_funcs.sh b/testcases/kernel/controllers/cpuset/cpuset_funcs.sh
>> index f4365af2c..b469140ca 100755
>> --- a/testcases/kernel/controllers/cpuset/cpuset_funcs.sh
>> +++ b/testcases/kernel/controllers/cpuset/cpuset_funcs.sh
>> @@ -28,10 +28,11 @@
>>
>>   NR_CPUS=`tst_ncpus`
>>   if [ -f "/sys/devices/system/node/has_high_memory" ]; then
>> -	N_NODES="`cat /sys/devices/system/node/has_high_memory | tr ',' ' '`"
>> +	mem_string="`cat /sys/devices/system/node/has_high_memory`"
>>   else
>> -	N_NODES="`cat /sys/devices/system/node/has_normal_memory | tr ',' ' '`"
>> +	mem_string="`cat /sys/devices/system/node/has_normal_memory`"
>>   fi
>> +N_NODES="`echo $mem_string | tr ',' ' '`"
>>   count=0
>>   for item in $N_NODES; do
>>   	delta=1
>> @@ -42,8 +43,6 @@ for item in $N_NODES; do
>>   done
>>   N_NODES=$count
>>
>> -mem_string="$N_NODES"
>> -
>>   CPUSET="/dev/cpuset"
>>   CPUSET_TMP="/tmp/cpuset_tmp"
>>   CLONE_CHILDREN="/dev/cpuset/cgroup.clone_children"
>>
>> diff --git a/testcases/kernel/controllers/cpuset/cpuset_inherit_test/cpuset_inherit_testset.sh b/testcases/kernel/controllers/cpuset/cpuset_inherit_test/cpuset_inherit_testset.sh
>> index 73eed2cb9..27ff19532 100755
>> --- a/testcases/kernel/controllers/cpuset/cpuset_inherit_test/cpuset_inherit_testset.sh
>> +++ b/testcases/kernel/controllers/cpuset/cpuset_inherit_test/cpuset_inherit_testset.sh
>> @@ -31,10 +31,8 @@ export TST_COUNT=1
>>   check 1 1
>>
>>   nr_cpus=$NR_CPUS
>> -nr_mems=$N_NODES
>>
>>   cpus_all="$(seq -s, 0 $((nr_cpus-1)))"
>> -mems_all="$(seq -s, 0 $((nr_mems-1)))"
>>
>>   exit_status=0
>>
>> @@ -134,10 +132,10 @@ test_mems()
>>   	done<<- EOF
>>   		0	NULL					EMPTY
>>   		0	0					EMPTY
>> -		0	$mems_all				EMPTY
>> +		0	$mem_string				EMPTY
>>   		1	NULL					EMPTY
>>   		1	0					0
>> -		1	$mems_all				$mem_string
>> +		1	$mems_string				$mem_string
here has a typo, mems_string->mem_string
>>   	EOF
>>   	# while read mems result
>>   }
>
> I guess that it looks okay to me. I guess that we can commit this before
> the release, so acked.
>
> But don't we have the same problem for cpus_all? If we, for instance,
> have a machine where cpus are not numbered continously we will fail as
> well, right?
Yes, it has the same problem for cpus_all.
>
> I guess that a proper fix would be to rewrite the tests to parse the
> strings into a bitflag arrays and compare these arrays instead of the
> string comparsion and hacks that keeps up pilling up.
Will do it in v2.
>

[LTP] [PATCH] cpuset_inherit: Use the original mem value instead of N_NODES

[Yang Xu]

Hi Cyril

When I look these cpuset cases(cpuset_base_ops_test, 
cpuset_hierarchy_test, cpuset_inherit_test...), these cases seems all 
not consider the situation(cpus/memory are not numbered continously). If 
we want to modify them to be situable for not numbered continously, it 
will be complexd(especially cpuset_base_ops_test). AFAIK, I rarely see 
not numbered continously for memory node. IMO, we just check whether 
memory/cpu numbered continously, if not, we just report TCONF and remind 
user to change their system to meet environment, so their system can be 
fully tested.

For cpu, maybe we can use the following script to detect

cpu_string="`cat /sys/devices/system/cpu/online`"
offline_string="`cat /sys/devices/system/cpu/online`"
NR_CPUS=`tst_ncpus`
ALL_CPUS=`tst_ncpus_conf`
if [ $NR_CPUS -eq $ALL_CPUS ]; then
        tst_resm TINFO "All($ALL_CPUS) logical cpus on your environment"
else
       tst_brkm TCONF "Not all logical cpus on, 
online($cpu_string),offline($offline_string)"
fi

I wonder if it's worth changing the stable cpuset/memory cases for these 
rared situation(memory/cpu are not numbered continously).

What do you think about it?

ps:I have modify cpuset_inherit case, but when I modify 
cpuset_base_ops_test, I find it needs lots of changes because we need to 
distinguish cpu or memory  whether numbered continously.

the change for cpuset_inherit as attach

Best Regards
Yang Xu
> Hi Cyril
>> Hi!
>>> Since ltp commit cf33086a1ca, we add cgroup.clone_children switch for
>>> cpuset.cpus and mems, we used the original memory value to set in
>>> cpuset_inherit case.
>>>
>>> After ltp commit 6872ad15a, we improve the node number calculation
>>> for N_NODES,
>>> so it can calculate for N_NODES obtained from the file contains only
>>> "0,8".
>>>
>>> But it doesn't think about this patch will affect mem_string value,
>>> so this
>>> cpuset_inherit case will fail on 4 numa nodes pc, as below:
>>>
>>> cpuset_inherit 1 TPASS: cpus: Inherited information is right!
>>> cpuset_inherit 3 TPASS: cpus: Inherited information is right!
>>> cpuset_inherit 5 TPASS: cpus: Inherited information is right!
>>> cpuset_inherit 7 TPASS: cpus: Inherited information is right!
>>> cpuset_inherit 9 TPASS: cpus: Inherited information is right!
>>> cpuset_inherit 11 TPASS: cpus: Inherited information is right!
>>> cpuset_inherit 13 TPASS: mems: Inherited information is right!
>>> cpuset_inherit 15 TPASS: mems: Inherited information is right!
>>> cpuset_inherit 17 TPASS: mems: Inherited information is right!
>>> cpuset_inherit 19 TPASS: mems: Inherited information is right!
>>> cpuset_inherit 21 TPASS: mems: Inherited information is right!
>>> cpuset_inherit 23 TFAIL: mems: Test result - 0-3 Expected string - "4"
>>>
>>> Fix this by using original mem value.
>>>
>>> Signed-off-by: Yang Xu<xuyang2018.jy@cn.fujitsu.com>
>>> ---
>>> testcases/kernel/controllers/cpuset/cpuset_funcs.sh | 7 +++----
>>> .../cpuset/cpuset_inherit_test/cpuset_inherit_testset.sh | 6 ++----
>>> 2 files changed, 5 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/testcases/kernel/controllers/cpuset/cpuset_funcs.sh
>>> b/testcases/kernel/controllers/cpuset/cpuset_funcs.sh
>>> index f4365af2c..b469140ca 100755
>>> --- a/testcases/kernel/controllers/cpuset/cpuset_funcs.sh
>>> +++ b/testcases/kernel/controllers/cpuset/cpuset_funcs.sh
>>> @@ -28,10 +28,11 @@
>>>
>>> NR_CPUS=`tst_ncpus`
>>> if [ -f "/sys/devices/system/node/has_high_memory" ]; then
>>> - N_NODES="`cat /sys/devices/system/node/has_high_memory | tr ',' ' '`"
>>> + mem_string="`cat /sys/devices/system/node/has_high_memory`"
>>> else
>>> - N_NODES="`cat /sys/devices/system/node/has_normal_memory | tr ',' '
>>> '`"
>>> + mem_string="`cat /sys/devices/system/node/has_normal_memory`"
>>> fi
>>> +N_NODES="`echo $mem_string | tr ',' ' '`"
>>> count=0
>>> for item in $N_NODES; do
>>> delta=1
>>> @@ -42,8 +43,6 @@ for item in $N_NODES; do
>>> done
>>> N_NODES=$count
>>>
>>> -mem_string="$N_NODES"
>>> -
>>> CPUSET="/dev/cpuset"
>>> CPUSET_TMP="/tmp/cpuset_tmp"
>>> CLONE_CHILDREN="/dev/cpuset/cgroup.clone_children"
>>>
>>> diff --git
>>> a/testcases/kernel/controllers/cpuset/cpuset_inherit_test/cpuset_inherit_testset.sh
>>> b/testcases/kernel/controllers/cpuset/cpuset_inherit_test/cpuset_inherit_testset.sh
>>>
>>> index 73eed2cb9..27ff19532 100755
>>> ---
>>> a/testcases/kernel/controllers/cpuset/cpuset_inherit_test/cpuset_inherit_testset.sh
>>>
>>> +++
>>> b/testcases/kernel/controllers/cpuset/cpuset_inherit_test/cpuset_inherit_testset.sh
>>>
>>> @@ -31,10 +31,8 @@ export TST_COUNT=1
>>> check 1 1
>>>
>>> nr_cpus=$NR_CPUS
>>> -nr_mems=$N_NODES
>>>
>>> cpus_all="$(seq -s, 0 $((nr_cpus-1)))"
>>> -mems_all="$(seq -s, 0 $((nr_mems-1)))"
>>>
>>> exit_status=0
>>>
>>> @@ -134,10 +132,10 @@ test_mems()
>>> done<<- EOF
>>> 0 NULL EMPTY
>>> 0 0 EMPTY
>>> - 0 $mems_all EMPTY
>>> + 0 $mem_string EMPTY
>>> 1 NULL EMPTY
>>> 1 0 0
>>> - 1 $mems_all $mem_string
>>> + 1 $mems_string $mem_string
> here has a typo, mems_string->mem_string
>>> EOF
>>> # while read mems result
>>> }
>>
>> I guess that it looks okay to me. I guess that we can commit this before
>> the release, so acked.
>>
>> But don't we have the same problem for cpus_all? If we, for instance,
>> have a machine where cpus are not numbered continously we will fail as
>> well, right?
> Yes, it has the same problem for cpus_all.
>>
>> I guess that a proper fix would be to rewrite the tests to parse the
>> strings into a bitflag arrays and compare these arrays instead of the
>> string comparsion and hacks that keeps up pilling up.
> Will do it in v2.
>>
>



-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: diff.txt
URL: <http://lists.linux.it/pipermail/ltp/attachments/20210112/c9f7e91e/attachment-0001.txt>

[LTP] [PATCH] cpuset_inherit: Use the original mem value instead of N_NODES

[Cyril Hrubis]

Hi!
> When I look these cpuset cases(cpuset_base_ops_test, 
> cpuset_hierarchy_test, cpuset_inherit_test...), these cases seems all 
> not consider the situation(cpus/memory are not numbered continously). If 
> we want to modify them to be situable for not numbered continously, it 
> will be complexd(especially cpuset_base_ops_test).

Thats why I said that these tests would need to be rewritten ideally
from a scratch. I guess that it would be easier to work with bitfields
in C as well.

> AFAIK, I rarely see not numbered continously for memory node. IMO, we
> just check whether memory/cpu numbered continously, if not, we just
> report TCONF and remind user to change their system to meet
> environment, so their system can be fully tested.

That would be better than unexpected failure at least.

> For cpu, maybe we can use the following script to detect
> 
> cpu_string="`cat /sys/devices/system/cpu/online`"
> offline_string="`cat /sys/devices/system/cpu/online`"
> NR_CPUS=`tst_ncpus`
> ALL_CPUS=`tst_ncpus_conf`
> if [ $NR_CPUS -eq $ALL_CPUS ]; then
>         tst_resm TINFO "All($ALL_CPUS) logical cpus on your environment"
> else
>        tst_brkm TCONF "Not all logical cpus on, online($cpu_string),offline($offline_string)"
> fi
> 
> I wonder if it's worth changing the stable cpuset/memory cases for these 
> rared situation(memory/cpu are not numbered continously).

It would allow us to offline CPUs in the middle of the test and checking
that offlined CPUs can no longer be added into the mask, which is
something we cannot test at the moment.

> What do you think about it?

To be honest I'm not sure if ncpus == ncpus_conf means that the cpu
numbering is continous.

I guess that the safest bet would be actually parsing the
/sys/devices/system/cpu/online instead. I.e. check that the file is in a
format 0-$(UINT), since that is what the testcases do expect, right?

> +#select the first one or two online cpu
> +select_online_cpus()
> +{
> +	ncpus_check ${1:-2}
> +	local cpus_array="$(seq -s' ' 0 $((ALL_CPUS-1)))"
> +	local cpuid=
> +	local iter=0
> +	for cpuid in $cpus_array
> +	do
> +		local file="/sys/devices/system/cpu/cpu$cpuid/online"
> +		local online="$(cat $file)"
> +		if [ $online -eq 1 ]; then
> +			iter=`expr $iter + 1`
> +			if [ $iter -eq 1 ]; then
> +				F_ONELINE_CPU=$cpu_id
> +			elif [ $iter -eq 2 ]; then
> +				S_ONLINE_CPU=$cpu_id
> +			else
> +				break
> +			fi
> +		fi
> +        done
> +}

Bitfields are akward in shell. So if I was writing these tests I would
write a function to parse the sysfs file into a cpuset bitfield and
second function to write the bitfield into a sysfs file. And after that
we would do all the operations on cpuset bitfields instead.

That way we can, for instance, get any subset of online CPUs easily,
since that is just one loop over the cpuset bitfield.

e.g. to get a subset with half of the online CPUs we would do:

	int flag = 0, i;

	for (i = 0; i < setsize; i++) {
		if (CPU_ISSET_S(i, setsize, inset)) {
			if (flag)
				GPU_SET_S(i, setsize, outset);

			flag = !flag;
		}
	}

We can probably reuse the code kernel uses to parse and print these, the
code to print a bitmap seems to be in bitmap_list_string() in
lib/vsprintf.c, the parsing seems to be implemented in
bitmap_parselist_user() in the lib/bitmap.c.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH] cpuset_inherit: Use the original mem value instead of N_NODES

[Yang Xu]

Hi Cyril
> Hi!
>> When I look these cpuset cases(cpuset_base_ops_test,
>> cpuset_hierarchy_test, cpuset_inherit_test...), these cases seems all
>> not consider the situation(cpus/memory are not numbered continously). If
>> we want to modify them to be situable for not numbered continously, it
>> will be complexd(especially cpuset_base_ops_test).
>
> Thats why I said that these tests would need to be rewritten ideally
> from a scratch. I guess that it would be easier to work with bitfields
> in C as well.
>
I see.
>> AFAIK, I rarely see not numbered continously for memory node. IMO, we
>> just check whether memory/cpu numbered continously, if not, we just
>> report TCONF and remind user to change their system to meet
>> environment, so their system can be fully tested.
>
> That would be better than unexpected failure at least.
>
Since we are just before a release, I think using this way is ok. I will 
send a patch for this soon.
>> For cpu, maybe we can use the following script to detect
>>
>> cpu_string="`cat /sys/devices/system/cpu/online`"
>> offline_string="`cat /sys/devices/system/cpu/online`"
>> NR_CPUS=`tst_ncpus`
>> ALL_CPUS=`tst_ncpus_conf`
>> if [ $NR_CPUS -eq $ALL_CPUS ]; then
>>          tst_resm TINFO "All($ALL_CPUS) logical cpus on your environment"
>> else
>>         tst_brkm TCONF "Not all logical cpus on, online($cpu_string),offline($offline_string)"
>> fi
>>
>> I wonder if it's worth changing the stable cpuset/memory cases for these
>> rared situation(memory/cpu are not numbered continously).
>
> It would allow us to offline CPUs in the middle of the test and checking
> that offlined CPUs can no longer be added into the mask, which is
> something we cannot test at the moment.
 From this point, it is meaningful.
>
>> What do you think about it?
>
> To be honest I'm not sure if ncpus == ncpus_conf means that the cpu
> numbering is continous.
I see ltp/lib/tst_cpu.c uses 
_SC_NPROCESSORS_ONLN/_SC_NPROCESSORS_CONF[1] sysconf. if ncpus == 
ncpus_conf, it means all cpu enable so the cpu numbering must be continous.

[1]https://www.gnu.org/software/libc/manual/html_node/Processor-Resources.html
>
> I guess that the safest bet would be actually parsing the
> /sys/devices/system/cpu/online instead. I.e. check that the file is in a
> format 0-$(UINT), since that is what the testcases do expect, right?
Yes.
my old way (using _SC_NPROCESSORS_ONLN/_SC_NPROCESSORS_CONF[1] macro) is 
to detect all logcial cpu enabled, your way can cover more situation.
>
>> +#select the first one or two online cpu
>> +select_online_cpus()
>> +{
>> +	ncpus_check ${1:-2}
>> +	local cpus_array="$(seq -s' ' 0 $((ALL_CPUS-1)))"
>> +	local cpuid=
>> +	local iter=0
>> +	for cpuid in $cpus_array
>> +	do
>> +		local file="/sys/devices/system/cpu/cpu$cpuid/online"
>> +		local online="$(cat $file)"
>> +		if [ $online -eq 1 ]; then
>> +			iter=`expr $iter + 1`
>> +			if [ $iter -eq 1 ]; then
>> +				F_ONELINE_CPU=$cpu_id
>> +			elif [ $iter -eq 2 ]; then
>> +				S_ONLINE_CPU=$cpu_id
>> +			else
>> +				break
>> +			fi
>> +		fi
>> +        done
>> +}
>
> Bitfields are akward in shell. So if I was writing these tests I would
> write a function to parse the sysfs file into a cpuset bitfield and
> second function to write the bitfield into a sysfs file. And after that
> we would do all the operations on cpuset bitfields instead.
>
> That way we can, for instance, get any subset of online CPUs easily,
> since that is just one loop over the cpuset bitfield.
Agree.
>
> e.g. to get a subset with half of the online CPUs we would do:
>
> 	int flag = 0, i;
>
> 	for (i = 0; i<  setsize; i++) {
> 		if (CPU_ISSET_S(i, setsize, inset)) {
> 			if (flag)
> 				GPU_SET_S(i, setsize, outset);
>
> 			flag = !flag;
> 		}
> 	}
>
> We can probably reuse the code kernel uses to parse and print these, the
> code to print a bitmap seems to be in bitmap_list_string() in
> lib/vsprintf.c, the parsing seems to be implemented in
> bitmap_parselist_user() in the lib/bitmap.c.
I will see these code when I am free.
>

[LTP] [PATCH v2] cpuset: skip test when cpu or nodes are not numbered continously from 0

[Yang Xu]

These cpuset cases(cpuset_base_ops_test, cpuset_hierarchy_test, cpuset_inherit_test...) seem all
not consider the situation(cpus/memory are not numbered continuously). It is continuously from 0
as default. Skip test if there are not numbered continuously to avoid unexpected error.

This patch also fix cpu_inherit error by using original mem value.

cpuset_inherit case fails on 4 numa nodes pc, as below:
cpuset_inherit 1 TPASS: cpus: Inherited information is right!
cpuset_inherit 3 TPASS: cpus: Inherited information is right!
cpuset_inherit 5 TPASS: cpus: Inherited information is right!
cpuset_inherit 7 TPASS: cpus: Inherited information is right!
cpuset_inherit 9 TPASS: cpus: Inherited information is right!
cpuset_inherit 11 TPASS: cpus: Inherited information is right!
cpuset_inherit 13 TPASS: mems: Inherited information is right!
cpuset_inherit 15 TPASS: mems: Inherited information is right!
cpuset_inherit 17 TPASS: mems: Inherited information is right!
cpuset_inherit 19 TPASS: mems: Inherited information is right!
cpuset_inherit 21 TPASS: mems: Inherited information is right!
cpuset_inherit 23 TFAIL: mems: Test result - 0-3 Expected string - "4"

Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
 .../kernel/controllers/cpuset/cpuset_funcs.sh | 29 +++++++++++++++++--
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/testcases/kernel/controllers/cpuset/cpuset_funcs.sh b/testcases/kernel/controllers/cpuset/cpuset_funcs.sh
index f4365af2c..5b1bde114 100755
--- a/testcases/kernel/controllers/cpuset/cpuset_funcs.sh
+++ b/testcases/kernel/controllers/cpuset/cpuset_funcs.sh
@@ -26,23 +26,34 @@
 
 . test.sh
 
+cpu_string="`cat /sys/devices/system/cpu/online`"
 NR_CPUS=`tst_ncpus`
+
 if [ -f "/sys/devices/system/node/has_high_memory" ]; then
-	N_NODES="`cat /sys/devices/system/node/has_high_memory | tr ',' ' '`"
+	mem_string="`cat /sys/devices/system/node/has_high_memory`"
 else
-	N_NODES="`cat /sys/devices/system/node/has_normal_memory | tr ',' ' '`"
+	mem_string="`cat /sys/devices/system/node/has_normal_memory`"
 fi
+N_NODES="`echo $mem_string | tr ',' ' '`"
 count=0
+final_node=0
 for item in $N_NODES; do
 	delta=1
 	if [ "${item#*-*}" != "$item" ]; then
 		delta=$((${item#*-*} - ${item%*-*} + 1))
 	fi
+	final_node=${item#*-*}
 	count=$((count + $delta))
 done
+final_node=`expr $final_node + 1`
 N_NODES=$count
 
-mem_string="$N_NODES"
+final_cpu=0
+N_CPUS="`echo $cpu_string | tr ',' ' '`"
+for item in $N_CPUS; do
+	final_cpu=${item#*-*}
+done
+final_cpu=`expr $final_cpu + 1`
 
 CPUSET="/dev/cpuset"
 CPUSET_TMP="/tmp/cpuset_tmp"
@@ -78,6 +89,12 @@ ncpus_check()
 	if [ $NR_CPUS -lt $1 ]; then
 		tst_brkm TCONF "The total of CPUs is less than $1"
 	fi
+	# check online cpu whether match 0-num
+	if [ $final_cpu -eq $NR_CPUS ]; then
+		tst_resm TINFO "cpus enable continuously from 0($cpu_string)"
+	else
+		tst_brkm TCONF "cpus don't enable continuously from 0($cpu_string)"
+	fi
 }
 
 nnodes_check()
@@ -85,6 +102,12 @@ nnodes_check()
 	if [ $N_NODES -lt $1 ]; then
 		tst_brkm TCONF "The total of nodes is less than $1"
 	fi
+	# check online whether match 0-num
+	if [ $final_node -eq $count ]; then
+		tst_resm TINFO "nodes enable continuously from 0($mem_string)"
+	else
+		tst_brkm TCONF "nodes don't enable continuously from 0($mem_string)"
+	fi
 }
 
 user_check()
-- 
2.23.0

[LTP] [PATCH v2] cpuset: skip test when cpu or nodes are not numbered continously from 0

[Cyril Hrubis]

Hi!
> diff --git a/testcases/kernel/controllers/cpuset/cpuset_funcs.sh b/testcases/kernel/controllers/cpuset/cpuset_funcs.sh
> index f4365af2c..5b1bde114 100755
> --- a/testcases/kernel/controllers/cpuset/cpuset_funcs.sh
> +++ b/testcases/kernel/controllers/cpuset/cpuset_funcs.sh
> @@ -26,23 +26,34 @@
>  
>  . test.sh
>  
> +cpu_string="`cat /sys/devices/system/cpu/online`"
>  NR_CPUS=`tst_ncpus`
> +
>  if [ -f "/sys/devices/system/node/has_high_memory" ]; then
> -	N_NODES="`cat /sys/devices/system/node/has_high_memory | tr ',' ' '`"
> +	mem_string="`cat /sys/devices/system/node/has_high_memory`"
>  else
> -	N_NODES="`cat /sys/devices/system/node/has_normal_memory | tr ',' ' '`"
> +	mem_string="`cat /sys/devices/system/node/has_normal_memory`"
>  fi
> +N_NODES="`echo $mem_string | tr ',' ' '`"
>  count=0
> +final_node=0
>  for item in $N_NODES; do
>  	delta=1
>  	if [ "${item#*-*}" != "$item" ]; then
>  		delta=$((${item#*-*} - ${item%*-*} + 1))
>  	fi
> +	final_node=${item#*-*}
>  	count=$((count + $delta))
>  done
> +final_node=`expr $final_node + 1`

Why don't we use $(()) as in the rest of the code?

>  N_NODES=$count
>  
> -mem_string="$N_NODES"
> +final_cpu=0
> +N_CPUS="`echo $cpu_string | tr ',' ' '`"
> +for item in $N_CPUS; do
> +	final_cpu=${item#*-*}
> +done
> +final_cpu=`expr $final_cpu + 1`

Here as well.

>  CPUSET="/dev/cpuset"
>  CPUSET_TMP="/tmp/cpuset_tmp"
> @@ -78,6 +89,12 @@ ncpus_check()
>  	if [ $NR_CPUS -lt $1 ]; then
>  		tst_brkm TCONF "The total of CPUs is less than $1"
>  	fi
> +	# check online cpu whether match 0-num
> +	if [ $final_cpu -eq $NR_CPUS ]; then
> +		tst_resm TINFO "cpus enable continuously from 0($cpu_string)"
                                     ^
				     This does not make much sense.

This should probably be:
	"CPUs numbered continuously starting at 0 ($cpu_string)"


> +	else
> +		tst_brkm TCONF "cpus don't enable continuously from 0($cpu_string)"

Here it should be:
	"CPUs are not numbered continuously starting at 0 ($cpu_string)"

> +	fi
>  }
>  
>  nnodes_check()
> @@ -85,6 +102,12 @@ nnodes_check()
>  	if [ $N_NODES -lt $1 ]; then
>  		tst_brkm TCONF "The total of nodes is less than $1"
>  	fi
> +	# check online whether match 0-num
> +	if [ $final_node -eq $count ]; then
                               ^
			I guess that N_NODES would be better here.

> +		tst_resm TINFO "nodes enable continuously from 0($mem_string)"
> +	else
> +		tst_brkm TCONF "nodes don't enable continuously from 0($mem_string)"

These two messages should be adjusted as well.

> +	fi
>  }
>  
>  user_check()
> -- 
> 2.23.0
> 
> 
> 

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2] cpuset: skip test when cpu or nodes are not numbered continously from 0

[Yang Xu]

Hi Cyril

I will send a v3 for these comments.

Best Regards
Yang Xu
> Hi!
>> diff --git a/testcases/kernel/controllers/cpuset/cpuset_funcs.sh b/testcases/kernel/controllers/cpuset/cpuset_funcs.sh
>> index f4365af2c..5b1bde114 100755
>> --- a/testcases/kernel/controllers/cpuset/cpuset_funcs.sh
>> +++ b/testcases/kernel/controllers/cpuset/cpuset_funcs.sh
>> @@ -26,23 +26,34 @@
>>
>>   . test.sh
>>
>> +cpu_string="`cat /sys/devices/system/cpu/online`"
>>   NR_CPUS=`tst_ncpus`
>> +
>>   if [ -f "/sys/devices/system/node/has_high_memory" ]; then
>> -	N_NODES="`cat /sys/devices/system/node/has_high_memory | tr ',' ' '`"
>> +	mem_string="`cat /sys/devices/system/node/has_high_memory`"
>>   else
>> -	N_NODES="`cat /sys/devices/system/node/has_normal_memory | tr ',' ' '`"
>> +	mem_string="`cat /sys/devices/system/node/has_normal_memory`"
>>   fi
>> +N_NODES="`echo $mem_string | tr ',' ' '`"
>>   count=0
>> +final_node=0
>>   for item in $N_NODES; do
>>   	delta=1
>>   	if [ "${item#*-*}" != "$item" ]; then
>>   		delta=$((${item#*-*} - ${item%*-*} + 1))
>>   	fi
>> +	final_node=${item#*-*}
>>   	count=$((count + $delta))
>>   done
>> +final_node=`expr $final_node + 1`
>
> Why don't we use $(()) as in the rest of the code?
>
>>   N_NODES=$count
>>
>> -mem_string="$N_NODES"
>> +final_cpu=0
>> +N_CPUS="`echo $cpu_string | tr ',' ' '`"
>> +for item in $N_CPUS; do
>> +	final_cpu=${item#*-*}
>> +done
>> +final_cpu=`expr $final_cpu + 1`
>
> Here as well.
>
>>   CPUSET="/dev/cpuset"
>>   CPUSET_TMP="/tmp/cpuset_tmp"
>> @@ -78,6 +89,12 @@ ncpus_check()
>>   	if [ $NR_CPUS -lt $1 ]; then
>>   		tst_brkm TCONF "The total of CPUs is less than $1"
>>   	fi
>> +	# check online cpu whether match 0-num
>> +	if [ $final_cpu -eq $NR_CPUS ]; then
>> +		tst_resm TINFO "cpus enable continuously from 0($cpu_string)"
>                                       ^
> 				     This does not make much sense.
>
> This should probably be:
> 	"CPUs numbered continuously starting at 0 ($cpu_string)"
>
>
>> +	else
>> +		tst_brkm TCONF "cpus don't enable continuously from 0($cpu_string)"
>
> Here it should be:
> 	"CPUs are not numbered continuously starting at 0 ($cpu_string)"
>
>> +	fi
>>   }
>>
>>   nnodes_check()
>> @@ -85,6 +102,12 @@ nnodes_check()
>>   	if [ $N_NODES -lt $1 ]; then
>>   		tst_brkm TCONF "The total of nodes is less than $1"
>>   	fi
>> +	# check online whether match 0-num
>> +	if [ $final_node -eq $count ]; then
>                                 ^
> 			I guess that N_NODES would be better here.
>
>> +		tst_resm TINFO "nodes enable continuously from 0($mem_string)"
>> +	else
>> +		tst_brkm TCONF "nodes don't enable continuously from 0($mem_string)"
>
> These two messages should be adjusted as well.
>
>> +	fi
>>   }
>>
>>   user_check()
>> --
>> 2.23.0
>>
>>
>>
>

[LTP] [PATCH v3 1/2] cpuset: skip test when cpu or nodes are not numbered continuously from 0

[Yang Xu]

These cpuset cases(cpuset_base_ops_test, cpuset_hierarchy_test, cpuset_inherit_test...)
seem all not consider the situation(cpus/memory are not numbered continuously). It is
continuously from 0 as default. Skip test if there are not numbered continuously to
avoid unexpected error.

This patch also fix cpu_inherit error by using original mem value.

cpuset_inherit case fails on 4 numa nodes pc, as below:
cpuset_inherit 1 TPASS: cpus: Inherited information is right!
cpuset_inherit 3 TPASS: cpus: Inherited information is right!
cpuset_inherit 5 TPASS: cpus: Inherited information is right!
cpuset_inherit 7 TPASS: cpus: Inherited information is right!
cpuset_inherit 9 TPASS: cpus: Inherited information is right!
cpuset_inherit 11 TPASS: cpus: Inherited information is right!
cpuset_inherit 13 TPASS: mems: Inherited information is right!
cpuset_inherit 15 TPASS: mems: Inherited information is right!
cpuset_inherit 17 TPASS: mems: Inherited information is right!
cpuset_inherit 19 TPASS: mems: Inherited information is right!
cpuset_inherit 21 TPASS: mems: Inherited information is right!
cpuset_inherit 23 TFAIL: mems: Test result - 0-3 Expected string - "4"

Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
 .../kernel/controllers/cpuset/cpuset_funcs.sh | 29 +++++++++++++++++--
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/testcases/kernel/controllers/cpuset/cpuset_funcs.sh b/testcases/kernel/controllers/cpuset/cpuset_funcs.sh
index f4365af2c..84b87d17e 100755
--- a/testcases/kernel/controllers/cpuset/cpuset_funcs.sh
+++ b/testcases/kernel/controllers/cpuset/cpuset_funcs.sh
@@ -26,23 +26,34 @@
 
 . test.sh
 
+cpu_string="`cat /sys/devices/system/cpu/online`"
 NR_CPUS=`tst_ncpus`
+
 if [ -f "/sys/devices/system/node/has_high_memory" ]; then
-	N_NODES="`cat /sys/devices/system/node/has_high_memory | tr ',' ' '`"
+	mem_string="`cat /sys/devices/system/node/has_high_memory`"
 else
-	N_NODES="`cat /sys/devices/system/node/has_normal_memory | tr ',' ' '`"
+	mem_string="`cat /sys/devices/system/node/has_normal_memory`"
 fi
+N_NODES="`echo $mem_string | tr ',' ' '`"
 count=0
+final_node=0
 for item in $N_NODES; do
 	delta=1
 	if [ "${item#*-*}" != "$item" ]; then
 		delta=$((${item#*-*} - ${item%*-*} + 1))
 	fi
+	final_node=${item#*-*}
 	count=$((count + $delta))
 done
+final_node=$((final_node + 1))
 N_NODES=$count
 
-mem_string="$N_NODES"
+final_cpu=0
+N_CPUS="`echo $cpu_string | tr ',' ' '`"
+for item in $N_CPUS; do
+	final_cpu=${item#*-*}
+done
+final_cpu=$((final_cpu + 1))
 
 CPUSET="/dev/cpuset"
 CPUSET_TMP="/tmp/cpuset_tmp"
@@ -78,6 +89,12 @@ ncpus_check()
 	if [ $NR_CPUS -lt $1 ]; then
 		tst_brkm TCONF "The total of CPUs is less than $1"
 	fi
+	# check online cpus whether match 0-num
+	if [ $final_cpu -eq $NR_CPUS ]; then
+		tst_resm TINFO "CPUs are numbered continuously starting at 0 ($cpu_string)"
+	else
+		tst_brkm TCONF "CPUs are not numbered continuously starting at 0 ($cpu_string)"
+	fi
 }
 
 nnodes_check()
@@ -85,6 +102,12 @@ nnodes_check()
 	if [ $N_NODES -lt $1 ]; then
 		tst_brkm TCONF "The total of nodes is less than $1"
 	fi
+	# check online nodes whether match 0-num
+	if [ $final_node -eq $N_NODES ]; then
+		tst_resm TINFO "Nodes are numbered continuously starting at 0 ($mem_string)"
+	else
+		tst_brkm TCONF "Nodes are not numbered continuously starting at 0 ($mem_string)"
+	fi
 }
 
 user_check()
-- 
2.23.0

[LTP] [PATCH v3 2/2] cpuset_inherit: Remove redundant cpu_string assignment

[Yang Xu]

Since the previous patch has got this string from "/sys/devices/system/cpu/online"
file, we don't need to assign value for cpu_string here. Remove it.

Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
 .../cpuset/cpuset_inherit_test/cpuset_inherit_testset.sh     | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/testcases/kernel/controllers/cpuset/cpuset_inherit_test/cpuset_inherit_testset.sh b/testcases/kernel/controllers/cpuset/cpuset_inherit_test/cpuset_inherit_testset.sh
index 73eed2cb9..650972d4b 100755
--- a/testcases/kernel/controllers/cpuset/cpuset_inherit_test/cpuset_inherit_testset.sh
+++ b/testcases/kernel/controllers/cpuset/cpuset_inherit_test/cpuset_inherit_testset.sh
@@ -106,11 +106,6 @@ inherit_test()
 test_cpus()
 {
 	cfile_name="cpus"
-	num=$((nr_cpus-1))
-	cpu_string="0-$num"
-	if [ $nr_cpus -eq 1 ]; then
-		cpu_string="0"
-	fi
 	while read inherit cpus result
 	do
 		inherit_test "$CPUSET/1/cpuset.cpus" "$inherit" "$cpus" "$result"
-- 
2.23.0

[LTP] [PATCH v3 1/2] cpuset: skip test when cpu or nodes are not numbered continuously from 0

[Yang Xu]

HI Cyril
ping.

Best Regards
Yang Xu
> These cpuset cases(cpuset_base_ops_test, cpuset_hierarchy_test, cpuset_inherit_test...)
> seem all not consider the situation(cpus/memory are not numbered continuously). It is
> continuously from 0 as default. Skip test if there are not numbered continuously to
> avoid unexpected error.
> 
> This patch also fix cpu_inherit error by using original mem value.
> 
> cpuset_inherit case fails on 4 numa nodes pc, as below:
> cpuset_inherit 1 TPASS: cpus: Inherited information is right!
> cpuset_inherit 3 TPASS: cpus: Inherited information is right!
> cpuset_inherit 5 TPASS: cpus: Inherited information is right!
> cpuset_inherit 7 TPASS: cpus: Inherited information is right!
> cpuset_inherit 9 TPASS: cpus: Inherited information is right!
> cpuset_inherit 11 TPASS: cpus: Inherited information is right!
> cpuset_inherit 13 TPASS: mems: Inherited information is right!
> cpuset_inherit 15 TPASS: mems: Inherited information is right!
> cpuset_inherit 17 TPASS: mems: Inherited information is right!
> cpuset_inherit 19 TPASS: mems: Inherited information is right!
> cpuset_inherit 21 TPASS: mems: Inherited information is right!
> cpuset_inherit 23 TFAIL: mems: Test result - 0-3 Expected string - "4"
> 
> Signed-off-by: Yang Xu<xuyang2018.jy@cn.fujitsu.com>
> ---
>   .../kernel/controllers/cpuset/cpuset_funcs.sh | 29 +++++++++++++++++--
>   1 file changed, 26 insertions(+), 3 deletions(-)
> 
> diff --git a/testcases/kernel/controllers/cpuset/cpuset_funcs.sh b/testcases/kernel/controllers/cpuset/cpuset_funcs.sh
> index f4365af2c..84b87d17e 100755
> --- a/testcases/kernel/controllers/cpuset/cpuset_funcs.sh
> +++ b/testcases/kernel/controllers/cpuset/cpuset_funcs.sh
> @@ -26,23 +26,34 @@
> 
>   . test.sh
> 
> +cpu_string="`cat /sys/devices/system/cpu/online`"
>   NR_CPUS=`tst_ncpus`
> +
>   if [ -f "/sys/devices/system/node/has_high_memory" ]; then
> -	N_NODES="`cat /sys/devices/system/node/has_high_memory | tr ',' ' '`"
> +	mem_string="`cat /sys/devices/system/node/has_high_memory`"
>   else
> -	N_NODES="`cat /sys/devices/system/node/has_normal_memory | tr ',' ' '`"
> +	mem_string="`cat /sys/devices/system/node/has_normal_memory`"
>   fi
> +N_NODES="`echo $mem_string | tr ',' ' '`"
>   count=0
> +final_node=0
>   for item in $N_NODES; do
>   	delta=1
>   	if [ "${item#*-*}" != "$item" ]; then
>   		delta=$((${item#*-*} - ${item%*-*} + 1))
>   	fi
> +	final_node=${item#*-*}
>   	count=$((count + $delta))
>   done
> +final_node=$((final_node + 1))
>   N_NODES=$count
> 
> -mem_string="$N_NODES"
> +final_cpu=0
> +N_CPUS="`echo $cpu_string | tr ',' ' '`"
> +for item in $N_CPUS; do
> +	final_cpu=${item#*-*}
> +done
> +final_cpu=$((final_cpu + 1))
> 
>   CPUSET="/dev/cpuset"
>   CPUSET_TMP="/tmp/cpuset_tmp"
> @@ -78,6 +89,12 @@ ncpus_check()
>   	if [ $NR_CPUS -lt $1 ]; then
>   		tst_brkm TCONF "The total of CPUs is less than $1"
>   	fi
> +	# check online cpus whether match 0-num
> +	if [ $final_cpu -eq $NR_CPUS ]; then
> +		tst_resm TINFO "CPUs are numbered continuously starting at 0 ($cpu_string)"
> +	else
> +		tst_brkm TCONF "CPUs are not numbered continuously starting at 0 ($cpu_string)"
> +	fi
>   }
> 
>   nnodes_check()
> @@ -85,6 +102,12 @@ nnodes_check()
>   	if [ $N_NODES -lt $1 ]; then
>   		tst_brkm TCONF "The total of nodes is less than $1"
>   	fi
> +	# check online nodes whether match 0-num
> +	if [ $final_node -eq $N_NODES ]; then
> +		tst_resm TINFO "Nodes are numbered continuously starting at 0 ($mem_string)"
> +	else
> +		tst_brkm TCONF "Nodes are not numbered continuously starting at 0 ($mem_string)"
> +	fi
>   }
> 
>   user_check()

[LTP] [PATCH v3 1/2] cpuset: skip test when cpu or nodes are not numbered continuously from 0

[Petr Vorel]

Hi Xu,

> > These cpuset cases(cpuset_base_ops_test, cpuset_hierarchy_test, cpuset_inherit_test...)
> > seem all not consider the situation(cpus/memory are not numbered continuously). It is
> > continuously from 0 as default. Skip test if there are not numbered continuously to
> > avoid unexpected error.

> > This patch also fix cpu_inherit error by using original mem value.

> > cpuset_inherit case fails on 4 numa nodes pc, as below:
> > cpuset_inherit 1 TPASS: cpus: Inherited information is right!
> > cpuset_inherit 3 TPASS: cpus: Inherited information is right!
> > cpuset_inherit 5 TPASS: cpus: Inherited information is right!
> > cpuset_inherit 7 TPASS: cpus: Inherited information is right!
> > cpuset_inherit 9 TPASS: cpus: Inherited information is right!
> > cpuset_inherit 11 TPASS: cpus: Inherited information is right!
> > cpuset_inherit 13 TPASS: mems: Inherited information is right!
> > cpuset_inherit 15 TPASS: mems: Inherited information is right!
> > cpuset_inherit 17 TPASS: mems: Inherited information is right!
> > cpuset_inherit 19 TPASS: mems: Inherited information is right!
> > cpuset_inherit 21 TPASS: mems: Inherited information is right!
> > cpuset_inherit 23 TFAIL: mems: Test result - 0-3 Expected string - "4"

Good catch :).

BTW how this happen? hot-unplug on lpar?
Maybe add brief note about it into commit message.

Reviewed-by: Petr Vorel <pvorel@suse.cz>

...
> > +++ b/testcases/kernel/controllers/cpuset/cpuset_funcs.sh
> > @@ -26,23 +26,34 @@

> >   . test.sh

> > +cpu_string="`cat /sys/devices/system/cpu/online`"
> >   NR_CPUS=`tst_ncpus`
> > +
> >   if [ -f "/sys/devices/system/node/has_high_memory" ]; then
> > -	N_NODES="`cat /sys/devices/system/node/has_high_memory | tr ',' ' '`"
> > +	mem_string="`cat /sys/devices/system/node/has_high_memory`"
> >   else
> > -	N_NODES="`cat /sys/devices/system/node/has_normal_memory | tr ',' ' '`"
> > +	mem_string="`cat /sys/devices/system/node/has_normal_memory`"
> >   fi
> > +N_NODES="`echo $mem_string | tr ',' ' '`"

nit: I'd personally do:

f="/sys/devices/system/node/has_high_memory"
[ -f "$f" ] || f="/sys/devices/system/node/has_normal_memory"
N_NODES="$(cat $f | tr ',' ' ')"

but that's a tiny detail.

It'd be great to rewrite these tests into C.

Kind regards,
Petr

[LTP] [PATCH v3 2/2] cpuset_inherit: Remove redundant cpu_string assignment

[Petr Vorel]

Hi Xu,

> Since the previous patch has got this string from "/sys/devices/system/cpu/online"
> file, we don't need to assign value for cpu_string here. Remove it.

Obviously ok.
Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

[LTP] [PATCH v3 1/2] cpuset: skip test when cpu or nodes are not numbered continuously from 0

[Cyril Hrubis]

Hi!
Looks good to me as well.

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v3 2/2] cpuset_inherit: Remove redundant cpu_string assignment

[Cyril Hrubis]

Hi!
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v3 1/2] cpuset: skip test when cpu or nodes are not numbered continuously from 0

[Yang Xu]

Hi Petr
> Hi Xu,
>
>>> These cpuset cases(cpuset_base_ops_test, cpuset_hierarchy_test, cpuset_inherit_test...)
>>> seem all not consider the situation(cpus/memory are not numbered continuously). It is
>>> continuously from 0 as default. Skip test if there are not numbered continuously to
>>> avoid unexpected error.
>
>>> This patch also fix cpu_inherit error by using original mem value.
>
>>> cpuset_inherit case fails on 4 numa nodes pc, as below:
>>> cpuset_inherit 1 TPASS: cpus: Inherited information is right!
>>> cpuset_inherit 3 TPASS: cpus: Inherited information is right!
>>> cpuset_inherit 5 TPASS: cpus: Inherited information is right!
>>> cpuset_inherit 7 TPASS: cpus: Inherited information is right!
>>> cpuset_inherit 9 TPASS: cpus: Inherited information is right!
>>> cpuset_inherit 11 TPASS: cpus: Inherited information is right!
>>> cpuset_inherit 13 TPASS: mems: Inherited information is right!
>>> cpuset_inherit 15 TPASS: mems: Inherited information is right!
>>> cpuset_inherit 17 TPASS: mems: Inherited information is right!
>>> cpuset_inherit 19 TPASS: mems: Inherited information is right!
>>> cpuset_inherit 21 TPASS: mems: Inherited information is right!
>>> cpuset_inherit 23 TFAIL: mems: Test result - 0-3 Expected string - "4"
>
> Good catch :).
>
> BTW how this happen? hot-unplug on lpar?
> Maybe add brief note about it into commit message.
I have added it and merged this patchset, thanks for you and cyril's review.
>
> Reviewed-by: Petr Vorel<pvorel@suse.cz>
>
> ...
>>> +++ b/testcases/kernel/controllers/cpuset/cpuset_funcs.sh
>>> @@ -26,23 +26,34 @@
>
>>>    . test.sh
>
>>> +cpu_string="`cat /sys/devices/system/cpu/online`"
>>>    NR_CPUS=`tst_ncpus`
>>> +
>>>    if [ -f "/sys/devices/system/node/has_high_memory" ]; then
>>> -	N_NODES="`cat /sys/devices/system/node/has_high_memory | tr ',' ' '`"
>>> +	mem_string="`cat /sys/devices/system/node/has_high_memory`"
>>>    else
>>> -	N_NODES="`cat /sys/devices/system/node/has_normal_memory | tr ',' ' '`"
>>> +	mem_string="`cat /sys/devices/system/node/has_normal_memory`"
>>>    fi
>>> +N_NODES="`echo $mem_string | tr ',' ' '`"
>
> nit: I'd personally do:
>
> f="/sys/devices/system/node/has_high_memory"
> [ -f "$f" ] || f="/sys/devices/system/node/has_normal_memory"
> N_NODES="$(cat $f | tr ',' ' ')"
>
> but that's a tiny detail.
>
> It'd be great to rewrite these tests into C.
Yes, I will create a issue to avoid we forget this.
>
> Kind regards,
> Petr
>
>
> .
>