public inbox for ltp@lists.linux.it
 help / color / mirror / Atom feed
From: Cyril Hrubis <chrubis@suse.cz>
To: ltp@lists.linux.it
Subject: [LTP] [PATCH] cpuset_inherit: Use the original mem value instead of N_NODES
Date: Fri, 8 Jan 2021 14:26:36 +0100	[thread overview]
Message-ID: <X/hdjNT81M9CxbaD@yuki.lan> (raw)
In-Reply-To: <1606701966-1596-1-git-send-email-xuyang2018.jy@cn.fujitsu.com>

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

  reply	other threads:[~2021-01-08 13:26 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 [this message]
2021-01-11  9:04   ` Yang Xu
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=X/hdjNT81M9CxbaD@yuki.lan \
    --to=chrubis@suse.cz \
    --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