diff mbox series

[1/1] RISC-V: add IPI tracepoints

Message ID 20240115071246.32675-1-yongxuan.wang@sifive.com
State New
Headers
Series [1/1] RISC-V: add IPI tracepoints |

Commit Message

Yong-Xuan Wang Jan. 15, 2024, 7:12 a.m. UTC 
  The strings used to list IPIs in /proc/interrupts are reused for tracing
purposes. Also slightly clean up the code by removing send_ipi_single().

Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
---
 arch/riscv/kernel/smp.c | 38 +++++++++++++++++++++-----------------
 1 file changed, 21 insertions(+), 17 deletions(-)

Comments

Valentin Schneider Jan. 15, 2024, 9:41 a.m. UTC | #1 
On 15/01/24 07:12, Yong-Xuan Wang wrote:
> The strings used to list IPIs in /proc/interrupts are reused for tracing
> purposes. Also slightly clean up the code by removing send_ipi_single().
>

So this is adding trace events for sending & receiving IPIs.

Regarding reception, AFAICT arm/arm64 had that historically because IPIs
were handled directly by the irqchip (see rambling on the first part of the
cover letter at [1]), but now that this is no longer the case they are
obsolete: they show up in trace_irq_handler* events.

I don't know riscv IRQ handling, but riscv_ipi_set_virq_range() looks
awfully similar to arm64's set_smp_ipi_range() and does a "proper"
request_percpu_irq() for handle_IPI(). So if one wants a trace footprint of
IPI reception, there already is infrastructure for it.

Regarding sending IPIs, there now is trace_ipi_send_cpu{mask} which is in
the core kernel and thus benefits all architectures, though it
only covers IPI_RESCHEDULE, IPI_CALL_FUNC and IPI_IRQ_WORK.

Long story short, I know the ipi_raise and ipi_{entry_exit} TPs already
exist and we can't remove them, but IMO they're not that helpful because
they just give a string rather than an actual function pointer, *and* they
have to be manually added to the right place in each architecture. I'd
rather see trace_ipi_send_cpu{mask} extended to cover the missing cases.

[1]: https://lore.kernel.org/lkml/20230307143558.294354-1-vschneid@redhat.com/
diff mbox series

Patch

diff --git a/arch/riscv/kernel/smp.c b/arch/riscv/kernel/smp.c
index 40420afbb1a0..32c653cd3433 100644
--- a/arch/riscv/kernel/smp.c
+++ b/arch/riscv/kernel/smp.c
@@ -26,6 +26,8 @@ 
 #include <asm/cacheflush.h>
 #include <asm/cpu_ops.h>
 
+#include <trace/events/ipi.h>
+
 enum ipi_message_type {
 	IPI_RESCHEDULE,
 	IPI_CALL_FUNC,
@@ -36,6 +38,15 @@  enum ipi_message_type {
 	IPI_MAX
 };
 
+static const char * const ipi_names[] __tracepoint_string = {
+	[IPI_RESCHEDULE]	= "Rescheduling interrupts",
+	[IPI_CALL_FUNC]		= "Function call interrupts",
+	[IPI_CPU_STOP]		= "CPU stop interrupts",
+	[IPI_CPU_CRASH_STOP]	= "CPU stop (for crash dump) interrupts",
+	[IPI_IRQ_WORK]		= "IRQ work interrupts",
+	[IPI_TIMER]		= "Timer broadcast interrupts",
+};
+
 unsigned long __cpuid_to_hartid_map[NR_CPUS] __ro_after_init = {
 	[0 ... NR_CPUS-1] = INVALID_HARTID
 };
@@ -96,18 +107,14 @@  static inline void ipi_cpu_crash_stop(unsigned int cpu, struct pt_regs *regs)
 
 static void send_ipi_mask(const struct cpumask *mask, enum ipi_message_type op)
 {
+	trace_ipi_raise(mask, ipi_names[op]);
 	__ipi_send_mask(ipi_desc[op], mask);
 }
 
-static void send_ipi_single(int cpu, enum ipi_message_type op)
-{
-	__ipi_send_mask(ipi_desc[op], cpumask_of(cpu));
-}
-
 #ifdef CONFIG_IRQ_WORK
 void arch_irq_work_raise(void)
 {
-	send_ipi_single(smp_processor_id(), IPI_IRQ_WORK);
+	send_ipi_mask(cpumask_of(smp_processor_id()), IPI_IRQ_WORK);
 }
 #endif
 
@@ -115,6 +122,9 @@  static irqreturn_t handle_IPI(int irq, void *data)
 {
 	int ipi = irq - ipi_virq_base;
 
+	if ((unsigned int)ipi < IPI_MAX)
+		trace_ipi_entry(ipi_names[ipi]);
+
 	switch (ipi) {
 	case IPI_RESCHEDULE:
 		scheduler_ipi();
@@ -141,6 +151,9 @@  static irqreturn_t handle_IPI(int irq, void *data)
 		break;
 	}
 
+	if ((unsigned int)ipi < IPI_MAX)
+		trace_ipi_exit(ipi_names[ipi]);
+
 	return IRQ_HANDLED;
 }
 
@@ -205,15 +218,6 @@  void riscv_ipi_set_virq_range(int virq, int nr, bool use_for_rfence)
 		static_branch_disable(&riscv_ipi_for_rfence);
 }
 
-static const char * const ipi_names[] = {
-	[IPI_RESCHEDULE]	= "Rescheduling interrupts",
-	[IPI_CALL_FUNC]		= "Function call interrupts",
-	[IPI_CPU_STOP]		= "CPU stop interrupts",
-	[IPI_CPU_CRASH_STOP]	= "CPU stop (for crash dump) interrupts",
-	[IPI_IRQ_WORK]		= "IRQ work interrupts",
-	[IPI_TIMER]		= "Timer broadcast interrupts",
-};
-
 void show_ipi_stats(struct seq_file *p, int prec)
 {
 	unsigned int cpu, i;
@@ -234,7 +238,7 @@  void arch_send_call_function_ipi_mask(struct cpumask *mask)
 
 void arch_send_call_function_single_ipi(int cpu)
 {
-	send_ipi_single(cpu, IPI_CALL_FUNC);
+	send_ipi_mask(cpumask_of(cpu), IPI_CALL_FUNC);
 }
 
 #ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
@@ -329,6 +333,6 @@  bool smp_crash_stop_failed(void)
 
 void arch_smp_send_reschedule(int cpu)
 {
-	send_ipi_single(cpu, IPI_RESCHEDULE);
+	send_ipi_mask(cpumask_of(cpu), IPI_RESCHEDULE);
 }
 EXPORT_SYMBOL_GPL(arch_smp_send_reschedule);