On 10/08/26 13:24, Tomas Glozar wrote:
> út 4. 8. 2026 v 19:43 odesílatel Valentin Schneider
> <[email protected]> napsal:
>>
>> A later commit will apply a filter to events recorded to the trace
>> output. To prevent any user confusion, remove pre-existing filters when
>> enabling an event provided via the '-e' command line argument.
>>
>> Suggested-by: Tomas Glozar <[email protected]>
>> Signed-off-by: Valentin Schneider <[email protected]>
>> ---
>
> I found that I missed one case when suggesting this: the user might
> supply an event twice. With this change, it will now clear the filter
> the second time the event is applied:
>
> $ rtla timerlat hist --no-aa --on-threshold trace \
> --on-threshold shell,command="grep sched_switch timerlat_trace.txt
> | grep -Fv '[000]' | head -n1" \
> -i 1 -e sched:sched_switch -e sched:sched_switch --filter "cpu == 0"
>
> Without this commit:
>
> ```
> Saving trace to timerlat_trace.txt
> # RTLA timerlat histogram
> ...
> ```
>
> With this commit:
>
> ```
> Saving trace to timerlat_trace.txt
> <idle>-0 [002] d..2. 423185.347008: sched_switch: ...
> # RTLA timerlat histogram
> ...
> ```
>
> (Note that events are processed in opposite order to the command line.)
>
> This is unexpected and might break scripts that for some reason enable
> an event twice. So I'm not sure if my suggestion was the best
> solution.
>
Hm, didn't think of that.
I would say having the last defined event+filter override any previous
filter would make the most sense.
trace-cmd does this partially:
bash-5.3# trace-cmd record -e sched_switch -f 'CPU==0' -e sched_switch -- bash
-c 'ls &>/dev/null'
CPU0 data recorded at offset=0x180000
109 bytes in size (8192 uncompressed)
CPU1 data recorded at offset=0x181000
0 bytes in size (0 uncompressed)
CPU2 data recorded at offset=0x181000
0 bytes in size (0 uncompressed)
CPU3 data recorded at offset=0x181000
0 bytes in size (0 uncompressed)
bash-5.3# trace-cmd record -e sched_switch -f 'CPU==0' -e sched_switch -f
'CPU==1' -- bash -c 'ls &>/dev/null'
CPU0 data recorded at offset=0x180000
0 bytes in size (0 uncompressed)
CPU1 data recorded at offset=0x180000
1428 bytes in size (237568 uncompressed)
CPU2 data recorded at offset=0x181000
0 bytes in size (0 uncompressed)
CPU3 data recorded at offset=0x181000
0 bytes in size (0 uncompressed)
Although I didn't realize that events were handled in reverse cmdline input
order until you pointed it out.
AIUI filers and triggers rely on the LIFO ordering to grab the
last-provided event; making the events list double-linked (but not
circular) would let us process them in FIFO order; something like the
barely tested:
---
diff --git a/tools/tracing/rtla/src/cli_p.h b/tools/tracing/rtla/src/cli_p.h
index 3c939de9abf02..4638cc317ea26 100644
--- a/tools/tracing/rtla/src/cli_p.h
+++ b/tools/tracing/rtla/src/cli_p.h
@@ -221,8 +221,10 @@ static int opt_event_cb(const struct option *opt, const
char *arg, int unset)
if (!tevent)
fatal("Error alloc trace event");
- if (*events)
+ if (*events) {
tevent->next = *events;
+ (*events)->prev = tevent;
+ }
*events = tevent;
return 0;
diff --git a/tools/tracing/rtla/src/trace.c b/tools/tracing/rtla/src/trace.c
index e407447773d04..35601a2e8d0c7 100644
--- a/tools/tracing/rtla/src/trace.c
+++ b/tools/tracing/rtla/src/trace.c
@@ -440,6 +440,18 @@ static void trace_event_disable_trigger(struct
trace_instance *instance,
tevent->event ? : "*", tevent->trigger);
}
+static inline struct trace_events *trace_events_tail(struct trace_events
*tevent)
+{
+ while (tevent && tevent->next)
+ tevent = tevent->next;
+
+ return tevent;
+}
+
+/* Events are stashed in LIFO order; flip that to FIFO processing */
+#define for_each_trace_event(tevent) \
+ for (tevent = trace_events_tail(tevent); tevent; tevent = tevent->prev)
+
/*
* trace_events_disable - disable all trace events
*/
@@ -451,7 +463,7 @@ void trace_events_disable(struct trace_instance *instance,
if (!events)
return;
- while (tevent) {
+ for_each_trace_event(tevent) {
debug_msg("Disabling event %s:%s\n", tevent->system,
tevent->event ? : "*");
if (tevent->enabled) {
trace_event_disable_filter(instance, tevent);
@@ -460,7 +472,6 @@ void trace_events_disable(struct trace_instance *instance,
}
tevent->enabled = 0;
- tevent = tevent->next;
}
}
@@ -544,7 +555,10 @@ int trace_events_enable(struct trace_instance *instance,
struct trace_events *tevent = events;
int retval;
- while (tevent) {
+ if (!events)
+ return 0;
+
+ for_each_trace_event(tevent) {
debug_msg("Enabling event %s:%s\n", tevent->system,
tevent->event ? : "*");
retval = tracefs_event_enable(instance->inst, tevent->system,
tevent->event);
if (retval < 0) {
@@ -562,7 +576,6 @@ int trace_events_enable(struct trace_instance *instance,
return 1;
tevent->enabled = 1;
- tevent = tevent->next;
}
return 0;
diff --git a/tools/tracing/rtla/src/trace.h b/tools/tracing/rtla/src/trace.h
index 95b911a2228b2..eacafc0c96b31 100644
--- a/tools/tracing/rtla/src/trace.h
+++ b/tools/tracing/rtla/src/trace.h
@@ -4,6 +4,7 @@
struct trace_events {
struct trace_events *next;
+ struct trace_events *prev;
char *system;
char *event;
char *filter;
