From: takahiro.akashi@linaro.org (AKASHI Takahiro)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC 2/3] arm64: refactor save_stack_trace()
Date: Thu, 16 Jul 2015 09:27:46 +0900 [thread overview]
Message-ID: <55A6FA82.9000901@linaro.org> (raw)
In-Reply-To: <20150715121337.3b31aa84@gandalf.local.home>
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.
- Takahiro AKASHI
> -- 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-16 0:27 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 [this message]
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
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=55A6FA82.9000901@linaro.org \
--to=takahiro.akashi@linaro.org \
--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).