[for-linus v2][PATCH 0/2] ftrace: Fixes for v6.13

Steven Rostedt posted 2 patches 1 year ago
There is a newer version of this series
[for-linus v2][PATCH 0/2] ftrace: Fixes for v6.13
Posted by Steven Rostedt 1 year ago
Ftrace fixes for 6.13:

- Always try to initialize the idle functions when graph tracer starts

  A bug was found that when a CPU is offline when graph tracing starts
  and then comes online, that CPU is not traced. The fix to that was
  to move the initialization of the idle shadow stack over to the
  hot plug online logic, which also handle onlined CPUs. The issue was
  that it removed the initialization of the shadow stack when graph tracing
  starts, but the callbacks to the hot plug logic do nothing if graph
  tracing isn't currently running. Although that fix fixed the onlining
  of a CPU during tracing, it broke the CPUs that were already online.

- Have microblaze not try to get the "true parent" in function tracing

  If function tracing and graph tracing are both enabled at the same time
  the parent of the functions traced by the function tracer may sometimes
  be the graph tracing trampoline. The graph tracing hijacks the return
  pointer of the function to trace it, but that can interfere with the
  function tracing parent output. This was fixed by using the
  ftrace_graph_ret_addr() function passing in the kernel stack pointer
  using the ftrace_regs_get_stack_pointer() function. But Al Viro reported
  that Microblaze does not implement the kernel_stack_pointer(regs)
  helper function that ftrace_regs_get_stack_pointer() uses and fails
  to compile when function graph tracing is enabled.

  It was first thought that this was a microblaze issue, but the real
  cause is that this only works when an architecture implements
  HAVE_DYNAMIC_FTRACE_WITH_ARGS, as a requirement for that config
  is to have ftrace always pass a valid ftrace_regs to the callbacks.
  That also means that the architecture supports ftrace_regs_get_stack_pointer()
  Microblaze does not set HAVE_DYNAMIC_FTRACE_WITH_ARGS nor does it
  implement ftrace_regs_get_stack_pointer() which caused it to fail to
  build. Only implement the "true parent" logic if an architecture has
  that config set.

Changes since v1: https://lore.kernel.org/all/20241214182138.4e7984a2@batman.local.home/

- Removed the hash-ptr fix as Linus was unhappy with the code it was
  fixing and I have another series to address that. It didn't even
  belong in this pull request, as this is the ftrace topic and that
  was a tracing topic.

- Properly fix the function_get_true_parent_ip(), which wasn't a
  microblaze issue at all, but an issue for any architecture that
  does not support HAVE_DYNAMIC_FTRACE_WITH_ARGS.
Re: [for-linus v2][PATCH 0/2] ftrace: Fixes for v6.13
Posted by Steven Rostedt 1 year ago
On Tue, 17 Dec 2024 11:18:40 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> Ftrace fixes for 6.13:
> 
> - Always try to initialize the idle functions when graph tracer starts
> 
>   A bug was found that when a CPU is offline when graph tracing starts
>   and then comes online, that CPU is not traced. The fix to that was
>   to move the initialization of the idle shadow stack over to the
>   hot plug online logic, which also handle onlined CPUs. The issue was
>   that it removed the initialization of the shadow stack when graph tracing
>   starts, but the callbacks to the hot plug logic do nothing if graph
>   tracing isn't currently running. Although that fix fixed the onlining
>   of a CPU during tracing, it broke the CPUs that were already online.
> 
> - Have microblaze not try to get the "true parent" in function tracing
> 
>   If function tracing and graph tracing are both enabled at the same time
>   the parent of the functions traced by the function tracer may sometimes
>   be the graph tracing trampoline. The graph tracing hijacks the return
>   pointer of the function to trace it, but that can interfere with the
>   function tracing parent output. This was fixed by using the
>   ftrace_graph_ret_addr() function passing in the kernel stack pointer
>   using the ftrace_regs_get_stack_pointer() function. But Al Viro reported
>   that Microblaze does not implement the kernel_stack_pointer(regs)
>   helper function that ftrace_regs_get_stack_pointer() uses and fails
>   to compile when function graph tracing is enabled.
> 
>   It was first thought that this was a microblaze issue, but the real
>   cause is that this only works when an architecture implements
>   HAVE_DYNAMIC_FTRACE_WITH_ARGS, as a requirement for that config
>   is to have ftrace always pass a valid ftrace_regs to the callbacks.
>   That also means that the architecture supports ftrace_regs_get_stack_pointer()
>   Microblaze does not set HAVE_DYNAMIC_FTRACE_WITH_ARGS nor does it
>   implement ftrace_regs_get_stack_pointer() which caused it to fail to
>   build. Only implement the "true parent" logic if an architecture has
>   that config set.
> 
> Changes since v1: https://lore.kernel.org/all/20241214182138.4e7984a2@batman.local.home/
> 
> - Removed the hash-ptr fix as Linus was unhappy with the code it was
>   fixing and I have another series to address that. It didn't even
>   belong in this pull request, as this is the ftrace topic and that
>   was a tracing topic.
> 
> - Properly fix the function_get_true_parent_ip(), which wasn't a
>   microblaze issue at all, but an issue for any architecture that
>   does not support HAVE_DYNAMIC_FTRACE_WITH_ARGS.


I forgot to append the diffstat and branch where this lives:

  git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace.git
ftrace/fixes

Head SHA1: 166438a432d76c68d3f0da60667248f3c2303d6c


Steven Rostedt (2):
      fgraph: Still initialize idle shadow stacks when starting
      ftrace: Do not find "true_parent" if HAVE_DYNAMIC_FTRACE_WITH_ARGS is not set

----
 kernel/trace/fgraph.c          | 8 +++++++-
 kernel/trace/trace_functions.c | 3 ++-
 2 files changed, 9 insertions(+), 2 deletions(-)