Re: [RFC v3 1/2] powerpc/cpuidle: cpuidle wakeup latency based on IPI and timer events

2023-09-21 Thread Michael Ellerman
Aboorva Devarajan  writes:
> On Wed, 2023-09-13 at 08:54 +1000, Michael Ellerman wrote:
>> Aboorva Devarajan  writes:
>> > 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
>> > +
>>   
>> I don't see anything here that's powerpc specific?
>> 
>> Which makes me wonder 1) could this be done with some existing
>> generic
>> mechanism?, and 2) if not can this test code be made generic.
>> 
>> At the very least this should be Cc'ed to the cpuidle lists &
>> maintainers given it's a test for cpuidle latency :)
>> 
>> cheers
>
> Hi Michael,
>
> Thanks a lot for taking a look at this.
>
> Yes, this test-case can be used as a generic benchmark for evaluating
> CPU idle latencies across different architectures, as it has thus far
> been exclusively tested and used on PowerPC, so we thought it would be
> more beneficial to incorporate it into a PowerPC specific self-test
> suite. But I will work on making it a generic self-test and send across
> a v4.

I'd suggest just posting v3 again but Cc'ing the cpuidle lists &
maintainers, to see if there is any interest in making it generic.

cheers


Re: [RFC v3 1/2] powerpc/cpuidle: cpuidle wakeup latency based on IPI and timer events

2023-09-21 Thread Aboorva Devarajan
On Wed, 2023-09-13 at 08:54 +1000, Michael Ellerman wrote:
> Aboorva Devarajan  writes:
> > 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
> > +
>   
> I don't see anything here that's powerpc specific?
> 
> Which makes me wonder 1) could this be done with some existing
> generic
> mechanism?, and 2) if not can this test code be made generic.
> 
> At the very least this should be Cc'ed to the cpuidle lists &
> maintainers given it's a test for cpuidle latency :)
> 
> cheers

Hi Michael,

Thanks a lot for taking a look at this.

Yes, this test-case can be used as a generic benchmark for evaluating
CPU idle latencies across different architectures, as it has thus far
been exclusively tested and used on PowerPC, so we thought it would be
more beneficial to incorporate it into a PowerPC specific self-test
suite. But I will work on making it a generic self-test and send across
a v4.



Re: [RFC v3 1/2] powerpc/cpuidle: cpuidle wakeup latency based on IPI and timer events

2023-09-12 Thread Michael Ellerman
Aboorva Devarajan  writes:
> 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 +
  
I don't see anything here that's powerpc specific?

Which makes me wonder 1) could this be done with some existing generic
mechanism?, and 2) if not can this test code be made generic.

At the very least this should be Cc'ed to the cpuidle lists &
maintainers given it's a test for cpuidle latency :)

cheers


[RFC v3 1/2] powerpc/cpuidle: cpuidle wakeup latency based on IPI and timer events

2023-09-10 Thread Aboorva Devarajan
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)
+{
+