linux-trace-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
To: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Florent Revest <revest@chromium.org>,
	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>,
	Peter Zijlstra <peterz@infradead.org>,
	Thomas Gleixner <tglx@linutronix.de>, Guo Ren <guoren@kernel.org>,
	wuqiang <wuqiang.matt@bytedance.com>
Subject: Re: [PATCH v5 00/12] tracing: fprobe: rethook: Use ftrace_regs instead of pt_regs
Date: Tue, 3 Oct 2023 00:39:23 +0900	[thread overview]
Message-ID: <20231003003923.eabc33bb3f4ffb8eac71f2af@kernel.org> (raw)
In-Reply-To: <20230930181435.6663ef5a6ad718548a1e414a@kernel.org>

On Sat, 30 Sep 2023 18:14:35 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:

> On Fri, 29 Sep 2023 17:12:07 -0700
> Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> 
> > On Thu, Sep 28, 2023 at 6:21 PM Masami Hiramatsu <mhiramat@kernel.org> wrote:
> > >
> > >
> > > Thus, what I need is to make fprobe to use function-graph tracer's shadow
> > > stack and trampoline instead of rethook. This may need to generalize its
> > > interface so that we can share it between fprobe and function-graph tracer,
> > > but we don't need to involve rethook and kretprobes anymore.
> > 
> > ...
> > 
> > > And need to add patches
> > >
> > >  - Introduce a generized function exit hook interface for ftrace.
> > >  - Replace rethook in fprobe with the function exit hook interface.
> > 
> > you mean that rethook will be removed after that?
> 
> No, it is too late. rethook is deeply integrated with kretprobe.
> So when we remove the kretprobe, rethook will be removed too.
> (fprobe and kretprobe provides similar functionality, so we can
> move to fprobe)
> 
> Even though, objpool(*) itself might be kept for some other use
> cases. As far as I can see, ftrace_ret_stack can not provide a context
> local storage between entry -> exit callbacks. (so this feature must
> be dropped from fprobe)
> 
> (*) https://lore.kernel.org/all/20230905015255.81545-1-wuqiang.matt@bytedance.com/

Oops, I rechecked the performance of objpool with prctl loop by
perf stat -a -I 10000 --interval-count 4 -e syscalls:sys_enter_prctl

And I found that with objpool, fprobe performance is the same
as function-graph!

	noprobe	kretprobe	fprobe	function-graph
T1	10706762	8506402	10475887	11249096
T2	28698960	20972543	22567923	22586848
T4	56634397	41500675	45042714	44932685
T8	114910972	85211522	91560078	90068034
T16	228519966	169212249	181582171	181181211
T32	448049923	330408645	361074536	356221873
T64	623779515	450932779	499909030	495516920

objpool consumes more memory than current freelist (because it is
just a list with counter) but that is limited. Usual use-case
(per-probe node size is 128, # of cpu: 8) one probe will allocate
22KB. (100 probes will need 2.2MB)
This is comparable to function graph ret-stack.

So now I'm reconsidering the strategy. I might better to keep
using rethook, but without ugly pt_regs casts. (e.g. use different
trampoline if rethook user requires ftrace_regs)

Sorry for confusing the direction.

Thank you,

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

      reply	other threads:[~2023-10-02 15:39 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-24 13:35 [PATCH v5 00/12] tracing: fprobe: rethook: Use ftrace_regs instead of pt_regs Masami Hiramatsu (Google)
2023-09-24 13:35 ` [PATCH v5 01/12] riscv: ftrace: Fix to pass correct ftrace_regs to ftrace_func_t functions Masami Hiramatsu (Google)
2023-09-24 13:36 ` [PATCH v5 02/12] Documentation: probes: Add a new ret_ip callback parameter Masami Hiramatsu (Google)
2023-09-24 13:36 ` [PATCH v5 03/12] tracing: Add a comment about the requirements of the ftrace_regs Masami Hiramatsu (Google)
2023-09-24 13:36 ` [PATCH v5 04/12] fprobe: Use ftrace_regs in fprobe entry handler Masami Hiramatsu (Google)
2023-09-25 10:41   ` Jiri Olsa
2023-09-25 12:15     ` Masami Hiramatsu
2023-09-25 22:14       ` Jiri Olsa
2023-09-26  0:20         ` Masami Hiramatsu
2023-09-26  7:23           ` Jiri Olsa
2023-09-26 11:49             ` Jiri Olsa
2023-09-26 14:47               ` Masami Hiramatsu
2023-09-24 13:36 ` [PATCH v5 05/12] tracing: Expose ftrace_regs regardless of CONFIG_FUNCTION_TRACER Masami Hiramatsu (Google)
2023-09-24 13:37 ` [PATCH v5 06/12] fprobe: rethook: Use ftrace_regs in fprobe exit handler and rethook Masami Hiramatsu (Google)
2023-09-24 13:37 ` [PATCH v5 07/12] tracing: Add ftrace_partial_regs() for converting ftrace_regs to pt_regs Masami Hiramatsu (Google)
2023-09-24 13:37 ` [PATCH v5 08/12] tracing: Add ftrace_fill_perf_regs() for perf event Masami Hiramatsu (Google)
2023-09-24 13:37 ` [PATCH v5 09/12] tracing/fprobe: Enable fprobe events with CONFIG_DYNAMIC_FTRACE_WITH_ARGS Masami Hiramatsu (Google)
2023-09-24 13:37 ` [PATCH v5 10/12] bpf: Enable kprobe_multi feature if CONFIG_FPROBE is enabled Masami Hiramatsu (Google)
2023-09-24 13:38 ` [PATCH v5 11/12] Documentation: probes: Update fprobe document to use ftrace_regs Masami Hiramatsu (Google)
2023-09-24 13:38 ` [PATCH v5 12/12] Documentation: tracing: Add a note about argument and retval access Masami Hiramatsu (Google)
2023-09-29  1:21 ` [PATCH v5 00/12] tracing: fprobe: rethook: Use ftrace_regs instead of pt_regs Masami Hiramatsu
2023-09-30  0:12   ` Alexei Starovoitov
2023-09-30  9:14     ` Masami Hiramatsu
2023-10-02 15:39       ` Masami Hiramatsu [this message]

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=20231003003923.eabc33bb3f4ffb8eac71f2af@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=guoren@kernel.org \
    --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 \
    --cc=wuqiang.matt@bytedance.com \
    /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).