Commit 72ac426a authored by Steven Rostedt (Red Hat)'s avatar Steven Rostedt (Red Hat) Committed by Steven Rostedt

tracing: Clean up stack tracing and fix fentry updates

Akashi Takahiro was porting the stack tracer to arm64 and found some
issues with it. One was that it repeats the top function, due to the
stack frame added by the mcount caller and added by itself. This
was added when fentry came in, and before fentry created its own stack
frame. But x86's fentry now creates its own stack frame, and there's
no need to insert the function again.

This also cleans up the code a bit, where it doesn't need to do something
special for fentry, and doesn't include insertion of a duplicate
entry for the called function being traced.

Link: http://lkml.kernel.org/r/55A646EE.6030402@linaro.orgSome-suggestions-by: default avatarJungseok Lee <jungseoklee85@gmail.com>
Some-suggestions-by: default avatarMark Rutland <mark.rutland@arm.com>
Reported-by: default avatarAKASHI Takahiro <takahiro.akashi@linaro.org>
Signed-off-by: default avatarSteven Rostedt <rostedt@goodmis.org>
parent d90fd774
...@@ -18,12 +18,6 @@ ...@@ -18,12 +18,6 @@
#define STACK_TRACE_ENTRIES 500 #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] = static unsigned long stack_dump_trace[STACK_TRACE_ENTRIES+1] =
{ [0 ... (STACK_TRACE_ENTRIES)] = ULONG_MAX }; { [0 ... (STACK_TRACE_ENTRIES)] = ULONG_MAX };
static unsigned stack_dump_index[STACK_TRACE_ENTRIES]; static unsigned stack_dump_index[STACK_TRACE_ENTRIES];
...@@ -35,7 +29,7 @@ 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 = { static struct stack_trace max_stack_trace = {
.max_entries = STACK_TRACE_ENTRIES - 1, .max_entries = STACK_TRACE_ENTRIES - 1,
.entries = &stack_dump_trace[1], .entries = &stack_dump_trace[0],
}; };
static unsigned long max_stack_size; static unsigned long max_stack_size;
...@@ -55,7 +49,7 @@ static inline void print_max_stack(void) ...@@ -55,7 +49,7 @@ static inline void print_max_stack(void)
pr_emerg(" Depth Size Location (%d entries)\n" pr_emerg(" Depth Size Location (%d entries)\n"
" ----- ---- --------\n", " ----- ---- --------\n",
max_stack_trace.nr_entries - 1); max_stack_trace.nr_entries);
for (i = 0; i < max_stack_trace.nr_entries; i++) { for (i = 0; i < max_stack_trace.nr_entries; i++) {
if (stack_dump_trace[i] == ULONG_MAX) if (stack_dump_trace[i] == ULONG_MAX)
...@@ -77,7 +71,7 @@ check_stack(unsigned long ip, unsigned long *stack) ...@@ -77,7 +71,7 @@ check_stack(unsigned long ip, unsigned long *stack)
unsigned long this_size, flags; unsigned long *p, *top, *start; unsigned long this_size, flags; unsigned long *p, *top, *start;
static int tracer_frame; static int tracer_frame;
int frame_size = ACCESS_ONCE(tracer_frame); int frame_size = ACCESS_ONCE(tracer_frame);
int i; int i, x;
this_size = ((unsigned long)stack) & (THREAD_SIZE-1); this_size = ((unsigned long)stack) & (THREAD_SIZE-1);
this_size = THREAD_SIZE - this_size; this_size = THREAD_SIZE - this_size;
...@@ -105,26 +99,20 @@ check_stack(unsigned long ip, unsigned long *stack) ...@@ -105,26 +99,20 @@ check_stack(unsigned long ip, unsigned long *stack)
max_stack_size = this_size; max_stack_size = this_size;
max_stack_trace.nr_entries = 0; max_stack_trace.nr_entries = 0;
max_stack_trace.skip = 3;
if (using_ftrace_ops_list_func())
max_stack_trace.skip = 4;
else
max_stack_trace.skip = 3;
save_stack_trace(&max_stack_trace); save_stack_trace(&max_stack_trace);
/* /* Skip over the overhead of the stack tracer itself */
* Add the passed in ip from the function tracer. for (i = 0; i < max_stack_trace.nr_entries; i++) {
* Searching for this on the stack will skip over if (stack_dump_trace[i] == ip)
* most of the overhead from the stack tracer itself. break;
*/ }
stack_dump_trace[0] = ip;
max_stack_trace.nr_entries++;
/* /*
* Now find where in the stack these are. * Now find where in the stack these are.
*/ */
i = 0; x = 0;
start = stack; start = stack;
top = (unsigned long *) top = (unsigned long *)
(((unsigned long)start & ~(THREAD_SIZE-1)) + THREAD_SIZE); (((unsigned long)start & ~(THREAD_SIZE-1)) + THREAD_SIZE);
...@@ -139,12 +127,15 @@ check_stack(unsigned long ip, unsigned long *stack) ...@@ -139,12 +127,15 @@ check_stack(unsigned long ip, unsigned long *stack)
while (i < max_stack_trace.nr_entries) { while (i < max_stack_trace.nr_entries) {
int found = 0; int found = 0;
stack_dump_index[i] = this_size; stack_dump_index[x] = this_size;
p = start; p = start;
for (; p < top && i < max_stack_trace.nr_entries; p++) { 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]) {
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); (top - p) * sizeof(unsigned long);
found = 1; found = 1;
/* Start the search from here */ /* Start the search from here */
...@@ -156,7 +147,7 @@ check_stack(unsigned long ip, unsigned long *stack) ...@@ -156,7 +147,7 @@ check_stack(unsigned long ip, unsigned long *stack)
* out what that is, then figure it out * out what that is, then figure it out
* now. * now.
*/ */
if (unlikely(!tracer_frame) && i == 1) { if (unlikely(!tracer_frame)) {
tracer_frame = (p - stack) * tracer_frame = (p - stack) *
sizeof(unsigned long); sizeof(unsigned long);
max_stack_size -= tracer_frame; max_stack_size -= tracer_frame;
...@@ -168,6 +159,10 @@ check_stack(unsigned long ip, unsigned long *stack) ...@@ -168,6 +159,10 @@ check_stack(unsigned long ip, unsigned long *stack)
i++; i++;
} }
max_stack_trace.nr_entries = x;
for (; x < i; x++)
stack_dump_trace[x] = ULONG_MAX;
if (task_stack_end_corrupted(current)) { if (task_stack_end_corrupted(current)) {
print_max_stack(); print_max_stack();
BUG(); BUG();
...@@ -192,24 +187,7 @@ stack_trace_call(unsigned long ip, unsigned long parent_ip, ...@@ -192,24 +187,7 @@ stack_trace_call(unsigned long ip, unsigned long parent_ip,
if (per_cpu(trace_active, cpu)++ != 0) if (per_cpu(trace_active, cpu)++ != 0)
goto out; goto out;
/* ip += MCOUNT_INSN_SIZE;
* 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;
check_stack(ip, &stack); check_stack(ip, &stack);
...@@ -284,7 +262,7 @@ __next(struct seq_file *m, loff_t *pos) ...@@ -284,7 +262,7 @@ __next(struct seq_file *m, loff_t *pos)
{ {
long n = *pos - 1; long n = *pos - 1;
if (n >= max_stack_trace.nr_entries || stack_dump_trace[n] == ULONG_MAX) if (n > max_stack_trace.nr_entries || stack_dump_trace[n] == ULONG_MAX)
return NULL; return NULL;
m->private = (void *)n; m->private = (void *)n;
...@@ -354,7 +332,7 @@ static int t_show(struct seq_file *m, void *v) ...@@ -354,7 +332,7 @@ static int t_show(struct seq_file *m, void *v)
seq_printf(m, " Depth Size Location" seq_printf(m, " Depth Size Location"
" (%d entries)\n" " (%d entries)\n"
" ----- ---- --------\n", " ----- ---- --------\n",
max_stack_trace.nr_entries - 1); max_stack_trace.nr_entries);
if (!stack_tracer_enabled && !max_stack_size) if (!stack_tracer_enabled && !max_stack_size)
print_disabled(m); print_disabled(m);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment