From: Steven Rostedt <rostedt@goodmis.org>
To: AKASHI Takahiro <takahiro.akashi@linaro.org>
Cc: Jungseok Lee <jungseoklee85@gmail.com>,
catalin.marinas@arm.com, will.deacon@arm.com, olof@lixom.net,
broonie@kernel.org, david.griego@linaro.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Subject: Re: [RFC 2/3] arm64: refactor save_stack_trace()
Date: Wed, 15 Jul 2015 12:13:37 -0400 [thread overview]
Message-ID: <20150715121337.3b31aa84@gandalf.local.home> (raw)
In-Reply-To: <20150715105536.42949ea9@gandalf.local.home>
On Wed, 15 Jul 2015 10:55:36 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> I'll take a look at it and try to clean up the code.
Does the following patch make sense for you?
-- Steve
diff --git a/kernel/trace/trace_stack.c b/kernel/trace/trace_stack.c
index 3f34496244e9..9384647d07c3 100644
--- a/kernel/trace/trace_stack.c
+++ b/kernel/trace/trace_stack.c
@@ -18,12 +18,6 @@
#define STACK_TRACE_ENTRIES 500
-#ifdef CC_USING_FENTRY
-# define fentry 1
-#else
-# define fentry 0
-#endif
-
static unsigned long stack_dump_trace[STACK_TRACE_ENTRIES+1] =
{ [0 ... (STACK_TRACE_ENTRIES)] = ULONG_MAX };
static unsigned stack_dump_index[STACK_TRACE_ENTRIES];
@@ -35,7 +29,7 @@ static unsigned stack_dump_index[STACK_TRACE_ENTRIES];
*/
static struct stack_trace max_stack_trace = {
.max_entries = STACK_TRACE_ENTRIES - 1,
- .entries = &stack_dump_trace[1],
+ .entries = &stack_dump_trace[0],
};
static unsigned long max_stack_size;
@@ -77,7 +71,7 @@ check_stack(unsigned long ip, unsigned long *stack)
unsigned long this_size, flags; unsigned long *p, *top, *start;
static int tracer_frame;
int frame_size = ACCESS_ONCE(tracer_frame);
- int i;
+ int i, x;
this_size = ((unsigned long)stack) & (THREAD_SIZE-1);
this_size = THREAD_SIZE - this_size;
@@ -105,26 +99,20 @@ check_stack(unsigned long ip, unsigned long *stack)
max_stack_size = this_size;
max_stack_trace.nr_entries = 0;
-
- if (using_ftrace_ops_list_func())
- max_stack_trace.skip = 4;
- else
- max_stack_trace.skip = 3;
+ max_stack_trace.skip = 0;
save_stack_trace(&max_stack_trace);
- /*
- * Add the passed in ip from the function tracer.
- * Searching for this on the stack will skip over
- * most of the overhead from the stack tracer itself.
- */
- stack_dump_trace[0] = ip;
- max_stack_trace.nr_entries++;
+ /* Skip over the overhead of the stack tracer itself */
+ for (i = 0; i < max_stack_trace.nr_entries; i++) {
+ if (stack_dump_trace[i] == ip)
+ break;
+ }
/*
* Now find where in the stack these are.
*/
- i = 0;
+ x = 0;
start = stack;
top = (unsigned long *)
(((unsigned long)start & ~(THREAD_SIZE-1)) + THREAD_SIZE);
@@ -139,12 +127,13 @@ check_stack(unsigned long ip, unsigned long *stack)
while (i < max_stack_trace.nr_entries) {
int found = 0;
- stack_dump_index[i] = this_size;
+ stack_dump_index[x] = this_size;
p = start;
for (; p < top && i < max_stack_trace.nr_entries; p++) {
if (*p == stack_dump_trace[i]) {
- this_size = stack_dump_index[i++] =
+ stack_dump_trace[x] = stack_dump_trace[i++];
+ this_size = stack_dump_index[x++] =
(top - p) * sizeof(unsigned long);
found = 1;
/* Start the search from here */
@@ -168,6 +157,9 @@ check_stack(unsigned long ip, unsigned long *stack)
i++;
}
+ for (; x < max_stack_trace.nr_entries; x++)
+ stack_dump_trace[x] = ULONG_MAX;
+
if (task_stack_end_corrupted(current)) {
print_max_stack();
BUG();
@@ -192,24 +184,7 @@ stack_trace_call(unsigned long ip, unsigned long parent_ip,
if (per_cpu(trace_active, cpu)++ != 0)
goto out;
- /*
- * When fentry is used, the traced function does not get
- * its stack frame set up, and we lose the parent.
- * The ip is pretty useless because the function tracer
- * was called before that function set up its stack frame.
- * In this case, we use the parent ip.
- *
- * By adding the return address of either the parent ip
- * or the current ip we can disregard most of the stack usage
- * caused by the stack tracer itself.
- *
- * The function tracer always reports the address of where the
- * mcount call was, but the stack will hold the return address.
- */
- if (fentry)
- ip = parent_ip;
- else
- ip += MCOUNT_INSN_SIZE;
+ ip += MCOUNT_INSN_SIZE;
check_stack(ip, &stack);
WARNING: multiple messages have this Message-ID (diff)
From: rostedt@goodmis.org (Steven Rostedt)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC 2/3] arm64: refactor save_stack_trace()
Date: Wed, 15 Jul 2015 12:13:37 -0400 [thread overview]
Message-ID: <20150715121337.3b31aa84@gandalf.local.home> (raw)
In-Reply-To: <20150715105536.42949ea9@gandalf.local.home>
On Wed, 15 Jul 2015 10:55:36 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> I'll take a look at it and try to clean up the code.
Does the following patch make sense for you?
-- Steve
diff --git a/kernel/trace/trace_stack.c b/kernel/trace/trace_stack.c
index 3f34496244e9..9384647d07c3 100644
--- a/kernel/trace/trace_stack.c
+++ b/kernel/trace/trace_stack.c
@@ -18,12 +18,6 @@
#define STACK_TRACE_ENTRIES 500
-#ifdef CC_USING_FENTRY
-# define fentry 1
-#else
-# define fentry 0
-#endif
-
static unsigned long stack_dump_trace[STACK_TRACE_ENTRIES+1] =
{ [0 ... (STACK_TRACE_ENTRIES)] = ULONG_MAX };
static unsigned stack_dump_index[STACK_TRACE_ENTRIES];
@@ -35,7 +29,7 @@ static unsigned stack_dump_index[STACK_TRACE_ENTRIES];
*/
static struct stack_trace max_stack_trace = {
.max_entries = STACK_TRACE_ENTRIES - 1,
- .entries = &stack_dump_trace[1],
+ .entries = &stack_dump_trace[0],
};
static unsigned long max_stack_size;
@@ -77,7 +71,7 @@ check_stack(unsigned long ip, unsigned long *stack)
unsigned long this_size, flags; unsigned long *p, *top, *start;
static int tracer_frame;
int frame_size = ACCESS_ONCE(tracer_frame);
- int i;
+ int i, x;
this_size = ((unsigned long)stack) & (THREAD_SIZE-1);
this_size = THREAD_SIZE - this_size;
@@ -105,26 +99,20 @@ check_stack(unsigned long ip, unsigned long *stack)
max_stack_size = this_size;
max_stack_trace.nr_entries = 0;
-
- if (using_ftrace_ops_list_func())
- max_stack_trace.skip = 4;
- else
- max_stack_trace.skip = 3;
+ max_stack_trace.skip = 0;
save_stack_trace(&max_stack_trace);
- /*
- * Add the passed in ip from the function tracer.
- * Searching for this on the stack will skip over
- * most of the overhead from the stack tracer itself.
- */
- stack_dump_trace[0] = ip;
- max_stack_trace.nr_entries++;
+ /* Skip over the overhead of the stack tracer itself */
+ for (i = 0; i < max_stack_trace.nr_entries; i++) {
+ if (stack_dump_trace[i] == ip)
+ break;
+ }
/*
* Now find where in the stack these are.
*/
- i = 0;
+ x = 0;
start = stack;
top = (unsigned long *)
(((unsigned long)start & ~(THREAD_SIZE-1)) + THREAD_SIZE);
@@ -139,12 +127,13 @@ check_stack(unsigned long ip, unsigned long *stack)
while (i < max_stack_trace.nr_entries) {
int found = 0;
- stack_dump_index[i] = this_size;
+ stack_dump_index[x] = this_size;
p = start;
for (; p < top && i < max_stack_trace.nr_entries; p++) {
if (*p == stack_dump_trace[i]) {
- this_size = stack_dump_index[i++] =
+ stack_dump_trace[x] = stack_dump_trace[i++];
+ this_size = stack_dump_index[x++] =
(top - p) * sizeof(unsigned long);
found = 1;
/* Start the search from here */
@@ -168,6 +157,9 @@ check_stack(unsigned long ip, unsigned long *stack)
i++;
}
+ for (; x < max_stack_trace.nr_entries; x++)
+ stack_dump_trace[x] = ULONG_MAX;
+
if (task_stack_end_corrupted(current)) {
print_max_stack();
BUG();
@@ -192,24 +184,7 @@ stack_trace_call(unsigned long ip, unsigned long parent_ip,
if (per_cpu(trace_active, cpu)++ != 0)
goto out;
- /*
- * When fentry is used, the traced function does not get
- * its stack frame set up, and we lose the parent.
- * The ip is pretty useless because the function tracer
- * was called before that function set up its stack frame.
- * In this case, we use the parent ip.
- *
- * By adding the return address of either the parent ip
- * or the current ip we can disregard most of the stack usage
- * caused by the stack tracer itself.
- *
- * The function tracer always reports the address of where the
- * mcount call was, but the stack will hold the return address.
- */
- if (fentry)
- ip = parent_ip;
- else
- ip += MCOUNT_INSN_SIZE;
+ ip += MCOUNT_INSN_SIZE;
check_stack(ip, &stack);
next prev parent reply other threads:[~2015-07-15 16:13 UTC|newest]
Thread overview: 106+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-07-13 5:29 [RFC 0/3] arm64: ftrace: fix incorrect output from stack tracer AKASHI Takahiro
2015-07-13 5:29 ` AKASHI Takahiro
2015-07-13 5:29 ` [RFC 1/3] ftrace: adjust a function's pc to search for in check_stack() for arm64 AKASHI Takahiro
2015-07-13 5:29 ` AKASHI Takahiro
2015-07-13 15:24 ` Steven Rostedt
2015-07-13 15:24 ` Steven Rostedt
2015-07-15 0:22 ` AKASHI Takahiro
2015-07-15 0:22 ` AKASHI Takahiro
2015-07-13 5:29 ` [RFC 2/3] arm64: refactor save_stack_trace() AKASHI Takahiro
2015-07-13 5:29 ` AKASHI Takahiro
2015-07-14 12:47 ` Jungseok Lee
2015-07-14 12:47 ` Jungseok Lee
2015-07-14 13:31 ` Steven Rostedt
2015-07-14 13:31 ` Steven Rostedt
2015-07-15 0:20 ` AKASHI Takahiro
2015-07-15 0:20 ` AKASHI Takahiro
2015-07-15 2:51 ` Steven Rostedt
2015-07-15 2:51 ` Steven Rostedt
2015-07-15 11:41 ` AKASHI Takahiro
2015-07-15 11:41 ` AKASHI Takahiro
2015-07-15 14:55 ` Steven Rostedt
2015-07-15 14:55 ` Steven Rostedt
2015-07-15 16:13 ` Steven Rostedt [this message]
2015-07-15 16:13 ` Steven Rostedt
2015-07-16 0:27 ` AKASHI Takahiro
2015-07-16 0:27 ` AKASHI Takahiro
2015-07-16 1:08 ` AKASHI Takahiro
2015-07-16 1:08 ` AKASHI Takahiro
2015-07-16 1:38 ` Steven Rostedt
2015-07-16 1:38 ` Steven Rostedt
2015-07-17 10:46 ` Will Deacon
2015-07-17 10:46 ` Will Deacon
2015-07-16 13:29 ` Jungseok Lee
2015-07-16 13:29 ` Jungseok Lee
2015-07-16 13:54 ` Jungseok Lee
2015-07-16 13:54 ` Jungseok Lee
2015-07-16 14:24 ` Steven Rostedt
2015-07-16 14:24 ` Steven Rostedt
2015-07-16 15:01 ` Jungseok Lee
2015-07-16 15:01 ` Jungseok Lee
2015-07-16 15:31 ` Steven Rostedt
2015-07-16 15:31 ` Steven Rostedt
2015-07-16 15:52 ` Jungseok Lee
2015-07-16 15:52 ` Jungseok Lee
2015-07-16 20:22 ` Steven Rostedt
2015-07-16 20:22 ` Steven Rostedt
2015-07-17 2:49 ` AKASHI Takahiro
2015-07-17 2:49 ` AKASHI Takahiro
2015-07-17 3:21 ` Steven Rostedt
2015-07-17 3:21 ` Steven Rostedt
2015-07-16 16:16 ` Steven Rostedt
2015-07-16 16:16 ` Steven Rostedt
2015-07-17 12:40 ` Mark Rutland
2015-07-17 12:40 ` Mark Rutland
2015-07-17 12:51 ` Steven Rostedt
2015-07-17 12:51 ` Steven Rostedt
2015-07-17 13:00 ` Steven Rostedt
2015-07-17 13:00 ` Steven Rostedt
2015-07-17 14:28 ` Jungseok Lee
2015-07-17 14:28 ` Jungseok Lee
2015-07-17 14:41 ` Steven Rostedt
2015-07-17 14:41 ` Steven Rostedt
2015-07-17 14:59 ` Jungseok Lee
2015-07-17 14:59 ` Jungseok Lee
2015-07-17 15:34 ` Jungseok Lee
2015-07-17 15:34 ` Jungseok Lee
2015-07-17 16:01 ` Steven Rostedt
2015-07-17 16:01 ` Steven Rostedt
2015-07-20 16:20 ` Will Deacon
2015-07-20 16:20 ` Will Deacon
2015-07-20 23:53 ` AKASHI Takahiro
2015-07-20 23:53 ` AKASHI Takahiro
2015-07-21 10:26 ` AKASHI Takahiro
2015-07-21 10:26 ` AKASHI Takahiro
2015-07-21 14:34 ` Jungseok Lee
2015-07-21 14:34 ` Jungseok Lee
2015-08-03 9:09 ` Will Deacon
2015-08-03 9:09 ` Will Deacon
2015-08-03 14:01 ` Steven Rostedt
2015-08-03 14:01 ` Steven Rostedt
2015-08-03 14:04 ` Will Deacon
2015-08-03 14:04 ` Will Deacon
2015-08-03 16:30 ` Jungseok Lee
2015-08-03 16:30 ` Jungseok Lee
2015-08-03 16:57 ` Steven Rostedt
2015-08-03 16:57 ` Steven Rostedt
2015-08-03 17:22 ` Jungseok Lee
2015-08-03 17:22 ` Jungseok Lee
2015-08-03 17:32 ` Steven Rostedt
2015-08-03 17:32 ` Steven Rostedt
2015-08-04 7:41 ` AKASHI Takahiro
2015-08-04 7:41 ` AKASHI Takahiro
2015-07-17 2:04 ` AKASHI Takahiro
2015-07-17 2:04 ` AKASHI Takahiro
2015-07-17 14:38 ` Jungseok Lee
2015-07-17 14:38 ` Jungseok Lee
2015-07-16 14:28 ` Mark Rutland
2015-07-16 14:28 ` Mark Rutland
2015-07-16 14:34 ` Steven Rostedt
2015-07-16 14:34 ` Steven Rostedt
2015-07-17 2:09 ` AKASHI Takahiro
2015-07-17 2:09 ` AKASHI Takahiro
2015-07-13 5:29 ` [RFC 3/3] arm64: ftrace: mcount() should not create a stack frame AKASHI Takahiro
2015-07-13 5:29 ` AKASHI Takahiro
2015-07-13 15:01 ` [RFC 0/3] arm64: ftrace: fix incorrect output from stack tracer Jungseok Lee
2015-07-13 15:01 ` 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=20150715121337.3b31aa84@gandalf.local.home \
--to=rostedt@goodmis.org \
--cc=broonie@kernel.org \
--cc=catalin.marinas@arm.com \
--cc=david.griego@linaro.org \
--cc=jungseoklee85@gmail.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=olof@lixom.net \
--cc=takahiro.akashi@linaro.org \
--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: 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.