2024-01-15 07:13:04

by Yong-Xuan Wang

[permalink] [raw]
Subject: [PATCH 1/1] RISC-V: add IPI tracepoints

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 <[email protected]>
---
arch/riscv/kernel/smp.c | 38 +++++++++++++++++++++-----------------
1 file changed, 21 insertions(+), 17 deletions(-)

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);
--
2.17.1



2024-01-15 09:42:10

by Valentin Schneider

[permalink] [raw]
Subject: Re: [PATCH 1/1] RISC-V: add IPI tracepoints

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/[email protected]/