public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
From: Jiri Olsa <olsajiri@gmail.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: "Jiri Olsa" <olsajiri@gmail.com>,
	"Mark Rutland" <mark.rutland@arm.com>,
	"Steven Rostedt" <rostedt@kernel.org>,
	"Florent Revest" <revest@google.com>,
	bpf@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-trace-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	"Alexei Starovoitov" <ast@kernel.org>,
	"Daniel Borkmann" <daniel@iogearbox.net>,
	"Andrii Nakryiko" <andrii@kernel.org>,
	"Menglong Dong" <menglong8.dong@gmail.com>,
	"Naveen N Rao" <naveen@kernel.org>,
	"Michael Ellerman" <mpe@ellerman.id.au>,
	"Björn Töpel" <bjorn@rivosinc.com>,
	"Andy Chiu" <andybnac@gmail.com>,
	"Alexandre Ghiti" <alexghiti@rivosinc.com>,
	"Palmer Dabbelt" <palmer@dabbelt.com>
Subject: Re: [RFC 00/10] ftrace,bpf: Use single direct ops for bpf trampolines
Date: Thu, 31 Jul 2025 22:40:07 +0200	[thread overview]
Message-ID: <aIvUp_88v84Uw-lQ@krava> (raw)
In-Reply-To: <20250730095641.660800b1@gandalf.local.home>

On Wed, Jul 30, 2025 at 09:56:41AM -0400, Steven Rostedt wrote:
> On Wed, 30 Jul 2025 13:19:51 +0200
> Jiri Olsa <olsajiri@gmail.com> wrote:
> 
> > so it's all work on PoC stage, the idea is to be able to attach many
> > (like 20,30,40k) functions to their trampolines quickly, which at the
> > moment is slow because all the involved interfaces work with just single
> > function/tracempoline relation
> 
> Sounds like you are reinventing the ftrace mechanism itself. Which I warned
> against when I first introduced direct trampolines, which were purposely
> designed to do a few functions, not thousands. But, oh well.
> 
> 
> > Steven, please correct me if/when I'm wrong ;-)
> > 
> > IIUC in x86_64, IF there's just single ftrace_ops defined for the function,
> > it will bypass ftrace trampoline and call directly the direct trampoline
> > for the function, like:
> > 
> >    <foo>:
> >      call direct_trampoline
> >      ...
> 
> Yes.
> 
> And it will also do the same for normal ftrace functions. If you have:
> 
> struct ftrace_ops {
> 	.func = myfunc;
> };
> 
> It will create a trampoline that has:
> 
>       <tramp>
> 	...
> 	call myfunc
> 	...
> 	ret
> 
> On x86, I believe the ftrace_ops for myfunc is added to the trampoline,
> where as in arm, it's part of the function header. To modify it, it
> requires converting to the list operation (which ignores the ops
> parameter), then the ops at the function gets changed before it goes to the
> new function.
> 
> And if it is the only ops attached to a function foo, the function foo
> would have:
> 
>       <foo>
> 	call tramp
> 	...
> 
> But what's nice about this is that if you have 12 different ftrace_ops that
> each attach to a 1000 different functions, but no two ftrace_ops attach to
> the same function, they all do the above. No hash needed!
> 
> > 
> > IF there are other ftrace_ops 'users' on the same function, we execute
> > each of them like:
> > 
> >   <foo>:
> >     call ftrace_trampoline
> >       call ftrace_ops_1->func
> >       call ftrace_ops_2->func
> >       ...
> > 
> > with our direct ftrace_ops->func currently using ftrace_ops->direct_call
> > to return direct trampoline for the function:
> > 
> > 	-static void call_direct_funcs(unsigned long ip, unsigned long pip,
> > 	-                             struct ftrace_ops *ops, struct ftrace_regs *fregs)
> > 	-{
> > 	-       unsigned long addr = READ_ONCE(ops->direct_call);
> > 	-
> > 	-       if (!addr)
> > 	-               return;
> > 	-
> > 	-       arch_ftrace_set_direct_caller(fregs, addr);
> > 	-}
> > 
> > in the new changes it will do hash lookup (based on ip) for the direct
> > trampoline we want to execute:
> > 
> > 	+static void call_direct_funcs_hash(unsigned long ip, unsigned long pip,
> > 	+                                  struct ftrace_ops *ops, struct ftrace_regs *fregs)
> > 	+{
> > 	+       unsigned long addr;
> > 	+
> > 	+       addr = ftrace_find_rec_direct(ip);
> > 	+       if (!addr)
> > 	+               return;
> > 	+
> > 	+       arch_ftrace_set_direct_caller(fregs, addr);
> > 	+}
> 
> I think the above will work.
> 
> > 
> > still this is the slow path for the case where multiple ftrace_ops objects use
> > same function.. for the fast path we have the direct attachment as described above
> > 
> > sorry I probably forgot/missed discussion on this, but doing the fast path like in
> > x86_64 is not an option in arm, right?
> 
> That's a question for Mark, right?

yes, thanks for the other details

jirka


  reply	other threads:[~2025-07-31 20:43 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-29 10:28 [RFC 00/10] ftrace,bpf: Use single direct ops for bpf trampolines Jiri Olsa
2025-07-29 10:28 ` [RFC 01/10] ftrace: Make alloc_and_copy_ftrace_hash direct friendly Jiri Olsa
2025-07-29 10:28 ` [RFC 02/10] ftrace: Add register_ftrace_direct_hash function Jiri Olsa
2025-07-29 10:28 ` [RFC 03/10] ftrace: Add unregister_ftrace_direct_hash function Jiri Olsa
2025-07-29 10:28 ` [RFC 04/10] ftrace: Add modify_ftrace_direct_hash function Jiri Olsa
2025-07-29 10:28 ` [RFC 05/10] ftrace: Export some of hash related functions Jiri Olsa
2025-07-29 10:28 ` [RFC 06/10] ftrace: Use direct hash interface in direct functions Jiri Olsa
2025-07-29 10:28 ` [RFC 07/10] bpf: Add trampoline ip hash table Jiri Olsa
2025-07-29 10:28 ` [RFC 08/10] ftrace: Factor ftrace_ops ops_func interface Jiri Olsa
2025-07-29 10:28 ` [RFC 09/10] bpf: Remove ftrace_ops from bpf_trampoline object Jiri Olsa
2025-07-29 10:28 ` [RFC 10/10] Revert "ftrace: Store direct called addresses in their ops" Jiri Olsa
2025-07-29 17:57 ` [RFC 00/10] ftrace,bpf: Use single direct ops for bpf trampolines Mark Rutland
2025-07-30 11:19   ` Jiri Olsa
2025-07-30 13:56     ` Steven Rostedt
2025-07-31 20:40       ` Jiri Olsa [this message]
2025-08-01  9:49     ` Mark Rutland
2025-08-02 21:26       ` Jiri Olsa
2025-08-06 10:20         ` Mark Rutland
2025-08-13 11:09           ` Jiri Olsa

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=aIvUp_88v84Uw-lQ@krava \
    --to=olsajiri@gmail.com \
    --cc=alexghiti@rivosinc.com \
    --cc=andrii@kernel.org \
    --cc=andybnac@gmail.com \
    --cc=ast@kernel.org \
    --cc=bjorn@rivosinc.com \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=menglong8.dong@gmail.com \
    --cc=mpe@ellerman.id.au \
    --cc=naveen@kernel.org \
    --cc=palmer@dabbelt.com \
    --cc=revest@google.com \
    --cc=rostedt@goodmis.org \
    --cc=rostedt@kernel.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