On 29/06/26 14:56, Tomas Glozar wrote:

st 17. 6. 2026 v 15:18 odesílatel Valentin Schneider
[off-list ref] napsal:

quoted

+/*
+ * 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, &params->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.

You're right, I fished out the src_cpu to have it available but it's not
being used ATM.

quoted

+
+       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, &params->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.

Right, AFAICT that's the "canonical" type for passing cpumasks around
between userspace and kernelspace. e.g. for sched_getaffinity():

manpage:

    int sched_getaffinity(pid_t pid, size_t cpusetsize,
                          cpu_set_t *mask);

kernelside:

    SYSCALL_DEFINE3(sched_getaffinity, pid_t, pid, unsigned int, len,
                    unsigned long __user *, user_mask_ptr)
    {
            cpumask_var_t mask;
            sched_getaffinity(pid, mask);
            copy_to_user(user_mask_ptr, cpumask_bits(mask), ...)
    }

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.

I couldn't find anything in tools/testing/* other than the CPU_*() helpers.

quoted

+
+       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.

Ack.

quoted

@@ -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.

Indeed, will do.

quoted

[truncated]

Tomas