Skip to contentSkip to conversation

tracing/function-return-tracer: store return stack into task_struct and allocate it dynamically

PATCH New Frederic Weisbecker 2 participants 3 messages Nov 23, 2008

Subsystems (6)
x86 MM core Tracing Scheduler +2 more subsystems
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 tracing/function-return-tracer: store return stack into task_struct and allocate it dynamically

Impact: use deeper tracing depth safely

Some tests showed that function return tracing needed a more deeper depth of function calls. But it could be unsafe to store these return addresses to the stack.
So these arrays will now be allocated dynamically into task_struct of current only when the tracer is activated.

Typical scheme when tracer is activated:
_ allocate a return stack for each task in global list. _ fork: allocate the return stack for the newly created task _ exit: free return stack of current
_ idle init: same as fork

I chose a default depth of 50. I don't have anymore overruns (didn't tested for a long time).

---
 arch/x86/include/asm/ftrace.h      |    1 -
 arch/x86/include/asm/thread_info.h |   29 -----------
 arch/x86/kernel/ftrace.c           |   29 ++++++-----
 include/linux/ftrace.h             |    5 ++
 include/linux/sched.h              |   23 ++++-----
 kernel/exit.c                      |    5 ++-
 kernel/fork.c                      |    4 ++
 kernel/sched.c                     |    3 +
 kernel/trace/ftrace.c              |   96 +++++++++++++++++++++++++++++++++++-
 9 files changed, 137 insertions(+), 58 deletions(-)
arch/x86/include/asm/ftrace.h +0 -1
index 2bb43b4..754a3e0 100644
@@ -29,7 +29,6 @@ struct dyn_arch_ftrace {
 #endif /* CONFIG_FUNCTION_TRACER */
 
 #ifdef CONFIG_FUNCTION_RET_TRACER
-#define FTRACE_RET_STACK_SIZE 20
 
 #ifndef __ASSEMBLY__
 
arch/x86/include/asm/thread_info.h +0 -29
index e90e81e..0921b40 100644
@@ -40,36 +40,8 @@ struct thread_info {
 						*/
 	__u8			supervisor_stack[0];
 #endif
-
-#ifdef CONFIG_FUNCTION_RET_TRACER
-	/* Index of current stored adress in ret_stack */
-	int		curr_ret_stack;
-	/* Stack of return addresses for return function tracing */
-	struct ftrace_ret_stack	ret_stack[FTRACE_RET_STACK_SIZE];
-	/*
-	 * Number of functions that haven't been traced
-	 * because of depth overrun.
-	 */
-	atomic_t	trace_overrun;
-#endif
 };
 
-#ifdef CONFIG_FUNCTION_RET_TRACER
-#define INIT_THREAD_INFO(tsk)			\
-{						\
-	.task		= &tsk,			\
-	.exec_domain	= &default_exec_domain,	\
-	.flags		= 0,			\
-	.cpu		= 0,			\
-	.preempt_count	= 1,			\
-	.addr_limit	= KERNEL_DS,		\
-	.restart_block = {			\
-		.fn = do_no_restart_syscall,	\
-	},					\
-	.curr_ret_stack = -1,\
-	.trace_overrun	= ATOMIC_INIT(0)	\
-}
-#else
 #define INIT_THREAD_INFO(tsk)			\
 {						\
 	.task		= &tsk,			\
@@ -82,7 +54,6 @@ struct thread_info {
 		.fn = do_no_restart_syscall,	\
 	},					\
 }
-#endif
 
 #define init_thread_info	(init_thread_union.thread_info)
 #define init_stack		(init_thread_union.stack)
arch/x86/kernel/ftrace.c +15 -14
index 356bb1e..bb137f7 100644
@@ -350,19 +350,21 @@ static int push_return_trace(unsigned long ret, unsigned long long time,
 				unsigned long func)
 {
 	int index;
-	struct thread_info *ti = current_thread_info();
+
+	if (!current->ret_stack)
+		return -EBUSY;
 
 	/* The return trace stack is full */
-	if (ti->curr_ret_stack == FTRACE_RET_STACK_SIZE - 1) {
-		atomic_inc(&ti->trace_overrun);
+	if (current->curr_ret_stack == FTRACE_RETFUNC_DEPTH - 1) {
+		atomic_inc(&current->trace_overrun);
 		return -EBUSY;
 	}
 
-	index = ++ti->curr_ret_stack;
+	index = ++current->curr_ret_stack;
 	barrier();
-	ti->ret_stack[index].ret = ret;
-	ti->ret_stack[index].func = func;
-	ti->ret_stack[index].calltime = time;
+	current->ret_stack[index].ret = ret;
+	current->ret_stack[index].func = func;
+	current->ret_stack[index].calltime = time;
 
 	return 0;
 }
@@ -373,13 +375,12 @@ static void pop_return_trace(unsigned long *ret, unsigned long long *time,
 {
 	int index;
 
-	struct thread_info *ti = current_thread_info();
-	index = ti->curr_ret_stack;
-	*ret = ti->ret_stack[index].ret;
-	*func = ti->ret_stack[index].func;
-	*time = ti->ret_stack[index].calltime;
-	*overrun = atomic_read(&ti->trace_overrun);
-	ti->curr_ret_stack--;
+	index = current->curr_ret_stack;
+	*ret = current->ret_stack[index].ret;
+	*func = current->ret_stack[index].func;
+	*time = current->ret_stack[index].calltime;
+	*overrun = atomic_read(&current->trace_overrun);
+	current->curr_ret_stack--;
 }
 
 /*
include/linux/ftrace.h +5 -0
index f7ba4ea..2ba259b 100644
@@ -323,6 +323,8 @@ struct ftrace_retfunc {
 };
 
 #ifdef CONFIG_FUNCTION_RET_TRACER
+#define FTRACE_RETFUNC_DEPTH 50
+#define FTRACE_RETSTACK_ALLOC_SIZE 32
 /* Type of a callback handler of tracing return function */
 typedef void (*trace_function_return_t)(struct ftrace_retfunc *);
 
@@ -330,6 +332,9 @@ extern int register_ftrace_return(trace_function_return_t func);
 /* The current handler in use */
 extern trace_function_return_t ftrace_function_return;
 extern void unregister_ftrace_return(void);
+
+extern void ftrace_retfunc_init_task(struct task_struct *t);
+extern void ftrace_retfunc_exit_task(struct task_struct *t);
 #endif
 
 #endif /* _LINUX_FTRACE_H */
include/linux/sched.h +11 -12
index deba0ed..e22cb72 100644
@@ -1349,6 +1349,17 @@ struct task_struct {
 	unsigned long default_timer_slack_ns;
 
 	struct list_head	*scm_work_list;
+#ifdef CONFIG_FUNCTION_RET_TRACER
+	/* Index of current stored adress in ret_stack */
+	int curr_ret_stack;
+	/* Stack of return addresses for return function tracing */
+	struct ftrace_ret_stack	*ret_stack;
+	/*
+	 * Number of functions that haven't been traced
+	 * because of depth overrun.
+	 */
+	atomic_t trace_overrun;
+#endif
 };
 
 /*
@@ -2003,18 +2014,6 @@ static inline void setup_thread_stack(struct task_struct *p, struct task_struct
 {
 	*task_thread_info(p) = *task_thread_info(org);
 	task_thread_info(p)->task = p;
-
-#ifdef CONFIG_FUNCTION_RET_TRACER
-	/*
-	 * When fork() creates a child process, this function is called.
-	 * But the child task may not inherit the return adresses traced
-	 * by the return function tracer because it will directly execute
-	 * in userspace and will not return to kernel functions its parent
-	 * used.
-	 */
-	task_thread_info(p)->curr_ret_stack = -1;
-	atomic_set(&task_thread_info(p)->trace_overrun, 0);
-#endif
 }
 
 static inline unsigned long *end_of_stack(struct task_struct *p)
kernel/exit.c +4 -1
index 1eb455c..2c98e8c 100644
@@ -47,6 +47,7 @@
 #include <linux/task_io_accounting_ops.h>
 #include <linux/tracehook.h>
 #include <trace/sched.h>
+#include <linux/ftrace.h>
 
 #include <asm/uaccess.h>
 #include <asm/unistd.h>
@@ -1124,7 +1125,9 @@ NORET_TYPE void do_exit(long code)
 	preempt_disable();
 	/* causes final put_task_struct in finish_task_switch(). */
 	tsk->state = TASK_DEAD;
-
+#ifdef CONFIG_FUNCTION_RET_TRACER
+	ftrace_retfunc_exit_task(tsk);
+#endif
 	schedule();
 	BUG();
 	/* Avoid "noreturn function does return".  */
kernel/fork.c +4 -0
index 775d62a..76afd0a 100644
@@ -61,6 +61,7 @@
 #include <linux/blkdev.h>
 #include <trace/sched.h>
 #include <linux/magic.h>
+#include <linux/ftrace.h>
 
 #include <asm/pgtable.h>
 #include <asm/pgalloc.h>
@@ -1274,6 +1275,9 @@ static struct task_struct *copy_process(unsigned long clone_flags,
 	total_forks++;
 	spin_unlock(&current->sighand->siglock);
 	write_unlock_irq(&tasklist_lock);
+#ifdef CONFIG_FUNCTION_RET_TRACER
+	ftrace_retfunc_init_task(p);
+#endif
 	proc_fork_connector(p);
 	cgroup_post_fork(p);
 	return p;
kernel/sched.c +3 -0
index 2b33c28..510fedc 100644
@@ -5864,6 +5864,9 @@ void __cpuinit init_idle(struct task_struct *idle, int cpu)
 	 * The idle tasks have their own, simple scheduling class:
 	 */
 	idle->sched_class = &idle_sched_class;
+#ifdef CONFIG_FUNCTION_RET_TRACER
+	ftrace_retfunc_init_task(idle);
+#endif
 }
 
 /*
kernel/trace/ftrace.c +95 -1

124 lines not on this page · raw message

Signature · 1 line
1.5.2.5
#1 lore raw

PATCH 1/1 tracing/function-return-tracer: clean up task start/exit callbacks

Quoted 19 lines · Frederic Weisbecker

Impact: use deeper tracing depth safely

Some tests showed that function return tracing needed a more deeper depth of function calls. But it could be unsafe to store these return addresses to the stack.
So these arrays will now be allocated dynamically into task_struct of current only when the tracer is activated.

Typical scheme when tracer is activated:
_ allocate a return stack for each task in global list. _ fork: allocate the return stack for the newly created task _ exit: free return stack of current
_ idle init: same as fork

I chose a default depth of 50. I don't have anymore overruns (didn't tested for a long time).

Quoted 11 lines
---
 arch/x86/include/asm/ftrace.h      |    1 -
 arch/x86/include/asm/thread_info.h |   29 -----------
 arch/x86/kernel/ftrace.c           |   29 ++++++-----
 include/linux/ftrace.h             |    5 ++
 include/linux/sched.h              |   23 ++++-----
 kernel/exit.c                      |    5 ++-
 kernel/fork.c                      |    4 ++
 kernel/sched.c                     |    3 +
 kernel/trace/ftrace.c              |   96 +++++++++++++++++++++++++++++++++++-
 9 files changed, 137 insertions(+), 58 deletions(-)

nice! Applied to tip/tracing/function-return-tracer and i started testing it as well.

I've done the small cleanup below on top of your patch - we try to avoid #ifdefs in core kernel code as much as possible, by hiding .config details into include files.

Ingo

-------------->

Quoted 1 line

From 82f60f0bc854aada696f27d863c03bef91f1509d Mon Sep 17 00:00:00 2001

From: Ingo Molnar <[email protected]>
Date: Sun, 23 Nov 2008 09:18:56 +0100
Subject: [PATCH] tracing/function-return-tracer: clean up task start/exit callbacks

Impact: cleanup

Eliminate #ifdefs in core code by using empty inline functions.

---
 include/linux/ftrace.h |    3 +++
 kernel/exit.c          |    2 --
 kernel/fork.c          |    2 --
 kernel/sched.c         |    2 --
 4 files changed, 3 insertions(+), 6 deletions(-)
include/linux/ftrace.h +3 -0
index 2ba259b..938ca19 100644
@@ -335,6 +335,9 @@ extern void unregister_ftrace_return(void);
 
 extern void ftrace_retfunc_init_task(struct task_struct *t);
 extern void ftrace_retfunc_exit_task(struct task_struct *t);
+#else
+static inline void ftrace_retfunc_init_task(struct task_struct *t) { }
+static inline void ftrace_retfunc_exit_task(struct task_struct *t) { }
 #endif
 
 #endif /* _LINUX_FTRACE_H */
kernel/exit.c +0 -2
index b9d4463..ef04d03 100644
@@ -1128,9 +1128,7 @@ NORET_TYPE void do_exit(long code)
 	preempt_disable();
 	/* causes final put_task_struct in finish_task_switch(). */
 	tsk->state = TASK_DEAD;
-#ifdef CONFIG_FUNCTION_RET_TRACER
 	ftrace_retfunc_exit_task(tsk);
-#endif
 	schedule();
 	BUG();
 	/* Avoid "noreturn function does return".  */
kernel/fork.c +0 -2
index d1eb30e..fbf4a4c 100644
@@ -1270,9 +1270,7 @@ static struct task_struct *copy_process(unsigned long clone_flags,
 	total_forks++;
 	spin_unlock(&current->sighand->siglock);
 	write_unlock_irq(&tasklist_lock);
-#ifdef CONFIG_FUNCTION_RET_TRACER
 	ftrace_retfunc_init_task(p);
-#endif
 	proc_fork_connector(p);
 	cgroup_post_fork(p);
 	return p;
kernel/sched.c +0 -2
index fb17205..388d9db 100644
@@ -5901,9 +5901,7 @@ void __cpuinit init_idle(struct task_struct *idle, int cpu)
 	 * The idle tasks have their own, simple scheduling class:
 	 */
 	idle->sched_class = &idle_sched_class;
-#ifdef CONFIG_FUNCTION_RET_TRACER
 	ftrace_retfunc_init_task(idle);
-#endif
 }
 
 /*
#2 lore raw

Re: [PATCH] tracing/function-return-tracer: clean up task start/exit callbacks

2008/11/23 Ingo Molnar <[email protected]>:

Quoted 34 lines · Frederic Weisbecker
Quoted 20 lines

Impact: use deeper tracing depth safely

Some tests showed that function return tracing needed a more deeper depth of function calls. But it could be unsafe to store these return addresses to the stack.
So these arrays will now be allocated dynamically into task_struct of current only when the tracer is activated.

Typical scheme when tracer is activated:
_ allocate a return stack for each task in global list. _ fork: allocate the return stack for the newly created task _ exit: free return stack of current
_ idle init: same as fork

I chose a default depth of 50. I don't have anymore overruns (didn't tested for a long time).

Quoted 11 lines
---
 arch/x86/include/asm/ftrace.h      |    1 -
 arch/x86/include/asm/thread_info.h |   29 -----------
 arch/x86/kernel/ftrace.c           |   29 ++++++-----
 include/linux/ftrace.h             |    5 ++
 include/linux/sched.h              |   23 ++++-----
 kernel/exit.c                      |    5 ++-
 kernel/fork.c                      |    4 ++
 kernel/sched.c                     |    3 +
 kernel/trace/ftrace.c              |   96 +++++++++++++++++++++++++++++++++++-
 9 files changed, 137 insertions(+), 58 deletions(-)
Quoted 8 lines

nice! Applied to tip/tracing/function-return-tracer and i started testing it as well.

I've done the small cleanup below on top of your patch - we try to avoid #ifdefs in core kernel code as much as possible, by hiding .config details into include files.

Ingo

Ok thanks!