[RFC v3 2/2] powerpc/selftest: Add support for cpuidle latency measurement
From: Pratik R. Sampat
The cpuidle latency selftest provides support to systematically extract,
analyse and present IPI and timer based wakeup latencies for each CPU
and each idle state available on the system.
The selftest leverages test_cpuidle_latency module's debugfs interface
to interact and extract latency information from the kernel.
The selftest inserts the module if already not inserted, disables all
the idle states and enables them one by one testing the following:
1. Keeping source CPU constant, iterate through all the cores and pick
a single CPU for each core measuring IPI latency for baseline
(CPU is busy with cat /dev/random > /dev/null workload) and then
when the CPU is idle.
2. Iterating through all the CPU cores and selecting one CPU for each
core, then, the expected timer durations to be equivalent to the
residency of the deepest idle state enabled is sent to the selected
target CPU, then the difference between the expected timer duration
and the time of wakeup is determined.
To run this test specifically:
$ sudo make -C tools/testing/selftests \
TARGETS="powerpc/cpuidle_latency" run_tests
There are a few optional arguments too that the script can take
[-h ]
[-i ]
[-m ]
[-s ]
[-o ]
[-v (run on all cpus)]
Default Output location in:
tools/testing/selftests/powerpc/cpuidle_latency/cpuidle_latency.log
To run the test without re-compiling:
$ cd tools/testing/selftest/powerpc/cpuidle_latency/
$ sudo ./cpuidle_latency.sh
Reviewed-by: Srikar Dronamraju
Signed-off-by: Pratik R. Sampat
Signed-off-by: Aboorva Devarajan
---
tools/testing/selftests/powerpc/Makefile | 1 +
.../powerpc/cpuidle_latency/.gitignore| 2 +
.../powerpc/cpuidle_latency/Makefile | 6 +
.../cpuidle_latency/cpuidle_latency.sh| 443 ++
.../powerpc/cpuidle_latency/settings | 1 +
5 files changed, 453 insertions(+)
create mode 100644 tools/testing/selftests/powerpc/cpuidle_latency/.gitignore
create mode 100644 tools/testing/selftests/powerpc/cpuidle_latency/Makefile
create mode 100755
tools/testing/selftests/powerpc/cpuidle_latency/cpuidle_latency.sh
create mode 100644 tools/testing/selftests/powerpc/cpuidle_latency/settings
diff --git a/tools/testing/selftests/powerpc/Makefile
b/tools/testing/selftests/powerpc/Makefile
index 49f2ad1793fd..efac7270ce1f 100644
--- a/tools/testing/selftests/powerpc/Makefile
+++ b/tools/testing/selftests/powerpc/Makefile
@@ -17,6 +17,7 @@ SUB_DIRS = alignment \
benchmarks \
cache_shape \
copyloops\
+ cpuidle_latency \
dexcr\
dscr \
mm \
diff --git a/tools/testing/selftests/powerpc/cpuidle_latency/.gitignore
b/tools/testing/selftests/powerpc/cpuidle_latency/.gitignore
new file mode 100644
index ..987f8852dc59
--- /dev/null
+++ b/tools/testing/selftests/powerpc/cpuidle_latency/.gitignore
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0-only
+cpuidle_latency.log
diff --git a/tools/testing/selftests/powerpc/cpuidle_latency/Makefile
b/tools/testing/selftests/powerpc/cpuidle_latency/Makefile
new file mode 100644
index ..04492b6d2582
--- /dev/null
+++ b/tools/testing/selftests/powerpc/cpuidle_latency/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0
+all:
+
+TEST_PROGS := cpuidle_latency.sh
+
+include ../../lib.mk
diff --git a/tools/testing/selftests/powerpc/cpuidle_latency/cpuidle_latency.sh
b/tools/testing/selftests/powerpc/cpuidle_latency/cpuidle_latency.sh
new file mode 100755
index ..c6b1beffa85f
--- /dev/null
+++ b/tools/testing/selftests/powerpc/cpuidle_latency/cpuidle_latency.sh
@@ -0,0 +1,443 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+#
+# CPU-Idle latency selftest enables systematic retrieval and presentation
+# of IPI and timer-triggered wake-up latencies for every CPU and available
+# system idle state by leveraging the test_cpuidle_latency module.
+#
+# Author: Pratik R. Sampat
+# Author: Aboorva Devarajan
+
+DISABLE=1
+ENABLE=0
+
+LOG=cpuidle_latency.log
+MODULE=/lib/modules/$(uname
-r)/kernel/arch/powerpc/kernel/test_cpuidle_latency.ko
+
+# Kselftest framework requirement - SKIP code is 4.
+ksft_skip=4
+exit_status=0
+
+RUN_TIMER_TEST=1
+TIMEOUT=100
+VERBOSE=0
+
+IPI_SRC_CPU=0
+
+helpme() {
+printf "Usage: %s [-h] [-todg args]
+ [-h ]
+ [-s (default: 0)]
+ [-m ]
+ [-o ]
+ [-v (execute test across all CPU threads)]
+ [-i ]
+ \n" "$0"
+exit 2
+}
+
+cpu_is_online() {
+local cpu=$1
+if [ ! -f "/sys/devices/system/cpu/cpu$cpu/online" ]; then
+printf "CPU %s: file not found: /sys/devices/system/cpu/cpu%s/online"
"$cpu" "$cpu"
+return 0
+fi
+status=$(cat /sys/devices/system/cpu/cpu"$cpu"/online)
+return "$status"
+}
+
[RFC v3 1/2] powerpc/cpuidle: cpuidle wakeup latency based on IPI and timer events
From: Pratik R. Sampat
Introduce a mechanism to fire directed IPIs from a source CPU to a
specified target CPU and measure the time incurred on waking up the
target CPU in response.
Also, introduce a mechanism to queue a hrtimer on a specified CPU and
subsequently measure the time taken to wakeup the CPU.
Define a simple debugfs interface that allows for adjusting the
settings to trigger IPI and timer events on a designated CPU, and to
observe the resulting cpuidle wakeup latencies.
Reviewed-by: Srikar Dronamraju
Signed-off-by: Pratik R. Sampat
Signed-off-by: Aboorva Devarajan
---
arch/powerpc/Kconfig.debug | 10 ++
arch/powerpc/kernel/Makefile | 1 +
arch/powerpc/kernel/test_cpuidle_latency.c | 154 +
3 files changed, 165 insertions(+)
create mode 100644 arch/powerpc/kernel/test_cpuidle_latency.c
diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug
index 2a54fadbeaf5..e175fc3028ac 100644
--- a/arch/powerpc/Kconfig.debug
+++ b/arch/powerpc/Kconfig.debug
@@ -391,3 +391,13 @@ config KASAN_SHADOW_OFFSET
default 0xe000 if PPC32
default 0xa80e if PPC_BOOK3S_64
default 0xa8001c00 if PPC_BOOK3E_64
+
+config CPUIDLE_LATENCY_SELFTEST
+ tristate "Cpuidle latency selftests"
+ depends on CPU_IDLE
+ help
+ Provides a kernel module that run tests using the IPI and
+ timers to measure cpuidle latency.
+
+ Say M if you want these self tests to build as a module.
+ Say N if you are unsure.
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 2919433be355..3c5a576bbcf2 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -94,6 +94,7 @@ obj-$(CONFIG_PPC_BARRIER_NOSPEC) += security.o
obj-$(CONFIG_PPC64)+= vdso64_wrapper.o
obj-$(CONFIG_ALTIVEC) += vecemu.o
obj-$(CONFIG_PPC_BOOK3S_IDLE) += idle_book3s.o
+obj-$(CONFIG_CPUIDLE_LATENCY_SELFTEST) += test_cpuidle_latency.o
procfs-y := proc_powerpc.o
obj-$(CONFIG_PROC_FS) += $(procfs-y)
rtaspci-$(CONFIG_PPC64)-$(CONFIG_PCI) := rtas_pci.o
diff --git a/arch/powerpc/kernel/test_cpuidle_latency.c
b/arch/powerpc/kernel/test_cpuidle_latency.c
new file mode 100644
index ..c93a8f76
--- /dev/null
+++ b/arch/powerpc/kernel/test_cpuidle_latency.c
@@ -0,0 +1,154 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Module-based API test facility for cpuidle latency using IPIs and timers
+ */
+
+#include
+#include
+#include
+
+/*
+ * IPI based wakeup latencies
+ * Measure time taken for a CPU to wakeup on a IPI sent from another CPU
+ * The latency measured also includes the latency of sending the IPI
+ */
+struct latency {
+ unsigned int src_cpu;
+ unsigned int dest_cpu;
+ ktime_t time_start;
+ ktime_t time_end;
+ u64 latency_ns;
+} ipi_wakeup;
+
+static void measure_latency(void *info)
+{
+ struct latency *v = (struct latency *)info;
+ ktime_t time_diff;
+
+ v->time_end = ktime_get();
+ time_diff = ktime_sub(v->time_end, v->time_start);
+ v->latency_ns = ktime_to_ns(time_diff);
+}
+
+void run_smp_call_function_test(unsigned int cpu)
+{
+ ipi_wakeup.src_cpu = smp_processor_id();
+ ipi_wakeup.dest_cpu = cpu;
+ ipi_wakeup.time_start = ktime_get();
+ smp_call_function_single(cpu, measure_latency, _wakeup, 1);
+}
+
+/*
+ * Timer based wakeup latencies
+ * Measure time taken for a CPU to wakeup on a timer being armed and fired
+ */
+struct timer_data {
+ unsigned int src_cpu;
+ u64 timeout;
+ ktime_t time_start;
+ ktime_t time_end;
+ struct hrtimer timer;
+ u64 timeout_diff_ns;
+} timer_wakeup;
+
+static enum hrtimer_restart hrtimer_callback(struct hrtimer *hrtimer)
+{
+ struct timer_data *w;
+ ktime_t time_diff;
+
+ w = container_of(hrtimer, struct timer_data, timer);
+ w->time_end = ktime_get();
+
+ time_diff = ktime_sub(w->time_end, w->time_start);
+ time_diff = ktime_sub(time_diff, ns_to_ktime(w->timeout));
+ w->timeout_diff_ns = ktime_to_ns(time_diff);
+ return HRTIMER_NORESTART;
+}
+
+static void run_timer_test(unsigned int ns)
+{
+ hrtimer_init(_wakeup.timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+ timer_wakeup.timer.function = hrtimer_callback;
+ timer_wakeup.src_cpu = smp_processor_id();
+ timer_wakeup.timeout = ns;
+ timer_wakeup.time_start = ktime_get();
+
+ hrtimer_start(_wakeup.timer, ns_to_ktime(ns),
+ HRTIMER_MODE_REL_PINNED);
+}
+
+static struct dentry *dir;
+
+static int cpu_read_op(void *data, u64 *dest_cpu)
+{
+ *dest_cpu = ipi_wakeup.dest_cpu;
+ return 0;
+}
+
+/*
+ * Send a directed IPI from the current CPU (source) to the destination CPU and
+ * measure the latency on wakeup.
+ */
+static int cpu_write_op(void *data, u64 value)
+{
+
[RFC v3 0/2] CPU-Idle latency selftest framework
Changelog: v2 -> v3 * Minimal code refactoring * Rebased on v6.6-rc1 RFC v1: https://lore.kernel.org/all/20210611124154.56427-1-psam...@linux.ibm.com/ RFC v2: https://lore.kernel.org/all/20230828061530.126588-2-aboor...@linux.vnet.ibm.com/ Other related RFC: https://lore.kernel.org/all/20210430082804.38018-1-psam...@linux.ibm.com/ Userspace selftest: https://lkml.org/lkml/2020/9/2/356 A kernel module + userspace driver to estimate the wakeup latency caused by going into stop states. The motivation behind this program is to find significant deviations behind advertised latency and residency values. The patchset measures latencies for two kinds of events. IPIs and Timers As this is a software-only mechanism, there will be additional latencies of the kernel-firmware-hardware interactions. To account for that, the program also measures a baseline latency on a 100 percent loaded CPU and the latencies achieved must be in view relative to that. To achieve this, we introduce a kernel module and expose its control knobs through the debugfs interface that the selftests can engage with. The kernel module provides the following interfaces within /sys/kernel/debug/powerpc/latency_test/ for, IPI test: ipi_cpu_dest = Destination CPU for the IPI ipi_cpu_src = Origin of the IPI ipi_latency_ns = Measured latency time in ns Timeout test: timeout_cpu_src = CPU on which the timer to be queued timeout_expected_ns = Timer duration timeout_diff_ns = Difference of actual duration vs expected timer Sample output is as follows: # --IPI Latency Test--- # Baseline Avg IPI latency(ns): 2720 # Observed Avg IPI latency(ns) - State snooze: 2565 # Observed Avg IPI latency(ns) - State stop0_lite: 3856 # Observed Avg IPI latency(ns) - State stop0: 3670 # Observed Avg IPI latency(ns) - State stop1: 3872 # Observed Avg IPI latency(ns) - State stop2: 17421 # Observed Avg IPI latency(ns) - State stop4: 1003922 # Observed Avg IPI latency(ns) - State stop5: 1058870 # # --Timeout Latency Test-- # Baseline Avg timeout diff(ns): 1435 # Observed Avg timeout diff(ns) - State snooze: 1709 # Observed Avg timeout diff(ns) - State stop0_lite: 2028 # Observed Avg timeout diff(ns) - State stop0: 1954 # Observed Avg timeout diff(ns) - State stop1: 1895 # Observed Avg timeout diff(ns) - State stop2: 14556 # Observed Avg timeout diff(ns) - State stop4: 873988 # Observed Avg timeout diff(ns) - State stop5: 959137 Aboorva Devarajan (2): powerpc/cpuidle: cpuidle wakeup latency based on IPI and timer events powerpc/selftest: Add support for cpuidle latency measurement arch/powerpc/Kconfig.debug| 10 + arch/powerpc/kernel/Makefile | 1 + arch/powerpc/kernel/test_cpuidle_latency.c| 154 ++ tools/testing/selftests/powerpc/Makefile | 1 + .../powerpc/cpuidle_latency/.gitignore| 2 + .../powerpc/cpuidle_latency/Makefile | 6 + .../cpuidle_latency/cpuidle_latency.sh| 443 ++ .../powerpc/cpuidle_latency/settings | 1 + 8 files changed, 618 insertions(+) create mode 100644 arch/powerpc/kernel/test_cpuidle_latency.c create mode 100644 tools/testing/selftests/powerpc/cpuidle_latency/.gitignore create mode 100644 tools/testing/selftests/powerpc/cpuidle_latency/Makefile create mode 100755 tools/testing/selftests/powerpc/cpuidle_latency/cpuidle_latency.sh create mode 100644 tools/testing/selftests/powerpc/cpuidle_latency/settings -- 2.25.1
Re: [PATCH V5 3/3] skiboot: Update IMC PMU node names for power10
> On 10-Aug-2023, at 3:28 AM, Reza Arbab wrote:
>
> On Mon, Jul 17, 2023 at 08:54:31AM +0530, Athira Rajeev wrote:
>> @@ -408,14 +469,129 @@ static void disable_unavailable_units(struct dt_node
>> *dev)
>> avl_vec = (0xffULL) << 56;
>> }
>>
>> - for (i = 0; i < ARRAY_SIZE(nest_pmus); i++) {
>> - if (!(PPC_BITMASK(i, i) & avl_vec)) {
>> - /* Check if the device node exists */
>> - target = dt_find_by_name_before_addr(dev, nest_pmus[i]);
>> - if (!target)
>> - continue;
>> - /* Remove the device node */
>> - dt_free(target);
>> + if (proc_gen == proc_gen_p9) {
>> + for (i = 0; i < ARRAY_SIZE(nest_pmus_p9); i++) {
>> + if (!(PPC_BITMASK(i, i) & avl_vec)) {
>
> I think all these PPC_BITMASK(i, i) can be changed to PPC_BIT(i).
Hi Reza,
Thanks for reviewing the changes.
Yes. I will add the change in next version
Thanks
Athira
>
>> + /* Check if the device node exists */
>> + target = dt_find_by_name_before_addr(dev, nest_pmus_p9[i]);
>> + if (!target)
>> + continue;
>> + /* Remove the device node */
>> + dt_free(target);
>> + }
>> + }
>> + } else if (proc_gen == proc_gen_p10) {
>> + int val;
>> + char name[8];
>> +
>> + for (i = 0; i < 11; i++) {
>> + if (!(PPC_BITMASK(i, i) & avl_vec)) {
>> + /* Check if the device node exists */
>> + target = dt_find_by_name_before_addr(dev, nest_pmus_p10[i]);
>> + if (!target)
>> + continue;
>> + /* Remove the device node */
>> + dt_free(target);
>> + }
>> + }
>> +
>> + for (i = 35; i < 41; i++) {
>> + if (!(PPC_BITMASK(i, i) & avl_vec)) {
>> + /* Check if the device node exists for phb */
>> + for (j = 0; j < 3; j++) {
>> + snprintf(name, sizeof(name), "phb%d_%d", (i-35), j);
>> + target = dt_find_by_name_before_addr(dev, name);
>> + if (!target)
>> + continue;
>> + /* Remove the device node */
>> + dt_free(target);
>> + }
>> + }
>> + }
>> +
>> + for (i = 41; i < 58; i++) {
>> + if (!(PPC_BITMASK(i, i) & avl_vec)) {
>> + /* Check if the device node exists */
>> + target = dt_find_by_name_before_addr(dev, nest_pmus_p10[i]);
>> + if (!target)
>> + continue;
>> + /* Remove the device node */
>> + dt_free(target);
>> + }
>> + }
>
> --
> Reza Arbab
Re: [PATCH V5 1/3] core/device: Add function to return child node using name at substring "@"
> On 10-Aug-2023, at 3:21 AM, Reza Arbab wrote:
>
> Hi Athira,
>
> I still have a couple of the same questions I asked in v4.
>
> On Mon, Jul 17, 2023 at 08:54:29AM +0530, Athira Rajeev wrote:
>> Add a function dt_find_by_name_before_addr() that returns the child node if
>> it matches till first occurrence at "@" of a given name, otherwise NULL.
>
> Given this summary, I don't userstand the following:
>
>> + assert(dt_find_by_name(root, "node@1") == addr1);
>> + assert(dt_find_by_name(root, "node0_1@2") == addr2);
>
> Is this behavior required? I don't think it makes sense to call this function
> with a second argument containing '@', so I wouldn't expect it to match
> anything in these cases. The function seems to specifically enable it:
Hi Reza,
Yes makes sense. dt_find_by_name can be removed in this test since its
intention is to find device by name.
I will remove these two checks.
>
>> +struct dt_node *dt_find_by_name_before_addr(struct dt_node *root, const
>> char *name)
>> +{
> [snip]
>> + node = strdup(name);
>> + if (!node)
>> + return NULL;
>> + node = strtok(node, "@");
>
> Seems like you could get rid of this and just use name as-is.
Ok Reza
>
> I was curious about something else; say we have 'node@1' and 'node@2'. Is
> there an expectation of which it should match?
>
>addr1 = dt_new_addr(root, "node", 0x1);
>addr2 = dt_new_addr(root, "node", 0x2);
>assert(dt_find_by_name_substr(root, "node") == ???);
> ^^^
In this case, dt_find_by_name_before_addr is not the right function to use.
We have other functions like dt_find_by_name_addr that can be made use of.
I will address other changes in next version
Thanks
Athira
>
> --
> Reza Arbab
Re: [PATCH 00/11] add missing of_node_put
Hello: This patch was applied to netdev/net.git (main) by David S. Miller : On Thu, 7 Sep 2023 11:55:10 +0200 you wrote: > Add of_node_put on a break out of an of_node loop. > > --- > > arch/powerpc/kexec/file_load_64.c|8 ++-- > arch/powerpc/platforms/powermac/low_i2c.c|4 +++- > arch/powerpc/platforms/powermac/smp.c|4 +++- > drivers/bus/arm-cci.c|4 +++- > drivers/genpd/ti/ti_sci_pm_domains.c |8 ++-- > drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c |4 +++- > drivers/gpu/drm/mediatek/mtk_drm_drv.c |4 +++- > drivers/media/platform/mediatek/mdp3/mtk-mdp3-comp.c |1 + > drivers/mmc/host/atmel-mci.c |8 ++-- > drivers/net/ethernet/broadcom/asp2/bcmasp.c |1 + > drivers/soc/dove/pmu.c |5 - > drivers/thermal/thermal_of.c |8 ++-- > sound/soc/sh/rcar/core.c |1 + > 13 files changed, 46 insertions(+), 14 deletions(-) Here is the summary with links: - [02/11] net: bcmasp: add missing of_node_put https://git.kernel.org/netdev/net/c/e73d1ab6cd7e You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
