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 <psam...@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.

Reply via email to