From: Gautham R Shenoy <ego@linux.vnet.ibm.com>
To: Pratik Rajesh Sampat <psampat@linux.ibm.com>
Cc: rjw@rjwysocki.net, daniel.lezcano@linaro.org, mpe@ellerman.id.au,
benh@kernel.crashing.org, paulus@samba.org,
srivatsa@csail.mit.edu, shuah@kernel.org, npiggin@gmail.com,
ego@linux.vnet.ibm.com, svaidy@linux.ibm.com,
linux-pm@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org
Subject: Re: [PATCH v2 2/2] selftest/cpuidle: Add support for cpuidle latency measurement
Date: Mon, 20 Jul 2020 11:22:42 +0530 [thread overview]
Message-ID: <20200720055242.GB31497@in.ibm.com> (raw)
In-Reply-To: <20200717091801.29289-3-psampat@linux.ibm.com>
Hi Pratik,
On Fri, Jul 17, 2020 at 02:48:01PM +0530, Pratik Rajesh Sampat wrote:
> This patch adds support to trace IPI based and timer based wakeup
> latency from idle states
>
> Latches onto the test-cpuidle_latency kernel module using the debugfs
> interface to send IPIs or schedule a timer based event, which in-turn
> populates the debugfs with the latency measurements.
>
> Currently for the IPI and timer tests; first disable all idle states
> and then test for latency measurements incrementally enabling each state
>
> Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
A few comments below.
> ---
> tools/testing/selftests/Makefile | 1 +
> tools/testing/selftests/cpuidle/Makefile | 6 +
> tools/testing/selftests/cpuidle/cpuidle.sh | 257 +++++++++++++++++++++
> tools/testing/selftests/cpuidle/settings | 1 +
> 4 files changed, 265 insertions(+)
> create mode 100644 tools/testing/selftests/cpuidle/Makefile
> create mode 100755 tools/testing/selftests/cpuidle/cpuidle.sh
> create mode 100644 tools/testing/selftests/cpuidle/settings
>
[..skip..]
> +
> +ins_mod()
> +{
> + if [ ! -f "$MODULE" ]; then
> + printf "$MODULE module does not exist. Exitting\n"
If the module has been compiled into the kernel (due to a
localyesconfig, for instance), then it is unlikely that we will find
it in /lib/modules. Perhaps you want to check if the debugfs
directories created by the module exist, and if so, print a message
saying that the modules is already loaded or some such?
> + exit $ksft_skip
> + fi
> + printf "Inserting $MODULE module\n\n"
> + insmod $MODULE
> + if [ $? != 0 ]; then
> + printf "Insmod $MODULE failed\n"
> + exit $ksft_skip
> + fi
> +}
> +
> +compute_average()
> +{
> + arr=("$@")
> + sum=0
> + size=${#arr[@]}
> + for i in "${arr[@]}"
> + do
> + sum=$((sum + i))
> + done
> + avg=$((sum/size))
It would be good to assert that "size" isn't 0 here.
> +}
> +
> +# Disable all stop states
> +disable_idle()
> +{
> + for ((cpu=0; cpu<NUM_CPUS; cpu++))
> + do
> + for ((state=0; state<NUM_STATES; state++))
> + do
> + echo 1 > /sys/devices/system/cpu/cpu$cpu/cpuidle/state$state/disable
So, on offlined CPUs, we won't see
/sys/devices/system/cpu/cpu$cpu/cpuidle/state$state directory. You
should probably perform this operation only on online CPUs.
> + done
> + done
> +}
> +
> +# Perform operation on each CPU for the given state
> +# $1 - Operation: enable (0) / disable (1)
> +# $2 - State to enable
> +op_state()
> +{
> + for ((cpu=0; cpu<NUM_CPUS; cpu++))
> + do
> + echo $1 > /sys/devices/system/cpu/cpu$cpu/cpuidle/state$2/disable
Ditto
> + done
> +}
This is a helper function. For better readability of the main code you
can define the following wrappers and use them.
cpuidle_enable_state()
{
state=$1
op_state 1 $state
}
cpuidle_disable_state()
{
state=$1
op_state 0 $state
}
> +
[..snip..]
> +run_ipi_tests()
> +{
> + extract_latency
> + disable_idle
> + declare -a avg_arr
> + echo -e "--IPI Latency Test---" >> $LOG
> +
> + echo -e "--Baseline IPI Latency measurement: CPU Busy--" >> $LOG
> + printf "%s %10s %12s\n" "SRC_CPU" "DEST_CPU" "IPI_Latency(ns)" >> $LOG
> + for ((cpu=0; cpu<NUM_CPUS; cpu++))
> + do
> + ipi_test_once "baseline" $cpu
> + printf "%-3s %10s %12s\n" $src_cpu $cpu $ipi_latency >> $LOG
> + avg_arr+=($ipi_latency)
> + done
> + compute_average "${avg_arr[@]}"
> + echo -e "Baseline Average IPI latency(ns): $avg" >> $LOG
> +
> + for ((state=0; state<NUM_STATES; state++))
> + do
> + unset avg_arr
> + echo -e "---Enabling state: $state---" >> $LOG
> + op_state 0 $state
> + printf "%s %10s %12s\n" "SRC_CPU" "DEST_CPU" "IPI_Latency(ns)" >> $LOG
> + for ((cpu=0; cpu<NUM_CPUS; cpu++))
> + do
If a CPU is offline, then we should skip it here.
> + # Running IPI test and logging results
> + sleep 1
> + ipi_test_once "test" $cpu
> + printf "%-3s %10s %12s\n" $src_cpu $cpu $ipi_latency >> $LOG
> + avg_arr+=($ipi_latency)
> + done
> + compute_average "${avg_arr[@]}"
> + echo -e "Expected IPI latency(ns): ${latency_arr[$state]}" >> $LOG
> + echo -e "Observed Average IPI latency(ns): $avg" >> $LOG
> + op_state 1 $state
> + done
> +}
> +
> +# Extract the residency in microseconds and convert to nanoseconds.
> +# Add 100 ns so that the timer stays for a little longer than the residency
> +extract_residency()
> +{
> + for ((state=0; state<NUM_STATES; state++))
> + do
> + residency=$(($(cat /sys/devices/system/cpu/cpu0/cpuidle/state$state/residency) * 1000 + 200))
> + residency_arr+=($residency)
> + done
> +}
> +
> +# Run the Timeout test
> +# $1 run for baseline - busy cpu or regular environment
> +# $2 destination cpu
> +# $3 timeout
> +timeout_test_once()
> +{
> + dest_cpu=$2
> + if [ "$1" = "baseline" ]; then
> + # Keep the CPU busy
> + taskset -c $dest_cpu cat /dev/random > /dev/null &
> + task_pid=$!
> + # Wait for the workload to achieve 100% CPU usage
> + sleep 1
> + fi
> + taskset -c $dest_cpu echo $3 > /sys/kernel/debug/latency_test/timeout_expected_ns
> + # Wait for the result to populate
> + sleep 0.1
> + timeout_diff=$(cat /sys/kernel/debug/latency_test/timeout_diff_ns)
> + src_cpu=$(cat /sys/kernel/debug/latency_test/timeout_cpu_src)
> + if [ "$1" = "baseline" ]; then
> + kill $task_pid
> + wait $task_pid 2>/dev/null
> + fi
> +}
> +
> +run_timeout_tests()
> +{
> + extract_residency
> + disable_idle
> + declare -a avg_arr
> + echo -e "\n--Timeout Latency Test--" >> $LOG
> +
> + echo -e "--Baseline Timeout Latency measurement: CPU Busy--" >> $LOG
> + printf "%s %10s %10s\n" "Wakeup_src" "Baseline_delay(ns)">> $LOG
> + for ((cpu=0; cpu<NUM_CPUS; cpu++))
> + do
Again only perform this on online CPUs.
> + timeout_test_once "baseline" $cpu ${residency_arr[0]}
> + printf "%-3s %13s\n" $src_cpu $timeout_diff >> $LOG
> + avg_arr+=($timeout_diff)
> + done
> + compute_average "${avg_arr[@]}"
> + echo -e "Baseline Average timeout diff(ns): $avg" >> $LOG
> +
> + for ((state=0; state<NUM_STATES; state++))
> + do
> + echo -e "---Enabling state: $state---" >> $LOG
> + op_state 0 $state
> + printf "%s %10s %10s\n" "Wakeup_src" "Baseline_delay(ns)" "Delay(ns)" >> $LOG
> + unset avg_arr
> + for ((cpu=0; cpu<NUM_CPUS; cpu++))
> + do
Ditto.
> + timeout_test_once "test" $cpu ${residency_arr[$state]}
> + printf "%-3s %13s %18s\n" $src_cpu $baseline_timeout_diff $timeout_diff >> $LOG
> + avg_arr+=($timeout_diff)
> + done
> + compute_average "${avg_arr[@]}"
> + echo -e "Expected timeout(ns): ${residency_arr[$state]}" >> $LOG
> + echo -e "Observed Average timeout diff(ns): $avg" >> $LOG
> + op_state 1 $state
> + done
> +}
> +
--
Thanks and Regards
gautham.
next prev parent reply other threads:[~2020-07-20 5:53 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-17 9:17 [PATCH v2 0/2] Selftest for cpuidle latency measurement Pratik Rajesh Sampat
2020-07-17 9:18 ` [PATCH v2 1/2] cpuidle: Trace IPI based and timer based wakeup latency from idle states Pratik Rajesh Sampat
2020-07-20 5:00 ` Gautham R Shenoy
2020-07-17 9:18 ` [PATCH v2 2/2] selftest/cpuidle: Add support for cpuidle latency measurement Pratik Rajesh Sampat
2020-07-20 5:52 ` Gautham R Shenoy [this message]
2020-07-21 11:56 ` Pratik Sampat
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=20200720055242.GB31497@in.ibm.com \
--to=ego@linux.vnet.ibm.com \
--cc=benh@kernel.crashing.org \
--cc=daniel.lezcano@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=mpe@ellerman.id.au \
--cc=npiggin@gmail.com \
--cc=paulus@samba.org \
--cc=psampat@linux.ibm.com \
--cc=rjw@rjwysocki.net \
--cc=shuah@kernel.org \
--cc=srivatsa@csail.mit.edu \
--cc=svaidy@linux.ibm.com \
/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;
as well as URLs for NNTP newsgroup(s).