From: Jiri Olsa <jolsa@redhat.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: bpf@vger.kernel.org, linux-kernel@vger.kernel.org,
Alexei Starovoitov <ast@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Andrii Nakryiko <andriin@fb.com>
Subject: Re: [PATCH 7/8] ftrace: Add multi direct modify interface
Date: Fri, 15 Oct 2021 14:05:25 +0200 [thread overview]
Message-ID: <YWluhdDMfkNGwlhz@krava> (raw)
In-Reply-To: <20211014162819.5c85618b@gandalf.local.home>
On Thu, Oct 14, 2021 at 04:28:19PM -0400, Steven Rostedt wrote:
> On Fri, 8 Oct 2021 11:13:35 +0200
> Jiri Olsa <jolsa@redhat.com> wrote:
>
> > + /*
> > + * Shutdown the ops, change 'direct' pointer for each
> > + * ops entry in direct_functions hash and startup the
> > + * ops back again.
> > + *
> > + * Note there is no callback called for @ops object after
> > + * this ftrace_shutdown call until ftrace_startup is called
> > + * later on.
> > + */
> > + err = ftrace_shutdown(ops, 0);
> > + if (err)
> > + goto out_unlock;
>
> I believe I said before that we can do this by adding a stub ops that match
> all the functions with the direct ops being modified. This will cause the
> loop function to be called, which will call the direct function helper,
> which will then call the direct function that is found. That is, there is
> no "pause" in calling the direct callers. Either the old direct is called,
> or the new one. When the function returns, all are calling the new one.
>
> That is, instead of:
>
> [ Changing direct call from my_direct_1 to my_direct_2 ]
>
> <traced_func>:
> call my_direct_1
>
> ||||||||||||||||||||
> vvvvvvvvvvvvvvvvvvvv
>
> <traced_func>:
> nop
>
> ||||||||||||||||||||
> vvvvvvvvvvvvvvvvvvvv
>
> <traced_func>:
> call my_direct_2
>
>
> We have it do:
>
> <traced_func>:
> call my_direct_1
>
> ||||||||||||||||||||
> vvvvvvvvvvvvvvvvvvvv
>
> <traced_func>:
> call ftrace_caller
>
>
> <ftrace_caller>:
> [..]
> call ftrace_ops_list_func
>
>
> ftrace_ops_list_func()
> {
> ops->func() -> direct_helper -> set rax to my_direct_1 or my_direct_2
> }
>
> call rax (to either my_direct_1 or my_direct_2
nice! :) I did not see that as a problem and something that can be
done later, thanks for doing this
>
> ||||||||||||||||||||
> vvvvvvvvvvvvvvvvvvvv
>
> <traced_func>:
> call my_direct_2
>
>
> I did this on top of this patch:
ATM I'm bit stuck on the bpf side of this whole change, I'll test
it with my other changes when I unstuck myself ;-)
thanks,
jirka
>
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
> kernel/trace/ftrace.c | 33 ++++++++++++++++++++-------------
> 1 file changed, 20 insertions(+), 13 deletions(-)
>
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 30120342176e..7ad1e8ae5855 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -5561,8 +5561,12 @@ EXPORT_SYMBOL_GPL(unregister_ftrace_direct_multi);
> */
> int modify_ftrace_direct_multi(struct ftrace_ops *ops, unsigned long addr)
> {
> - struct ftrace_hash *hash = ops->func_hash->filter_hash;
> + struct ftrace_hash *hash;
> struct ftrace_func_entry *entry, *iter;
> + static struct ftrace_ops tmp_ops = {
> + .func = ftrace_stub,
> + .flags = FTRACE_OPS_FL_STUB,
> + };
> int i, size;
> int err;
>
> @@ -5572,21 +5576,22 @@ int modify_ftrace_direct_multi(struct ftrace_ops *ops, unsigned long addr)
> return -EINVAL;
>
> mutex_lock(&direct_mutex);
> - mutex_lock(&ftrace_lock);
> +
> + /* Enable the tmp_ops to have the same functions as the direct ops */
> + ftrace_ops_init(&tmp_ops);
> + tmp_ops.func_hash = ops->func_hash;
> +
> + err = register_ftrace_function(&tmp_ops);
> + if (err)
> + goto out_direct;
>
> /*
> - * Shutdown the ops, change 'direct' pointer for each
> - * ops entry in direct_functions hash and startup the
> - * ops back again.
> - *
> - * Note there is no callback called for @ops object after
> - * this ftrace_shutdown call until ftrace_startup is called
> - * later on.
> + * Now the ftrace_ops_list_func() is called to do the direct callers.
> + * We can safely change the direct functions attached to each entry.
> */
> - err = ftrace_shutdown(ops, 0);
> - if (err)
> - goto out_unlock;
> + mutex_lock(&ftrace_lock);
>
> + hash = ops->func_hash->filter_hash;
> size = 1 << hash->size_bits;
> for (i = 0; i < size; i++) {
> hlist_for_each_entry(iter, &hash->buckets[i], hlist) {
> @@ -5597,10 +5602,12 @@ int modify_ftrace_direct_multi(struct ftrace_ops *ops, unsigned long addr)
> }
> }
>
> - err = ftrace_startup(ops, 0);
> + /* Removing the tmp_ops will add the updated direct callers to the functions */
> + unregister_ftrace_function(&tmp_ops);
>
> out_unlock:
> mutex_unlock(&ftrace_lock);
> + out_direct:
> mutex_unlock(&direct_mutex);
> return err;
> }
> --
> 2.31.1
>
next prev parent reply other threads:[~2021-10-15 12:05 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-08 9:13 [PATCHv2 0/8] x86/ftrace: Add direct batch interface Jiri Olsa
2021-10-08 9:13 ` [PATCH 1/8] x86/ftrace: Remove extra orig rax move Jiri Olsa
2021-10-08 9:13 ` [PATCH 2/8] x86/ftrace: Remove fault protection code in prepare_ftrace_return Jiri Olsa
2021-10-08 9:13 ` [PATCH 3/8] x86/ftrace: Make function graph use ftrace directly Jiri Olsa
2021-10-08 9:13 ` [PATCH 4/8] tracing: Add trampoline/graph selftest Jiri Olsa
2021-10-08 9:13 ` [PATCH 5/8] ftrace: Add ftrace_add_rec_direct function Jiri Olsa
2021-10-08 9:13 ` [PATCH 6/8] ftrace: Add multi direct register/unregister interface Jiri Olsa
2021-10-08 9:13 ` [PATCH 7/8] ftrace: Add multi direct modify interface Jiri Olsa
2021-10-14 20:28 ` Steven Rostedt
2021-10-15 12:05 ` Jiri Olsa [this message]
2021-10-15 14:05 ` Steven Rostedt
2021-10-16 11:39 ` Jiri Olsa
2021-10-19 2:10 ` Steven Rostedt
2021-10-19 13:19 ` Jiri Olsa
2021-10-19 13:26 ` Jiri Olsa
2021-10-19 13:32 ` Steven Rostedt
2021-10-19 14:03 ` Jiri Olsa
2021-10-19 14:44 ` Steven Rostedt
2021-10-19 14:47 ` Jiri Olsa
2021-10-08 9:13 ` [PATCH 8/8] ftrace/samples: Add multi direct interface test module Jiri Olsa
2021-10-26 19:23 ` Guenter Roeck
2021-10-26 19:29 ` Steven Rostedt
2021-10-13 9:01 ` [PATCHv2 0/8] x86/ftrace: Add direct batch interface Heiko Carstens
2021-10-13 12:25 ` Jiri Olsa
-- strict thread matches above, loose matches on Subject: below --
2021-08-31 9:50 [PATCH " Jiri Olsa
2021-08-31 9:50 ` [PATCH 7/8] ftrace: Add multi direct modify interface Jiri Olsa
2021-09-14 21:41 ` Steven Rostedt
2021-09-15 21:47 ` Steven Rostedt
2021-09-16 19:49 ` Jiri Olsa
2021-09-16 20:41 ` Steven Rostedt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YWluhdDMfkNGwlhz@krava \
--to=jolsa@redhat.com \
--cc=andriin@fb.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=linux-kernel@vger.kernel.org \
--cc=rostedt@goodmis.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).