All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mark Rutland <mark.rutland@arm.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: linux-kernel@vger.kernel.org, catalin.marinas@arm.com,
	linux-arm-kernel@lists.infradead.org, mhiramat@kernel.org,
	revest@chromium.org, will@kernel.org
Subject: Re: [PATCH v2 0/4] arm64/ftrace: move to DYNAMIC_FTRACE_WITH_ARGS
Date: Tue, 15 Nov 2022 15:48:49 +0000	[thread overview]
Message-ID: <Y3O04U1NydlIOddW@FVFF77S0Q05N.cambridge.arm.com> (raw)
In-Reply-To: <20221115100148.08475da0@gandalf.local.home>

On Tue, Nov 15, 2022 at 10:01:48AM -0500, Steven Rostedt wrote:
> On Thu,  3 Nov 2022 17:05:16 +0000
> Mark Rutland <mark.rutland@arm.com> wrote:
> 
> > This series replaces arm64's support for FTRACE_WITH_REGS with support
> > for FTRACE_WITH_ARGS. This removes some overhead and complexity, and
> > removes some latent issues with inconsistent presentation of struct
> > pt_regs (which can only be reliably saved/restored at exception
> > boundaries).
> > 
> > The existing FTRACE_WITH_REGS support was added for two major reasons:
> > 
> > (1) To make it possible to use the ftrace graph tracer with pointer
> >     authentication, where it's necessary to snapshot/manipulate the LR
> >     before it is signed by the instrumented function.
> > 
> > (2) To make it possible to implement LIVEPATCH in future, where we need
> >     to hook function entry before an instrumented function manipulates
> >     the stack or argument registers. Practically speaking, we need to
> >     preserve the argument/return registers, PC, LR, and SP.
> > 
> > Neither of these requires the full set of pt_regs, and only requires us
> > to save/restore a subset of registers used for passing
> > arguments/return-values and context/return information (which is the
> > minimum set we always need to save/restore today).
> > 
> > As there is no longer a need to save different sets of registers for
> > different features, we no longer need distinct `ftrace_caller` and
> > `ftrace_regs_caller` trampolines. This allows the trampoline assembly to
> > be simpler, and simplifies code which previously had to handle the two
> > trampolines.
> > 
> > I've tested this with the ftrace selftests, where there are no
> > unexpected failures.
> 
> Were there any "expected" failures?

Ah; sorry, I had meant to include the results here.

With this series applied atop v6.1-rc4 and using the ftrace selftests from that
tree, my results were the same as with baseline v6.1-rc4:

| # of passed:  104
| # of failed:  0
| # of unresolved:  7
| # of untested:  0
| # of unsupported:  2
| # of xfailed:  1
| # of undefined(test bug):  0

Where the non-passing tests were:

| [8] Test ftrace direct functions against tracers        [UNRESOLVED]
| [9] Test ftrace direct functions against kprobes        [UNRESOLVED]

... as direct functions aren't supported on arm64 (both before and after this
series).

| [16] Generic dynamic event - check if duplicate events are caught       [UNSUPPORTED]
| [74] event trigger - test inter-event histogram trigger eprobe on synthetic event       [UNSUPPORTED]

... which are due to a bug in the tests fixed by:

  https://lore.kernel.org/all/20221010074207.714077-1-svens@linux.ibm.com/

... and they both pass with that applied.

| [22] Test trace_printk from module      [UNRESOLVED]
| [31] ftrace - function trace on module  [UNRESOLVED]
| [51] Kprobe dynamic event - probing module      [UNRESOLVED]
| [61] test for the preemptirqsoff tracer [UNRESOLVED]

... which are because my test environment didn't have modules.

| [62] Meta-selftest: Checkbashisms       [UNRESOLVED]

... which is irrelevant for this series.

| [65] event trigger - test inter-event histogram trigger expected fail actions   [XFAIL]

... which is expected.

[...]

> So I ran this on top of my code through all my ftrace tests (for x86) and
> it didn't cause any regressions.
> 
> Reviewed-by: Steven Rostedt (Google) <rostedt@goodmis.org>

Thanks!

Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Mark Rutland <mark.rutland@arm.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: linux-kernel@vger.kernel.org, catalin.marinas@arm.com,
	linux-arm-kernel@lists.infradead.org, mhiramat@kernel.org,
	revest@chromium.org, will@kernel.org
Subject: Re: [PATCH v2 0/4] arm64/ftrace: move to DYNAMIC_FTRACE_WITH_ARGS
Date: Tue, 15 Nov 2022 15:48:49 +0000	[thread overview]
Message-ID: <Y3O04U1NydlIOddW@FVFF77S0Q05N.cambridge.arm.com> (raw)
In-Reply-To: <20221115100148.08475da0@gandalf.local.home>

On Tue, Nov 15, 2022 at 10:01:48AM -0500, Steven Rostedt wrote:
> On Thu,  3 Nov 2022 17:05:16 +0000
> Mark Rutland <mark.rutland@arm.com> wrote:
> 
> > This series replaces arm64's support for FTRACE_WITH_REGS with support
> > for FTRACE_WITH_ARGS. This removes some overhead and complexity, and
> > removes some latent issues with inconsistent presentation of struct
> > pt_regs (which can only be reliably saved/restored at exception
> > boundaries).
> > 
> > The existing FTRACE_WITH_REGS support was added for two major reasons:
> > 
> > (1) To make it possible to use the ftrace graph tracer with pointer
> >     authentication, where it's necessary to snapshot/manipulate the LR
> >     before it is signed by the instrumented function.
> > 
> > (2) To make it possible to implement LIVEPATCH in future, where we need
> >     to hook function entry before an instrumented function manipulates
> >     the stack or argument registers. Practically speaking, we need to
> >     preserve the argument/return registers, PC, LR, and SP.
> > 
> > Neither of these requires the full set of pt_regs, and only requires us
> > to save/restore a subset of registers used for passing
> > arguments/return-values and context/return information (which is the
> > minimum set we always need to save/restore today).
> > 
> > As there is no longer a need to save different sets of registers for
> > different features, we no longer need distinct `ftrace_caller` and
> > `ftrace_regs_caller` trampolines. This allows the trampoline assembly to
> > be simpler, and simplifies code which previously had to handle the two
> > trampolines.
> > 
> > I've tested this with the ftrace selftests, where there are no
> > unexpected failures.
> 
> Were there any "expected" failures?

Ah; sorry, I had meant to include the results here.

With this series applied atop v6.1-rc4 and using the ftrace selftests from that
tree, my results were the same as with baseline v6.1-rc4:

| # of passed:  104
| # of failed:  0
| # of unresolved:  7
| # of untested:  0
| # of unsupported:  2
| # of xfailed:  1
| # of undefined(test bug):  0

Where the non-passing tests were:

| [8] Test ftrace direct functions against tracers        [UNRESOLVED]
| [9] Test ftrace direct functions against kprobes        [UNRESOLVED]

... as direct functions aren't supported on arm64 (both before and after this
series).

| [16] Generic dynamic event - check if duplicate events are caught       [UNSUPPORTED]
| [74] event trigger - test inter-event histogram trigger eprobe on synthetic event       [UNSUPPORTED]

... which are due to a bug in the tests fixed by:

  https://lore.kernel.org/all/20221010074207.714077-1-svens@linux.ibm.com/

... and they both pass with that applied.

| [22] Test trace_printk from module      [UNRESOLVED]
| [31] ftrace - function trace on module  [UNRESOLVED]
| [51] Kprobe dynamic event - probing module      [UNRESOLVED]
| [61] test for the preemptirqsoff tracer [UNRESOLVED]

... which are because my test environment didn't have modules.

| [62] Meta-selftest: Checkbashisms       [UNRESOLVED]

... which is irrelevant for this series.

| [65] event trigger - test inter-event histogram trigger expected fail actions   [XFAIL]

... which is expected.

[...]

> So I ran this on top of my code through all my ftrace tests (for x86) and
> it didn't cause any regressions.
> 
> Reviewed-by: Steven Rostedt (Google) <rostedt@goodmis.org>

Thanks!

Mark.

  reply	other threads:[~2022-11-15 15:50 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-03 17:05 [PATCH v2 0/4] arm64/ftrace: move to DYNAMIC_FTRACE_WITH_ARGS Mark Rutland
2022-11-03 17:05 ` Mark Rutland
2022-11-03 17:05 ` [PATCH v2 1/4] ftrace: pass fregs to arch_ftrace_set_direct_caller() Mark Rutland
2022-11-03 17:05   ` Mark Rutland
2022-11-03 17:05 ` [PATCH v2 2/4] ftrace: rename ftrace_instruction_pointer_set() -> ftrace_regs_set_instruction_pointer() Mark Rutland
2022-11-03 17:05   ` Mark Rutland
2022-11-03 17:05 ` [PATCH v2 3/4] ftrace: abstract DYNAMIC_FTRACE_WITH_ARGS accesses Mark Rutland
2022-11-03 17:05   ` Mark Rutland
2022-11-03 17:05 ` [PATCH v2 4/4] ftrace: arm64: move from REGS to ARGS Mark Rutland
2022-11-03 17:05   ` Mark Rutland
2022-11-15 11:27   ` Will Deacon
2022-11-15 11:27     ` Will Deacon
2022-11-17 10:52     ` Mark Rutland
2022-11-17 10:52       ` Mark Rutland
2022-11-18 12:31       ` Will Deacon
2022-11-18 12:31         ` Will Deacon
2022-11-18 13:57         ` Mark Rutland
2022-11-18 13:57           ` Mark Rutland
2022-11-18 14:09           ` Will Deacon
2022-11-18 14:09             ` Will Deacon
2022-11-15 15:01 ` [PATCH v2 0/4] arm64/ftrace: move to DYNAMIC_FTRACE_WITH_ARGS Steven Rostedt
2022-11-15 15:01   ` Steven Rostedt
2022-11-15 15:48   ` Mark Rutland [this message]
2022-11-15 15:48     ` Mark Rutland
2022-11-18  0:04 ` Masami Hiramatsu
2022-11-18  0:04   ` Masami Hiramatsu
2022-11-18 19:40 ` Will Deacon
2022-11-18 19:40   ` Will Deacon

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=Y3O04U1NydlIOddW@FVFF77S0Q05N.cambridge.arm.com \
    --to=mark.rutland@arm.com \
    --cc=catalin.marinas@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhiramat@kernel.org \
    --cc=revest@chromium.org \
    --cc=rostedt@goodmis.org \
    --cc=will@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 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.