All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Jungseok Lee <jungseoklee85@gmail.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Mark Rutland <mark.rutland@arm.com>,
	Catalin Marinas <Catalin.Marinas@arm.com>,
	Will Deacon <Will.Deacon@arm.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	AKASHI Takahiro <takahiro.akashi@linaro.org>,
	"broonie@kernel.org" <broonie@kernel.org>,
	"david.griego@linaro.org" <david.griego@linaro.org>,
	"olof@lixom.net" <olof@lixom.net>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [RFC 2/3] arm64: refactor save_stack_trace()
Date: Sat, 18 Jul 2015 00:34:21 +0900	[thread overview]
Message-ID: <1357EA74-B972-4B99-ADB0-BC7E8F06DDB5@gmail.com> (raw)
In-Reply-To: <0886A996-40E1-49E9-823C-85E55A858716@gmail.com>

On Jul 17, 2015, at 11:59 PM, Jungseok Lee wrote:
> On Jul 17, 2015, at 11:41 PM, Steven Rostedt wrote:
>> On Fri, 17 Jul 2015 23:28:13 +0900
>> Jungseok Lee <jungseoklee85@gmail.com> wrote:
>> 
>> 
>>> 
>>> I have reviewed and tested the kernel including this patch and only [RFC 1/3].
>> 
>> Thanks! Can you repost patch 1 with the changes I recommended, so that
>> I can get an Acked-by from the arm64 maintainers and pull all the
>> changes in together. This is fine for a 4.3 release, right? That is, it
>> doesn't need to go into 4.2-rcs.
>> 
> 
> Welcome!
> 
> It's not hard to repost a patch, but I feel like we have to wait for Akashi's response.
> Also, it might be needed to consider Mark's comment on arch part.
> 
> If they are okay, I will proceed.

Akashi and Mark,

The [RFC 1/3] patch used in my environment is shaped as follows.
I leave the hunk for *only* clear synchronization. This is why I choose this format
instead of reposting a patch. I hope it would help to track down this thread.

Steve,

I think this is my best at this point.

Best Regards
Jungseok Lee

----8<----
diff --git a/arch/arm64/include/asm/ftrace.h b/arch/arm64/include/asm/ftrace.h
index c5534fa..2b43e20 100644
--- a/arch/arm64/include/asm/ftrace.h
+++ b/arch/arm64/include/asm/ftrace.h
@@ -13,8 +13,9 @@
 
 #include <asm/insn.h>
 
-#define MCOUNT_ADDR		((unsigned long)_mcount)
-#define MCOUNT_INSN_SIZE	AARCH64_INSN_SIZE
+#define MCOUNT_ADDR			((unsigned long)_mcount)
+#define MCOUNT_INSN_SIZE		AARCH64_INSN_SIZE
+#define FTRACE_STACK_FRAME_OFFSET	AARCH64_INSN_SIZE
 
 #ifndef __ASSEMBLY__
 #include <linux/compat.h>
diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c
index 407991b..9ab67af 100644
--- a/arch/arm64/kernel/stacktrace.c
+++ b/arch/arm64/kernel/stacktrace.c
@@ -20,6 +20,7 @@
 #include <linux/sched.h>
 #include <linux/stacktrace.h>
 
+#include <asm/insn.h>
 #include <asm/stacktrace.h>
 
 /*
@@ -52,7 +53,7 @@ int notrace unwind_frame(struct stackframe *frame)
 	 * -4 here because we care about the PC at time of bl,
 	 * not where the return will go.
 	 */
-	frame->pc = *(unsigned long *)(fp + 8) - 4;
+	frame->pc = *(unsigned long *)(fp + 8) - AARCH64_INSN_SIZE;
 
 	return 0;
 }
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 1da6029..6566201 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -260,6 +260,9 @@ static inline void ftrace_kill(void) { }
 #endif /* CONFIG_FUNCTION_TRACER */
 
 #ifdef CONFIG_STACK_TRACER
+#ifndef FTRACE_STACK_FRAME_OFFSET
+#define FTRACE_STACK_FRAME_OFFSET 0
+#endif
 extern int stack_tracer_enabled;
 int
 stack_trace_sysctl(struct ctl_table *table, int write,
diff --git a/kernel/trace/trace_stack.c b/kernel/trace/trace_stack.c
index b746399..30521ea 100644
--- a/kernel/trace/trace_stack.c
+++ b/kernel/trace/trace_stack.c
@@ -105,7 +105,7 @@ check_stack(unsigned long ip, unsigned long *stack)
 
 	/* 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)
+		if ((stack_dump_trace[i] + FTRACE_STACK_FRAME_OFFSET) == ip)
 			break;
 	}
 
@@ -133,7 +133,8 @@ check_stack(unsigned long ip, unsigned long *stack)
 		for (; p < top && i < max_stack_trace.nr_entries; p++) {
 			if (stack_dump_trace[i] == ULONG_MAX)
 				break;
-			if (*p == stack_dump_trace[i]) {
+			if (*p == (stack_dump_trace[i]
+					+ FTRACE_STACK_FRAME_OFFSET)) {
 				stack_dump_trace[x] = stack_dump_trace[i++];
 				this_size = stack_dump_index[x++] =
 					(top - p) * sizeof(unsigned long);
----8<

WARNING: multiple messages have this Message-ID (diff)
From: jungseoklee85@gmail.com (Jungseok Lee)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC 2/3] arm64: refactor save_stack_trace()
Date: Sat, 18 Jul 2015 00:34:21 +0900	[thread overview]
Message-ID: <1357EA74-B972-4B99-ADB0-BC7E8F06DDB5@gmail.com> (raw)
In-Reply-To: <0886A996-40E1-49E9-823C-85E55A858716@gmail.com>

On Jul 17, 2015, at 11:59 PM, Jungseok Lee wrote:
> On Jul 17, 2015, at 11:41 PM, Steven Rostedt wrote:
>> On Fri, 17 Jul 2015 23:28:13 +0900
>> Jungseok Lee <jungseoklee85@gmail.com> wrote:
>> 
>> 
>>> 
>>> I have reviewed and tested the kernel including this patch and only [RFC 1/3].
>> 
>> Thanks! Can you repost patch 1 with the changes I recommended, so that
>> I can get an Acked-by from the arm64 maintainers and pull all the
>> changes in together. This is fine for a 4.3 release, right? That is, it
>> doesn't need to go into 4.2-rcs.
>> 
> 
> Welcome!
> 
> It's not hard to repost a patch, but I feel like we have to wait for Akashi's response.
> Also, it might be needed to consider Mark's comment on arch part.
> 
> If they are okay, I will proceed.

Akashi and Mark,

The [RFC 1/3] patch used in my environment is shaped as follows.
I leave the hunk for *only* clear synchronization. This is why I choose this format
instead of reposting a patch. I hope it would help to track down this thread.

Steve,

I think this is my best at this point.

Best Regards
Jungseok Lee

----8<----
diff --git a/arch/arm64/include/asm/ftrace.h b/arch/arm64/include/asm/ftrace.h
index c5534fa..2b43e20 100644
--- a/arch/arm64/include/asm/ftrace.h
+++ b/arch/arm64/include/asm/ftrace.h
@@ -13,8 +13,9 @@
 
 #include <asm/insn.h>
 
-#define MCOUNT_ADDR		((unsigned long)_mcount)
-#define MCOUNT_INSN_SIZE	AARCH64_INSN_SIZE
+#define MCOUNT_ADDR			((unsigned long)_mcount)
+#define MCOUNT_INSN_SIZE		AARCH64_INSN_SIZE
+#define FTRACE_STACK_FRAME_OFFSET	AARCH64_INSN_SIZE
 
 #ifndef __ASSEMBLY__
 #include <linux/compat.h>
diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c
index 407991b..9ab67af 100644
--- a/arch/arm64/kernel/stacktrace.c
+++ b/arch/arm64/kernel/stacktrace.c
@@ -20,6 +20,7 @@
 #include <linux/sched.h>
 #include <linux/stacktrace.h>
 
+#include <asm/insn.h>
 #include <asm/stacktrace.h>
 
 /*
@@ -52,7 +53,7 @@ int notrace unwind_frame(struct stackframe *frame)
 	 * -4 here because we care about the PC at time of bl,
 	 * not where the return will go.
 	 */
-	frame->pc = *(unsigned long *)(fp + 8) - 4;
+	frame->pc = *(unsigned long *)(fp + 8) - AARCH64_INSN_SIZE;
 
 	return 0;
 }
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 1da6029..6566201 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -260,6 +260,9 @@ static inline void ftrace_kill(void) { }
 #endif /* CONFIG_FUNCTION_TRACER */
 
 #ifdef CONFIG_STACK_TRACER
+#ifndef FTRACE_STACK_FRAME_OFFSET
+#define FTRACE_STACK_FRAME_OFFSET 0
+#endif
 extern int stack_tracer_enabled;
 int
 stack_trace_sysctl(struct ctl_table *table, int write,
diff --git a/kernel/trace/trace_stack.c b/kernel/trace/trace_stack.c
index b746399..30521ea 100644
--- a/kernel/trace/trace_stack.c
+++ b/kernel/trace/trace_stack.c
@@ -105,7 +105,7 @@ check_stack(unsigned long ip, unsigned long *stack)
 
 	/* 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)
+		if ((stack_dump_trace[i] + FTRACE_STACK_FRAME_OFFSET) == ip)
 			break;
 	}
 
@@ -133,7 +133,8 @@ check_stack(unsigned long ip, unsigned long *stack)
 		for (; p < top && i < max_stack_trace.nr_entries; p++) {
 			if (stack_dump_trace[i] == ULONG_MAX)
 				break;
-			if (*p == stack_dump_trace[i]) {
+			if (*p == (stack_dump_trace[i]
+					+ FTRACE_STACK_FRAME_OFFSET)) {
 				stack_dump_trace[x] = stack_dump_trace[i++];
 				this_size = stack_dump_index[x++] =
 					(top - p) * sizeof(unsigned long);
----8<----

  reply	other threads:[~2015-07-17 15:34 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
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 [this message]
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=1357EA74-B972-4B99-ADB0-BC7E8F06DDB5@gmail.com \
    --to=jungseoklee85@gmail.com \
    --cc=Catalin.Marinas@arm.com \
    --cc=Will.Deacon@arm.com \
    --cc=broonie@kernel.org \
    --cc=david.griego@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=olof@lixom.net \
    --cc=rostedt@goodmis.org \
    --cc=takahiro.akashi@linaro.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.