st 17. 6. 2026 v 15:18 odesílatel Valentin Schneider
<[email protected]> napsal:
>
> Instead of post-processing the events in the tracefs_iterate_raw_events()
> callbacks, leverage the kernel event filtering infrastructure to only emit
> IPI events if they target CPUs that are being traced, as specified by the
> -c cmdline option.
>
> Note that some post-processing is still required for the ipi_send_cpumask
> event, as the event being emitted means *some* CPUs targeted by that event
> are monitored, but not all of them - userspace has to recompute that
> intersection.
>

Nit: I'd drop the "Instead of post-processing the events in the
tracefs_iterate_raw_events() callbacks" sentence. I find it a bit
confusing, as "instead of" is quite a strong wording implying
post-processing is removed (at least to my perception), but in the
next paragraph, you contradict it by saying that some post-processing
is still done. Also the commit message is perfectly understandable
without it.

> Signed-off-by: Valentin Schneider <[email protected]>
> ---
>  tools/tracing/rtla/src/osnoise_top.c | 37 +++++++++++++++++++++++++---
>  1 file changed, 33 insertions(+), 4 deletions(-)
>
> diff --git a/tools/tracing/rtla/src/osnoise_top.c 
> b/tools/tracing/rtla/src/osnoise_top.c
> index 5b462a3543b97..8040521710884 100644
> --- a/tools/tracing/rtla/src/osnoise_top.c
> +++ b/tools/tracing/rtla/src/osnoise_top.c
> @@ -93,18 +93,15 @@ 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);
> +       account_ipi(tool, src_cpu, dst_cpu);
>
>         return 0;
>  }
> @@ -141,6 +138,11 @@ osnoise_ipi_cpumask_handler(struct trace_seq *s, struct 
> tep_record *record,
>                 return 0;
>         }
>
> +       /*
> +        * Despite already filtering for such an intersection, we need to 
> compute
> +        * the intersection here as the @cpumask field may contain 
> non-monitered

Typo: non-monitered -> non-monitored

> +        * CPUs.
> +        */
>         CPU_AND(&cpumask_tmp_cpus, event_cpus, 
> &params->common.monitored_cpus);
>
>         /*
> @@ -406,6 +408,33 @@ struct osnoise_tool *osnoise_init_top(struct 
> common_params *params)
>                 goto out_err;
>         }
>
> +       /*
> +        * If tracing on a subset of possible CPUs, leverage the kernel 
> filtering
> +        * infrastructure to only generate events on traced CPUs.
> +        */
> +       if (params->cpus) {
> +               char filter[MAX_PATH];
> +
> +               snprintf(filter, ARRAY_SIZE(filter), "cpu & CPUS{%s}\n", 
> params->cpus);
> +               retval = tracefs_event_file_write(tool->trace.inst,
> +                                                 "ipi", "ipi_send_cpu", 
> "filter",
> +                                                 filter);
> +               if (retval) {

retval is the number of bytes written here, so this should be "retval
< 0" like in trace_event_enable_filter() in trace.c. Same below.

> +                       err_msg("Could not set ipi_send_cpu CPU filter\n");
> +                       goto out_err;

It would be useful to have --ipi work even on older kernels that don't
yet have your cpumask trace event filter patchset [1], for example, by
printing a debug message that filtering is disabled and setting a flag
instead of erroring out here. Then the code in
osnoise_ipi_cpu_handler() can preserve the CPU_ISSET check if the flag
is set.

As --ipi is optional, we can choose to only support it on newer
kernels, but it would be nice to have it working without the filter,
too.

[1] 
https://lore.kernel.org/linux-trace-kernel/[email protected]/T/#u

> +               }
> +
> +
> +               snprintf(filter, ARRAY_SIZE(filter), "cpumask & CPUS{%s}\n", 
> params->cpus);
> +               retval = tracefs_event_file_write(tool->trace.inst,
> +                                                 "ipi", "ipi_send_cpumask", 
> "filter",
> +                                                 filter);
> +               if (retval) {
> +                       err_msg("Could not set ipi_send_cpumask CPU 
> filter\n");
> +                       goto out_err;
> +               }

Same two comments above apply here.

> +       }
> +
>         tep_register_event_handler(tool->trace.tep, -1, "ipi", "ipi_send_cpu",
>                                    osnoise_ipi_cpu_handler, NULL);
>
> --
> 2.54.0
>

I was thinking that it might make sense to enable the filters also for
the trace output instance. On the other hand, it would make it
difficult to enable the event without the filter then, as specifying
"-e ipi" or similar only re-enables the event but does not remove the
filter. Maybe the better idea is to implement an option to filter any
event enabled through -e/--event only to the measurement CPU, as a
separate feature.

Tomas


Reply via email to