All of lore.kernel.org
 help / color / mirror / Atom feed
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>,
	linux-trace-kernel@vger.kernel.org,
	LKML <linux-kernel@vger.kernel.org>,
	Martin KaFai Lau <martin.lau@linux.dev>,
	bpf <bpf@vger.kernel.org>, Sven Schnelle <svens@linux.ibm.com>,
	Alexei Starovoitov <ast@kernel.org>, Jiri Olsa <jolsa@kernel.org>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Alan Maguire <alan.maguire@oracle.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Florent Revest <revest@chromium.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [PATCH v4 3/9] bpf/btf: Add a function to search a member of a struct/union
Date: Wed, 2 Aug 2023 22:56:34 +0900	[thread overview]
Message-ID: <20230802225634.f520080cd9de759d687a2b0a@kernel.org> (raw)
In-Reply-To: <20230801204054.3884688e@rorschach.local.home>

On Tue, 1 Aug 2023 20:40:54 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Wed, 2 Aug 2023 09:21:46 +0900
> Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> 
> > > Then use kprobes. When I asked Masami what the difference between fprobes
> > > and kprobes was, he told me that it would be that it would no longer rely
> > > on the slower FTRACE_WITH_REGS. But currently, it still does.  
> > 
> > kprobes needs to keep using pt_regs because software-breakpoint exception
> > handler gets that. And fprobe is used for bpf multi-kprobe interface,
> > but I think it can be optional.
> > 
> > So until user-land tool supports the ftrace_regs, you can just disable
> > using fprobes if CONFIG_DYNAMIC_FTRACE_WITH_REGS=n
> 
> I'm confused. I asked about the difference between kprobes on ftrace
> and fprobes, and you said it was to get rid of the requirement of
> FTRACE_WITH_REGS.
> 
>  https://lore.kernel.org/all/20230120205535.98998636329ca4d5f8325bc3@kernel.org/

Yes, it is for enabling fprobe (and fprobe-event) on more architectures.
I don't think it's possible to change everything at once. So, it will be
changed step by step. At the first step, I will replace pt_regs with
ftrace_regs, and make bpf_trace.c and fprobe_event depends on
FTRACE_WITH_REGS.

At this point, we can split the problem into two, how to move bpf on
ftrace_regs and how to move fprobe-event on ftrace_regs. fprobe-event
change is not hard because it is closing in the kernel and I can do it.
But for BPF, I need to ask BPF user-land tools to support ftrace_regs.

> 
> > 
> > Then you can safely use 
> > 
> > struct pt_regs *regs = ftrace_get_regs(fregs);
> > 
> > I think we can just replace the CONFIG_FPROBE ifdefs with
> > CONFIG_DYNAMIC_FTRACE_WITH_REGS in kernel/trace/bpf_trace.c
> > And that will be the first version of using ftrace_regs in fprobe.
> 
> But it is still slow. The FTRACE_WITH_REGS gives us the full pt_regs
> and saves all registers including flags, which is a very slow operation
> (and noticeable in profilers).

Yes, to solve this part, we need to work with BPF user-land people.
I guess the BPF is accessing registers from pt_regs with fixed offset
which is calculated from pt_regs layout in the user-space.

> 
> And this still doesn't work on arm64.

Yes, and this makes more motivation to move on ftrace_regs.

Thank you,


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

  parent reply	other threads:[~2023-08-02 13:56 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-31  7:30 [PATCH v4 0/9] tracing: Improbe BTF support on probe events Masami Hiramatsu (Google)
2023-07-31  7:30 ` [PATCH v4 1/9] tracing/probes: Support BTF argument on module functions Masami Hiramatsu (Google)
2023-07-31  7:30 ` [PATCH v4 2/9] bpf/btf: tracing: Move finding func-proto API and getting func-param API to BTF Masami Hiramatsu (Google)
2023-07-31  7:30 ` [PATCH v4 3/9] bpf/btf: Add a function to search a member of a struct/union Masami Hiramatsu (Google)
2023-07-31 21:59   ` Alexei Starovoitov
2023-07-31 23:57     ` Masami Hiramatsu
2023-08-01  0:29       ` Alexei Starovoitov
2023-08-01 15:02         ` Masami Hiramatsu
2023-08-01 15:20           ` Steven Rostedt
2023-08-01 15:32             ` Steven Rostedt
2023-08-01 22:18               ` Alexei Starovoitov
2023-08-01 23:09                 ` Steven Rostedt
2023-08-01 23:44                   ` Alexei Starovoitov
2023-08-02  0:21                   ` Masami Hiramatsu
2023-08-02  0:40                     ` Steven Rostedt
2023-08-02  0:44                       ` Steven Rostedt
2023-08-02  2:22                         ` Alexei Starovoitov
2023-08-02  2:32                           ` Steven Rostedt
2023-08-02 14:07                           ` Masami Hiramatsu
2023-08-02 15:08                             ` Florent Revest
2023-08-02 13:56                       ` Masami Hiramatsu [this message]
2023-08-02 14:48                         ` Florent Revest
2023-08-02 15:47                         ` Florent Revest
2023-08-03  1:55                           ` Masami Hiramatsu
2023-08-02 18:24                         ` Alexei Starovoitov
2023-08-02 18:38                           ` Steven Rostedt
2023-08-02 19:48                             ` Alexei Starovoitov
2023-08-02 20:12                               ` Steven Rostedt
2023-08-02 21:28                                 ` Alexei Starovoitov
2023-08-02 14:44                   ` Florent Revest
2023-08-02 16:11                     ` Steven Rostedt
2023-08-03 15:42                     ` Masami Hiramatsu
2023-08-03 16:37                       ` Florent Revest
2023-08-07 20:48                       ` Jiri Olsa
2023-08-08 14:32                         ` Masami Hiramatsu
2023-08-01  1:15     ` Steven Rostedt
2023-08-01  2:24       ` Alexei Starovoitov
2023-08-01 13:35         ` Steven Rostedt
2023-08-01 15:18         ` Masami Hiramatsu
2023-08-01 22:21           ` Alexei Starovoitov
2023-08-01 23:17             ` Masami Hiramatsu
2023-07-31  7:30 ` [PATCH v4 4/9] tracing/probes: Support BTF based data structure field access Masami Hiramatsu (Google)
2023-07-31  7:30 ` [PATCH v4 5/9] tracing/probes: Support BTF field access from $retval Masami Hiramatsu (Google)
2023-07-31  7:31 ` [PATCH v4 6/9] tracing/probes: Add string type check with BTF Masami Hiramatsu (Google)
2023-07-31  7:31 ` [PATCH v4 7/9] tracing/fprobe-event: Assume fprobe is a return event by $retval Masami Hiramatsu (Google)
2023-07-31  7:31 ` [PATCH v4 8/9] selftests/ftrace: Add BTF fields access testcases Masami Hiramatsu (Google)
2023-07-31  7:31 ` [PATCH v4 9/9] Documentation: tracing: Update fprobe event example with BTF field Masami Hiramatsu (Google)

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=20230802225634.f520080cd9de759d687a2b0a@kernel.org \
    --to=mhiramat@kernel.org \
    --cc=acme@kernel.org \
    --cc=alan.maguire@oracle.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=martin.lau@linux.dev \
    --cc=peterz@infradead.org \
    --cc=revest@chromium.org \
    --cc=rostedt@goodmis.org \
    --cc=svens@linux.ibm.com \
    --cc=tglx@linutronix.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.