All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mark Rutland <mark.rutland@arm.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Ard Biesheuvel <ardb@kernel.org>,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, will@kernel.org,
	samitolvanen@google.com, keescook@chromium.org,
	mhiramat@kernel.org
Subject: Re: [PATCH] ftrace: Allow WITH_ARGS flavour of graph tracer with shadow call stack
Date: Mon, 12 Dec 2022 10:36:23 +0000	[thread overview]
Message-ID: <Y5cEJ55sEvs0QsNF@FVFF77S0Q05N> (raw)
In-Reply-To: <20221209165139.37d033ee@gandalf.local.home>

On Fri, Dec 09, 2022 at 04:51:39PM -0500, Steven Rostedt wrote:
> On Fri, 9 Dec 2022 14:40:25 +0000
> Mark Rutland <mark.rutland@arm.com> wrote:
> 
> > On Fri, Dec 09, 2022 at 03:34:02PM +0100, Ard Biesheuvel wrote:
> > > The recent switch on arm64 from DYNAMIC_FTRACE_WITH_REGS to
> > > DYNAMIC_FTRACE_WITH_ARGS failed to take into account that we currently
> > > require the former in order to allow the function graph tracer to be
> > > enabled in combination with shadow call stacks. This means that this is
> > > no longer permitted at all, in spite of the fact that either flavour of
> > > ftrace works perfectly fine in this combination.
> > > 
> > > Given that arm64 is the only arch that implements shadow call stacks in
> > > the first place, let's update the condition to just reflect the arm64
> > > change. When other architectures adopt shadow call stack support, this
> > > can be revisited if needed.
> > > 
> > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>  
> > 
> > My bad; sorry about this, and thanks for the fix!
> > 
> > Acked-by: Mark Rutland <mark.rutland@arm.com>
> > 
> > We should probably also add:
> > 
> > Fixes: 26299b3f6ba26bfc ("ftrace: arm64: move from REGS to ARGS")
> 
> Actually, I believe it is:
> 
> Fixes: ddc9863e9e90 ("scs: Disable when function graph tracing is enabled")

Ah; it's slightly more subtle since these were on different branches that got
merged. Either's correct in its own branch, and the merge is where things went
wrong.

I think the overall least confusing thing is to bite the bullet and list both
REGS and ARGS, i.e.

  depends on DYNAMIC_FTRACE_WITH_ARGS || DYNAMIC_FTRACE_WITH_REGS || !FUNCTION_GRAPH_TRACER

... and for the fixes tag have:

  Fixes: ddc9863e9e90 ("scs: Disable when function graph tracing is enabled")

That way the commit is correct regardless of the REGS -> ARGS conversion, and
will work if backported independently.

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: Ard Biesheuvel <ardb@kernel.org>,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, will@kernel.org,
	samitolvanen@google.com, keescook@chromium.org,
	mhiramat@kernel.org
Subject: Re: [PATCH] ftrace: Allow WITH_ARGS flavour of graph tracer with shadow call stack
Date: Mon, 12 Dec 2022 10:36:23 +0000	[thread overview]
Message-ID: <Y5cEJ55sEvs0QsNF@FVFF77S0Q05N> (raw)
In-Reply-To: <20221209165139.37d033ee@gandalf.local.home>

On Fri, Dec 09, 2022 at 04:51:39PM -0500, Steven Rostedt wrote:
> On Fri, 9 Dec 2022 14:40:25 +0000
> Mark Rutland <mark.rutland@arm.com> wrote:
> 
> > On Fri, Dec 09, 2022 at 03:34:02PM +0100, Ard Biesheuvel wrote:
> > > The recent switch on arm64 from DYNAMIC_FTRACE_WITH_REGS to
> > > DYNAMIC_FTRACE_WITH_ARGS failed to take into account that we currently
> > > require the former in order to allow the function graph tracer to be
> > > enabled in combination with shadow call stacks. This means that this is
> > > no longer permitted at all, in spite of the fact that either flavour of
> > > ftrace works perfectly fine in this combination.
> > > 
> > > Given that arm64 is the only arch that implements shadow call stacks in
> > > the first place, let's update the condition to just reflect the arm64
> > > change. When other architectures adopt shadow call stack support, this
> > > can be revisited if needed.
> > > 
> > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>  
> > 
> > My bad; sorry about this, and thanks for the fix!
> > 
> > Acked-by: Mark Rutland <mark.rutland@arm.com>
> > 
> > We should probably also add:
> > 
> > Fixes: 26299b3f6ba26bfc ("ftrace: arm64: move from REGS to ARGS")
> 
> Actually, I believe it is:
> 
> Fixes: ddc9863e9e90 ("scs: Disable when function graph tracing is enabled")

Ah; it's slightly more subtle since these were on different branches that got
merged. Either's correct in its own branch, and the merge is where things went
wrong.

I think the overall least confusing thing is to bite the bullet and list both
REGS and ARGS, i.e.

  depends on DYNAMIC_FTRACE_WITH_ARGS || DYNAMIC_FTRACE_WITH_REGS || !FUNCTION_GRAPH_TRACER

... and for the fixes tag have:

  Fixes: ddc9863e9e90 ("scs: Disable when function graph tracing is enabled")

That way the commit is correct regardless of the REGS -> ARGS conversion, and
will work if backported independently.

Thanks,
Mark.

  reply	other threads:[~2022-12-12 10:38 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-09 14:34 [PATCH] ftrace: Allow WITH_ARGS flavour of graph tracer with shadow call stack Ard Biesheuvel
2022-12-09 14:34 ` Ard Biesheuvel
2022-12-09 14:40 ` Mark Rutland
2022-12-09 14:40   ` Mark Rutland
2022-12-09 21:51   ` Steven Rostedt
2022-12-09 21:51     ` Steven Rostedt
2022-12-12 10:36     ` Mark Rutland [this message]
2022-12-12 10:36       ` Mark Rutland
2022-12-13 11:36       ` Will Deacon
2022-12-13 11:36         ` Will Deacon
2022-12-11  3:27 ` Masami Hiramatsu
2022-12-11  3:27   ` Masami Hiramatsu
2022-12-12 10:46   ` Mark Rutland
2022-12-12 10:46     ` Mark Rutland

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=Y5cEJ55sEvs0QsNF@FVFF77S0Q05N \
    --to=mark.rutland@arm.com \
    --cc=ardb@kernel.org \
    --cc=keescook@chromium.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhiramat@kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=samitolvanen@google.com \
    --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.