Skip to content

Commit 4df2971

Browse files
rostedtrostedt
committed
tracing: Remove most or all of stack tracer stack size from stack_max_size
Currently, the depth reported in the stack tracer stack_trace file does not match the stack_max_size file. This is because the stack_max_size includes the overhead of stack tracer itself while the depth does not. The first time a max is triggered, a calculation is not performed that figures out the overhead of the stack tracer and subtracts it from the stack_max_size variable. The overhead is stored and is subtracted from the reported stack size for comparing for a new max. Now the stack_max_size corresponds to the reported depth: # cat stack_max_size 4640 # cat stack_trace Depth Size Location (48 entries) ----- ---- -------- 0) 4640 32 _raw_spin_lock+0x18/0x24 1) 4608 112 ____cache_alloc+0xb7/0x22d 2) 4496 80 kmem_cache_alloc+0x63/0x12f 3) 4416 16 mempool_alloc_slab+0x15/0x17 [...] While testing against and older gcc on x86 that uses mcount instead of fentry, I found that pasing in ip + MCOUNT_INSN_SIZE let the stack trace show one more function deep which was missing before. Cc: stable@vger.kernel.org Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
1 parent d4ecbfc commit 4df2971

1 file changed

Lines changed: 54 additions & 21 deletions

File tree

‎kernel/trace/trace_stack.c‎

Lines changed: 54 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -20,27 +20,24 @@
2020

2121
#define STACK_TRACE_ENTRIES 500
2222

23-
/*
24-
* If fentry is used, then the function being traced will
25-
* jump to fentry directly before it sets up its stack frame.
26-
* We need to ignore that one and record the parent. Since
27-
* the stack frame for the traced function wasn't set up yet,
28-
* the stack_trace wont see the parent. That needs to be added
29-
* manually to stack_dump_trace[] as the first element.
30-
*/
3123
#ifdef CC_USING_FENTRY
32-
# define add_func 1
24+
# define fentry 1
3325
#else
34-
# define add_func 0
26+
# define fentry 0
3527
#endif
3628

3729
static unsigned long stack_dump_trace[STACK_TRACE_ENTRIES+1] =
3830
{ [0 ... (STACK_TRACE_ENTRIES)] = ULONG_MAX };
3931
static unsigned stack_dump_index[STACK_TRACE_ENTRIES];
4032

33+
/*
34+
* Reserve one entry for the passed in ip. This will allow
35+
* us to remove most or all of the stack size overhead
36+
* added by the stack tracer itself.
37+
*/
4138
static struct stack_trace max_stack_trace = {
42-
.max_entries = STACK_TRACE_ENTRIES - add_func,
43-
.entries = &stack_dump_trace[add_func],
39+
.max_entries = STACK_TRACE_ENTRIES - 1,
40+
.entries = &stack_dump_trace[1],
4441
};
4542

4643
static unsigned long max_stack_size;
@@ -58,10 +55,14 @@ check_stack(unsigned long ip, unsigned long *stack)
5855
{
5956
unsigned long this_size, flags;
6057
unsigned long *p, *top, *start;
58+
static int tracer_frame;
59+
int frame_size = ACCESS_ONCE(tracer_frame);
6160
int i;
6261

6362
this_size = ((unsigned long)stack) & (THREAD_SIZE-1);
6463
this_size = THREAD_SIZE - this_size;
64+
/* Remove the frame of the tracer */
65+
this_size -= frame_size;
6566

6667
if (this_size <= max_stack_size)
6768
return;
@@ -73,6 +74,10 @@ check_stack(unsigned long ip, unsigned long *stack)
7374
local_irq_save(flags);
7475
arch_spin_lock(&max_stack_lock);
7576

77+
/* In case another CPU set the tracer_frame on us */
78+
if (unlikely(!frame_size))
79+
this_size -= tracer_frame;
80+
7681
/* a race could have already updated it */
7782
if (this_size <= max_stack_size)
7883
goto out;
@@ -85,15 +90,12 @@ check_stack(unsigned long ip, unsigned long *stack)
8590
save_stack_trace(&max_stack_trace);
8691

8792
/*
88-
* When fentry is used, the traced function does not get
89-
* its stack frame set up, and we lose the parent.
90-
* Add that one in manally. We set up save_stack_trace()
91-
* to not touch the first element in this case.
93+
* Add the passed in ip from the function tracer.
94+
* Searching for this on the stack will skip over
95+
* most of the overhead from the stack tracer itself.
9296
*/
93-
if (add_func) {
94-
stack_dump_trace[0] = ip;
95-
max_stack_trace.nr_entries++;
96-
}
97+
stack_dump_trace[0] = ip;
98+
max_stack_trace.nr_entries++;
9799

98100
/*
99101
* Now find where in the stack these are.
@@ -123,6 +125,18 @@ check_stack(unsigned long ip, unsigned long *stack)
123125
found = 1;
124126
/* Start the search from here */
125127
start = p + 1;
128+
/*
129+
* We do not want to show the overhead
130+
* of the stack tracer stack in the
131+
* max stack. If we haven't figured
132+
* out what that is, then figure it out
133+
* now.
134+
*/
135+
if (unlikely(!tracer_frame) && i == 1) {
136+
tracer_frame = (p - stack) *
137+
sizeof(unsigned long);
138+
max_stack_size -= tracer_frame;
139+
}
126140
}
127141
}
128142

@@ -149,7 +163,26 @@ stack_trace_call(unsigned long ip, unsigned long parent_ip,
149163
if (per_cpu(trace_active, cpu)++ != 0)
150164
goto out;
151165

152-
check_stack(parent_ip, &stack);
166+
/*
167+
* When fentry is used, the traced function does not get
168+
* its stack frame set up, and we lose the parent.
169+
* The ip is pretty useless because the function tracer
170+
* was called before that function set up its stack frame.
171+
* In this case, we use the parent ip.
172+
*
173+
* By adding the return address of either the parent ip
174+
* or the current ip we can disregard most of the stack usage
175+
* caused by the stack tracer itself.
176+
*
177+
* The function tracer always reports the address of where the
178+
* mcount call was, but the stack will hold the return address.
179+
*/
180+
if (fentry)
181+
ip = parent_ip;
182+
else
183+
ip += MCOUNT_INSN_SIZE;
184+
185+
check_stack(ip, &stack);
153186

154187
out:
155188
per_cpu(trace_active, cpu)--;

0 commit comments

Comments
 (0)