From: jungseoklee85@gmail.com (Jungseok Lee)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC 2/3] arm64: refactor save_stack_trace()
Date: Fri, 17 Jul 2015 23:38:28 +0900 [thread overview]
Message-ID: <10104998-CA54-4150-9228-2ED8C327911D@gmail.com> (raw)
In-Reply-To: <55A862C0.4040101@linaro.org>
On Jul 17, 2015, at 11:04 AM, AKASHI Takahiro wrote:
> Jungseok,
>
> Thank you for your testing and reviews.
You're welcome.
> On 07/16/2015 10:29 PM, Jungseok Lee wrote:
>> On Jul 16, 2015, at 10:08 AM, AKASHI Takahiro wrote:
>>
>> Hi, AKASHI
>>
>>> On 07/16/2015 09:27 AM, AKASHI Takahiro wrote:
>>>> On 07/16/2015 01:13 AM, Steven Rostedt wrote:
>>>>> 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?
>>>>
>>>> Looks nice. The patch greatly simplifies changes on arm64 side.
>>>
>>> As follows:
>>>
>>> - Takahiro AKASHI
>>>
>>> diff --git a/arch/arm64/include/asm/ftrace.h b/arch/arm64/include/asm/ftrace.h
>>> index c5534fa..868d6f1 100644
>>> --- a/arch/arm64/include/asm/ftrace.h
>>> +++ b/arch/arm64/include/asm/ftrace.h
>>> @@ -15,6 +15,7 @@
>>>
>>> #define MCOUNT_ADDR ((unsigned long)_mcount)
>>> #define MCOUNT_INSN_SIZE AARCH64_INSN_SIZE
>>> +#define FTRACE_STACK_FRAME_OFFSET 4 /* sync it up with stacktrace.c */
>
> Well,
> #define FTRACE_STACK_FRAME_OFFSET AARCH64_INSN_SIZE
> might be better.
Agree.
>
>> How about binding it to -4 in unwind_frame function?
>
> Do you mean like this?
> In unwind_frame(),
> frame->pc = *(unsigned long*)(fp + 8) - AARCH64_INSN_SIZE;
Exactly.
>
>> IMHO, it would help other developers trying to change stack trace code
>> be aware of this stack tracer feature.
>>
>>> #ifndef __ASSEMBLY__
>>> #include <linux/compat.h>
>>> diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
>>> index 1da6029..2c1bf7d 100644
>>> --- a/include/linux/ftrace.h
>>> +++ b/include/linux/ftrace.h
>>> @@ -260,6 +260,13 @@ static inline void ftrace_kill(void) { }
>>> #endif /* CONFIG_FUNCTION_TRACER */
>>>
>>> #ifdef CONFIG_STACK_TRACER
>>> +/*
>>> + * the offset value to add to return address from save_stack_trace()
>>> + */
>>> +#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 9384647..c5b9748 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;
>>> }
>>>
>>> @@ -131,7 +131,8 @@ check_stack(unsigned long ip, unsigned long *stack)
>>> p = start;
>>>
>>> for (; p < top && i < max_stack_trace.nr_entries; p++) {
>>> - 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);
>>> --
>>
>> I've prepared a kernel with the following patches and reviewed them.
>>
>> 1) Steve's clean up patch
>> 2) This patch
>> 3) [RFC 2/3]
>>
>> AFAIU, [RFC 3/3] is not needed any more thanks to Steve's patch.
>
> We don't need [2/3].
Okay. So, I've played a kernel including Steve's latest patch and only [1/3].
Best Regards
Jungseok Lee
next prev parent reply other threads:[~2015-07-17 14:38 UTC|newest]
Thread overview: 53+ 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 ` [RFC 1/3] ftrace: adjust a function's pc to search for in check_stack() for arm64 AKASHI Takahiro
2015-07-13 15:24 ` Steven Rostedt
2015-07-15 0:22 ` AKASHI Takahiro
2015-07-13 5:29 ` [RFC 2/3] arm64: refactor save_stack_trace() AKASHI Takahiro
2015-07-14 12:47 ` Jungseok Lee
2015-07-14 13:31 ` Steven Rostedt
2015-07-15 0:20 ` AKASHI Takahiro
2015-07-15 2:51 ` Steven Rostedt
2015-07-15 11:41 ` AKASHI Takahiro
2015-07-15 14:55 ` Steven Rostedt
2015-07-15 16:13 ` Steven Rostedt
2015-07-16 0:27 ` AKASHI Takahiro
2015-07-16 1:08 ` AKASHI Takahiro
2015-07-16 1:38 ` Steven Rostedt
2015-07-17 10:46 ` Will Deacon
2015-07-16 13:29 ` Jungseok Lee
2015-07-16 13:54 ` Jungseok Lee
2015-07-16 14:24 ` Steven Rostedt
2015-07-16 15:01 ` Jungseok Lee
2015-07-16 15:31 ` Steven Rostedt
2015-07-16 15:52 ` Jungseok Lee
2015-07-16 20:22 ` Steven Rostedt
2015-07-17 2:49 ` AKASHI Takahiro
2015-07-17 3:21 ` Steven Rostedt
2015-07-16 16:16 ` Steven Rostedt
2015-07-17 12:40 ` Mark Rutland
2015-07-17 12:51 ` Steven Rostedt
2015-07-17 13:00 ` Steven Rostedt
2015-07-17 14:28 ` Jungseok Lee
2015-07-17 14:41 ` Steven Rostedt
2015-07-17 14:59 ` Jungseok Lee
2015-07-17 15:34 ` Jungseok Lee
2015-07-17 16:01 ` Steven Rostedt
2015-07-20 16:20 ` Will Deacon
2015-07-20 23:53 ` AKASHI Takahiro
2015-07-21 10:26 ` AKASHI Takahiro
2015-07-21 14:34 ` Jungseok Lee
2015-08-03 9:09 ` Will Deacon
2015-08-03 14:01 ` Steven Rostedt
2015-08-03 14:04 ` Will Deacon
2015-08-03 16:30 ` Jungseok Lee
2015-08-03 16:57 ` Steven Rostedt
2015-08-03 17:22 ` Jungseok Lee
2015-08-03 17:32 ` Steven Rostedt
2015-08-04 7:41 ` AKASHI Takahiro
2015-07-17 2:04 ` AKASHI Takahiro
2015-07-17 14:38 ` Jungseok Lee [this message]
2015-07-16 14:28 ` Mark Rutland
2015-07-16 14:34 ` Steven Rostedt
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 15:01 ` [RFC 0/3] arm64: ftrace: fix incorrect output from stack tracer 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=10104998-CA54-4150-9228-2ED8C327911D@gmail.com \
--to=jungseoklee85@gmail.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 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).