diff mbox series

libtracefs: Fix mistaken update to TRACEFS_STACKTRACE macro

Message ID 20250407172013.6aa58191@gandalf.local.home (mailing list archive)
State Accepted
Commit a8d57d26a13542a969d8834f34860aa054cfa8c1
Headers show
Series libtracefs: Fix mistaken update to TRACEFS_STACKTRACE macro | expand

Commit Message

Steven Rostedt April 7, 2025, 9:20 p.m. UTC
From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

The change that converted generic names to variables to start with "tfs_"
was a bit too aggressive and ended up changing the string name of
"common_stacktrace" which broke trace_sql():

When running sqlhist, it would get the following error:

  # ./bin/sqlhist -n wake_lat 'select start.pid, start.common_stacktrace, TIMESTAMP_DELTA_USECS as delta from sched_waking as start join sched_switch as end on start.pid = end.next_pid'
  Failed creating synthetic event!: Success
  select start.pid, start.common_stacktrace, TIMESTAMP_DELTA_USECS as delta from sched_waking as start join sched_switch as end on start.pid = end.next_pid
                    ^
  ERROR: 'start.common_stacktrace'
  Field 'common_stacktrace' not part of event sched_waking

When it should have produced:

  # ./bin/sqlhist -n wake_lat 'select start.pid, start.common_stacktrace, TIMESTAMP_DELTA_USECS as delta from sched_waking as start join sched_switch as end on start.pid = end.next_pid'
  echo 's:wake_lat pid_t pid; unsigned long common_stacktrace[]; u64 delta;' >> /sys/kernel/tracing/dynamic_events
  echo 'hist:keys=pid:__arg_15169_2=pid,__arg_15169_4=common_stacktrace,__arg_15169_5=common_timestamp.usecs' >> /sys/kernel/tracing/events/sched/sched_waking/trigger
  echo 'hist:keys=next_pid:__pid_15169_1=$__arg_15169_2,__common_stacktrace_15169_3=$__arg_15169_4,__delta_15169_6=common_timestamp.usecs-$__arg_15169_5:onmatch(sched.sched_waking).trace(wake_lat,$__pid_15169_1,$__common_stacktrace_15169_3,$__delta_15169_6)' >> /sys/kernel/tracing/events/sched/sched_switch/trigger

Fixes: a2bfb49f ("libtracefs: utest: Rename private functions to fix static building")
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 include/tracefs.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Metin Kaya April 8, 2025, 7:44 a.m. UTC | #1
On 07/04/2025 10:20 PM, Steven Rostedt wrote:
> From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
>
> The change that converted generic names to variables to start with "tfs_"
> was a bit too aggressive and ended up changing the string name of
> "common_stacktrace" which broke trace_sql():
>
> When running sqlhist, it would get the following error:
>
>    # ./bin/sqlhist -n wake_lat 'select start.pid, start.common_stacktrace, TIMESTAMP_DELTA_USECS as delta from sched_waking as start join sched_switch as end on start.pid = end.next_pid'
>    Failed creating synthetic event!: Success
>    select start.pid, start.common_stacktrace, TIMESTAMP_DELTA_USECS as delta from sched_waking as start join sched_switch as end on start.pid = end.next_pid
>                      ^
>    ERROR: 'start.common_stacktrace'
>    Field 'common_stacktrace' not part of event sched_waking
>
> When it should have produced:
>
>    # ./bin/sqlhist -n wake_lat 'select start.pid, start.common_stacktrace, TIMESTAMP_DELTA_USECS as delta from sched_waking as start join sched_switch as end on start.pid = end.next_pid'
>    echo 's:wake_lat pid_t pid; unsigned long common_stacktrace[]; u64 delta;' >> /sys/kernel/tracing/dynamic_events
>    echo 'hist:keys=pid:__arg_15169_2=pid,__arg_15169_4=common_stacktrace,__arg_15169_5=common_timestamp.usecs' >> /sys/kernel/tracing/events/sched/sched_waking/trigger
>    echo 'hist:keys=next_pid:__pid_15169_1=$__arg_15169_2,__common_stacktrace_15169_3=$__arg_15169_4,__delta_15169_6=common_timestamp.usecs-$__arg_15169_5:onmatch(sched.sched_waking).trace(wake_lat,$__pid_15169_1,$__common_stacktrace_15169_3,$__delta_15169_6)' >> /sys/kernel/tracing/events/sched/sched_switch/trigger
>

test_instance_trace_sql() has similar test cases, but they seem to pass
even without the fix in this patch, IIUC. If that's the case, should we
extend the unit tests to catch this kind of issues earlier?

> Fixes: a2bfb49f ("libtracefs: utest: Rename private functions to fix static building")
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> ---
>   include/tracefs.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/tracefs.h b/include/tracefs.h
> index b4e2e30..b6e0f6b 100644
> --- a/include/tracefs.h
> +++ b/include/tracefs.h
> @@ -593,7 +593,7 @@ int tracefs_event_verify_filter(struct tep_event *event, const char *filter,
>   #define TRACEFS_TIMESTAMP "common_timestamp"
>   #define TRACEFS_TIMESTAMP_USECS "common_timestamp.usecs"
>
> -#define TRACEFS_STACKTRACE "tfs_common_stacktrace"
> +#define TRACEFS_STACKTRACE "common_stacktrace"
>
>   enum tracefs_synth_handler {
>       TRACEFS_SYNTH_HANDLE_NONE       = 0,

Reviewed-by: Metin Kaya <metin.kaya@arm.com>
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
diff mbox series

Patch

diff --git a/include/tracefs.h b/include/tracefs.h
index b4e2e30..b6e0f6b 100644
--- a/include/tracefs.h
+++ b/include/tracefs.h
@@ -593,7 +593,7 @@  int tracefs_event_verify_filter(struct tep_event *event, const char *filter,
 #define TRACEFS_TIMESTAMP "common_timestamp"
 #define TRACEFS_TIMESTAMP_USECS "common_timestamp.usecs"
 
-#define TRACEFS_STACKTRACE "tfs_common_stacktrace"
+#define TRACEFS_STACKTRACE "common_stacktrace"
 
 enum tracefs_synth_handler {
 	TRACEFS_SYNTH_HANDLE_NONE	= 0,