public inbox for ltp@lists.linux.it
 help / color / mirror / Atom feed
From: Petr Vorel <pvorel@suse.cz>
To: ltp@lists.linux.it
Subject: [LTP] [PATCH v3] rcu/rcu_torture.sh: Rewrite test
Date: Thu, 11 Jul 2019 08:03:57 +0200	[thread overview]
Message-ID: <20190711060357.GA28190@dell5510> (raw)
In-Reply-To: <1562769310-4918-1-git-send-email-ice_yangxiao@163.com>

Hi Xiao,

> 1) Cleanup and convert to new API
> 2) Update valid rcutorture types(rcu, srcu, srcud, tasks)

> Note:
> Exclude valid busted* types(busted, busted_srcud) that check
> the test itself and expect failures, suggested by:
> https://www.spinics.net/lists/kernel/msg3045252.html

> Signed-off-by: Xiao Yang <ice_yangxiao@163.com>
Acked-by: Petr Vorel <pvorel@suse.cz>

Only few comments below (nits).

>  # default options
> -test_time=60
> +test_time=30
You shortened the test to half, good :).

...
> +rcutorture_setup()
> +{
> +	local module=1

> -	if tst_kvcmp -lt "3.11"; then
> -		rcu_type="$rcu_type srcu_raw srcu_raw_sync"
> -	fi
> -fi
> +	# check if rcutorture is built as a kernel module by inserting
> +	# and then removing it
> +	modprobe rcutorture >/dev/null 2>&1 ||  module=0
> +	modprobe -r rcutorture >/dev/null 2>&1 || module=0

This can be just module=1 or module= (none value).
(+ -q is enough instead of redirections). i.e.:

modprobe rcutorture -q || module=
modprobe -r rcutorture -q || module=
[ -n "$module" ] && \

(or even [ "$module" ] && \ )

We usually don't use 0 for shell scripts in LTP.
But that's a nit.

> -TST_TOTAL=$(echo "$rcu_type" | wc -w)
> +	[ $module -eq 0 ] && \
> +		tst_brk TCONF "rcutorture is built-in, non-existent or in use"
> +}

> -est_time=`echo "scale=2; $test_time * $TST_TOTAL / 60 " | bc`
> -tst_resm TINFO "estimate time $est_time min"
> +rcutorture_test()
> +{
> +	local rcu_type=$1

> -for type in $rcu_type; do
> +	tst_res TINFO "${rcu_type}-torture: running ${test_time} sec..."
	tst_res TINFO "${rcu_type}-torture: running $test_time sec..."
(I prefer using brackets ${foo} only when needed - $foo is more readable).

> -	tst_resm TINFO "$type: running $test_time sec..."
> +	modprobe rcutorture nfakewriters=${num_writers} \
> +		torture_type=${rcu_type} >/dev/null 2>&1
Again, use -q:
modprobe -q rcutorture nfakewriters=$num_writers torture_type=$rcu_type

> +	if [ $? -ne 0 ]; then
> +		dmesg | grep -q "invalid torture type: \"${rcu_type}\"" && \
> +			tst_brk TCONF "invalid ${rcu_type} type"
and here ${foo} => $foo

> -	modprobe rcutorture nfakewriters=$num_writers \
> -	         stat_interval=60 test_no_idle_hz=1 shuffle_interval=3 \
> -	         stutter=5 irqreader=1 fqs_duration=0 fqs_holdoff=0 \
> -	         fqs_stutter=3 test_boost=1 test_boost_interval=7 \
> -	         test_boost_duration=4 shutdown_secs=0 \
> -	         stall_cpu=0 stall_cpu_holdoff=10 n_barrier_cbs=0 \
> -	         onoff_interval=0 onoff_holdoff=0 torture_type=$type \
> -	         > /dev/null 2>&1 || tst_brkm TBROK "failed to load module"
> +		tst_brk TBROK "failed to load module"
And here. But I'd prefer to keep stderr (in case of failure it's better
to see immediately what the problem was).
> +	fi

> -	sleep $test_time
> +	sleep ${test_time}
	sleep $test_time

> -	rmmod rcutorture > /dev/null 2>&1 || \
> -		tst_brkm TBROK "failed to unload module"
> +	modprobe -r rcutorture >/dev/null 2>&1 || \
> +		tst_brk TBROK "failed to unload module"
The same here (here I'd also keep stdout)

>  	# check module status in dmesg
> -	result_str=`dmesg | sed -nE '$s/.*End of test: ([A-Z]+):.*/\1/p'`
> -	if [ "$result_str" = "SUCCESS" ]; then
> -		tst_resm TPASS "$type: completed"
> +	local res=$(dmesg | sed -nE "s/.* ${rcu_type}-torture:.* End of test: (.*): .*/\1/p" | tail -n1)
> +	if [ "$res" = "SUCCESS" ]; then
> +		tst_res TPASS "${rcu_type}-torture: $res"
>  	else
> -		tst_resm TFAIL "$type: $result_str, see dmesg"
> +		tst_res TFAIL "${rcu_type}-torture: $res, see dmesg"
Maybe print dmesg (for automated tests, when you don't have direct access to
machine).

Kind regards,
Petr

  reply	other threads:[~2019-07-11  6:03 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-10 14:35 [LTP] [PATCH v3] rcu/rcu_torture.sh: Rewrite test Xiao Yang
2019-07-11  6:03 ` Petr Vorel [this message]
2019-07-11  8:32   ` Xiao Yang
2019-07-11 10:42     ` Petr Vorel
2019-07-11 10:59       ` Xiao Yang

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=20190711060357.GA28190@dell5510 \
    --to=pvorel@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