út 30. 6. 2026 v 16:00 odesílatel Valentin Schneider
[off-list ref] napsal:

On 30/06/26 12:14, Tomas Glozar wrote:

quoted

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

quoted

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

I double-checked that and you are correct that the docstring says so,
but it's an error in the docstring. According to the manpage, it
returns the number of bytes written (i.e. positive on success, not
zero) [1]:

"RETURN VALUE
...
tracefs_event_file_write() and tracefs_event_file_append() returns
*the number of bytes written to the system/event file* or negative on
error."

The code agrees as well: in tracefs_event_file_write() there's the
wrong docstring (likely copied from another function) [2]:

/*
 * tracefs_event_file_write - write to an event file
 * ...
 * Return 0 on success, and -1 on error.
 */
int tracefs_event_file_write(struct tracefs_instance *instance,
    const char *system, const char *event,
    const char *file, const char *str)
{
        ....
        ret = tracefs_instance_file_write(instance, path, str);
        free(path);
        return ret;
}

but the source of the return value is tracefs_instance_file_write(),
where the docstring is correct [3]:

/**
 * tracefs_instance_file_write - Write in trace file of specific instance.
 * ...
 * Returns the number of written bytes, or -1 in case of an error
 */
int tracefs_instance_file_write(struct tracefs_instance *instance,
const char *file, const char *str)
{
        return instance_file_write(instance, file, str, O_WRONLY | O_TRUNC);
}

instance_file_write() gets the return value from write_file() [4]
which returns the return value of write() [5].

[1] https://man7.org/linux/man-pages/man3/tracefs_event_get_file.3.html
[2] https://git.kernel.org/pub/scm/libs/libtrace/libtracefs.git/tree/src/tracefs-events.c#n686
[3] https://git.kernel.org/pub/scm/libs/libtrace/libtracefs.git/tree/src/tracefs-instance.c#n532
[4] https://git.kernel.org/pub/scm/libs/libtrace/libtracefs.git/tree/src/tracefs-instance.c#n514
[5] https://git.kernel.org/pub/scm/libs/libtrace/libtracefs.git/tree/src/tracefs-instance.c#n496

So the "< 0" is required, the CPU filter doesn't work without it:

[tglozar@fedora rtla]$ uname -a
Linux fedora 7.0.12-101.fc43.x86_64 #1 SMP PREEMPT_DYNAMIC Thu Jun 11
01:32:26 UTC 2026 x86_64 GNU/Linux
[tglozar@fedora rtla]$ sudo ./rtla osnoise top -q -c 0 --ipi
Could not set ipi_send_cpu CPU filter, return value: 14
Could not init osnoise tool
[tglozar@fedora rtla]$ sudo ./rtla osnoise top -q -d 5s --ipi
                                         Operating System Noise
...
[tglozar@fedora rtla]$

(output with additional debug print)

quoted

quoted

+                       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 (local)

Makes sense, will do.

Thanks!

[truncated]

quoted

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.

Thanks. I gave it some more thought and realized enabling the event
without the filter should not be complicated at all. We can just
remove existing filters in trace_events_enable(), as
trace_events_enable() is called after osnoise_init_trace_tool() in
run_tool(). So that will make an explicit "-e ipi" drop the filter
from "--ipi" on the trace instance and show all IPI events. So you can
disregard my note about filtering -e options, it's not relevant here.

Tomas