[PATCH v2] Add new case for stress test
"Zhou, Wenjian/周文剑"
zhouwj-fnst@cn.fujitsu.com
Tue Jan 12 01:43:00 GMT 2016
On 01/12/2016 05:58 AM, David Smith wrote:
> On 01/04/2016 12:37 AM, Zhou Wenjian wrote:
>> * testsuite/lib/gen_load.exp: New test file
>> * testsuite/systemtap.stress/all_kernel_functions.exp: Use gen_load.exp
>> * testsuite/systemtap.stress/current.exp: Use gen_load.exp
>> * testsuite/systemtap.stress/parallel_exec.exp: New test case
>> ---
>> testsuite/lib/gen_load.exp | 19 ++++++++++
>> .../systemtap.stress/all_kernel_functions.exp | 16 +--------
>> testsuite/systemtap.stress/current.exp | 19 ++--------
>> testsuite/systemtap.stress/parallel_exec.exp | 42 ++++++++++++++++++++++
>> 4 files changed, 64 insertions(+), 32 deletions(-)
>> create mode 100644 testsuite/lib/gen_load.exp
>> create mode 100644 testsuite/systemtap.stress/parallel_exec.exp
>>
>> diff --git a/testsuite/lib/gen_load.exp b/testsuite/lib/gen_load.exp
>> new file mode 100644
>> index 0000000..43b9df1
>> --- /dev/null
>> +++ b/testsuite/lib/gen_load.exp
>> @@ -0,0 +1,19 @@
>> +# genload is used by stress test
>> +proc gen_load {} {
>> + # if 'genload' from the ltp exists, use it to create a real load
>> + catch {exec which genload 2>/dev/null} genload
>> + if {![file executable $genload]} {
>> + set genload {/usr/local/ltp/testcases/bin/genload}
>> + }
>> + if [file executable $genload] {
>> + exec $genload -c 10 -i 10 -m 10 -t 10
>> + # ^^^^^ run for 10 seconds
>> + # ^^^^^ 10 procs spinning on malloc
>> + # ^^^^^ 10 procs spinning on sync
>> + # ^^^^^ 10 procs spinning on sqrt
>> + } else {
>> + # sleep for a bit
>> + wait_n_secs 10
>> + }
>> + return 0
>> +}
>
> I know you didn't write the code in gen_load.exp, but I'd modify the
> comment before 'wait_n_secs' to remind readers that wait_n_secs runs
> 'stress' if available.
>
>> diff --git a/testsuite/systemtap.stress/all_kernel_functions.exp b/testsuite/systemtap.stress/all_kernel_functions.exp
>> index d7c2fbc..d546cfc 100644
>> --- a/testsuite/systemtap.stress/all_kernel_functions.exp
>> +++ b/testsuite/systemtap.stress/all_kernel_functions.exp
>> @@ -1,18 +1,4 @@
>> -proc genload {} {
>> - # if 'genload' from the ltp exists, use it to create a real load
>> - set genload {/usr/local/ltp/testcases/bin/genload}
>> - if [file executable $genload] {
>> - exec $genload -c 10 -i 10 -m 10 -t 10
>> - # ^^^^^ run for 10 seconds
>> - # ^^^^^ 10 procs spinning on malloc
>> - # ^^^^^ 10 procs spinning on sync
>> - # ^^^^^ 10 procs spinning on sqrt
>> - } else {
>> - # sleep for a bit
>> - wait_n_secs 10
>> - }
>> - return 0
>> -}
>> +load_lib "gen_load.exp"
>
> OK, this change (calling 'load_lib') I don't get. That shouldn't be
> necessary. All the code in testsuite/lib should be available to all
> testcases with calling load_lib.
>
> Did you find this necessary?
>
load_lib should be called either here or in systemtap/testsuite/config/unix.exp.
Otherwise it can't be found.
Since the gen_load.exp is only used by the stress test, I didn't choose calling it
in systemtap/testsuite/config/unix.exp.
>> proc probe_ok {probepoint} {
>> set cmd {exec stap -p2 -e}
>> diff --git a/testsuite/systemtap.stress/current.exp b/testsuite/systemtap.stress/current.exp
>> index 186baa3..83814b6 100644
>> --- a/testsuite/systemtap.stress/current.exp
>> +++ b/testsuite/systemtap.stress/current.exp
>> @@ -2,23 +2,8 @@
>> # function, install it, and get some output.
>>
>> set test "current"
>> -
>> -proc current_load {} {
>> - # if 'genload' from the ltp exists, use it to create a real load
>> - set genload {/usr/local/ltp/testcases/bin/genload}
>> - if [file executable $genload] {
>> - exec $genload -c 10 -i 10 -m 10 -t 10
>> - # ^^^^^ run for 10 seconds
>> - # ^^^^^ 10 procs spinning on malloc
>> - # ^^^^^ 10 procs spinning on sync
>> - # ^^^^^ 10 procs spinning on sqrt
>> - } else {
>> - # sleep for a bit
>> - wait_n_secs 10
>> - }
>> - return 0
>> -}
>> +load_lib "gen_load.exp"
>
> Ditto comment about calling 'load_lib' here.
>
>> set output_string "(\\w+ = \\d+\r\n){5}(${all_pass_string}){2}(WARNING.*skipped.*)?"
>>
>> -stap_run $srcdir/$subdir/$test.stp current_load $output_string -g -w
>> +stap_run $srcdir/$subdir/$test.stp gen_load $output_string -g -w
>> diff --git a/testsuite/systemtap.stress/parallel_exec.exp b/testsuite/systemtap.stress/parallel_exec.exp
>> new file mode 100644
>> index 0000000..9b6769e
>> --- /dev/null
>> +++ b/testsuite/systemtap.stress/parallel_exec.exp
>> @@ -0,0 +1,42 @@
>> +#!/bin/expect -f
>> +
>> +set test "parallel execute"
>> +
>> +load_lib "gen_load.exp"
>> +
>> +set process_num 10
>> +
>> +set script {
>> + probe syscall.open
>> + {
>> + if (pid() != target()) next
>> + log("open")
>> + }
>> +
>> + probe syscall.close
>> + {
>> + if (pid() != target()) next
>> + log("close")
>> + }
>> +}
>> +
>> +for { set i 0 } { $i < $process_num } { incr i } {
>> + exec stap -e "$script" -c "cat /etc/hosts > /dev/null" >>/tmp/parallel_exec &
>> +}
>> +
>> +if {[eval gen_load] == 0} then {
>> + pass "$test load generation"
>> +} else {
>> + fail "$test load generation"
>> +}
>
> I'm not sure I understand the above. As far as I can see, gen_load
> always returns a 0, so this test will always pass.
>
Perhaps it will always pass.
But it is the same of code in stap_run.exp:
if {$LOAD_GEN_FUNCTION != ""} then {
#run the interesting test here
if {[eval $LOAD_GEN_FUNCTION] == 0} then {
pass "$TEST_NAME load generation"
} else {
fail "$TEST_NAME load generation"
}
}
> In addition, I'd remove /tmp/parallel_exec before you start, otherwise
> you might be appending to a file from the previous run. You might think
> about giving your temporary file a unique name using mktemp.
>
I see.
> I think you are really hoping there that waiting 10 seconds will be
> enough time for all the systemtap scripts to finish. I'm not sure you
> can depend on that, especially on a slow system like an arm/arm64. One
> thing that would help here is to precompile your script (stap -p4 -e
> '$script") before your run of 10, so that all 10 can use the precompiled
> systemtap module from the cache.
>
Previously, I think it is not important whether systemtap will finish
in 10 seconds or not, for starting 10 systemtap processes in 10 seconds
is enough.
But now, I'm confused. I not sure if it is necessary to test the
parallel compiling. Maybe compiling and executing should be test
separately.
How do you think?
>> +set num1 [exec stap -e "$script" -c "cat /etc/hosts > /dev/null" | grep \[open|close\] | wc -l]
>> +set num2 [exec grep \[open|close\] /tmp/parallel_exec | wc -l]
>
> I'm not sure I understand this logic, perhaps I'm missing something.
>
> I'd think we'd see num1 as 10 (since there are 10 invocations of stap).
> Why are you grepping for open|close for num1?
>
> I'd think we'd you'd want to see num2 as 20 (10 lines of 'open' and 10
> lines of 'close'), but I'd guess that your regexp is going to find the
> stap invocation lines also. Why not grep for something like
> '^(open|close)$'?
>
>> +
>> +exec rm -f /tmp/parallel_exec
>> +
>> +if {$num1 * $process_num != $num2} {
>> + fail $test
>> +} else {
>> + pass $test
>> +}
>
> I'm lost on your math here. I'd think you'd want num1 as 10 (10
> invocations of stap) and num2 as 20 (10 lines of 'open', 10 lines of
> 'close'), but num1 (10) * process_num (10) == 100, which isn't close to
> num2 (20). How do you see this working?
>
I'm afraid you have misunderstood it.
num1 is the number of [open|close] in the output by 1 systemtap process.
num2 is the number of [open|close] in the output by 10 systemtap processes.
10 processes' outputs are all in one file.
So if there is no problem in parallel executing, num2 should equal num1 * 10.
--
Thanks
Zhou
More information about the Systemtap
mailing list