[GIT PULL] ftrace: Rewrite of function graph to allow multiple users

[GIT PULL] ftrace: Rewrite of function graph to allow multiple users

[Steven Rostedt]

Linus,

ftrace: Rewrite of function graph tracer

Up until now, the function graph tracer could only have a single user
attached to it. If another user tried to attach to the function graph
tracer while one was already attached, it would fail. Allowing function
graph tracer to have more than one user has been asked for since 2009, but
it required a rewrite to the logic to pull it off so it never happened.
Until now!

There's three systems that trace the return of a function. That is
kretprobes, function graph tracer, and BPF. kretprobes and function graph
tracing both do it similarly. The difference is that kretprobes uses a
shadow stack per callback and function graph tracer creates a shadow stack
for all tasks. The function graph tracer method makes it possible to trace
the return of all functions. As kretprobes now needs that feature too,
allowing it to use function graph tracer was needed. BPF also wants to
trace the return of many probes and its method doesn't scale either.
Having it use function graph tracer would improve that.

By allowing function graph tracer to have multiple users allows both
kretprobes and BPF to use function graph tracer in these cases. This will
allow kretprobes code to be removed in the future as it's version will no
longer be needed. Note, function graph tracer is only limited to 16
simultaneous users, due to shadow stack size and allocated slots.


Please pull the latest ftrace-v6.11 tree, which can be found at:


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

Tag SHA1: d370074e4673a7f2dca52f93552a28714e8f65d7
Head SHA1: b576d375b536568c85d42c15a189f6b6fdd75b74


Jiapeng Chong (2):
      fgraph: Remove some unused functions
      fgraph: Use str_plural() in test_graph_storage_single()

Marilene A Garcia (1):
      ftrace: Add missing kerneldoc parameters to unregister_ftrace_direct()

Masami Hiramatsu (Google) (3):
      function_graph: Handle tail calls for stack unwinding
      function_graph: Use a simple LRU for fgraph_array index number
      ftrace: Add multiple fgraph storage selftest

Steven Rostedt (Google) (27):
      ftrace: Add subops logic to allow one ops to manage many
      ftrace: Allow subops filtering to be modified
      function_graph: Add pid tracing back to function graph tracer
      function_graph: Use for_each_set_bit() in __ftrace_return_to_handler()
      function_graph: Use bitmask to loop on fgraph entry
      function_graph: Use static_call and branch to optimize entry function
      function_graph: Use static_call and branch to optimize return function
      selftests/ftrace: Add function_graph tracer to func-filter-pid test
      selftests/ftrace: Add fgraph-multi.tc test
      ftrace: Add back ftrace_update_trampoline() to ftrace_update_pid_func()
      ftrace/selftests: Fix pid test with function graph not showing pids
      ftrace: Rename dup_hash() and comment it
      ftrace: Remove "filter_hash" parameter from __ftrace_hash_rec_update()
      ftrace: Add comments to ftrace_hash_rec_disable/enable()
      ftrace: Convert "inc" parameter to bool in ftrace_hash_rec_update_modify()
      ftrace: Add comments to ftrace_hash_move() and friends
      ftrace: Declare function_trace_op in header to quiet sparse warning
      ftrace: Assign ftrace_list_end to ftrace_ops_list type cast to RCU
      ftrace: Assign RCU list variable with rcu_assign_ptr()
      ftrace: Fix prototypes for ftrace_startup/shutdown_subops()
      function_graph: Make fgraph_do_direct static key static
      function_graph: Do not update pid func if CONFIG_DYNAMIC_FTRACE not enabled
      function_graph: Rename BYTE_NUMBER to CHAR_NUMBER in selftests
      function_graph: Make fgraph_update_pid_func() a stub for !DYNAMIC_FTRACE
      function_graph: Fix up ftrace_graph_ret_addr()
      function_graph: Everyone uses HAVE_FUNCTION_GRAPH_RET_ADDR_PTR, remove it
      function_graph: Add READ_ONCE() when accessing fgraph_array[]

Steven Rostedt (VMware) (15):
      function_graph: Convert ret_stack to a series of longs
      fgraph: Use BUILD_BUG_ON() to make sure we have structures divisible by long
      function_graph: Add an array structure that will allow multiple callbacks
      function_graph: Allow multiple users to attach to function graph
      function_graph: Remove logic around ftrace_graph_entry and return
      ftrace/function_graph: Pass fgraph_ops to function graph callbacks
      ftrace: Allow function_graph tracer to be enabled in instances
      ftrace: Allow ftrace startup flags to exist without dynamic ftrace
      function_graph: Have the instances use their own ftrace_ops for filtering
      function_graph: Add "task variables" per task for fgraph_ops
      function_graph: Move set_graph_function tests to shadow stack global var
      function_graph: Move graph depth stored data to shadow stack global var
      function_graph: Move graph notrace bit to shadow stack global var
      function_graph: Implement fgraph_reserve_data() and fgraph_retrieve_data()
      function_graph: Add selftest for passing local variables

Tatsuya S (1):
      ftrace: Hide one more entry in stack trace when ftrace_pid is enabled

----
 Documentation/trace/ftrace-design.rst              |   12 -
 arch/arm64/include/asm/ftrace.h                    |   11 -
 arch/csky/include/asm/ftrace.h                     |    2 -
 arch/loongarch/include/asm/ftrace.h                |    1 -
 arch/powerpc/include/asm/ftrace.h                  |    2 -
 arch/riscv/include/asm/ftrace.h                    |    1 -
 arch/s390/include/asm/ftrace.h                     |    1 -
 arch/x86/include/asm/ftrace.h                      |    2 -
 include/linux/ftrace.h                             |   48 +-
 include/linux/sched.h                              |    2 +-
 include/linux/trace_recursion.h                    |   39 -
 kernel/trace/fgraph.c                              | 1054 ++++++++++++++++----
 kernel/trace/ftrace.c                              |  688 +++++++++++--
 kernel/trace/ftrace_internal.h                     |   18 +-
 kernel/trace/trace.h                               |   93 +-
 kernel/trace/trace_functions.c                     |   15 +-
 kernel/trace/trace_functions_graph.c               |   96 +-
 kernel/trace/trace_irqsoff.c                       |   10 +-
 kernel/trace/trace_sched_wakeup.c                  |   10 +-
 kernel/trace/trace_selftest.c                      |  259 ++++-
 .../selftests/ftrace/test.d/ftrace/fgraph-multi.tc |  103 ++
 .../ftrace/test.d/ftrace/func-filter-pid.tc        |   29 +-
 22 files changed, 2055 insertions(+), 441 deletions(-)
 create mode 100644 tools/testing/selftests/ftrace/test.d/ftrace/fgraph-multi.tc
---------------------------

Re: [GIT PULL] ftrace: Rewrite of function graph to allow multiple users

[Linus Torvalds]

On Tue, 16 Jul 2024 at 13:27, Steven Rostedt <rostedt@goodmis.org> wrote:
>
> Masami Hiramatsu (Google) (3):
> Steven Rostedt (Google) (27):
> Steven Rostedt (VMware) (15):

So this is getting annoying. Now the company name literally means that
"git shortlog" shows you as two different people.

You're not the only one to do it, but the list of people who do it is
not all that long, and it doesn't actually seem to be a company rule
since (a) you've done it across companies and (b) other people at
google don't do it.

The company you work at does not define you as a person. Really. Is
there some reason why this is happening?

Can it be useful to see where people are working? Sure. But does it
have to be in every commit and affect things like 'git shortlog"? No.

Is there a company rule that just most others are happily ignoring?

               Linus

Re: [GIT PULL] ftrace: Rewrite of function graph to allow multiple users

[pr-tracker-bot]

The pull request you sent on Tue, 16 Jul 2024 16:27:14 -0400:

> git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace.git ftrace-v6.11

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/70045bfc4cd5fef44ada25fa3367329eba98731a

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html

Re: [GIT PULL] ftrace: Rewrite of function graph to allow multiple users

[Steven Rostedt]

On Thu, 18 Jul 2024 14:01:13 -0700
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Tue, 16 Jul 2024 at 13:27, Steven Rostedt <rostedt@goodmis.org> wrote:
> >
> > Masami Hiramatsu (Google) (3):
> > Steven Rostedt (Google) (27):
> > Steven Rostedt (VMware) (15):  
> 
> So this is getting annoying. Now the company name literally means that
> "git shortlog" shows you as two different people.
> 
> You're not the only one to do it, but the list of people who do it is
> not all that long, and it doesn't actually seem to be a company rule
> since (a) you've done it across companies and (b) other people at
> google don't do it.
> 
> The company you work at does not define you as a person. Really. Is
> there some reason why this is happening?
> 
> Can it be useful to see where people are working? Sure. But does it
> have to be in every commit and affect things like 'git shortlog"? No.
> 
> Is there a company rule that just most others are happily ignoring?

I started this when I was at Red Hat because there was pressure for me
to use my Red Hat email. I only use my "goodmis.org" email, and that's
the one I use for my upstream work no matter who I work for. But the
company is the one that pays me to do the work.

When I started at VMware, it was company policy to use the vmware
email. Dirk for his part was able to get an exception for me and
others, but we had to add that "(VMware)" to our commits. It's a way to
show who owns the code.

So no, I'm not different people, but I wrote that code while at
different companies, and the code with "(VMware)" is to be copyright
VMware, and the code with "(Google)" is copyright Google.

Basically, it's just to show who owns the copyright.

-- Steve

Re: [GIT PULL] ftrace: Rewrite of function graph to allow multiple users

[Linus Torvalds]

On Thu, 18 Jul 2024 at 14:29, Steven Rostedt <rostedt@goodmis.org> wrote:
>
> Basically, it's just to show who owns the copyright.

.. but since basically nobody else does that, it's worthless.

You'd have to go back to the email address and date range *anyway* for
everybody else, so you doing it adds no value - and everybody doing it
would be just hugely annoying.

End result: unless there's some actual company policy, please just
don't do this. Because your "copyright" excuse is clearly just empty
words.

          Linus

Re: [GIT PULL] ftrace: Rewrite of function graph to allow multiple users

[Steven Rostedt]

On Thu, 18 Jul 2024 14:54:11 -0700
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Thu, 18 Jul 2024 at 14:29, Steven Rostedt <rostedt@goodmis.org> wrote:
> >
> > Basically, it's just to show who owns the copyright.  
> 
> .. but since basically nobody else does that, it's worthless.
> 
> You'd have to go back to the email address and date range *anyway* for
> everybody else, so you doing it adds no value - and everybody doing it
> would be just hugely annoying.
> 
> End result: unless there's some actual company policy, please just
> don't do this. Because your "copyright" excuse is clearly just empty
> words.

It was also a way to give credit to the ones that pay me. But whatever.
I've been doing this since 2013, not sure why it is now such an annoyance.
Was it just because patches that have been living in my repo since I was at
VMware finally made it forward so that both companies showed up in one pull
request? Or has this always been an annoyance?

I'll remove it, but it just feels wrong to me, as I'm no longer giving
credit to the one funding my work, and I have no plans on switching to my
google email.

Note, I have commits with it already, so this change will only happen for
new code.

-- Steve

Re: [GIT PULL] ftrace: Rewrite of function graph to allow multiple users

[Mathieu Desnoyers]

On 18-Jul-2024 06:55:47 PM, Steven Rostedt wrote:
> On Thu, 18 Jul 2024 14:54:11 -0700
> Linus Torvalds <torvalds@linux-foundation.org> wrote:
> 
> > On Thu, 18 Jul 2024 at 14:29, Steven Rostedt <rostedt@goodmis.org> wrote:
> > >
> > > Basically, it's just to show who owns the copyright.  
> > 
> > .. but since basically nobody else does that, it's worthless.
> > 
> > You'd have to go back to the email address and date range *anyway* for
> > everybody else, so you doing it adds no value - and everybody doing it
> > would be just hugely annoying.
> > 
> > End result: unless there's some actual company policy, please just
> > don't do this. Because your "copyright" excuse is clearly just empty
> > words.
> 
> It was also a way to give credit to the ones that pay me. But whatever.
> I've been doing this since 2013, not sure why it is now such an annoyance.
> Was it just because patches that have been living in my repo since I was at
> VMware finally made it forward so that both companies showed up in one pull
> request? Or has this always been an annoyance?
> 
> I'll remove it, but it just feels wrong to me, as I'm no longer giving
> credit to the one funding my work, and I have no plans on switching to my
> google email.
> 
> Note, I have commits with it already, so this change will only happen for
> new code.

There appears to be three sets of constraints here:

1) The code author/maintainer wishes to use an email address that stays
   valid across employers (name@kernel.org is another example of this),
   which is a way to stay reliably reachable over his entire career,

2) The employer requires attribution for tracking copyright ownership.
   This is typically done by requiring employees to use their corporate
   email address for code submissions, which is incompatible with (1).

3) Adding the employer in the form "First Last (Employer)" as a
   work-around creates separate author entries in the git shortlog.

I see a few possible solutions there:

- We can teach git to know about this "First Last (Employer)" formatting
  and combine duplicates.

- We can move the employer attribution to the email address instead,
  e.g.: name.corp@kernel.org or name+corp@kernel.org (or whatever we
  figure out looks OK).

Thoughts ?

Thanks,

Mathieu

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

Re: [GIT PULL] ftrace: Rewrite of function graph to allow multiple users

[Steven Rostedt]

On Fri, 19 Jul 2024 09:43:11 -0400
Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:

> I see a few possible solutions there:
> 
> - We can teach git to know about this "First Last (Employer)" formatting
>   and combine duplicates.
> 
> - We can move the employer attribution to the email address instead,
>   e.g.: name.corp@kernel.org or name+corp@kernel.org (or whatever we
>   figure out looks OK).
> 
> Thoughts ?

For now I changed my git config to:

  [author]
        name = Steven Rostedt
  [commiter]
        name = Steven Rostedt
  [user]
        name = Steven Rostedt (Google)
        email = rostedt@goodmis.org

Which makes what shows up in the shortlog with just my name (and git
blame as well), but the commits still have:

  Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>

Which may be a good compromise.

-- Steve