[v2,0/7] Refactor ftrace direct call APIs

  Differences since v1 [1]:
- Use a READ_ONCE() to read the direct call address in call_direct_funcs
- Added an Acked-by from Mark

This series refactors ftrace direct call APIs in preparation for arm64 support.
It is roughly a subset of [2] rebased on v6.3-rc2 and meant to be taken by
Steven's tree before all the arm64 specific bits.

The first patch was suggested by Steven in a review of [1], it makes it more
obvious to the caller that filters probably need to be freed when unregistering
a direct call.

The next three patches consolidate the two existing ftrace APIs for registering
direct calls. They are only split to make the reviewer's life easier.
Currently, there is both a _ftrace_direct and _ftrace_direct_multi API. Apart
from samples and selftests, there are no users of the _ftrace_direct API left
in-tree so this deletes it and renames the _ftrace_direct_multi API to
_ftrace_direct for simplicity.

The main benefit of this refactoring is that, with the API that's left, an
ftrace_ops backing a direct call will only ever point to one direct call. We can
therefore store the direct called trampoline address in the ops (patch 5) and,
in the future arm64 series, look it up from the ftrace trampoline. (in the
meantime, it makes call_direct_funcs a bit simpler too)

Ftrace direct calls technically don't need DYNAMIC_FTRACE_WITH_REGS so this
extends its support to DYNAMIC_FTRACE_WITH_ARGS (patch 6). arm64 won't support
DYNAMIC_FTRACE_WITH_REGS.

Finally, it fixes the ABI of the stub direct call trampoline used in ftrace
selftests.

This has been tested on x86_64 with:
1- CONFIG_FTRACE_SELFTEST
2- samples/ftrace/*.ko

1: https://lore.kernel.org/all/20230316173811.1223508-1-revest@chromium.org/T/#t
2: https://lore.kernel.org/all/20230207182135.2671106-1-revest@chromium.org/T/#t

Florent Revest (6):
  ftrace: Let unregister_ftrace_direct_multi() call ftrace_free_filter()
  ftrace: Replace uses of _ftrace_direct APIs with _ftrace_direct_multi
  ftrace: Remove the legacy _ftrace_direct API
  ftrace: Rename _ftrace_direct_multi APIs to _ftrace_direct APIs
  ftrace: Store direct called addresses in their ops
  ftrace: Make DIRECT_CALLS work WITH_ARGS and !WITH_REGS

Mark Rutland (1):
  ftrace: selftest: remove broken trace_direct_tramp

 arch/s390/kernel/mcount.S                   |   5 +
 arch/x86/kernel/ftrace_32.S                 |   5 +
 arch/x86/kernel/ftrace_64.S                 |   4 +
 include/linux/ftrace.h                      |  61 +--
 kernel/bpf/trampoline.c                     |  12 +-
 kernel/trace/Kconfig                        |   2 +-
 kernel/trace/ftrace.c                       | 438 ++------------------
 kernel/trace/trace_selftest.c               |  19 +-
 samples/Kconfig                             |   2 +-
 samples/ftrace/ftrace-direct-modify.c       |  10 +-
 samples/ftrace/ftrace-direct-multi-modify.c |   9 +-
 samples/ftrace/ftrace-direct-multi.c        |   5 +-
 samples/ftrace/ftrace-direct-too.c          |  10 +-
 samples/ftrace/ftrace-direct.c              |  10 +-
 14 files changed, 101 insertions(+), 491 deletions(-)
  

On Tue, Mar 21, 2023 at 03:04:17PM +0100, Florent Revest wrote:
> Differences since v1 [1]:
> - Use a READ_ONCE() to read the direct call address in call_direct_funcs
> - Added an Acked-by from Mark
> 
> This series refactors ftrace direct call APIs in preparation for arm64 support.
> It is roughly a subset of [2] rebased on v6.3-rc2 and meant to be taken by
> Steven's tree before all the arm64 specific bits.
> 
> The first patch was suggested by Steven in a review of [1], it makes it more
> obvious to the caller that filters probably need to be freed when unregistering
> a direct call.
> 
> The next three patches consolidate the two existing ftrace APIs for registering
> direct calls. They are only split to make the reviewer's life easier.
> Currently, there is both a _ftrace_direct and _ftrace_direct_multi API. Apart
> from samples and selftests, there are no users of the _ftrace_direct API left
> in-tree so this deletes it and renames the _ftrace_direct_multi API to
> _ftrace_direct for simplicity.
> 
> The main benefit of this refactoring is that, with the API that's left, an
> ftrace_ops backing a direct call will only ever point to one direct call. We can
> therefore store the direct called trampoline address in the ops (patch 5) and,
> in the future arm64 series, look it up from the ftrace trampoline. (in the
> meantime, it makes call_direct_funcs a bit simpler too)
> 
> Ftrace direct calls technically don't need DYNAMIC_FTRACE_WITH_REGS so this
> extends its support to DYNAMIC_FTRACE_WITH_ARGS (patch 6). arm64 won't support
> DYNAMIC_FTRACE_WITH_REGS.
> 
> Finally, it fixes the ABI of the stub direct call trampoline used in ftrace
> selftests.
> 
> This has been tested on x86_64 with:
> 1- CONFIG_FTRACE_SELFTEST
> 2- samples/ftrace/*.ko
> 
> 1: https://lore.kernel.org/all/20230316173811.1223508-1-revest@chromium.org/T/#t
> 2: https://lore.kernel.org/all/20230207182135.2671106-1-revest@chromium.org/T/#t
> 
> Florent Revest (6):
>   ftrace: Let unregister_ftrace_direct_multi() call ftrace_free_filter()
>   ftrace: Replace uses of _ftrace_direct APIs with _ftrace_direct_multi
>   ftrace: Remove the legacy _ftrace_direct API
>   ftrace: Rename _ftrace_direct_multi APIs to _ftrace_direct APIs
>   ftrace: Store direct called addresses in their ops
>   ftrace: Make DIRECT_CALLS work WITH_ARGS and !WITH_REGS

lgtm

Acked-by: Jiri Olsa <jolsa@kernel.org>

jirka

> 
> Mark Rutland (1):
>   ftrace: selftest: remove broken trace_direct_tramp
> 
>  arch/s390/kernel/mcount.S                   |   5 +
>  arch/x86/kernel/ftrace_32.S                 |   5 +
>  arch/x86/kernel/ftrace_64.S                 |   4 +
>  include/linux/ftrace.h                      |  61 +--
>  kernel/bpf/trampoline.c                     |  12 +-
>  kernel/trace/Kconfig                        |   2 +-
>  kernel/trace/ftrace.c                       | 438 ++------------------
>  kernel/trace/trace_selftest.c               |  19 +-
>  samples/Kconfig                             |   2 +-
>  samples/ftrace/ftrace-direct-modify.c       |  10 +-
>  samples/ftrace/ftrace-direct-multi-modify.c |   9 +-
>  samples/ftrace/ftrace-direct-multi.c        |   5 +-
>  samples/ftrace/ftrace-direct-too.c          |  10 +-
>  samples/ftrace/ftrace-direct.c              |  10 +-
>  14 files changed, 101 insertions(+), 491 deletions(-)
> 
> -- 
> 2.40.0.rc2.332.ga46443480c-goog
>
  

So I applied this to my for-next branch as the first patches starting from
v6.3-rc3. Last commit (patch 7) is fee86a4ed536f4e521f3a4530242e152dd2a466b

The branch is here:

  git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace.git
    trace/for-next

  https://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace.git/commit/?h=trace/for-next&id=fee86a4ed536f4e521f3a4530242e152dd2a466b

Since that commit only contains the updates for the direct trampolines that
ARM64 needs, and I'm not going to rebase that branch, you can just merge it
into the ARM64 tree so that you can base your changes on it. Make sure you
merge that commit, not the branch, as I have more tracing specific patches
on top of that commit.

Make sure to write a nice change log message in why you are merging that
branch, so that Linus knows about it. I'm Cc'ing Linus now so that he's
aware, but the change log should remind him why you pulled in my branch.

Again, at the above mentioned commit, that branch only contains the updates
you need to make the direct trampolines work on ARM64.

-- Steve


On Tue, 21 Mar 2023 15:04:17 +0100
Florent Revest <revest@chromium.org> wrote:

> Differences since v1 [1]:
> - Use a READ_ONCE() to read the direct call address in call_direct_funcs
> - Added an Acked-by from Mark
> 
> This series refactors ftrace direct call APIs in preparation for arm64 support.
> It is roughly a subset of [2] rebased on v6.3-rc2 and meant to be taken by
> Steven's tree before all the arm64 specific bits.
> 
> The first patch was suggested by Steven in a review of [1], it makes it more
> obvious to the caller that filters probably need to be freed when unregistering
> a direct call.
> 
> The next three patches consolidate the two existing ftrace APIs for registering
> direct calls. They are only split to make the reviewer's life easier.
> Currently, there is both a _ftrace_direct and _ftrace_direct_multi API. Apart
> from samples and selftests, there are no users of the _ftrace_direct API left
> in-tree so this deletes it and renames the _ftrace_direct_multi API to
> _ftrace_direct for simplicity.
> 
> The main benefit of this refactoring is that, with the API that's left, an
> ftrace_ops backing a direct call will only ever point to one direct call. We can
> therefore store the direct called trampoline address in the ops (patch 5) and,
> in the future arm64 series, look it up from the ftrace trampoline. (in the
> meantime, it makes call_direct_funcs a bit simpler too)
> 
> Ftrace direct calls technically don't need DYNAMIC_FTRACE_WITH_REGS so this
> extends its support to DYNAMIC_FTRACE_WITH_ARGS (patch 6). arm64 won't support
> DYNAMIC_FTRACE_WITH_REGS.
> 
> Finally, it fixes the ABI of the stub direct call trampoline used in ftrace
> selftests.
> 
> This has been tested on x86_64 with:
> 1- CONFIG_FTRACE_SELFTEST
> 2- samples/ftrace/*.ko
> 
> 1: https://lore.kernel.org/all/20230316173811.1223508-1-revest@chromium.org/T/#t
> 2: https://lore.kernel.org/all/20230207182135.2671106-1-revest@chromium.org/T/#t
> 
> Florent Revest (6):
>   ftrace: Let unregister_ftrace_direct_multi() call ftrace_free_filter()
>   ftrace: Replace uses of _ftrace_direct APIs with _ftrace_direct_multi
>   ftrace: Remove the legacy _ftrace_direct API
>   ftrace: Rename _ftrace_direct_multi APIs to _ftrace_direct APIs
>   ftrace: Store direct called addresses in their ops
>   ftrace: Make DIRECT_CALLS work WITH_ARGS and !WITH_REGS
> 
> Mark Rutland (1):
>   ftrace: selftest: remove broken trace_direct_tramp
> 
>  arch/s390/kernel/mcount.S                   |   5 +
>  arch/x86/kernel/ftrace_32.S                 |   5 +
>  arch/x86/kernel/ftrace_64.S                 |   4 +
>  include/linux/ftrace.h                      |  61 +--
>  kernel/bpf/trampoline.c                     |  12 +-
>  kernel/trace/Kconfig                        |   2 +-
>  kernel/trace/ftrace.c                       | 438 ++------------------
>  kernel/trace/trace_selftest.c               |  19 +-
>  samples/Kconfig                             |   2 +-
>  samples/ftrace/ftrace-direct-modify.c       |  10 +-
>  samples/ftrace/ftrace-direct-multi-modify.c |   9 +-
>  samples/ftrace/ftrace-direct-multi.c        |   5 +-
>  samples/ftrace/ftrace-direct-too.c          |  10 +-
>  samples/ftrace/ftrace-direct.c              |  10 +-
>  14 files changed, 101 insertions(+), 491 deletions(-)
>
  

On Thu, 23 Mar 2023 19:43:55 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> Since that commit only contains the updates for the direct trampolines that
> ARM64 needs, and I'm not going to rebase that branch, you can just merge it
> into the ARM64 tree so that you can base your changes on it. Make sure you
> merge that commit, not the branch, as I have more tracing specific patches
> on top of that commit.

I just made it easier for you. I created a signed tag: trace-direct-v6.3-rc3

So just pull from:

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

tag: trace-direct-v6.3-rc3

-- Steve
  

On Thu, Mar 23, 2023 at 10:00:42PM -0400, Steven Rostedt wrote:
> On Thu, 23 Mar 2023 19:43:55 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > Since that commit only contains the updates for the direct trampolines that
> > ARM64 needs, and I'm not going to rebase that branch, you can just merge it
> > into the ARM64 tree so that you can base your changes on it. Make sure you
> > merge that commit, not the branch, as I have more tracing specific patches
> > on top of that commit.
> 
> I just made it easier for you. I created a signed tag: trace-direct-v6.3-rc3
> 
> So just pull from:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace.git
> 
> tag: trace-direct-v6.3-rc3

Thanks Steve; much appreciated!

Florent, can you post a new spinf of the remaining arm64 bits rebased atop
that?

Thanks,
Mark.
  

On Fri, Mar 24, 2023 at 12:46 PM Mark Rutland <mark.rutland@arm.com> wrote:
>
> On Thu, Mar 23, 2023 at 10:00:42PM -0400, Steven Rostedt wrote:
> > On Thu, 23 Mar 2023 19:43:55 -0400
> > Steven Rostedt <rostedt@goodmis.org> wrote:
> >
> > > Since that commit only contains the updates for the direct trampolines that
> > > ARM64 needs, and I'm not going to rebase that branch, you can just merge it
> > > into the ARM64 tree so that you can base your changes on it. Make sure you
> > > merge that commit, not the branch, as I have more tracing specific patches
> > > on top of that commit.
> >
> > I just made it easier for you. I created a signed tag: trace-direct-v6.3-rc3
> >
> > So just pull from:
> >
> >   git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace.git
> >
> > tag: trace-direct-v6.3-rc3

Awesome! Thank you Steve :D

> Thanks Steve; much appreciated!
>
> Florent, can you post a new spinf of the remaining arm64 bits rebased atop
> that?

Done! :)
https://lore.kernel.org/bpf/20230324171451.2752302-1-revest@chromium.org/