live-patching.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mark Brown <broonie@kernel.org>
To: "Madhavan T. Venkataraman" <madvenka@linux.microsoft.com>
Cc: mark.rutland@arm.com, jpoimboe@redhat.com, jthierry@redhat.com,
	catalin.marinas@arm.com, will@kernel.org,
	linux-arm-kernel@lists.infradead.org,
	live-patching@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH v1 3/4] arm64: Detect FTRACE cases that make the stack trace unreliable
Date: Thu, 1 Apr 2021 19:28:11 +0100	[thread overview]
Message-ID: <20210401182810.GO4758@sirena.org.uk> (raw)
In-Reply-To: <0bece48b-5fee-2bd1-752e-66d2b89cc5ad@linux.microsoft.com>

[-- Attachment #1: Type: text/plain, Size: 1581 bytes --]

On Thu, Apr 01, 2021 at 12:43:25PM -0500, Madhavan T. Venkataraman wrote:

> >> +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
> >> +	{ (unsigned long) &ftrace_graph_call, 0 },
> >> +#ifdef CONFIG_FUNCTION_GRAPH_TRACER
> >> +	{ (unsigned long) ftrace_graph_caller, 0 },

> > It's weird that we take the address of ftrace_graph_call but not the
> > other functions - we should be consistent or explain why.  It'd probably
> > also look nicer to not nest the ifdefs, the dependencies in Kconfig will
> > ensure we only get things when we should.

> I have explained it in the comment in the FTRACE trampoline right above
> ftrace_graph_call().

Ah, right - it's a result of it being an inner label.  I'd suggest
putting a brief note right at that line of code explaining this (eg,
"Inner label, not a function"), it wasn't confusing due to the use of
that symbol but rather due to it being different from everything else
in the list and that's kind of lost in the main comment.

> So, it is only defined if CONFIG_FUNCTION_GRAPH_TRACER is defined. I can address
> this as well as your comment by defining another label whose name is more meaningful
> to our use:

> +SYM_INNER_LABEL(ftrace_trampoline, SYM_L_GLOBAL) // checked by the unwinder
> #ifdef CONFIG_FUNCTION_GRAPH_TRACER
> SYM_INNER_LABEL(ftrace_graph_call, SYM_L_GLOBAL) // ftrace_graph_caller();
>         nop                             // If enabled, this will be replaced
>                                         // "b ftrace_graph_caller"
> #endif

I'm not sure we need to bother with that, you'd still need the & I think.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

  reply	other threads:[~2021-04-01 18:37 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <77bd5edeea72d44533c769b1e8c0fea7a9d7eb3a>
2021-03-30 19:09 ` [RFC PATCH v1 0/4] arm64: Implement stack trace reliability checks madvenka
2021-03-30 19:09   ` [RFC PATCH v1 1/4] arm64: Implement infrastructure for " madvenka
2021-04-01 15:27     ` Mark Brown
2021-04-01 17:44       ` Madhavan T. Venkataraman
2021-03-30 19:09   ` [RFC PATCH v1 2/4] arm64: Mark a stack trace unreliable if an EL1 exception frame is detected madvenka
2021-04-01 17:21     ` Mark Brown
2021-03-30 19:09   ` [RFC PATCH v1 3/4] arm64: Detect FTRACE cases that make the stack trace unreliable madvenka
2021-04-01 14:27     ` Mark Brown
2021-04-01 17:43       ` Madhavan T. Venkataraman
2021-04-01 18:28         ` Mark Brown [this message]
2021-04-01 18:40           ` Madhavan T. Venkataraman
2021-04-01 18:53             ` Madhavan T. Venkataraman
2021-04-01 19:47               ` Madhavan T. Venkataraman
2021-04-06 11:02                 ` Mark Brown
2021-04-01 17:48       ` Madhavan T. Venkataraman
2021-03-30 19:09   ` [RFC PATCH v1 4/4] arm64: Mark stack trace as unreliable if kretprobed functions are present madvenka
2021-04-01 17:23     ` Mark Brown
2021-04-03 17:01   ` [RFC PATCH v1 0/4] arm64: Implement stack trace reliability checks Josh Poimboeuf
2021-04-04  3:29     ` Madhavan T. Venkataraman
2021-04-05 13:24       ` Masami Hiramatsu
2021-04-05 13:46         ` Madhavan T. Venkataraman
2021-04-05 14:56         ` Madhavan T. Venkataraman
2021-04-05 17:12           ` Madhavan T. Venkataraman
2021-04-05 23:39             ` Masami Hiramatsu
2021-04-05 23:40           ` Masami Hiramatsu

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=20210401182810.GO4758@sirena.org.uk \
    --to=broonie@kernel.org \
    --cc=catalin.marinas@arm.com \
    --cc=jpoimboe@redhat.com \
    --cc=jthierry@redhat.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=live-patching@vger.kernel.org \
    --cc=madvenka@linux.microsoft.com \
    --cc=mark.rutland@arm.com \
    --cc=will@kernel.org \
    --subject='Re: [RFC PATCH v1 3/4] arm64: Detect FTRACE cases that make the stack trace unreliable' \
    /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

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).