All of lore.kernel.org
 help / color / mirror / Atom feed
From: will.deacon@arm.com (Will Deacon)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC v2 1/4] ftrace: allow arch-specific check_stack()
Date: Tue, 18 Aug 2015 09:21:36 +0100	[thread overview]
Message-ID: <20150818082136.GA10301@arm.com> (raw)
In-Reply-To: <55D17A04.7020800@linaro.org>

On Mon, Aug 17, 2015 at 07:07:00AM +0100, AKASHI Takahiro wrote:
> On 08/12/2015 02:03 AM, Will Deacon wrote:
> > On Tue, Aug 04, 2015 at 08:44:06AM +0100, AKASHI Takahiro wrote:
> >> A stack frame pointer may be used in a different way depending on
> >> cpu architecture. Thus it is not always appropriate to slurp the stack
> >> contents, as currently done in check_stack(), in order to calcurate
> >> a stack index (height) at a given function call. At least not on arm64.
> >>
> >> This patch extract potentially arch-specific code from check_stack()
> >> and puts it into a new arch_check_stack(), which is declared as weak.
> >> So we will be able to add arch-specific and most efficient way of
> >> stack traversing Later.
> >>
> >> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> >
> > If arm64 is the only architecture behaving differently, then I'm happy
> > to reconsider the fix to unwind_frame that we merged in e306dfd06fcb
> > ("ARM64: unwind: Fix PC calculation"). I'd have thought any architecture
> > with a branch-and-link instruction would potentially have the same issue,
> > so we could just be fixing things in the wrong place if ftrace works
> > everywhere else.
> 
> I'm not the right person to answer for other architectures (and ftrace
> behavior on them.) The only thing I know is that current ftrace stack tracer
> works correctly only if the addresses stored and found on stack match to
> the ones returned by save_stack_trace().
> 
> Anyway, the fix above is not the only reason that I want to introduce arch-specific
> arch_check_stack(). Other issues to fix include
>    - combined case of stack tracer and function graph tracer (common across arch's)
>    - exception entries (as I'm trying to address in RFC 4/4)
>    - in-accurate stack size (for each function, my current fix is not perfect though.)

Ok, if you have other reasons for the callback, then fine. I just didn't
want you to think that you had to work around e306dfd06fcb at all costs.

Will

WARNING: multiple messages have this Message-ID (diff)
From: Will Deacon <will.deacon@arm.com>
To: AKASHI Takahiro <takahiro.akashi@linaro.org>
Cc: Catalin Marinas <Catalin.Marinas@arm.com>,
	"rostedt@goodmis.org" <rostedt@goodmis.org>,
	"jungseoklee85@gmail.com" <jungseoklee85@gmail.com>,
	"olof@lixom.net" <olof@lixom.net>,
	"broonie@kernel.org" <broonie@kernel.org>,
	"david.griego@linaro.org" <david.griego@linaro.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [RFC v2 1/4] ftrace: allow arch-specific check_stack()
Date: Tue, 18 Aug 2015 09:21:36 +0100	[thread overview]
Message-ID: <20150818082136.GA10301@arm.com> (raw)
In-Reply-To: <55D17A04.7020800@linaro.org>

On Mon, Aug 17, 2015 at 07:07:00AM +0100, AKASHI Takahiro wrote:
> On 08/12/2015 02:03 AM, Will Deacon wrote:
> > On Tue, Aug 04, 2015 at 08:44:06AM +0100, AKASHI Takahiro wrote:
> >> A stack frame pointer may be used in a different way depending on
> >> cpu architecture. Thus it is not always appropriate to slurp the stack
> >> contents, as currently done in check_stack(), in order to calcurate
> >> a stack index (height) at a given function call. At least not on arm64.
> >>
> >> This patch extract potentially arch-specific code from check_stack()
> >> and puts it into a new arch_check_stack(), which is declared as weak.
> >> So we will be able to add arch-specific and most efficient way of
> >> stack traversing Later.
> >>
> >> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> >
> > If arm64 is the only architecture behaving differently, then I'm happy
> > to reconsider the fix to unwind_frame that we merged in e306dfd06fcb
> > ("ARM64: unwind: Fix PC calculation"). I'd have thought any architecture
> > with a branch-and-link instruction would potentially have the same issue,
> > so we could just be fixing things in the wrong place if ftrace works
> > everywhere else.
> 
> I'm not the right person to answer for other architectures (and ftrace
> behavior on them.) The only thing I know is that current ftrace stack tracer
> works correctly only if the addresses stored and found on stack match to
> the ones returned by save_stack_trace().
> 
> Anyway, the fix above is not the only reason that I want to introduce arch-specific
> arch_check_stack(). Other issues to fix include
>    - combined case of stack tracer and function graph tracer (common across arch's)
>    - exception entries (as I'm trying to address in RFC 4/4)
>    - in-accurate stack size (for each function, my current fix is not perfect though.)

Ok, if you have other reasons for the callback, then fine. I just didn't
want you to think that you had to work around e306dfd06fcb at all costs.

Will

  reply	other threads:[~2015-08-18  8:21 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-04  7:44 [RFC v2 0/4] arm64: ftrace: fix incorrect output from stack tracer AKASHI Takahiro
2015-08-04  7:44 ` AKASHI Takahiro
2015-08-04  7:44 ` [RFC v2 1/4] ftrace: allow arch-specific check_stack() AKASHI Takahiro
2015-08-04  7:44   ` AKASHI Takahiro
2015-08-11 17:03   ` Will Deacon
2015-08-11 17:03     ` Will Deacon
2015-08-17  6:07     ` AKASHI Takahiro
2015-08-17  6:07       ` AKASHI Takahiro
2015-08-18  8:21       ` Will Deacon [this message]
2015-08-18  8:21         ` Will Deacon
2015-08-04  7:44 ` [RFC v2 2/4] arm64: ftrace: add arch-specific stack tracer AKASHI Takahiro
2015-08-04  7:44   ` AKASHI Takahiro
2015-08-04  7:44 ` [RFC v2 3/4] arm64: ftrace: fix a stack trace result under function graph tracer AKASHI Takahiro
2015-08-04  7:44   ` AKASHI Takahiro
2015-08-04  7:44 ` [RFC v2 4/4] arm64: ftrace: add a stack frame for exception handler AKASHI Takahiro
2015-08-04  7:44   ` AKASHI Takahiro
2015-08-11 14:57   ` Jungseok Lee
2015-08-11 14:57     ` Jungseok Lee
2015-08-17  5:21     ` AKASHI Takahiro
2015-08-17  5:21       ` AKASHI Takahiro
2015-08-11 14:52 ` [RFC v2 0/4] arm64: ftrace: fix incorrect output from stack tracer Jungseok Lee
2015-08-11 14:52   ` Jungseok Lee
2015-08-17  4:50   ` AKASHI Takahiro
2015-08-17  4:50     ` AKASHI Takahiro
2015-08-17 15:29     ` Jungseok Lee
2015-08-17 15:29       ` Jungseok Lee

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=20150818082136.GA10301@arm.com \
    --to=will.deacon@arm.com \
    --cc=linux-arm-kernel@lists.infradead.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.