Skip to content

Commit 868baf0

Browse files
Steven Rostedtrostedt
Steven Rostedt
authored andcommitted
ftrace: Fix memory leak with function graph and cpu hotplug
When the fuction graph tracer starts, it needs to make a special stack for each task to save the real return values of the tasks. All running tasks have this stack created, as well as any new tasks. On CPU hot plug, the new idle task will allocate a stack as well when init_idle() is called. The problem is that cpu hotplug does not create a new idle_task. Instead it uses the idle task that existed when the cpu went down. ftrace_graph_init_task() will add a new ret_stack to the task that is given to it. Because a clone will make the task have a stack of its parent it does not check if the task's ret_stack is already NULL or not. When the CPU hotplug code starts a CPU up again, it will allocate a new stack even though one already existed for it. The solution is to treat the idle_task specially. In fact, the function_graph code already does, just not at init_idle(). Instead of using the ftrace_graph_init_task() for the idle task, which that function expects the task to be a clone, have a separate ftrace_graph_init_idle_task(). Also, we will create a per_cpu ret_stack that is used by the idle task. When we call ftrace_graph_init_idle_task() it will check if the idle task's ret_stack is NULL, if it is, then it will assign it the per_cpu ret_stack. Reported-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> Suggested-by: Peter Zijlstra <a.p.zijlstra@chello.nl> Cc: Stable Tree <stable@kernel.org> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
1 parent 862b6f6 commit 868baf0

File tree

3 files changed

+48
-8
lines changed

3 files changed

+48
-8
lines changed

include/linux/ftrace.h

+2
Original file line numberDiff line numberDiff line change
@@ -428,6 +428,7 @@ extern void unregister_ftrace_graph(void);
428428

429429
extern void ftrace_graph_init_task(struct task_struct *t);
430430
extern void ftrace_graph_exit_task(struct task_struct *t);
431+
extern void ftrace_graph_init_idle_task(struct task_struct *t, int cpu);
431432

432433
static inline int task_curr_ret_stack(struct task_struct *t)
433434
{
@@ -451,6 +452,7 @@ static inline void unpause_graph_tracing(void)
451452

452453
static inline void ftrace_graph_init_task(struct task_struct *t) { }
453454
static inline void ftrace_graph_exit_task(struct task_struct *t) { }
455+
static inline void ftrace_graph_init_idle_task(struct task_struct *t, int cpu) { }
454456

455457
static inline int register_ftrace_graph(trace_func_graph_ret_t retfunc,
456458
trace_func_graph_ent_t entryfunc)

kernel/sched.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -5571,7 +5571,7 @@ void __cpuinit init_idle(struct task_struct *idle, int cpu)
55715571
* The idle tasks have their own, simple scheduling class:
55725572
*/
55735573
idle->sched_class = &idle_sched_class;
5574-
ftrace_graph_init_task(idle);
5574+
ftrace_graph_init_idle_task(idle, cpu);
55755575
}
55765576

55775577
/*

kernel/trace/ftrace.c

+45-7
Original file line numberDiff line numberDiff line change
@@ -3328,7 +3328,7 @@ static int start_graph_tracing(void)
33283328
/* The cpu_boot init_task->ret_stack will never be freed */
33293329
for_each_online_cpu(cpu) {
33303330
if (!idle_task(cpu)->ret_stack)
3331-
ftrace_graph_init_task(idle_task(cpu));
3331+
ftrace_graph_init_idle_task(idle_task(cpu), cpu);
33323332
}
33333333

33343334
do {
@@ -3418,6 +3418,49 @@ void unregister_ftrace_graph(void)
34183418
mutex_unlock(&ftrace_lock);
34193419
}
34203420

3421+
static DEFINE_PER_CPU(struct ftrace_ret_stack *, idle_ret_stack);
3422+
3423+
static void
3424+
graph_init_task(struct task_struct *t, struct ftrace_ret_stack *ret_stack)
3425+
{
3426+
atomic_set(&t->tracing_graph_pause, 0);
3427+
atomic_set(&t->trace_overrun, 0);
3428+
t->ftrace_timestamp = 0;
3429+
/* make curr_ret_stack visable before we add the ret_stack */
3430+
smp_wmb();
3431+
t->ret_stack = ret_stack;
3432+
}
3433+
3434+
/*
3435+
* Allocate a return stack for the idle task. May be the first
3436+
* time through, or it may be done by CPU hotplug online.
3437+
*/
3438+
void ftrace_graph_init_idle_task(struct task_struct *t, int cpu)
3439+
{
3440+
t->curr_ret_stack = -1;
3441+
/*
3442+
* The idle task has no parent, it either has its own
3443+
* stack or no stack at all.
3444+
*/
3445+
if (t->ret_stack)
3446+
WARN_ON(t->ret_stack != per_cpu(idle_ret_stack, cpu));
3447+
3448+
if (ftrace_graph_active) {
3449+
struct ftrace_ret_stack *ret_stack;
3450+
3451+
ret_stack = per_cpu(idle_ret_stack, cpu);
3452+
if (!ret_stack) {
3453+
ret_stack = kmalloc(FTRACE_RETFUNC_DEPTH
3454+
* sizeof(struct ftrace_ret_stack),
3455+
GFP_KERNEL);
3456+
if (!ret_stack)
3457+
return;
3458+
per_cpu(idle_ret_stack, cpu) = ret_stack;
3459+
}
3460+
graph_init_task(t, ret_stack);
3461+
}
3462+
}
3463+
34213464
/* Allocate a return stack for newly created task */
34223465
void ftrace_graph_init_task(struct task_struct *t)
34233466
{
@@ -3433,12 +3476,7 @@ void ftrace_graph_init_task(struct task_struct *t)
34333476
GFP_KERNEL);
34343477
if (!ret_stack)
34353478
return;
3436-
atomic_set(&t->tracing_graph_pause, 0);
3437-
atomic_set(&t->trace_overrun, 0);
3438-
t->ftrace_timestamp = 0;
3439-
/* make curr_ret_stack visable before we add the ret_stack */
3440-
smp_wmb();
3441-
t->ret_stack = ret_stack;
3479+
graph_init_task(t, ret_stack);
34423480
}
34433481
}
34443482

0 commit comments

Comments
 (0)