From: Steven Rostedt <rostedt@goodmis.org> To: Will Deacon <will.deacon@arm.com> Cc: linux-kernel@vger.kernel.org, Ingo Molnar <mingo@kernel.org>, Andrew Morton <akpm@linux-foundation.org>, Thomas Gleixner <tglx@linutronix.de>, Peter Zijlstra <peterz@infradead.org>, Masami Hiramatsu <mhiramat@kernel.org>, Josh Poimboeuf <jpoimboe@redhat.com>, Frederic Weisbecker <frederic@kernel.org>, Joel Fernandes <joel@joelfernandes.org>, Andy Lutomirski <luto@kernel.org>, Mark Rutland <mark.rutland@arm.com>, Catalin Marinas <catalin.marinas@arm.com>, linux-arm-kernel@lists.infradead.org Subject: Re: [RFC][PATCH 03/14] arm64: function_graph: Remove use of FTRACE_NOTRACE_DEPTH Date: Tue, 27 Nov 2018 14:50:44 -0500 [thread overview] Message-ID: <20181127145044.53ddb58a@gandalf.local.home> (raw) In-Reply-To: <20181127193149.GJ5641@arm.com> On Tue, 27 Nov 2018 19:31:50 +0000 Will Deacon <will.deacon@arm.com> wrote: > Hi Steve, > > On Wed, Nov 21, 2018 at 08:27:11PM -0500, Steven Rostedt wrote: > > From: "Steven Rostedt (VMware)" <rostedt@goodmis.org> > > > > The curr_ret_stack is no longer set to -1 when not tracing a function. It is > > now done differently, and the FTRACE_NOTRACE_DEPTH value is no longer used. > > Remove the reference to it. > > Do you have a pointer to the commit that changed that behaviour? I just want > to make sure we're not missing something in our unwind_frame() code. The commit log is slightly wrong. The -1 goes away in patch 11 of this series (and this is patch 3). But the FTRACE_NOTRACE_DEPTH is going away, which is why we need this patch. > > > Cc: Catalin Marinas <catalin.marinas@arm.com> > > Cc: Will Deacon <will.deacon@arm.com> > > Cc: linux-arm-kernel@lists.infradead.org > > Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org> > > --- > > arch/arm64/kernel/stacktrace.c | 3 --- > > 1 file changed, 3 deletions(-) > > > > diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c > > index 4989f7ea1e59..7723dadf25be 100644 > > --- a/arch/arm64/kernel/stacktrace.c > > +++ b/arch/arm64/kernel/stacktrace.c > > @@ -61,9 +61,6 @@ int notrace unwind_frame(struct task_struct *tsk, struct stackframe *frame) > > (frame->pc == (unsigned long)return_to_handler)) { > > if (WARN_ON_ONCE(frame->graph == -1)) > > return -EINVAL; > > Hmm, so is this code redundant too ^^ ? Actually, it is fine before patch 11 (which I'm only going to take the first 10 patches for the next merge window, after they are cleaned up). > > > - if (frame->graph < -1) > > - frame->graph += FTRACE_NOTRACE_DEPTH; > > - > > Do we still need to initialise frame->graph in __save_stack_trace()? You are fine till patch 11, which will probably break all this (another reason why I'm not going to push that in the next merge window). Ideally, we need to get rid of all users of curr_ret_stack outside of the function graph logic. As it's going to change drastically. We need this in order to combine function graph and kretprobes. I'm working on a v2 that's going to be aimed at the next merge window, that will only contain patches 1-10 of this series (and some other updates). I'll Cc you on that entire set. -- Steve
WARNING: multiple messages have this Message-ID (diff)
From: rostedt@goodmis.org (Steven Rostedt) To: linux-arm-kernel@lists.infradead.org Subject: [RFC][PATCH 03/14] arm64: function_graph: Remove use of FTRACE_NOTRACE_DEPTH Date: Tue, 27 Nov 2018 14:50:44 -0500 [thread overview] Message-ID: <20181127145044.53ddb58a@gandalf.local.home> (raw) In-Reply-To: <20181127193149.GJ5641@arm.com> On Tue, 27 Nov 2018 19:31:50 +0000 Will Deacon <will.deacon@arm.com> wrote: > Hi Steve, > > On Wed, Nov 21, 2018 at 08:27:11PM -0500, Steven Rostedt wrote: > > From: "Steven Rostedt (VMware)" <rostedt@goodmis.org> > > > > The curr_ret_stack is no longer set to -1 when not tracing a function. It is > > now done differently, and the FTRACE_NOTRACE_DEPTH value is no longer used. > > Remove the reference to it. > > Do you have a pointer to the commit that changed that behaviour? I just want > to make sure we're not missing something in our unwind_frame() code. The commit log is slightly wrong. The -1 goes away in patch 11 of this series (and this is patch 3). But the FTRACE_NOTRACE_DEPTH is going away, which is why we need this patch. > > > Cc: Catalin Marinas <catalin.marinas@arm.com> > > Cc: Will Deacon <will.deacon@arm.com> > > Cc: linux-arm-kernel at lists.infradead.org > > Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org> > > --- > > arch/arm64/kernel/stacktrace.c | 3 --- > > 1 file changed, 3 deletions(-) > > > > diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c > > index 4989f7ea1e59..7723dadf25be 100644 > > --- a/arch/arm64/kernel/stacktrace.c > > +++ b/arch/arm64/kernel/stacktrace.c > > @@ -61,9 +61,6 @@ int notrace unwind_frame(struct task_struct *tsk, struct stackframe *frame) > > (frame->pc == (unsigned long)return_to_handler)) { > > if (WARN_ON_ONCE(frame->graph == -1)) > > return -EINVAL; > > Hmm, so is this code redundant too ^^ ? Actually, it is fine before patch 11 (which I'm only going to take the first 10 patches for the next merge window, after they are cleaned up). > > > - if (frame->graph < -1) > > - frame->graph += FTRACE_NOTRACE_DEPTH; > > - > > Do we still need to initialise frame->graph in __save_stack_trace()? You are fine till patch 11, which will probably break all this (another reason why I'm not going to push that in the next merge window). Ideally, we need to get rid of all users of curr_ret_stack outside of the function graph logic. As it's going to change drastically. We need this in order to combine function graph and kretprobes. I'm working on a v2 that's going to be aimed at the next merge window, that will only contain patches 1-10 of this series (and some other updates). I'll Cc you on that entire set. -- Steve
next prev parent reply other threads:[~2018-11-27 19:50 UTC|newest] Thread overview: 58+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-11-22 1:27 [RFC][PATCH 00/14] function_graph: Rewrite to allow multiple users Steven Rostedt 2018-11-22 1:27 ` [RFC][PATCH 01/14] fgraph: Create a fgraph.c file to store function graph infrastructure Steven Rostedt 2018-11-22 1:27 ` [RFC][PATCH 02/14] fgraph: Have set_graph_notrace only affect function_graph tracer Steven Rostedt 2018-11-23 0:01 ` Namhyung Kim 2018-11-23 17:37 ` Steven Rostedt 2018-11-24 5:49 ` Namhyung Kim 2018-11-24 18:41 ` Steven Rostedt 2018-11-26 4:54 ` Namhyung Kim 2018-11-22 1:27 ` [RFC][PATCH 03/14] arm64: function_graph: Remove use of FTRACE_NOTRACE_DEPTH Steven Rostedt 2018-11-22 1:27 ` Steven Rostedt 2018-11-27 19:31 ` Will Deacon 2018-11-27 19:31 ` Will Deacon 2018-11-27 19:50 ` Steven Rostedt [this message] 2018-11-27 19:50 ` Steven Rostedt 2018-12-05 21:50 ` [PATCH 03/14 v2] " Steven Rostedt 2018-12-05 21:50 ` Steven Rostedt 2018-11-22 1:27 ` [RFC][PATCH 04/14] function_graph: Remove the " Steven Rostedt 2018-11-22 1:27 ` [RFC][PATCH 05/14] ftrace: Create new ftrace-internal.h header Steven Rostedt 2018-11-22 1:27 ` [RFC][PATCH 06/14] fgraph: Move function graph specific code into fgraph.c Steven Rostedt 2018-11-23 6:11 ` Joel Fernandes 2018-11-23 17:58 ` Steven Rostedt 2018-11-23 18:11 ` Steven Rostedt 2018-11-23 22:13 ` Joel Fernandes 2018-11-26 7:25 ` Masami Hiramatsu 2018-11-22 1:27 ` [RFC][PATCH 07/14] fgraph: Add new fgraph_ops structure to enable function graph hooks Steven Rostedt 2018-11-23 2:59 ` Joel Fernandes 2018-11-23 18:25 ` Steven Rostedt 2018-11-26 11:30 ` Masami Hiramatsu 2018-11-26 21:06 ` Steven Rostedt 2018-11-22 1:27 ` [RFC][PATCH 08/14] function_graph: Remove unused task_curr_ret_stack() Steven Rostedt 2018-11-26 7:40 ` Masami Hiramatsu 2018-11-26 21:26 ` Steven Rostedt 2018-11-26 10:02 ` Joey Pabalinas 2018-11-26 21:27 ` Steven Rostedt 2018-11-26 21:37 ` Joey Pabalinas 2018-11-22 1:27 ` [RFC][PATCH 09/14] function_graph: Move ftrace_graph_get_addr() to fgraph.c Steven Rostedt 2018-11-23 3:13 ` Joel Fernandes 2018-11-23 19:25 ` Steven Rostedt 2018-11-22 1:27 ` [RFC][PATCH 10/14] function_graph: Have profiler use new helper ftrace_graph_get_ret_stack() Steven Rostedt 2018-11-22 1:27 ` [RFC][PATCH 11/14] function_graph: Convert ret_stack to a series of longs Steven Rostedt 2018-11-24 5:31 ` Joel Fernandes 2018-11-26 16:07 ` Masami Hiramatsu 2018-11-26 16:26 ` Steven Rostedt 2018-11-28 1:38 ` Joel Fernandes 2018-11-26 21:31 ` Steven Rostedt 2018-11-22 1:27 ` [RFC][PATCH 12/14] function_graph: Add an array structure that will allow multiple callbacks Steven Rostedt 2018-11-22 1:27 ` [RFC][PATCH 13/14] function_graph: Allow multiple users to attach to function graph Steven Rostedt 2018-11-22 1:27 ` [RFC][PATCH 14/14] function_graph: Allow for more than one callback to be registered Steven Rostedt 2018-11-22 10:08 ` [RFC][PATCH 00/14] function_graph: Rewrite to allow multiple users Peter Zijlstra 2018-11-22 12:46 ` Steven Rostedt 2018-11-22 13:42 ` Peter Zijlstra 2018-11-26 9:21 ` Masami Hiramatsu 2018-11-26 16:32 ` Steven Rostedt 2018-11-29 14:29 ` Masami Hiramatsu 2018-11-29 16:46 ` Steven Rostedt 2018-11-30 2:26 ` Masami Hiramatsu 2018-11-30 3:24 ` Steven Rostedt 2018-11-30 14:11 ` 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=20181127145044.53ddb58a@gandalf.local.home \ --to=rostedt@goodmis.org \ --cc=akpm@linux-foundation.org \ --cc=catalin.marinas@arm.com \ --cc=frederic@kernel.org \ --cc=joel@joelfernandes.org \ --cc=jpoimboe@redhat.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=luto@kernel.org \ --cc=mark.rutland@arm.com \ --cc=mhiramat@kernel.org \ --cc=mingo@kernel.org \ --cc=peterz@infradead.org \ --cc=tglx@linutronix.de \ --cc=will.deacon@arm.com \ /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: linkBe 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.