Re: [RFC PATCH v2 4/4] rtla/osnoise: Leverage IPI event filters when tracing a subset of CPUs

[Valentin Schneider <vschneid@redhat.com>]

On 30/06/26 12:14, Tomas Glozar wrote:
> st 17. 6. 2026 v 15:18 odesílatel Valentin Schneider
> <vschneid@redhat.com> napsal:
>> @@ -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.
>

According to the docstring:

 * Return 0 on success, and -1 on error.

but regardless yes that should be a '< 0' check to match existing code.

>> +                       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/20230707172155.70873-1-vschneid@redhat.com/T/#u
>

Makes sense, will do.

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

I had actually forgotten about applying the filters for the output
instance... I'll look into it.

> Tomas