LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: Jiri Olsa <jolsa@redhat.com>
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: Thu, 14 Oct 2021 16:28:19 -0400	[thread overview]
Message-ID: <20211014162819.5c85618b@gandalf.local.home> (raw)
In-Reply-To: <20211008091336.33616-8-jolsa@kernel.org>

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

 ||||||||||||||||||||
 vvvvvvvvvvvvvvvvvvvv

  <traced_func>:
     call my_direct_2


I did this on top of this patch:

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


  reply	other threads:[~2021-10-14 20:28 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 [this message]
2021-10-15 12:05     ` Jiri Olsa
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=20211014162819.5c85618b@gandalf.local.home \
    --to=rostedt@goodmis.org \
    --cc=andriin@fb.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --subject='Re: [PATCH 7/8] ftrace: Add multi direct modify interface' \
    /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

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).