| Message ID | 1606701966-1596-1-git-send-email-xuyang2018.jy@cn.fujitsu.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | cpuset_inherit: Use the original mem value instead of N_NODES | expand |
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.
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. >
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.
>>
>
diff --git a/testcases/kernel/controllers/cpuset/cpuset_funcs.sh b/testcases/kernel/controllers/cpuset/cpuset_funcs.sh
index f4365af2c..e6a05c5e0 100755
--- a/testcases/kernel/controllers/cpuset/cpuset_funcs.sh
+++ b/testcases/kernel/controllers/cpuset/cpuset_funcs.sh
@@ -26,12 +26,26 @@
. test.sh
+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
+ ALL_ON_FLAG=1
+ tst_resm TINFO "All($ALL_CPUS) logical cpus on your environment"
+else
+ ALL_ON_FLAG=0
+ tst_resm TINFO "Not all logical cpus on, online($cpu_string),offline($offline_string)"
+fi
+
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
+tst_resm TINFO "memory node situation(%mem_string)"
+
+N_NODES="`echo $mem_string | tr ',' ' '`"
count=0
for item in $N_NODES; do
delta=1
@@ -42,13 +56,37 @@ 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"
CHILDREN_VALUE="0"
HOTPLUG_CPU="1"
+F_ONLINE_CPU=
+S_ONLINE_CPU=
+
+#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
+}
cpuset_log()
{
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..7c77b7c31 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
@@ -30,12 +30,6 @@ 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
cfile_name=
@@ -106,21 +100,17 @@ 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
+ select_online_cpus 1
while read inherit cpus result
do
inherit_test "$CPUSET/1/cpuset.cpus" "$inherit" "$cpus" "$result"
done <<- EOF
0 NULL EMPTY
0 0 EMPTY
- 0 $cpus_all EMPTY
+ 0 $cpus_string EMPTY
1 NULL EMPTY
- 1 0 0
- 1 $cpus_all $cpu_string
+ 1 $F_ONLINE_CPU $F_ONLINE_CPU
+ 1 $cpus_string $cpus_string
EOF
# while read cpus result
}
@@ -134,10 +124,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 $mem_string $mem_string
EOF
# while read mems result
}
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.
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. >
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 }
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(-)