public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Masami Hiramatsu <mhiramat@kernel.org>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: linux-kernel@vger.kernel.org,
	Masami Hiramatsu <mhiramat@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Jiri Kosina <jikos@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@kernel.org>,
	Alexei Starovoitov <alexei.starovoitov@gmail.com>,
	Jiri Olsa <jolsa@redhat.com>,
	Josh Poimboeuf <jpoimboe@redhat.com>
Subject: Re: [RFC][PATCH 1/2 v2] ftrace/x86: Allow for arguments to be passed in to REGS by default
Date: Thu, 29 Oct 2020 17:07:36 +0900	[thread overview]
Message-ID: <20201029170736.938ded569b436c858ebc2555@kernel.org> (raw)
In-Reply-To: <20201028112916.50bcbc69@oasis.local.home>

On Wed, 28 Oct 2020 11:29:16 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> Hi Masami,
> 
> Talking with Peter and Thomas on IRC, where they really don't like
> passing a partial pt_regs around, got me thinking of redoing the REGS
> parameter of ftrace. Kprobes is the only user that requires the full
> registers being saved, and that's only because some kprobe user might
> want them.

Yes, kprobes can be used for debugging use case. I think we can skip
coller-saved registers (because ftrace is embedded by compiler and it
may save such registers correctly), but we still need a pt_regs on memory
to access it.

> 
> On Wed, 28 Oct 2020 10:25:02 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
> >
> >  typedef void (*ftrace_func_t)(unsigned long ip, unsigned long parent_ip,
> >  			      struct ftrace_ops *op, struct pt_regs *regs);
> >  
> 
> 
> Most registers of pt_regs at a start of a function is rather useless.
> What if we got rid of FL_SAVE_REGS all together and had a "ftrace_regs"
> structure passed in that would have only access to all the argument
> registers, the stack pointer and the instruction pointer?

Yeah, that's OK to me. If someone wants to investigate suspicious compiler
bug, they should not put a kprobe on the fentry call site. (in most cases
symbol+5 will put a breakpoint right after fentry nop)

> Then kprobes could just create its own pt_regs, fill in all the data
> from ftrace_regs and then fill the rest with zeros or possibly whatever
> the values currently are (does it really matter what those registers
> are?), including flags.

That sounds good to me.

> 
> Not only would this simplify the code, it would probably allow moving
> more of the kprobe code from the arch specific to the generic code, and
> remove a lot of duplication.

Ah, right.

> 
> This would also help speed up the processing of live kernel patching.
> 
> And best of all, it would give everything access to the arguments of a
> function and a stack pointer with out (ab)using pt_regs.
> 
> Do you think this would be feasible?

Yes, I agreed.

Thank you,

> 
> -- Steve


-- 
Masami Hiramatsu <mhiramat@kernel.org>

  parent reply	other threads:[~2020-10-29  8:07 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-28 13:15 [RFC][PATCH 0/2] ftrace: Add access to function arguments for all callbacks Steven Rostedt
2020-10-28 13:15 ` [RFC][PATCH 1/2] ftrace/x86: Allow for arguments to be passed in to REGS by default Steven Rostedt
2020-10-28 14:25   ` [RFC][PATCH 1/2 v2] " Steven Rostedt
2020-10-28 15:29     ` Steven Rostedt
2020-10-28 20:36       ` Steven Rostedt
2020-10-28 21:14         ` Steven Rostedt
2020-10-29  8:07       ` Masami Hiramatsu [this message]
2020-10-28 13:15 ` [RFC][PATCH 2/2] ftrace: Test arguments by adding trace_printk in function tracer Steven Rostedt
2020-10-28 21:33 ` [RFC][PATCH 0/2] ftrace: Add access to function arguments for all callbacks Alexei Starovoitov
2020-10-28 22:07   ` Steven Rostedt
2020-10-28 23:03     ` Alexei Starovoitov
2020-10-28 23:16       ` 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=20201029170736.938ded569b436c858ebc2555@kernel.org \
    --to=mhiramat@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=alexei.starovoitov@gmail.com \
    --cc=jikos@kernel.org \
    --cc=jolsa@redhat.com \
    --cc=jpoimboe@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox