On Sat, 19 Oct 2024 11:57:48 +0000
Tatsuya S <[email protected]> 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 <[email protected]>
> ---
> 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


> +
>  #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');
>       }


Reply via email to