st 17. 6. 2026 v 15:18 odesÃlatel Valentin Schneider
<[email protected]> napsal:
>
> Leverage the ipi_send_cpu and ipi_send_cpumask trace events to record the
> count of IPIs sent to monitored CPUs. These interferences are already
> accounted by the IRQ count, but this split gives a better overall picture.
>
> This uses the newly added -i cmdline option.
>
> Signed-off-by: Valentin Schneider <[email protected]>
> ---
> tools/tracing/rtla/src/osnoise_top.c | 124 ++++++++++++++++++++++++++-
> 1 file changed, 123 insertions(+), 1 deletion(-)
>
Overall, looks good to me (see small comments below). The reported
numbers make sense:
[tglozar@cs9 rtla]$ sudo ./rtla osnoise top -q -c 0,1 -d 5s --ipi
Operating System Noise
duration: 0 00:00:05 | time is in us
CPU Period Runtime Noise % CPU Aval Max Noise Max
Single HW NMI IRQ Softirq Thread
IPI
0 #4 4000000 28481 99.28797 8977
248 6756 0 4002 18 1
42
1 #5 5000000 38025 99.23950 8120
185 8403 0 5260 153 141
49
(It looks good in the terminal, I'm sure Gmail will garble it...)
I'll compare with trace output on the next patch.
> diff --git a/tools/tracing/rtla/src/osnoise_top.c
> b/tools/tracing/rtla/src/osnoise_top.c
> index 512a6299cb018..5b462a3543b97 100644
> --- a/tools/tracing/rtla/src/osnoise_top.c
> +++ b/tools/tracing/rtla/src/osnoise_top.c
>
> [truncated]
>
> @@ -70,6 +72,91 @@ static struct osnoise_top_data *osnoise_alloc_top(void)
> return NULL;
> }
> +static void account_ipi(struct osnoise_tool *tool,
> + unsigned long long src_cpu, unsigned long long
> dst_cpu)
> +{
> + struct osnoise_top_cpu *cpu_data;
> + struct osnoise_top_data *data;
> + unsigned long long inc = 1;
> +
> + data = tool->data;
> + cpu_data = &data->cpu_data[dst_cpu];
> +
> + update_sum(&cpu_data->ipi_count, &inc);
> +}
> +
> +/*
> + * osnoise_ipi_cpu_handler - this is the handler for single CPU IPI events.
> + */
> +static int
> +osnoise_ipi_cpu_handler(struct trace_seq *s, struct tep_record *record,
> + struct tep_event *event, void *context)
> +{
> + struct osnoise_tool *tool;
> + struct osnoise_params *params;
> + unsigned long long src_cpu, dst_cpu;
> + struct trace_instance *trace = context;
> +
> + tool = container_of(trace, struct osnoise_tool, trace);
> + params = to_osnoise_params(tool->params);
> +
> + src_cpu = record->cpu;
> + tep_get_field_val(s, event, "cpu", record, &dst_cpu, 1);
> +
> + if (CPU_ISSET(dst_cpu, ¶ms->common.monitored_cpus))
> + account_ipi(tool, src_cpu, dst_cpu);
Do we need to retrieve and pass the src_cpu here? I get it if you plan
on using it in the future, but as far as I understand, you are
specifically tracking the destination CPU, not the source CPU. Same
note applies to osnoise_ipi_cpumask_handler() below.
> +
> + return 0;
> +}
> +
> +static cpu_set_t cpumask_tmp_cpus;
> +
> +/*
> + * osnoise_ipi_cpumask_handler - this is the handler for broadcasted IPI
> events.
> + */
> +static int
> +osnoise_ipi_cpumask_handler(struct trace_seq *s, struct tep_record *record,
> + struct tep_event *event, void *context)
> +{
> + struct trace_instance *trace = context;
> + struct osnoise_tool *tool;
> + struct osnoise_params *params;
> + struct tep_format_field *field;
> + unsigned long long src_cpu;
> + cpu_set_t *event_cpus;
> + int len;
> +
> + tool = container_of(trace, struct osnoise_tool, trace);
> + params = to_osnoise_params(tool->params);
> +
> + src_cpu = record->cpu;
> +
> + field = tep_find_field(event, "cpumask");
> + if (!field)
> + return 0;
> +
> + event_cpus = tep_get_field_raw(s, event, "cpumask", record, &len, 1);
> + if (!event_cpus) {
> + err_msg("Failed to get cpumask field\n");
> + return 0;
> + }
> +
> + CPU_AND(&cpumask_tmp_cpus, event_cpus,
> ¶ms->common.monitored_cpus);
> +
> + /*
> + * Computing the mask weight is overkill but there is no leaner option
> + * provided by glibc, e.g cpumask_first() or somesuch.
> + */
> + if (CPU_COUNT(&cpumask_tmp_cpus)) {
> + for (int cpu = 0; cpu < nr_cpus; cpu++) {
> + if (CPU_ISSET(cpu, &cpumask_tmp_cpus))
> + account_ipi(tool, src_cpu, cpu);
> + }
> + }
Technically, the existing code already relies on the glibc cpumask
implementation (cpu_set_t) matching the kernel "cpumask_t" type, as
the "cpumask" field is the latter (per
/sys/kernel/tracing/events/ipi/ipi_send_cpumask/format), not the
former. So I wouldn't worry about the opaqueness of cpu_set_t much.
Not sure how this is handled in other tracing tools that need to use
cpumask, I'd have to look around a bit. It might even make sense to
have a "tools" version of the cpumask functions like cpumask_first(),
I guess, like we already do for e.g. lists and container_of.
> +
> + return 0;
> +}
> +
> /*
> * osnoise_top_handler - this is the handler for osnoise tracer events
> */
Nit: As this is extra functionality, it'd be more readable to have the
IPI handling after the main top handler, so that someone not familiar
with the source code will see the core logic first. That would also
match IPI being displayed to the right of the other numbers in the top
output.
> @@ -164,6 +251,8 @@ static void osnoise_top_header(struct osnoise_tool *top)
> goto eol;
>
> trace_seq_printf(s, " IRQ Softirq Thread");
> + if (params->common.ipi)
> + trace_seq_printf(s, " IPI");
>
> eol:
> if (pretty)
> @@ -218,7 +307,13 @@ static void osnoise_top_print(struct osnoise_tool *tool,
> int cpu)
>
> trace_seq_printf(s, "%12llu ", cpu_data->irq_count);
> trace_seq_printf(s, "%12llu ", cpu_data->softirq_count);
> - trace_seq_printf(s, "%12llu\n", cpu_data->thread_count);
> + trace_seq_printf(s, "%12llu", cpu_data->thread_count);
> + if (!params->common.ipi) {
> + trace_seq_printf(s, "\n");
> + return;
> + }
> +
> + trace_seq_printf(s, " %12llu\n", cpu_data->ipi_count);
Maybe at this point it is worth it to print the "\n" in a separate
statement, readability-wise:
trace_seq_printf(s, "%12llu ", cpu_data->irq_count);
trace_seq_printf(s, "%12llu ", cpu_data->softirq_count);
trace_seq_printf(s, "%12llu", cpu_data->thread_count);
if (params->common.ipi)
trace_seq_printf(s, " %12llu", cpu_data->ipi_count);
trace_seq_printf(s, "\n");
It would also make diffs nicer when adding new options.
> [truncated]
Tomas
