From: Steven Rostedt <rostedt@goodmis.org>
To: Chengming Zhou <zhouchengming@bytedance.com>
Cc: mingo@redhat.com, catalin.marinas@arm.com, will@kernel.org,
mark.rutland@arm.com, broonie@kernel.org,
songmuchun@bytedance.com, qirui.001@bytedance.com,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Subject: Re: [External] Re: [PATCH] arm64/ftrace: Make function graph use ftrace directly
Date: Wed, 23 Feb 2022 20:30:01 -0500 [thread overview]
Message-ID: <20220223203001.1158d9d4@rorschach.local.home> (raw)
In-Reply-To: <5d64733b-8d2a-4928-c4dc-ce4f3b5fc561@bytedance.com>
On Wed, 23 Feb 2022 16:00:27 +0800
Chengming Zhou <zhouchengming@bytedance.com> wrote:
> Yes, it would be better to implement DYNAMIC_FTRACE_WITH_ARGS on arm64 too,
> and this patch just use DYNAMIC_FTRACE_WITH_REGS to install return_hooker
> for graph tracer, so it's a code cleanup, no performance optimization.
I'm worried that a clean up is either breaking the design or hurting
performance.
You have:
> +void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
> + struct ftrace_ops *op, struct ftrace_regs *fregs)
> +{
> + struct pt_regs *regs = arch_ftrace_get_regs(fregs);
Now, technically, arch_ftrace_get_regs() is to return NULL if the
ftrace_ops was not registered with ops->flags |= FTRACE_OPS_FL_SAVE_REGS.
Which function graph does not do.
But this is in arch specific code so you have more control of this
"undefined behavior". But you really should have a comment saying that
this needs to be fixed when DYNAMIC_FTRACE_WITH_ARGS is implemented.
-- Steve
> + unsigned long *parent = (unsigned long *)&procedure_link_pointer(regs);
> +
> + prepare_ftrace_return(ip, parent, frame_pointer(regs));
> +}
-- Steve
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2022-02-24 1:31 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-22 13:00 [PATCH] arm64/ftrace: Make function graph use ftrace directly Chengming Zhou
2022-02-22 15:52 ` Steven Rostedt
2022-02-22 16:07 ` Mark Rutland
2022-02-23 8:00 ` [External] " Chengming Zhou
2022-02-24 1:30 ` Steven Rostedt [this message]
2022-02-24 2:03 ` Chengming Zhou
2022-02-22 15:54 ` Mark Rutland
2022-02-22 16:05 ` Steven Rostedt
2022-02-23 7:55 ` [External] " Chengming Zhou
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=20220223203001.1158d9d4@rorschach.local.home \
--to=rostedt@goodmis.org \
--cc=broonie@kernel.org \
--cc=catalin.marinas@arm.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mingo@redhat.com \
--cc=qirui.001@bytedance.com \
--cc=songmuchun@bytedance.com \
--cc=will@kernel.org \
--cc=zhouchengming@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