Re: [PATCH v2 0/7] Refactor ftrace direct call APIs

From: Jiri Olsa <olsajiri@gmail.com>
To: Florent Revest <revest@chromium.org>
Cc: linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org,
	rostedt@goodmis.org, mhiramat@kernel.org, mark.rutland@arm.com,
	ast@kernel.org, daniel@iogearbox.net, kpsingh@kernel.org
Subject: Re: [PATCH v2 0/7] Refactor ftrace direct call APIs
Date: Tue, 21 Mar 2023 18:06:31 +0100	[thread overview]
Message-ID: <ZBnkFz4aXwOjqInu@krava> (raw)
In-Reply-To: <20230321140424.345218-1-revest@chromium.org>

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
>