From: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
To: ltp@lists.linux.it
Subject: [LTP] [PATCH] cpuset_inherit: Use the original mem value instead of N_NODES
Date: Mon, 11 Jan 2021 17:04:38 +0800 [thread overview]
Message-ID: <5FFC14A6.3030408@cn.fujitsu.com> (raw)
In-Reply-To: <X/hdjNT81M9CxbaD@yuki.lan>
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 prev parent reply other threads:[~2021-01-11 9:04 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-30 2:06 [LTP] [PATCH] cpuset_inherit: Use the original mem value instead of N_NODES Yang Xu
2021-01-08 13:26 ` Cyril Hrubis
2021-01-11 9:04 ` Yang Xu [this message]
2021-01-12 10:44 ` Yang Xu
2021-01-12 15:17 ` Cyril Hrubis
2021-01-13 9:24 ` Yang Xu
2021-01-13 12:03 ` [LTP] [PATCH v2] cpuset: skip test when cpu or nodes are not numbered continously from 0 Yang Xu
2021-01-13 16:23 ` Cyril Hrubis
2021-01-14 1:40 ` Yang Xu
2021-01-14 2:18 ` [LTP] [PATCH v3 1/2] cpuset: skip test when cpu or nodes are not numbered continuously " Yang Xu
2021-01-14 2:18 ` [LTP] [PATCH v3 2/2] cpuset_inherit: Remove redundant cpu_string assignment Yang Xu
2021-01-19 11:22 ` Petr Vorel
2021-01-19 11:51 ` Cyril Hrubis
2021-01-18 8:16 ` [LTP] [PATCH v3 1/2] cpuset: skip test when cpu or nodes are not numbered continuously from 0 Yang Xu
2021-01-19 11:21 ` Petr Vorel
2021-01-20 1:45 ` Yang Xu
2021-01-19 11:50 ` Cyril Hrubis
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5FFC14A6.3030408@cn.fujitsu.com \
--to=xuyang2018.jy@cn.fujitsu.com \
--cc=ltp@lists.linux.it \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox