Re: [PATCH v3] ftrace: Fix function name for trampoline

[ts]

On 10/20/24 09:18, Steven Rostedt wrote:

On Sat, 19 Oct 2024 11:57:48 +0000
Tatsuya S <tatsuya.s2...@gmail.com> wrote:


Thanks, this is getting close. One small fix.

Also note, the subject should be "tracing:" and not "ftrace:".
"ftrace:" is used for the function hooking mechanism (basically the
ftrace.c file). This is just the tracing infrastructure. I know it's
confusing because the tracing infrastructure uses the name "ftrace_" as
a prefix, but that's mostly due to legacy reasons when the tracing
infrastructure basically only did function tracing.

The issue that unrelated function name is shown on stack trace like
following even though it should be trampoline code address is caused by
the creation of trampoline code in the area where .init.text section
of module was freed after module is loaded.

bash-1344    [002] .....    43.644608: <stack trace>
=> (MODULE INIT FUNCTION)
=> vfs_write
=> ksys_write
=> do_syscall_64
=> entry_SYSCALL_64_after_hwframe

To resolve this, when function address of stack trace entry is in
trampoline, output without looking up symbol name.

Signed-off-by: Tatsuya S <tatsuya.s2...@gmail.com>
---
V2 -> V3: Instead of saving the trampoline address, marker was introduced.
V1 -> V2: Instead of checking trampoline when displaying "trace" results,
          it stores trampoline when generating the stacktrace entry.

  include/linux/ftrace.h      |  7 +++++++
  kernel/trace/trace.c        | 33 +++++++++++++++++++++++++--------
  kernel/trace/trace_output.c |  4 ++++
  3 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index fd5e84d0ec47..39a32fd7b116 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -1188,4 +1188,11 @@ unsigned long arch_syscall_addr(int nr);

#endif /* CONFIG_FTRACE_SYSCALLS */ +/*

+ * This is used only to distinguish
+ * function address from trampoline code.
+ * So this value has no meaning.
+ */
+#define FTRACE_TRAMPOLINE_MARKER  ((unsigned long) INT_MAX)

This doesn't need to be in include/linux, please move this to
kernel/trace/trace.h.

Thanks,

-- Steve

Thank you for your review.


OK, I will fix it.

+
  #endif /* _LINUX_FTRACE_H */
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 1c69ca1f1088..60d156c34e43 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -988,7 +988,8 @@ static inline void trace_access_lock_init(void)
  #endif

#ifdef CONFIG_STACKTRACE

-static void __ftrace_trace_stack(struct trace_buffer *buffer,
+static void __ftrace_trace_stack(struct trace_array *tr,
+                                struct trace_buffer *buffer,
                                 unsigned int trace_ctx,
                                 int skip, struct pt_regs *regs);
  static inline void ftrace_trace_stack(struct trace_array *tr,
@@ -997,7 +998,8 @@ static inline void ftrace_trace_stack(struct trace_array 
*tr,
                                      int skip, struct pt_regs *regs);

#else

-static inline void __ftrace_trace_stack(struct trace_buffer *buffer,
+static inline void __ftrace_trace_stack(struct trace_array *tr,
+                                       struct trace_buffer *buffer,
                                        unsigned int trace_ctx,
                                        int skip, struct pt_regs *regs)
  {
@@ -2928,7 +2930,8 @@ struct ftrace_stacks {
  static DEFINE_PER_CPU(struct ftrace_stacks, ftrace_stacks);
  static DEFINE_PER_CPU(int, ftrace_stack_reserve);

-static void __ftrace_trace_stack(struct trace_buffer *buffer,

+static void __ftrace_trace_stack(struct trace_array *tr,
+                                struct trace_buffer *buffer,
                                 unsigned int trace_ctx,
                                 int skip, struct pt_regs *regs)
  {
@@ -2975,6 +2978,20 @@ static void __ftrace_trace_stack(struct trace_buffer 
*buffer,
                nr_entries = stack_trace_save(fstack->calls, size, skip);
        }

+#ifdef CONFIG_DYNAMIC_FTRACE

+       /* Mark entry of stack trace as trampoline code */
+       if (tr->ops && tr->ops->trampoline) {
+               unsigned long tramp_start = tr->ops->trampoline;
+               unsigned long tramp_end = tramp_start + 
tr->ops->trampoline_size;
+               unsigned long *calls = fstack->calls;
+
+               for (int i = 0; i < nr_entries; i++) {
+                       if (calls[i] >= tramp_start && calls[i] < tramp_end)
+                               calls[i] = FTRACE_TRAMPOLINE_MARKER;
+               }
+       }
+#endif
+
        event = __trace_buffer_lock_reserve(buffer, TRACE_STACK,
                                    struct_size(entry, caller, nr_entries),
                                    trace_ctx);
@@ -3005,7 +3022,7 @@ static inline void ftrace_trace_stack(struct trace_array 
*tr,
        if (!(tr->trace_flags & TRACE_ITER_STACKTRACE))
                return;

- __ftrace_trace_stack(buffer, trace_ctx, skip, regs);

+       __ftrace_trace_stack(tr, buffer, trace_ctx, skip, regs);
  }

void __trace_stack(struct trace_array *tr, unsigned int trace_ctx,

@@ -3014,7 +3031,7 @@ void __trace_stack(struct trace_array *tr, unsigned int 
trace_ctx,
        struct trace_buffer *buffer = tr->array_buffer.buffer;

if (rcu_is_watching()) {

-               __ftrace_trace_stack(buffer, trace_ctx, skip, NULL);
+               __ftrace_trace_stack(tr, buffer, trace_ctx, skip, NULL);
                return;
        }

@@ -3031,7 +3048,7 @@ void __trace_stack(struct trace_array *tr, unsigned int trace_ctx,

                return;

ct_irq_enter_irqson();

-       __ftrace_trace_stack(buffer, trace_ctx, skip, NULL);
+       __ftrace_trace_stack(tr, buffer, trace_ctx, skip, NULL);
        ct_irq_exit_irqson();
  }

@@ -3048,8 +3065,8 @@ void trace_dump_stack(int skip)

        /* Skip 1 to skip this function. */
        skip++;
  #endif
-       __ftrace_trace_stack(printk_trace->array_buffer.buffer,
-                            tracing_gen_ctx(), skip, NULL);
+       __ftrace_trace_stack(printk_trace, printk_trace->array_buffer.buffer,
+                               tracing_gen_ctx(), skip, NULL);
  }
  EXPORT_SYMBOL_GPL(trace_dump_stack);

diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c

index 868f2f912f28..c14573e5a903 100644
--- a/kernel/trace/trace_output.c
+++ b/kernel/trace/trace_output.c
@@ -1246,6 +1246,10 @@ static enum print_line_t trace_stack_print(struct 
trace_iterator *iter,
                        break;

trace_seq_puts(s, " => ");

+               if ((*p) == FTRACE_TRAMPOLINE_MARKER) {
+                       trace_seq_puts(s, "[FTRACE TRAMPOLINE]\n");
+                       continue;
+               }
                seq_print_ip_sym(s, (*p) + delta, flags);
                trace_seq_putc(s, '\n');
        }