Menu
▾
▴
Re: [Patch/RFC] Oprofile-testsuite, handle power-savings-mode changes during scaling test
|
From: Carl L. <ce...@us...> - 2015-01-08 16:41:02
|
On Wed, 2015-01-07 at 10:51 -0600, Will Schmidt wrote:
> The scaling tests will fail if the power-savings-mode changes while the
> test is running. (idle/slow <-> non-idle/fast).
> This adds some logic to check the power state before and after running
> the ocount scaling workload, and reruns the workload if the state has
> changed before it attempts to process results.
>
> Signed-off-by: Will Schmidt <wil...@vn...>
> ---
>
Will:
The patch appears to be functionally correct. I don't see any logic
errors. I applied the patch and ran it on a Power machine.
Unfortunately, I was not able to capture an event where the
power-savings-mode changed.
The issues I note below is the indentation of the code does not match
the standard. Typically we indent by four spaces each time not with
tabs. Some of the lines are inconsistently indented.
Carl Love
> lib/op_util.exp | 36 ++++++++++++++++++++++++++++++
> oprofile-ocount/ocount-run.exp | 49 ++++++++++++++++++++++++++++++++---------
> 2 files changed, 75 insertions(+), 10 deletions(-)
>
>
> diff --git a/testsuite/lib/op_util.exp b/testsuite/lib/op_util.exp
> index 08eaa6c..cb0da1e 100644
> --- a/testsuite/lib/op_util.exp
> +++ b/testsuite/lib/op_util.exp
> @@ -27,6 +27,42 @@ proc found_error {msg} {
> exit 1
> }
>
> +# collect the current power_mode_data value. We collect it from
> +# the lparcfg interface. The field value ultimately comes from firmware.
> +proc get_baseline_power_savings_mode {} {
> + global initial_power_mode_data_string
> + set initial_power_mode_data_string [ local_exec "grep ^power_mode_data /proc/ppc64/lparcfg" "" "" 100 ]
> + if { [ regexp -line "power_mode_data=\[0-9\]+" $initial_power_mode_data_string line] != 1 } {
> + verbose "Did not find a power_mode_data value.\n"
> + set initial_power_mode_data_string "unset"
> + }
> +}
> +
> +# verify the current power_mode_data value is unchanged. A change in this value
> +# indicates the systems power saving state has changed, which will affect the
> +# test results.
> +proc confirm_power_savings_mode {} {
> + global initial_power_mode_data_string
indent should be 4 spaces not a tab
> + global power_mode_has_changed
> + # Pause a few seconds here. This allows the lparcfg reported value to
> + # catch up with the current setting. In practice, the lparcfg reported
> + # value does not immediately reflect the change in power modes.
> + local_exec "sleep 1" "" "" 5
> + set current_power_mode_data_string [ local_exec "grep ^power_mode_data /proc/ppc64/lparcfg" "" "" 100 ]
> + if { [ regexp -line "power_mode_data=\[0-9\]+" $current_power_mode_data_string line] != 1 } {
> + verbose "did not find a power_mode_data value.\n"
> + set current_power_mode_data_string "unset"
> + set power_mode_has_changed 0
Indent should be 8 spaces not two tabs....
> +
> } elseif { $initial_power_mode_data_string != $current_power_mode_data_string } {
> + verbose " power_mode_data value has changed. was: \[$initial_power_mode_data_string\] \n"
> + verbose " now: \[$current_power_mode_data_string\] \n"
> + set power_mode_has_changed 1
> + } else {
> + verbose "power_mode_data value has not changed since the last baseline check.\n"
> + set power_mode_has_changed 0
> + }
> +}
> +
> # read contents of file and return
> proc op_get_file_contents {fname} {
> set fd [open $fname]
> diff --git a/testsuite/oprofile-ocount/ocount-run.exp b/testsuite/oprofile-ocount/ocount-run.exp
> index 9c75fda..6e4942c 100644
> --- a/testsuite/oprofile-ocount/ocount-run.exp
> +++ b/testsuite/oprofile-ocount/ocount-run.exp
> @@ -110,16 +110,45 @@ proc do_test_ocount_scaling {workload_exec} {
>
> set event [get_cycles_event]
>
> - set pid [exec $workload_exec > /tmp/junky.xwyesu.xse &]
> -
> - # allow workload to reach steady state
> - local_exec "sleep 1" "" "" 10
> -
> - set result1 [local_exec "taskset -c 0 ocount -e ${event} --time-interval $run_time_interval1:1 -p $pid" "" "" 60]
> - set result2 [local_exec "taskset -c 0 ocount -e ${event} --time-interval $run_time_interval2:1 -p $pid" "" "" 60]
> - set result4 [local_exec "taskset -c 0 ocount -e ${event} --time-interval $run_time_interval4:1 -p $pid" "" "" 60]
> -
> - test_kill_workload $pid
> +# Logic here to handle a scenario where the system is switching between
> +# power states. Since a change in power saving modes will throw off
> +# the scaling test results, if we detect the state has changed, we
> +# will re-run the tests before attempting to process any results.
Indenting is off, two exta spaces.
> + set retry_limit 3
> + set retry_count 0
> + global power_mode_has_changed
> + set power_mode_has_changed 99
Indenting should be four spaces here not tab.
> +
> + # loop to validate power_savings_mode status begins here
> + while {$power_mode_has_changed > 0} {
> + incr retry_count
> + get_baseline_power_savings_mode
indenting should be 8 spaces not two tabs
> +
> + set pid [exec $workload_exec > /tmp/junky.xwyesu.xse &]
> +
> + # allow workload to reach steady state
> + local_exec "sleep 1" "" "" 10
> +
> + set result1 [local_exec "taskset -c 0 ocount -e ${event} --time-interval $run_time_interval1:1 -p $pid" "" "" 60]
> + set result2 [local_exec "taskset -c 0 ocount -e ${event} --time-interval $run_time_interval2:1 -p $pid" "" "" 60]
> + set result4 [local_exec "taskset -c 0 ocount -e ${event} --time-interval $run_time_interval4:1 -p $pid" "" "" 60]
> +
> + test_kill_workload $pid
indenting is off, should be 8 spaces
> +
> + confirm_power_savings_mode
> +
> + if { $power_mode_has_changed == 0 } {
> + # no change detected. Good to go.
> + verbose "valid run (The power_savings_mode value was stable, or not present)\n"
> + } elseif {$retry_count > $retry_limit} {
> + # exceeded our arbitrary retry limit.
> + send "Warning: Unable to verify a clean run due to power_savings_mode fluctuations.\n"
> + break
> + } else {
> + verbose "Informational: Detected power_savings_mode fluctuation. (iteration #$retry_count)\n"
> + }
> + # end of power_savings_mode loop.
> + }
>
> set count1 [get_event_count $result1 $event]
> set count2 [get_event_count $result2 $event]
>
>
>
> ------------------------------------------------------------------------------
> Dive into the World of Parallel Programming! The Go Parallel Website,
> sponsored by Intel and developed in partnership with Slashdot Media, is your
> hub for all things parallel software development, from weekly thought
> leadership blogs to news, videos, case studies, tutorials and more. Take a
> look and join the conversation now. http://goparallel.sourceforge.net
> _______________________________________________
> oprofile-list mailing list
> opr...@li...
> https://lists.sourceforge.net/lists/listinfo/oprofile-list
>
|