ftrace: rebase patches on top of tip/master + some extras
Subsystems (5)
Steven Rostedt
maintainercover ftrace: rebase patches on top of tip/master + some extras
Ingo,
Here's my patches rebased against latest tip/master. I also added a couple of goodies at the end to enhance the function-graph tracer.
The first patch is a repost of the set_ftrace_pid, so I am Cc'ing the container guys so they can abuse me some more ;-)
The following patches are in:
git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
branch: tip/devel
Steven Rostedt (5):
ftrace: add function tracing to single thread
ftrace: use code patching for ftrace graph tracer
ftrace: let function tracing and function return run together
ftrace: add thread comm to function graph tracer
ftrace: add cpu annotation for function graph tracer
----
Documentation/ftrace.txt | 79 ++++++++++ arch/x86/kernel/entry_32.S | 5 + arch/x86/kernel/ftrace.c | 48 ++++++- include/linux/ftrace.h | 5 + kernel/trace/ftrace.c | 264 +++++++++++++++++++++++++-------- kernel/trace/trace.c | 2 +- kernel/trace/trace.h | 1 + kernel/trace/trace_functions_graph.c | 51 +++++-- 8 files changed, 373 insertions(+), 82 deletions(-)
Steven Rostedt
maintainerPATCH 1/5 ftrace: add function tracing to single thread
From: Steven Rostedt <[email protected]>
Impact: feature to function trace a single thread
This patch adds the ability to function trace a single thread. The file:
/debugfs/tracing/set_ftrace_pid
contains the pid to trace. Valid pids are any positive integer. Writing any negative number to this file will disable the pid tracing and the function tracer will go back to tracing all of threads.
This feature works with both static and dynamic function tracing.
--- Documentation/ftrace.txt | 79 +++++++++++++++++ kernel/trace/ftrace.c | 209 ++++++++++++++++++++++++++++++++++++++++------ 2 files changed, 262 insertions(+), 26 deletions(-)
Documentation/ftrace.txt +79 -0
@@ -127,6 +127,8 @@ of ftrace. Here is a list of some of the key files: be traced. If a function exists in both set_ftrace_filter and set_ftrace_notrace, the function will _not_ be traced. + set_ftrace_pid: Have the function tracer only trace a single thread. + available_filter_functions: This lists the functions that ftrace has processed and can trace. These are the function names that you can pass to "set_ftrace_filter" or
@@ -1073,6 +1075,83 @@ For simple one time traces, the above is sufficent. For anything else, a search through /proc/mounts may be needed to find where the debugfs file-system is mounted. + +Single thread tracing +--------------------- + +By writing into /debug/tracing/set_ftrace_pid you can trace a +single thread. For example: + +# cat /debug/tracing/set_ftrace_pid +no pid +# echo 3111 > /debug/tracing/set_ftrace_pid +# cat /debug/tracing/set_ftrace_pid +3111 +# echo function > /debug/tracing/current_tracer +# cat /debug/tracing/trace | head + # tracer: function + # + # TASK-PID CPU# TIMESTAMP FUNCTION + # | | | | | + yum-updatesd-3111 [003] 1637.254676: finish_task_switch <-thread_return + yum-updatesd-3111 [003] 1637.254681: hrtimer_cancel <-schedule_hrtimeout_range + yum-updatesd-3111 [003] 1637.254682: hrtimer_try_to_cancel <-hrtimer_cancel + yum-updatesd-3111 [003] 1637.254683: lock_hrtimer_base <-hrtimer_try_to_cancel + yum-updatesd-3111 [003] 1637.254685: fget_light <-do_sys_poll + yum-updatesd-3111 [003] 1637.254686: pipe_poll <-do_sys_poll +# echo -1 > /debug/tracing/set_ftrace_pid +# cat /debug/tracing/trace |head + # tracer: function + # + # TASK-PID CPU# TIMESTAMP FUNCTION + # | | | | | + ##### CPU 3 buffer started #### + yum-updatesd-3111 [003] 1701.957688: free_poll_entry <-poll_freewait + yum-updatesd-3111 [003] 1701.957689: remove_wait_queue <-free_poll_entry + yum-updatesd-3111 [003] 1701.957691: fput <-free_poll_entry + yum-updatesd-3111 [003] 1701.957692: audit_syscall_exit <-sysret_audit + yum-updatesd-3111 [003] 1701.957693: path_put <-audit_syscall_exit + +If you want to trace a function when executing, you could use +something like this simple program: + +#include <stdio.h> +#include <stdlib.h> +#include <sys/types.h> +#include <sys/stat.h> +#include <fcntl.h> +#include <unistd.h> + +int main (int argc, char **argv) +{ + if (argc < 1) + exit(-1); + + if (fork() > 0) { + int fd, ffd; + char line[64]; + int s; + + ffd = open("/debug/tracing/current_tracer", O_WRONLY); + if (ffd < 0) + exit(-1); + write(ffd, "nop", 3); + + fd = open("/debug/tracing/set_ftrace_pid", O_WRONLY); + s = sprintf(line, "%d\n", getpid()); + write(fd, line, s); + + write(ffd, "function", 8); + + close(fd); + close(ffd); + + execvp(argv[1], argv+1); + } + + return 0; +} + dynamic ftrace --------------
kernel/trace/ftrace.c +183 -26
303 lines not on this page · raw message
Signature · 3 lines
1.5.6.5 --
Re: [PATCH 1/5] ftrace: add function tracing to single thread
Quoted 47 lines · On Wed, 26 Nov 2008 00:16:23 -0500 Steven Rostedt <[email protected]> wrote:
From: Steven Rostedt <[email protected]>
Impact: feature to function trace a single thread
This patch adds the ability to function trace a single thread. The file:
/debugfs/tracing/set_ftrace_pid
contains the pid to trace. Valid pids are any positive integer. Writing any negative number to this file will disable the pid tracing and the function tracer will go back to tracing all of threads.
This feature works with both static and dynamic function tracing.
...
kernel/trace/ftrace.c +5
-0@@ -47,6 +47,9 @@int ftrace_enabled__read_mostly ;static int last_ftrace_enabled; +/* ftrace_pid_trace >= 0 will only trace threads with this pid */ +static int ftrace_pid_trace =-1 ; +/* Quick disabling of function tracer. */ int function_trace_stop;@@ -61,6 +64,7 @@ static int ftrace_disabled __read_mostly;static DEFINE_SPINLOCK (ftrace_lock);static DEFINE_MUTEX (ftrace_sysctl_lock); +static DEFINE_MUTEX (ftrace_start_lock);static struct ftrace_ops ftrace_list_end__read_mostly = {@@ -70,6 +74,7 @@ static struct ftrace_ops ftrace_list_end __read_mostly =static struct ftrace_ops *ftrace_list__read_mostly = &ftrace_list_end;ftrace_func_t ftrace_trace_function__read_mostly = ftrace_stub;ftrace_func_t __ftrace_trace_function__read_mostly = ftrace_stub; +ftrace_func_t ftrace_pid_function__read_mostly = ftrace_stub;static void ftrace_list_func (unsigned long ip,unsigned long parent_ip) {
kernel/trace/ftrace.c static void ftrace_list_func(unsigned long ip, unsigned long parent_ip) · line 97
Quoted 7 lines
kernel/trace/ftrace.c +4
-0@@ -86,6 +91,21 @@ static void ftrace_list_func(unsigned long ip, unsigned long parent_ip) }; } +static void ftrace_pid_func (unsigned long ip,unsigned long parent_ip) +{ +if (current->pid != ftrace_pid_trace) +return ;
What happened with this?
It would reeeeely help if the changelog was updated to cover such frequently-occurring controversies as this.
kernel/trace/ftrace.c static int __unregister_ftrace_function(struct ftrace_ops *ops) · line 223
Quoted 5 lines
kernel/trace/ftrace.c +5
-0+#ifdef CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST + ftrace_trace_function = func; +#else + __ftrace_trace_function = func; +#endif
Pulling this assignment out into a helper fuction would clean things up. It happens at least twice.
kernel/trace/ftrace.c static int __init ftrace_nodyn_init(void) · line 1470
Quoted 12 lines
...
kernel/trace/ftrace.c +9
-0+static ssize_t +ftrace_pid_read (struct file *file,char __user *ubuf, +size_t cnt,loff_t *ppos) +{ +char buf[64 ]; +int r; + +if (ftrace_pid_trace >=0 ) + r =sprintf (buf,"%u\n" , ftrace_pid_trace);
ftrace_pid_trace is signed, and we print it as unsigned. Can this be improved?
Quoted 11 lines
kernel/trace/ftrace.c +11
-0+else + r =sprintf (buf,"no pid\n" ); + +return simple_read_from_buffer (ubuf, cnt, ppos, buf, r); +} + +static ssize_t +ftrace_pid_write (struct file *filp,const char __user *ubuf, +size_t cnt,loff_t *ppos) +{ +char buf[64 ];
kernel/trace/ftrace.c static int __init ftrace_nodyn_init(void) · line 1491
Quoted 10 lines
kernel/trace/ftrace.c +10
-0+long val; +int ret; + +if (cnt >=sizeof (buf)) +return -EINVAL ; + +if (copy_from_user (&buf, ubuf, cnt)) +return -EFAULT ; + + buf[cnt] =0 ;
Might be able to use strncpy_from_user() here?
Quoted 20 lines
kernel/trace/ftrace.c +20
-0+ ret =strict_strtol (buf,10 , &val); +if (ret <0 ) +return ret; + +mutex_lock (&ftrace_start_lock); +if (ret <0 ) { +/* disable pid tracing */ +if (ftrace_pid_trace <0 ) + goto out; + ftrace_pid_trace =-1 ; + + }else { + +if (ftrace_pid_trace == val) + goto out; + + ftrace_pid_trace = val; + } + +/* update the function call */
kernel/trace/ftrace.c static int __init ftrace_nodyn_init(void) · line 1522
Quoted 10 lines
kernel/trace/ftrace.c +10
-0+ftrace_update_pid_func (); +ftrace_startup_enable (0 ); + + out: +mutex_unlock (&ftrace_start_lock); + +return cnt; +} + +static struct file_operations ftrace_pid_fops = {
const, please.
Quoted 4 lines
kernel/trace/ftrace.c +4
-0+ .read = ftrace_pid_read, + .write = ftrace_pid_write, +}; +
Steven Rostedt
maintainerRe: [PATCH 1/5] ftrace: add function tracing to single thread
Quoted 11 lines · Andrew Morton
kernel/trace/ftrace.c static void ftrace_list_func(unsigned long ip, unsigned long parent_ip) · line 97
Quoted 5 lines · On Wed, 26 Nov 2008 00:16:23 -0500 Steven Rostedt <[email protected]> wrote:
kernel/trace/ftrace.c +4
-0+static void ftrace_pid_func (unsigned long ip,unsigned long parent_ip) +{ +if (current->pid != ftrace_pid_trace) +return ;What happened with this?
It would reeeeely help if the changelog was updated to cover such frequently-occurring controversies as this.
I just posted this again because Ingo did not pull it the first time. This patch did not change (nor did the change log) from my first posting, except that I rebased it on top of tip.
I would like to add new patches to solve this controversy. This way I can focus on the approach without cluttering up the patch itself. Also, this way works for the cases I currently care about, and should not break any other case. That is, the side effect of not selecting the right pid is that we either trace a process we do not want to trace, or we do not trace a process we want to trace. Nothing that will bring down the system ;-)
Quoted 9 lines
kernel/trace/ftrace.c static int __unregister_ftrace_function(struct ftrace_ops *ops) · line 223
Quoted 6 lines
kernel/trace/ftrace.c +5
-0+#ifdef CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST + ftrace_trace_function = func; +#else + __ftrace_trace_function = func; +#endif Pulling this assignment out into a helper fuction would clean things up. It happens at least twice.
Yes, I agree. I want to get this over to my PPC box where it does not have the "CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST" set. This way I can test all cases. And yes, I want to make a wrapper function for that too.
Quoted 4 lines
Quoted 4 lines
...
Quoted 12 lines
kernel/trace/ftrace.c static int __init ftrace_nodyn_init(void) · line 1470
Quoted 9 lines
kernel/trace/ftrace.c +9
-0+static ssize_t +ftrace_pid_read (struct file *file,char __user *ubuf, +size_t cnt,loff_t *ppos) +{ +char buf[64 ]; +int r; + +if (ftrace_pid_trace >=0 ) + r =sprintf (buf,"%u\n" , ftrace_pid_trace);
ftrace_pid_traceis signed, and we print it as unsigned. Can this be improved?
We only print it if it is greater than or equal to 0. Does this matter? It needs to be signed, because we print "no pid" when negative.
Quoted 16 lines
Quoted 16 lines
kernel/trace/ftrace.c +15
-0+else + r =sprintf (buf,"no pid\n" ); + +return simple_read_from_buffer (ubuf, cnt, ppos, buf, r); +} + +static ssize_t +ftrace_pid_write (struct file *filp,const char __user *ubuf, +size_t cnt,loff_t *ppos) +{ +char buf[64 ]; +long val; +int ret; + +if (cnt >=sizeof (buf))
Quoted 8 lines
kernel/trace/ftrace.c static int __init ftrace_nodyn_init(void) · line 1491
Quoted 6 lines
kernel/trace/ftrace.c +6
-0+return -EINVAL ; + +if (copy_from_user (&buf, ubuf, cnt)) +return -EFAULT ; + + buf[cnt] =0 ;Might be able to use
strncpy_from_user()here?
We are reading a number. But we might later add a string. The amount to be read has a limit. Should I switch?
Quoted 25 lines
Quoted 25 lines
kernel/trace/ftrace.c +24
-0+ ret =strict_strtol (buf,10 , &val); +if (ret <0 ) +return ret; + +mutex_lock (&ftrace_start_lock); +if (ret <0 ) { +/* disable pid tracing */ +if (ftrace_pid_trace <0 ) + goto out; + ftrace_pid_trace =-1 ; + + }else { + +if (ftrace_pid_trace == val) + goto out; + + ftrace_pid_trace = val; + } + +/* update the function call */ +ftrace_update_pid_func (); +ftrace_startup_enable (0 ); + + out:
Quoted 8 lines
kernel/trace/ftrace.c static int __init ftrace_nodyn_init(void) · line 1522
Quoted 6 lines
kernel/trace/ftrace.c +6
-0+mutex_unlock (&ftrace_start_lock); + +return cnt; +} + +static struct file_operations ftrace_pid_fops = {const, please.
Ouch, I cut and pasted. I need to fix all of them.
Thanks,
-- Steve
Quoted 8 lines
Quoted 5 lines
kernel/trace/ftrace.c +4
-0+ .read = ftrace_pid_read, + .write = ftrace_pid_write, +}; +
Re: [PATCH 1/5] ftrace: add function tracing to single thread
In reply to Steven RostedtQuoted 10 lines · Steven Rostedt
From: Steven Rostedt <[email protected]>
Impact: feature to function trace a single thread
This patch adds the ability to function trace a single thread. The file:
/debugfs/tracing/set_ftrace_pid
contains the pid to trace. Valid pids are any positive integer.
Please look below. Using find_get_pid causes this to use the same logic as the rest of the kernel on what a valid pid is. i.e. 0 is not a valid pid.
I believe I have sketched in what is needed to use the struct pid api properly. Certainly enough for you to get the jist of the idea.
Quoted 115 lines
Writing any negative number to this file will disable the pid tracing and the function tracer will go back to tracing all of threads.
This feature works with both static and dynamic function tracing.
Signed-off-by Steven Rostedt <[email protected]>--- Documentation/ftrace.txt | 79 +++++++++++++++++ kernel/trace/ftrace.c | 209 ++++++++++++++++++++++++++++++++++++++++------ 2 files changed, 262 insertions(+), 26 deletions(-)Documentation/ftrace.txt +79
-0@@ -127,6 +127,8 @@ of ftrace. Here is a list of some of the key files: be traced. If a function exists in both set_ftrace_filter and set_ftrace_notrace, the function will _not_ be traced. + set_ftrace_pid: Have the function tracer only trace a single thread. + available_filter_functions: This lists the functions that ftrace has processed and can trace. These are the function names that you can pass to "set_ftrace_filter" or@@ -1073,6 +1075,83 @@ For simple one time traces, the above is sufficent. For anything else, a search through /proc/mounts may be needed to find where the debugfs file-system is mounted. + +Single thread tracing +--------------------- + +By writing into /debug/tracing/set_ftrace_pid you can trace a +single thread. For example: + +# cat /debug/tracing/set_ftrace_pid +no pid +# echo 3111 > /debug/tracing/set_ftrace_pid +# cat /debug/tracing/set_ftrace_pid +3111 +# echo function > /debug/tracing/current_tracer +# cat /debug/tracing/trace | head + # tracer: function + # + # TASK-PID CPU# TIMESTAMP FUNCTION + # | | | | | + yum-updatesd-3111 [003] 1637.254676: finish_task_switch <-thread_return + yum-updatesd-3111 [003] 1637.254681: hrtimer_cancel <-schedule_hrtimeout_range + yum-updatesd-3111 [003] 1637.254682: hrtimer_try_to_cancel <-hrtimer_cancel + yum-updatesd-3111 [003] 1637.254683: lock_hrtimer_base <-hrtimer_try_to_cancel + yum-updatesd-3111 [003] 1637.254685: fget_light <-do_sys_poll + yum-updatesd-3111 [003] 1637.254686: pipe_poll <-do_sys_poll +# echo -1 > /debug/tracing/set_ftrace_pid +# cat /debug/tracing/trace |head + # tracer: function + # + # TASK-PID CPU# TIMESTAMP FUNCTION + # | | | | | + ##### CPU 3 buffer started #### + yum-updatesd-3111 [003] 1701.957688: free_poll_entry <-poll_freewait + yum-updatesd-3111 [003] 1701.957689: remove_wait_queue <-free_poll_entry + yum-updatesd-3111 [003] 1701.957691: fput <-free_poll_entry + yum-updatesd-3111 [003] 1701.957692: audit_syscall_exit <-sysret_audit + yum-updatesd-3111 [003] 1701.957693: path_put <-audit_syscall_exit + +If you want to trace a function when executing, you could use +something like this simple program: + +#include <stdio.h> +#include <stdlib.h> +#include <sys/types.h> +#include <sys/stat.h> +#include <fcntl.h> +#include <unistd.h> + +int main (int argc, char **argv) +{ + if (argc < 1) + exit(-1); + + if (fork() > 0) { + int fd, ffd; + char line[64]; + int s; + + ffd = open("/debug/tracing/current_tracer", O_WRONLY); + if (ffd < 0) + exit(-1); + write(ffd, "nop", 3); + + fd = open("/debug/tracing/set_ftrace_pid", O_WRONLY); + s = sprintf(line, "%d\n", getpid()); + write(fd, line, s); + + write(ffd, "function", 8); + + close(fd); + close(ffd); + + execvp(argv[1], argv+1); + } + + return 0; +} + dynamic ftrace --------------kernel/trace/ftrace.c +0
-0
kernel/trace/ftrace.c line 51
Quoted 5 lines
kernel/trace/ftrace.c +2
-0@@ -47,6 +47,9 @@int ftrace_enabled__read_mostly ;static int last_ftrace_enabled; +/* ftrace_pid_trace >= 0 will only trace threads with this pid */ +static int ftrace_pid_trace =-1 ;
Why not?
static struct pid *ftrace_pid_trace = NULL;
Quoted 21 lines
kernel/trace/ftrace.c +3
-0+/* Quick disabling of function tracer. */ int function_trace_stop;@@ -61,6 +64,7 @@ static int ftrace_disabled __read_mostly;static DEFINE_SPINLOCK (ftrace_lock);static DEFINE_MUTEX (ftrace_sysctl_lock); +static DEFINE_MUTEX (ftrace_start_lock);static struct ftrace_ops ftrace_list_end__read_mostly = {@@ -70,6 +74,7 @@ static struct ftrace_ops ftrace_list_end __read_mostly =static struct ftrace_ops *ftrace_list__read_mostly = &ftrace_list_end;ftrace_func_t ftrace_trace_function__read_mostly = ftrace_stub;ftrace_func_t __ftrace_trace_function__read_mostly = ftrace_stub; +ftrace_func_t ftrace_pid_function__read_mostly = ftrace_stub;static void ftrace_list_func (unsigned long ip,unsigned long parent_ip) {
kernel/trace/ftrace.c static void ftrace_list_func(unsigned long ip, unsigned long · line 98
Quoted 8 lines
kernel/trace/ftrace.c +4
-0@@ -86,6 +91,21 @@ static void ftrace_list_func(unsigned long ip, unsigned long parent_ip) }; } +static void ftrace_pid_func (unsigned long ip,unsigned long parent_ip) +{ +if (current->pid != ftrace_pid_trace) +return ;
And why not?
if (task_pid(current) != ftrace_pid_trace)
return;Racy or not. This is only comparing a different field in struct task_struct. So there should be not real difference.
Quoted 25 lines
kernel/trace/ftrace.c +11
-0+ +ftrace_pid_function (ip, parent_ip); +} + +static void set_ftrace_pid_function (ftrace_func_t func) +{ +/* do not set ftrace_pid_function to itself! */ +if (func != ftrace_pid_func) + ftrace_pid_function = func; +} +/** * clear_ftrace_function - reset the ftrace function *
kernel/trace/ftrace.c +1
-0@@ -96,6 +116,7 @@ void clear_ftrace_function(void) { ftrace_trace_function = ftrace_stub; __ftrace_trace_function = ftrace_stub; + ftrace_pid_function = ftrace_stub; }#ifndef CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST @@ -128,20 +149,26 @@ static int __register_ftrace_function(struct ftrace_ops *ops) ftrace_list = ops;
kernel/trace/ftrace.c static int __register_ftrace_function(struct ftrace_ops · line 160
Quoted 10 lines
kernel/trace/ftrace.c +8
-0if (ftrace_enabled) { +ftrace_func_t func; + +if (ops->next == &ftrace_list_end) + func = ops->func; +else + func = ftrace_list_func; + +if (ftrace_pid_trace >=0 ) {
And why not:
if (ftrace_pid_trace) {Quoted 24 lines
kernel/trace/ftrace.c +6
-8+set_ftrace_pid_function (func); + func = ftrace_pid_func; + } +/* * For one func, simply call it directly. * For more than one func, call the chain. */ #ifdef CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST -if (ops->next == &ftrace_list_end) - ftrace_trace_function = ops->func; -else - ftrace_trace_function = ftrace_list_func; + ftrace_trace_function = func;#else -if (ops->next == &ftrace_list_end) - __ftrace_trace_function = ops->func; -else - __ftrace_trace_function = ftrace_list_func; + __ftrace_trace_function = func; ftrace_trace_function = ftrace_test_stop_func;#endif kernel/trace/ftrace.c +0
-0}
kernel/trace/ftrace.c static int __unregister_ftrace_function(struct ftrace_ops · line 216
Quoted 10 lines
kernel/trace/ftrace.c +4
-2@@ -182,8 +209,19 @@ static int __unregister_ftrace_function(struct ftrace_ops *ops)if (ftrace_enabled) {/* If we only have one func left, then call that directly */ -if (ftrace_list->next == &ftrace_list_end) - ftrace_trace_function = ftrace_list->func; +if (ftrace_list->next == &ftrace_list_end) { +ftrace_func_t func = ftrace_list->func; + +if (ftrace_pid_trace >=0 ) {
And why not:
if (ftrace_pid_trace) {Quoted 20 lines
kernel/trace/ftrace.c +9
-0+set_ftrace_pid_function (func); + func = ftrace_pid_func; + } +#ifdef CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST + ftrace_trace_function = func; +#else + __ftrace_trace_function = func; +#endif + } }out:
kernel/trace/ftrace.c +3
-0@@ -192,6 +230,38 @@ static int __unregister_ftrace_function(struct ftrace_ops *ops)return ret; } +static void ftrace_update_pid_func (void ) +{ +ftrace_func_t func;
kernel/trace/ftrace.c static int __unregister_ftrace_function(struct ftrace_ops · line 246
Quoted 10 lines
kernel/trace/ftrace.c +10
-0+ +/* should not be called from interrupt context */ +spin_lock (&ftrace_lock); + +if (ftrace_trace_function == ftrace_stub) + goto out; + + func = ftrace_trace_function; + +if (ftrace_pid_trace >=0 ) {
And why not:
if (ftrace_pid_trace) {Quoted 97 lines
kernel/trace/ftrace.c +19
-0+set_ftrace_pid_function (func); + func = ftrace_pid_func; + }else { +if (func != ftrace_pid_func) + goto out; + +set_ftrace_pid_function (func); + } + +#ifdef CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST + ftrace_trace_function = func; +#else + __ftrace_trace_function = func; +#endif + + out: +spin_unlock (&ftrace_lock); +} +#ifdef CONFIG_DYNAMIC_FTRACE #ifndef CONFIG_FTRACE_MCOUNT_RECORD # error Dynamic ftrace depends on MCOUNT_RECORDkernel/trace/ftrace.c +17
-16@@ -545,7 +615,19 @@ static void ftrace_run_update_code(int command)static ftrace_func_t saved_ftrace_func;static int ftrace_start_up; -static DEFINE_MUTEX (ftrace_start_lock); + +static void ftrace_startup_enable (int command) +{ +if (saved_ftrace_func != ftrace_trace_function) { + saved_ftrace_func = ftrace_trace_function; + command |=FTRACE_UPDATE_TRACE_FUNC ; + } + +if (!command || !ftrace_enabled) +return ; + +ftrace_run_update_code (command); +}static void ftrace_startup (void ) {@@ -558,16 +640,8 @@ static void ftrace_startup(void) ftrace_start_up++; command |=FTRACE_ENABLE_CALLS ; -if (saved_ftrace_func != ftrace_trace_function) { - saved_ftrace_func = ftrace_trace_function; - command |=FTRACE_UPDATE_TRACE_FUNC ; - } - -if (!command || !ftrace_enabled) - goto out; +ftrace_startup_enable (command); -ftrace_run_update_code (command); - out:mutex_unlock (&ftrace_start_lock); }@@ -1262,13 +1336,10 @@ static struct file_operations ftrace_notrace_fops = { .release = ftrace_notrace_release, }; -static __init int ftrace_init_debugfs (void ) +static __init int ftrace_init_dyn_debugfs (struct dentry *d_tracer) { -struct dentry *d_tracer;struct dentry *entry; - d_tracer =tracing_init_dentry (); - entry =debugfs_create_file ("available_filter_functions" ,0444 , d_tracer,NULL , &ftrace_avail_fops);if (!entry)@@ -1295,8 +1366,6 @@ static __init int ftrace_init_debugfs(void)return 0 ; } -fs_initcall (ftrace_init_debugfs); -static int ftrace_convert_nops (struct module *mod,unsigned long *start,unsigned long *end)@@ -1382,12 +1451,100 @@ static int __init ftrace_nodyn_init(void) }device_initcall (ftrace_nodyn_init); +static inline int ftrace_init_dyn_debugfs (struct dentry *d_tracer) {return 0 ; } +static inline void ftrace_startup_enable (int command) { }# define ftrace_startup () do { } while (0)# define ftrace_shutdown () do { } while (0)# define ftrace_startup_sysctl () do { } while (0)# define ftrace_shutdown_sysctl () do { } while (0)#endif /* CONFIG_DYNAMIC_FTRACE */
kernel/trace/ftrace.c static int __init ftrace_nodyn_init(void) · line 1471
Quoted 10 lines
kernel/trace/ftrace.c +9
-0+static ssize_t +ftrace_pid_read (struct file *file,char __user *ubuf, +size_t cnt,loff_t *ppos) +{ +char buf[64 ]; +int r; + +if (ftrace_pid_trace >=0 ) + r =sprintf (buf,"%u\n" , ftrace_pid_trace);
And why not:
if (ftrace_pid_trace) r = sprintf(buf, "%u\n", pid_vmr(ftrace_pid_trace))
Quoted 15 lines
kernel/trace/ftrace.c +15
-0+else + r =sprintf (buf,"no pid\n" ); + +return simple_read_from_buffer (ubuf, cnt, ppos, buf, r); +} + +static ssize_t +ftrace_pid_write (struct file *filp,const char __user *ubuf, +size_t cnt,loff_t *ppos) +{ +char buf[64 ]; +long val; +int ret; + +if (cnt >=sizeof (buf))
kernel/trace/ftrace.c static int __init ftrace_nodyn_init(void) · line 1495
Quoted 10 lines
kernel/trace/ftrace.c +10
-0+return -EINVAL ; + +if (copy_from_user (&buf, ubuf, cnt)) +return -EFAULT ; + + buf[cnt] =0 ; + + ret =strict_strtol (buf,10 , &val); +if (ret <0 ) +return ret;
Why not?
pid = find_get_pid(ret);
mutex_lock(&ftrace_start_lock);
if (pid == ftrace_pid_trace)
goto out;
struct pid *tmp;
tmp = ftrace_pid_trace;
ftrace_pid_trace = pid;
pid = tmp;
/* update the function call */
ftrace_update_pid_func();
ftrace_startup_enable(0);
out:
mutex_unlock(&ftrace_start_lock);
put_pid(pid);Quoted 59 lines
kernel/trace/ftrace.c +52
-0+ +mutex_lock (&ftrace_start_lock); +if (ret <0 ) { +/* disable pid tracing */ +if (ftrace_pid_trace <0 ) + goto out; + ftrace_pid_trace =-1 ; + + }else { + +if (ftrace_pid_trace == val) + goto out; + + ftrace_pid_trace = val; + } + +/* update the function call */ +ftrace_update_pid_func (); +ftrace_startup_enable (0 ); + + out: +mutex_unlock (&ftrace_start_lock); + +return cnt; +} + +static struct file_operations ftrace_pid_fops = { + .read = ftrace_pid_read, + .write = ftrace_pid_write, +}; + +static __init int ftrace_init_debugfs (void ) +{ +struct dentry *d_tracer; +struct dentry *entry; + + d_tracer =tracing_init_dentry (); +if (!d_tracer) +return 0 ; + +ftrace_init_dyn_debugfs (d_tracer); + + entry =debugfs_create_file ("set_ftrace_pid" ,0644 , d_tracer, +NULL , &ftrace_pid_fops); +if (!entry) +pr_warning ("Could not create debugfs " +"'set_ftrace_pid' entry\n" ); +return 0 ; +} + +fs_initcall (ftrace_init_debugfs); +/** * ftrace_kill - kill ftrace *
Signature · 3 lines
1.5.6.5 --
Re: [PATCH 1/5] ftrace: add function tracing to single thread
Quoted 5 lines · Eric W. Biederman
kernel/trace/ftrace.c line 51
Quoted 2 lines
kernel/trace/ftrace.c +2
-0+/* ftrace_pid_trace >= 0 will only trace threads with this pid */ +static int ftrace_pid_trace =-1 ;Why not?
static struct pid*ftrace_pid_trace= NULL;
ok, looks reasonably simple at first sight.
Steve, mind sending the conversion of this facility to struct pid as a delta patch? (that would have education purposes as well, for similar situations)
Ingo
Steven Rostedt
maintainerRe: [PATCH 1/5] ftrace: add function tracing to single thread
Quoted 9 lines · Ingo Molnar
Quoted 5 lines · Eric W. Biederman
kernel/trace/ftrace.c line 51
Quoted 2 lines
kernel/trace/ftrace.c +2
-0+/* ftrace_pid_trace >= 0 will only trace threads with this pid */ +static int ftrace_pid_trace =-1 ;Why not?
static struct pid*ftrace_pid_trace= NULL;
Quoted 5 lines
ok, looks reasonably simple at first sight.
Steve, mind sending the conversion of this facility to struct pid as a delta patch? (that would have education purposes as well, for similar situations)
If I can trace specific kernel threads too, then I have no problem with it.
-- Steve
Steven Rostedt
maintainerRe: [PATCH 1/5] ftrace: add function tracing to single thread
In reply to Eric W. BiedermanQuoted 9 lines · Eric W. Biederman
Quoted 7 lines · Steven Rostedt
From: Steven Rostedt <[email protected]>
Impact: feature to function trace a single thread
This patch adds the ability to function trace a single thread. The file:
Quoted 7 lines
Quoted 3 lines
/debugfs/tracing/set_ftrace_pid
contains the pid to trace. Valid pids are any positive integer.
Please look below. Using
find_get_pidcauses this to use the same logic as the rest of the kernel on what a valid pid is. i.e. 0 is not a valid pid.
0 is not? What happens if I want to trace what the swapper task is doing? I want the ability to trace kernel threads as well.
Quoted 159 lines
I believe I have sketched in what is needed to use the struct pid api properly. Certainly enough for you to get the jist of the idea.
Quoted 115 lines
Writing any negative number to this file will disable the pid tracing and the function tracer will go back to tracing all of threads.
This feature works with both static and dynamic function tracing.
Signed-off-by Steven Rostedt <[email protected]>--- Documentation/ftrace.txt | 79 +++++++++++++++++ kernel/trace/ftrace.c | 209 ++++++++++++++++++++++++++++++++++++++++------ 2 files changed, 262 insertions(+), 26 deletions(-)Documentation/ftrace.txt +79
-0@@ -127,6 +127,8 @@ of ftrace. Here is a list of some of the key files: be traced. If a function exists in both set_ftrace_filter and set_ftrace_notrace, the function will _not_ be traced. + set_ftrace_pid: Have the function tracer only trace a single thread. + available_filter_functions: This lists the functions that ftrace has processed and can trace. These are the function names that you can pass to "set_ftrace_filter" or@@ -1073,6 +1075,83 @@ For simple one time traces, the above is sufficent. For anything else, a search through /proc/mounts may be needed to find where the debugfs file-system is mounted. + +Single thread tracing +--------------------- + +By writing into /debug/tracing/set_ftrace_pid you can trace a +single thread. For example: + +# cat /debug/tracing/set_ftrace_pid +no pid +# echo 3111 > /debug/tracing/set_ftrace_pid +# cat /debug/tracing/set_ftrace_pid +3111 +# echo function > /debug/tracing/current_tracer +# cat /debug/tracing/trace | head + # tracer: function + # + # TASK-PID CPU# TIMESTAMP FUNCTION + # | | | | | + yum-updatesd-3111 [003] 1637.254676: finish_task_switch <-thread_return + yum-updatesd-3111 [003] 1637.254681: hrtimer_cancel <-schedule_hrtimeout_range + yum-updatesd-3111 [003] 1637.254682: hrtimer_try_to_cancel <-hrtimer_cancel + yum-updatesd-3111 [003] 1637.254683: lock_hrtimer_base <-hrtimer_try_to_cancel + yum-updatesd-3111 [003] 1637.254685: fget_light <-do_sys_poll + yum-updatesd-3111 [003] 1637.254686: pipe_poll <-do_sys_poll +# echo -1 > /debug/tracing/set_ftrace_pid +# cat /debug/tracing/trace |head + # tracer: function + # + # TASK-PID CPU# TIMESTAMP FUNCTION + # | | | | | + ##### CPU 3 buffer started #### + yum-updatesd-3111 [003] 1701.957688: free_poll_entry <-poll_freewait + yum-updatesd-3111 [003] 1701.957689: remove_wait_queue <-free_poll_entry + yum-updatesd-3111 [003] 1701.957691: fput <-free_poll_entry + yum-updatesd-3111 [003] 1701.957692: audit_syscall_exit <-sysret_audit + yum-updatesd-3111 [003] 1701.957693: path_put <-audit_syscall_exit + +If you want to trace a function when executing, you could use +something like this simple program: + +#include <stdio.h> +#include <stdlib.h> +#include <sys/types.h> +#include <sys/stat.h> +#include <fcntl.h> +#include <unistd.h> + +int main (int argc, char **argv) +{ + if (argc < 1) + exit(-1); + + if (fork() > 0) { + int fd, ffd; + char line[64]; + int s; + + ffd = open("/debug/tracing/current_tracer", O_WRONLY); + if (ffd < 0) + exit(-1); + write(ffd, "nop", 3); + + fd = open("/debug/tracing/set_ftrace_pid", O_WRONLY); + s = sprintf(line, "%d\n", getpid()); + write(fd, line, s); + + write(ffd, "function", 8); + + close(fd); + close(ffd); + + execvp(argv[1], argv+1); + } + + return 0; +} + dynamic ftrace --------------kernel/trace/ftrace.c +0
-0kernel/trace/ftrace.c line 51
Quoted 5 lines
kernel/trace/ftrace.c +2
-0@@ -47,6 +47,9 @@int ftrace_enabled__read_mostly ;static int last_ftrace_enabled; +/* ftrace_pid_trace >= 0 will only trace threads with this pid */ +static int ftrace_pid_trace =-1 ;Why not?
static struct pid*ftrace_pid_trace= NULL;Quoted 21 lines
kernel/trace/ftrace.c +3
-0+/* Quick disabling of function tracer. */ int function_trace_stop;@@ -61,6 +64,7 @@ static int ftrace_disabled __read_mostly;static DEFINE_SPINLOCK (ftrace_lock);static DEFINE_MUTEX (ftrace_sysctl_lock); +static DEFINE_MUTEX (ftrace_start_lock);static struct ftrace_ops ftrace_list_end__read_mostly = {@@ -70,6 +74,7 @@ static struct ftrace_ops ftrace_list_end __read_mostly =static struct ftrace_ops *ftrace_list__read_mostly = &ftrace_list_end;ftrace_func_t ftrace_trace_function__read_mostly = ftrace_stub;ftrace_func_t __ftrace_trace_function__read_mostly = ftrace_stub; +ftrace_func_t ftrace_pid_function__read_mostly = ftrace_stub;static void ftrace_list_func (unsigned long ip,unsigned long parent_ip) {kernel/trace/ftrace.c static void ftrace_list_func(unsigned long ip, unsigned long · line 98
Quoted 8 lines
kernel/trace/ftrace.c +4
-0@@ -86,6 +91,21 @@ static void ftrace_list_func(unsigned long ip, unsigned long parent_ip) }; } +static void ftrace_pid_func (unsigned long ip,unsigned long parent_ip) +{ +if (current->pid != ftrace_pid_trace) +return ;And why not?
Quoted 5 lines
if (task_pid(current) != ftrace_pid_trace) return;Racy or not. This is only comparing a different field in struct
task_struct. So there should be not real difference.
There's no race here. Do kernel threads have their own unique "task_pid"s?
I understand where you are going with this, and I have no issue with it. As long as I can also pick kernel threads to trace including the idle thread.
Thanks,
-- Steve
Steven Rostedt
maintainerPATCH 2/5 ftrace: use code patching for ftrace graph tracer
In reply to Steven RostedtFrom: Steven Rostedt <[email protected]>
Impact: more efficient code for ftrace graph tracer
This patch uses the dynamic patching, when available, to patch the function graph code into the kernel.
This patch will ease the way for letting both function tracing and function graph tracing run together.
--- arch/x86/kernel/entry_32.S | 5 ++++ arch/x86/kernel/ftrace.c | 48 ++++++++++++++++++++++++++++++++++++++++++- include/linux/ftrace.h | 5 ++++ kernel/trace/ftrace.c | 35 ++++++++++++++----------------- 4 files changed, 72 insertions(+), 21 deletions(-)
arch/x86/kernel/entry_32.S +5 -0
@@ -1185,6 +1185,11 @@ ftrace_call: popl %edx popl %ecx popl %eax +#ifdef CONFIG_FUNCTION_GRAPH_TRACER +.globl ftrace_graph_call +ftrace_graph_call: + jmp ftrace_stub +#endif .globl ftrace_stub ftrace_stub:
arch/x86/kernel/ftrace.c +46 -2
66 lines not on this page · raw message
include/linux/ftrace.h +5 -0
@@ -117,6 +117,11 @@ extern void ftrace_call(void);extern void mcount_call (void );#ifdef CONFIG_FUNCTION_GRAPH_TRACER extern void ftrace_graph_caller (void ); +extern int ftrace_enable_ftrace_graph_caller (void ); +extern int ftrace_disable_ftrace_graph_caller (void ); +#else +static inline int ftrace_enable_ftrace_graph_caller (void ) {return 0 ; } +static inline int ftrace_disable_ftrace_graph_caller (void ) {return 0 ; }#endif /**
kernel/trace/ftrace.c +16 -19
@@ -281,6 +281,8 @@ enum {FTRACE_UPDATE_TRACE_FUNC = (1 <<2 ),FTRACE_ENABLE_MCOUNT = (1 <<3 ),FTRACE_DISABLE_MCOUNT = (1 <<4 ), +FTRACE_START_FUNC_RET = (1 <<5 ), + FTRACE_STOP_FUNC_RET = (1 << 6), };static int ftrace_filtered;
@@ -465,14 +467,7 @@ __ftrace_replace_code(struct dyn_ftrace *rec, int enable)unsigned long ip, fl;unsigned long ftrace_addr; -#ifdef CONFIG_FUNCTION_GRAPH_TRACER -if (ftrace_tracing_type ==FTRACE_TYPE_ENTER ) - ftrace_addr = (unsigned long )ftrace_caller; -else - ftrace_addr = (unsigned long )ftrace_graph_caller; -#else ftrace_addr = (unsigned long )ftrace_caller; -#endif ip = rec->ip;
@@ -605,6 +600,11 @@ static int __ftrace_modify_code(void *data)if (*command &FTRACE_UPDATE_TRACE_FUNC )ftrace_update_ftrace_func (ftrace_trace_function); +if (*command &FTRACE_START_FUNC_RET ) +ftrace_enable_ftrace_graph_caller (); +else if (*command &FTRACE_STOP_FUNC_RET ) +ftrace_disable_ftrace_graph_caller (); +return 0 ; }
@@ -629,10 +629,8 @@ static void ftrace_startup_enable(int command)ftrace_run_update_code (command); } -static void ftrace_startup (void ) +static void ftrace_startup (int command) { -int command =0 ; -if (unlikely (ftrace_disabled))return ;
@@ -645,10 +643,8 @@ static void ftrace_startup(void)mutex_unlock (&ftrace_start_lock); } -static void ftrace_shutdown (void ) +static void ftrace_shutdown (int command) { -int command =0 ; -if (unlikely (ftrace_disabled))return ;
@@ -1453,8 +1449,9 @@ device_initcall(ftrace_nodyn_init);static inline int ftrace_init_dyn_debugfs (struct dentry *d_tracer) {return 0 ; }static inline void ftrace_startup_enable (int command) { } -# define ftrace_startup () do { } while (0) -# define ftrace_shutdown () do { } while (0) +/* Keep as macros so we do not need to define the commands */ +# define ftrace_startup (command) do { } while (0) +# define ftrace_shutdown (command) do { } while (0)# define ftrace_startup_sysctl () do { } while (0)# define ftrace_shutdown_sysctl () do { } while (0)#endif /* CONFIG_DYNAMIC_FTRACE */
@@ -1585,7 +1582,7 @@ int register_ftrace_function(struct ftrace_ops *ops) } ret =__register_ftrace_function (ops); -ftrace_startup (); +ftrace_startup (0 ); out:mutex_unlock (&ftrace_sysctl_lock);
@@ -1604,7 +1601,7 @@ int unregister_ftrace_function(struct ftrace_ops *ops)mutex_lock (&ftrace_sysctl_lock); ret =__unregister_ftrace_function (ops); -ftrace_shutdown (); +ftrace_shutdown (0 );mutex_unlock (&ftrace_sysctl_lock);return ret;
@@ -1751,7 +1748,7 @@ int register_ftrace_graph(trace_func_graph_ret_t retfunc, ftrace_tracing_type =FTRACE_TYPE_RETURN ; ftrace_graph_return = retfunc; ftrace_graph_entry = entryfunc; -ftrace_startup (); +ftrace_startup (FTRACE_START_FUNC_RET ); out:mutex_unlock (&ftrace_sysctl_lock);
@@ -1765,7 +1762,7 @@ void unregister_ftrace_graph(void)atomic_dec (&ftrace_graph_active); ftrace_graph_return = (trace_func_graph_ret_t )ftrace_stub; ftrace_graph_entry = (trace_func_graph_ent_t )ftrace_stub; -ftrace_shutdown (); +ftrace_shutdown (FTRACE_STOP_FUNC_RET );/* Restore normal tracing type */ ftrace_tracing_type =FTRACE_TYPE_ENTER ;
Signature · 3 lines
1.5.6.5 --
Re: [PATCH 2/5] ftrace: use code patching for ftrace graph tracer
Quoted 12 lines · On Wed, 26 Nov 2008 00:16:24 -0500 Steven Rostedt…
From: Steven Rostedt <[email protected]>
Impact: more efficient code for ftrace graph tracer
This patch uses the dynamic patching, when available, to patch the function graph code into the kernel.
This patch will ease the way for letting both function tracing and function graph tracing run together.
...
arch/x86/kernel/ftrace.c int __init ftrace_dyn_arch_init(void *data) · line 338
Quoted 9 lines
arch/x86/kernel/ftrace.c +9
-0+static int ftrace_mod_jmp (unsigned long ip, +int old_offset,int new_offset) +{ +unsigned char code[MCOUNT_INSN_SIZE ]; + +if (probe_kernel_read (code, (void *)ip,MCOUNT_INSN_SIZE )) +return -EFAULT ; + +if (code[0 ] !=0xe9 || old_offset != *(int *)(&code[1 ]))
erk. I suspect that there's a nicer way of doing this amongst our forest of get_unaligned_foo() interfaces. Harvey will know.
arch/x86/kernel/ftrace.c int __init ftrace_dyn_arch_init(void *data) · line 341
Quoted 3 lines
arch/x86/kernel/ftrace.c +3
-0+return -EINVAL ; + + *(int *)(&code[1 ]) = new_offset;
Might be able to use put_unaligned_foo() here.
The problem is that these functions use sizeof(*ptr) to work out what to do, so a cast is still needed. A get_unaligned32(ptr) would be nice. One which takes a void* and assumes CPU ordering.
Quoted 6 lines
arch/x86/kernel/ftrace.c +6
-0+if (do_ftrace_mod_code (ip, &code)) +return -EPERM ; + +return 0 ; +} +
Re: [PATCH 2/5] ftrace: use code patching for ftrace graph tracer
Quoted 14 lines · Andrew Morton
Quoted 12 lines · On Wed, 26 Nov 2008 00:16:24 -0500 Steven Rostedt <[email protected]> wrote:
From: Steven Rostedt <[email protected]>
Impact: more efficient code for ftrace graph tracer
This patch uses the dynamic patching, when available, to patch the function graph code into the kernel.
This patch will ease the way for letting both function tracing and function graph tracing run together.
...
Quoted 13 lines
arch/x86/kernel/ftrace.c int __init ftrace_dyn_arch_init(void *data) · line 338
Quoted 9 lines
arch/x86/kernel/ftrace.c +9
-0+static int ftrace_mod_jmp (unsigned long ip, +int old_offset,int new_offset) +{ +unsigned char code[MCOUNT_INSN_SIZE ]; + +if (probe_kernel_read (code, (void *)ip,MCOUNT_INSN_SIZE )) +return -EFAULT ; + +if (code[0 ] !=0xe9 || old_offset != *(int *)(&code[1 ]))erk. I suspect that there's a nicer way of doing this amongst our forest of
get_unaligned_foo()interfaces. Harvey will know.
if (code[0] != 0xe9 || old_offset != get_unaligned((int *)(&code[1])))
Quoted 6 lines
arch/x86/kernel/ftrace.c int __init ftrace_dyn_arch_init(void *data) · line 341
Quoted 3 lines
arch/x86/kernel/ftrace.c +3
-0+return -EINVAL ; + + *(int *)(&code[1 ]) = new_offset;Might be able to use
put_unaligned_foo()here.
put_unaligned(new_offset, (int *)(&code[1]));
Quoted 3 lines
The problem is that these functions use
sizeof(*ptr)to work out what to do, so a cast is still needed. Aget_unaligned32(ptr)would be nice. One which takes a void* and assumes CPU ordering.
I've been thinking similarly, I could investigate something that goes in with the _noalign stuff?
I'll finish the documentation patch for the _noalign stuff and then see about doing the host-order bits to fit in as well.
Harvey
Re: [PATCH 2/5] ftrace: use code patching for ftrace graph tracer
Quoted 29 lines · On Tue, 25 Nov 2008 22:52:29 -0800 Harvey Harrison…
Quoted 14 lines · Andrew Morton
Quoted 12 lines · On Wed, 26 Nov 2008 00:16:24 -0500 Steven Rostedt…
From: Steven Rostedt <[email protected]>
Impact: more efficient code for ftrace graph tracer
This patch uses the dynamic patching, when available, to patch the function graph code into the kernel.
This patch will ease the way for letting both function tracing and function graph tracing run together.
...
Quoted 13 lines
arch/x86/kernel/ftrace.c int __init ftrace_dyn_arch_init(void *data) · line 338
Quoted 9 lines
arch/x86/kernel/ftrace.c +9
-0+static int ftrace_mod_jmp (unsigned long ip, +int old_offset,int new_offset) +{ +unsigned char code[MCOUNT_INSN_SIZE ]; + +if (probe_kernel_read (code, (void *)ip,MCOUNT_INSN_SIZE )) +return -EFAULT ; + +if (code[0 ] !=0xe9 || old_offset != *(int *)(&code[1 ]))erk. I suspect that there's a nicer way of doing this amongst our forest of
get_unaligned_foo()interfaces. Harvey will know.
Quoted 1 line
if (code[0] !=
0xe9||old_offset!=get_unaligned((int *)(&code[1])))
urgh, OK, that didn't really improve anything except to document something which was already rather obvious.
Quoted 7 lines
Quoted 6 lines
arch/x86/kernel/ftrace.c int __init ftrace_dyn_arch_init(void *data) · line 341
Quoted 3 lines
arch/x86/kernel/ftrace.c +3
-0+return -EINVAL ; + + *(int *)(&code[1 ]) = new_offset;Might be able to use
put_unaligned_foo()here.
Quoted 8 lines
put_unaligned(new_offset, (int *)(&code[1]));Quoted 3 lines
The problem is that these functions use
sizeof(*ptr)to work out what to do, so a cast is still needed. Aget_unaligned32(ptr)would be nice. One which takes a void* and assumes CPU ordering.I've been thinking similarly, I could investigate something that goes in with the
_noalignstuff?
If it's a commonly used pattern (you'd know better than I) then sure, it would be clean to have
u32 just_gimme_the_u32_at(void *this_address);
Quoted 4 lines
I'll finish the documentation patch for the
_noalignstuff and then see about doing the host-order bits to fit in as well.Harvey
Re: [PATCH 2/5] ftrace: use code patching for ftrace graph tracer
Quoted 7 lines · Andrew Morton
Quoted 6 lines · On Tue, 25 Nov 2008 22:52:29 -0800 Harvey Harrison <[email protected]>…
Quoted 5 lines
Review
Quoted 1 line
(quoted diff) +1
-0+ if (code[0] != 0xe9 || old_offset != *(int *)(&code[1]))erk. I suspect that there's a nicer way of doing this amongst our forest of
get_unaligned_foo()interfaces. Harvey will know.
Quoted 5 lines
Quoted 1 line
if (code[0] !=
0xe9||old_offset!=get_unaligned((int *)(&code[1])))urgh, OK, that didn't really improve anything except to document something which was already rather obvious.
Yeah, it really isn't that great. It could be somewhat nicer if we had the API for host-endian taking void and being explicit about the size:
if (code[0] != 0xe9 || old_offset != load32_noalign(&code[1]))
This is similar to the new API in -mm load_le32_noalign, but I don't think it would be worth load_u32_noalign...load32 should be enough.
Quoted 9 lines
Quoted 9 lines
Quoted 6 lines
Review
Quoted 3 lines
(quoted diff) +3
-0+ return -EINVAL; + + *(int *)(&code[1]) = new_offset;Might be able to use
put_unaligned_foo()here.put_unaligned(new_offset, (int *)(&code[1]));
In a similar vein to above, this becomes:
store32_noalign(&code[1], new_offset);
I rather like this as it would allow most (if not all) of the figure out what size based on pointer type versions to be chucked.
Turns out all the pieces are there in -mm that the patch is pretty trivial (not for application, just a heads-up):
include/asm-generic/unaligned.h | 14 ++++++++++++++ 1 files changed, 14 insertions(+), 0 deletions(-)
include/asm-generic/unaligned.h +14 -0
@@ -343,9 +343,23 @@ extern void __bad_unaligned_access_size(void);#ifdef __LITTLE_ENDIAN# define get_unaligned __get_unaligned_le# define put_unaligned __put_unaligned_le + +# define load16_noalign (p) load_le16_noalign((void *)(p)) +# define load32_noalign (p) load_le32_noalign((void *)(p)) +# define load64_noalign (p) load_le64_noalign((void *)(p)) +# define store16_noalign (p, val) store_le16_noalign((void *)(p), (val)) +# define store32_noalign (p, val) store_le32_noalign((void *)(p), (val)) +# define store64_noalign (p, val) store_le64_noalign((void *)(p), (val))#else # define get_unaligned __get_unaligned_be# define put_unaligned __put_unaligned_be + +# define load16_noalign (p) load_be16_noalign((void *)(p)) +# define load32_noalign (p) load_be32_noalign((void *)(p)) +# define load64_noalign (p) load_be64_noalign((void *)(p)) +# define store16_noalign (p, val) store_be16_noalign((void *)(p), (val)) +# define store32_noalign (p, val) store_be32_noalign((void *)(p), (val)) +# define store64_noalign (p, val) store_be64_noalign((void *)(p), (val))#endif #endif /* _ASM_GENERIC_UNALIGNED_H */
Signature · 1 line
1.6.0.4.1044.g77718
Re: [PATCH 2/5] ftrace: use code patching for ftrace graph tracer
Quoted 13 lines · On Wed, 26 Nov 2008 00:27:04 -0800 Harvey Harrison <[email protected]>…
if (code[0] !=
0xe9||old_offset!=load32_noalign(&code[1]))This is similar to the new API in -mm
load_le32_noalign, but I don't think it would be worth load_u32_noalign...load32 should be enough.Quoted 7 lines
Quoted 7 lines
Quoted 6 lines
Review
Quoted 3 lines
(quoted diff) +3
-0+ return -EINVAL; + + *(int *)(&code[1]) = new_offset;Might be able to use
put_unaligned_foo()here.
Quoted 7 lines
Quoted 2 lines
Quoted 2 lines
put_unaligned(new_offset, (int *)(&code[1]));In a similar vein to above, this becomes:
store32_noalign(&code[1], new_offset);
yes, that's much better than the party tricks with magical sizeof, which forces you to run around and check the types of everything.
I've seen people doing get_user() on `long' types and such things occasionally.
Re: [PATCH 2/5] ftrace: use code patching for ftrace graph tracer
Quoted 20 lines · Andrew Morton
Quoted 18 lines · On Wed, 26 Nov 2008 00:27:04 -0800 Harvey Harrison…
if (code[0] !=
0xe9||old_offset!=load32_noalign(&code[1]))This is similar to the new API in -mm
load_le32_noalign, but I don't think it would be worth load_u32_noalign...load32 should be enough.Quoted 9 lines
Quoted 9 lines
Quoted 6 lines
Review
Quoted 3 lines
(quoted diff) +3
-0+ return -EINVAL; + + *(int *)(&code[1]) = new_offset;Might be able to use
put_unaligned_foo()here.put_unaligned(new_offset, (int *)(&code[1]));In a similar vein to above, this becomes:
Quoted 9 lines
Quoted 2 lines
store32_noalign(&code[1], new_offset);yes, that's much better than the party tricks with magical sizeof, which forces you to run around and check the types of everything.
I've seen people doing
get_user()on `long' types and such things occasionally.
Do you want to carry the patches to move to the new helpers until they hit mainline, or would you rather I waited until they can go through maintainer trees?
I've got the pile ready removing all of the get/put_{endian} and moving to the load/store API. Not much left after that to just remove the magical sizeof versions too. Just let me know what timing you'd prefer.
Also, all of this ends up being so intertwined in the aligned/unaligned cases that I'd like to move most of Documentation/unaligned_memory_access.txt into a new alignment_and_byteorder.txt to cover all of these new helpers.
I started most of a byteorder document, but constantly referring to the other file made it a bit tiresome, would you mind a consolidated document?
Harvey
Re: [PATCH 2/5] ftrace: use code patching for ftrace graph tracer
Quoted 27 lines · On Wed, 26 Nov 2008 00:44:48 -0800 Harvey Harrison…
Quoted 26 lines · Andrew Morton
Quoted 13 lines · On Wed, 26 Nov 2008 00:27:04 -0800 Harvey Harrison…
if (code[0] !=
0xe9||old_offset!=load32_noalign(&code[1]))This is similar to the new API in -mm
load_le32_noalign, but I don't think it would be worth load_u32_noalign...load32 should be enough.Quoted 7 lines
Quoted 7 lines
Quoted 6 lines
Review
Quoted 3 lines
(quoted diff) +3
-0+ return -EINVAL; + + *(int *)(&code[1]) = new_offset;Might be able to use
put_unaligned_foo()here.Quoted 7 lines
Quoted 2 lines
Quoted 2 lines
put_unaligned(new_offset, (int *)(&code[1]));In a similar vein to above, this becomes:
store32_noalign(&code[1], new_offset);yes, that's much better than the party tricks with magical sizeof, which forces you to run around and check the types of everything.
Quoted 7 lines
Quoted 3 lines
I've seen people doing
get_user()on `long' types and such things occasionally.Do you want to carry the patches to move to the new helpers until they hit mainline, or would you rather I waited until they can go through maintainer trees?
I don't know what that means, really.
Quoted 3 lines
I've got the pile ready removing all of the get/put_{endian} and moving to the load/store API. Not much left after that to just remove the magical sizeof versions too. Just let me know what timing you'd prefer.
I don't understand the dependencies and relationships here.
If you have a bunch of patches which are applicable to current mainline then just spray 'em out with suitable cc's and we'll see which bits stick where. Or is it more complicated than that?
Quoted 6 lines
Also, all of this ends up being so intertwined in the aligned/unaligned cases that I'd like to move most of
Documentation/unaligned_memory_access.txtinto a new alignment_and_byteorder.txt to cover all of these new helpers.I started most of a byteorder document, but constantly referring to the other file made it a bit tiresome, would you mind a consolidated document?
Whatever you think is best. Propose something...
Re: [PATCH 2/5] ftrace: use code patching for ftrace graph tracer
Quoted 5 lines · Andrew Morton
Quoted 4 lines
Do you want to carry the patches to move to the new helpers until they hit mainline, or would you rather I waited until they can go through maintainer trees?
Quoted 7 lines
I don't know what that means, really.
Quoted 3 lines
I've got the pile ready removing all of the get/put_{endian} and moving to the load/store API. Not much left after that to just remove the magical sizeof versions too. Just let me know what timing you'd prefer.
I don't understand the dependencies and relationships here.
The pile I refer to depends on the full set of patches in -mm which includes (I can give all the patch names if you want):
1) byteorder patches moving all remaining arches to use include/linux/byteorder.h
2) unaligned access patches consolidating arches in asm-generic/unaligned.h with no changes
3) move the memset-using arches and ARM to the asm-generic version by moving __packed onto the struct rather than the struct members
4) introduce the load/store_{endian} API
Timing:
1) - 2) I hope all gets into mainline in 2.6.29.
3) I believe could go in 2.6.29, and I'm pretty confident it is OK after talking to some compiler folks.
4) can go in with 3)
My pile is just removing users of get_unaligned/put_unaligned/{endian}_to_cpup and using the new typesafe versions. At the end of the series we remove the old API.
Quoted 4 lines
If you have a bunch of patches which are applicable to current mainline then just spray 'em out with suitable cc's and we'll see which bits stick where. Or is it more complicated than that?
See above, not suitable for mainline.
Quoted 9 lines
Quoted 7 lines
Also, all of this ends up being so intertwined in the aligned/unaligned cases that I'd like to move most of
Documentation/unaligned_memory_access.txtinto a new alignment_and_byteorder.txt to cover all of these new helpers.I started most of a byteorder document, but constantly referring to the other file made it a bit tiresome, would you mind a consolidated document?
Whatever you think is best. Propose something...
Thinking some more, I'll just dump the new endian docs into unaligned_access.txt and that at least gets the documentation out there. Later on it be renamed to something more reflective of its contents.
Harvey
Steven Rostedt
maintainerRe: [PATCH 2/5] ftrace: use code patching for ftrace graph tracer
In reply to Andrew MortonQuoted 14 lines · Andrew Morton
Quoted 12 lines · On Wed, 26 Nov 2008 00:16:24 -0500 Steven Rostedt <[email protected]> wrote:
From: Steven Rostedt <[email protected]>
Impact: more efficient code for ftrace graph tracer
This patch uses the dynamic patching, when available, to patch the function graph code into the kernel.
This patch will ease the way for letting both function tracing and function graph tracing run together.
...
Quoted 12 lines
arch/x86/kernel/ftrace.c int __init ftrace_dyn_arch_init(void *data) · line 338
Quoted 9 lines
arch/x86/kernel/ftrace.c +9
-0+static int ftrace_mod_jmp (unsigned long ip, +int old_offset,int new_offset) +{ +unsigned char code[MCOUNT_INSN_SIZE ]; + +if (probe_kernel_read (code, (void *)ip,MCOUNT_INSN_SIZE )) +return -EFAULT ; + +if (code[0 ] !=0xe9 || old_offset != *(int *)(&code[1 ]))erk. I suspect that there's a nicer way of doing this amongst our forest of
get_unaligned_foo()interfaces. Harvey will know.
Hmm, I may be able to make a struct out of code.
struct {
unsigned char op;
unsigned int offset;} code __attribute__((packed));
Would that look better?
Quoted 10 lines
arch/x86/kernel/ftrace.c int __init ftrace_dyn_arch_init(void *data) · line 341
Quoted 4 lines
arch/x86/kernel/ftrace.c +3
-0+return -EINVAL ; + + *(int *)(&code[1 ]) = new_offset;Might be able to use
put_unaligned_foo()here.The problem is that these functions use
sizeof(*ptr)to work out what to do, so a cast is still needed. Aget_unaligned32(ptr)would be nice. One which takes a void* and assumes CPU ordering.
Is there a correctness concern here? This is arch specific code, so I'm not worried about other archs.
-- Steve
Quoted 10 lines
Quoted 7 lines
arch/x86/kernel/ftrace.c +6
-0+if (do_ftrace_mod_code (ip, &code)) +return -EPERM ; + +return 0 ; +} +
Re: [PATCH 2/5] ftrace: use code patching for ftrace graph tracer
Quoted 31 lines · On Wed, 26 Nov 2008 11:46:58 -0500 (EST) Steven Rostedt <[email protected]>…
Quoted 14 lines · Andrew Morton
Quoted 12 lines · On Wed, 26 Nov 2008 00:16:24 -0500 Steven Rostedt <[email protected]> wrote:
From: Steven Rostedt <[email protected]>
Impact: more efficient code for ftrace graph tracer
This patch uses the dynamic patching, when available, to patch the function graph code into the kernel.
This patch will ease the way for letting both function tracing and function graph tracing run together.
...
Quoted 12 lines
arch/x86/kernel/ftrace.c int __init ftrace_dyn_arch_init(void *data) · line 338
Quoted 9 lines
arch/x86/kernel/ftrace.c +9
-0+static int ftrace_mod_jmp (unsigned long ip, +int old_offset,int new_offset) +{ +unsigned char code[MCOUNT_INSN_SIZE ]; + +if (probe_kernel_read (code, (void *)ip,MCOUNT_INSN_SIZE )) +return -EFAULT ; + +if (code[0 ] !=0xe9 || old_offset != *(int *)(&code[1 ]))erk. I suspect that there's a nicer way of doing this amongst our forest of
get_unaligned_foo()interfaces. Harvey will know.
Quoted 8 lines
Hmm, I may be able to make a struct out of code.
struct { unsigned char op; unsigned int offset;} code __attribute__((packed));
Would that look better?
nah, let's do something more generic for this.
Quoted 7 lines
Quoted 7 lines
arch/x86/kernel/ftrace.c int __init ftrace_dyn_arch_init(void *data) · line 341
Quoted 4 lines
arch/x86/kernel/ftrace.c +3
-0+return -EINVAL ; + + *(int *)(&code[1 ]) = new_offset;Might be able to use
put_unaligned_foo()here.
Quoted 6 lines
Quoted 3 lines
The problem is that these functions use
sizeof(*ptr)to work out what to do, so a cast is still needed. Aget_unaligned32(ptr)would be nice. One which takes a void* and assumes CPU ordering.Is there a correctness concern here? This is arch specific code, so I'm not worried about other archs.
No, the code is OK as-is.
It's just that "read a word from an [maybe-]unaligned address" is such a common operation that there should be a nice clean simple function to do it, rather than doing open-coded (and different) weird C tricks at each and\ every site.
Re: [PATCH 2/5] ftrace: use code patching for ftrace graph tracer
Quoted 14 lines · Andrew Morton
Quoted 7 lines
Quoted 7 lines
arch/x86/kernel/ftrace.c int __init ftrace_dyn_arch_init(void *data) · line 341
Quoted 4 lines
arch/x86/kernel/ftrace.c +3
-0+return -EINVAL ; + + *(int *)(&code[1 ]) = new_offset;Might be able to use
put_unaligned_foo()here.Quoted 6 lines
Quoted 3 lines
The problem is that these functions use
sizeof(*ptr)to work out what to do, so a cast is still needed. Aget_unaligned32(ptr)would be nice. One which takes a void* and assumes CPU ordering.Is there a correctness concern here? This is arch specific code, so I'm not worried about other archs.
Quoted 7 lines
No, the code is OK as-is.
It's just that "read a word from an [maybe-]unaligned address" is such a common operation that there should be a nice clean simple function to do it, rather than doing open-coded (and different) weird C tricks at each and\ every site.
Also it is arch-specific, so if you know unaligned access is OK, just doing the cast+deref is OK.
Harvey
Steven Rostedt
maintainerPATCH 3/5 ftrace: let function tracing and function return run together
In reply to Steven RostedtFrom: Steven Rostedt <[email protected]>
Impact: feature
This patch enables function tracing and function return to run together. I've tested this by enabling the stack tracer and return tracer, where both the function entry and function return are used together with dynamic ftrace.
--- kernel/trace/ftrace.c | 22 ++-------------------- 1 files changed, 2 insertions(+), 20 deletions(-)
kernel/trace/ftrace.c +2 -20
@@ -53,9 +53,6 @@ static int ftrace_pid_trace = -1;/* Quick disabling of function tracer. */ int function_trace_stop; -/* By default, current tracing type is normal tracing. */ -enum ftrace_tracing_type_t ftrace_tracing_type =FTRACE_TYPE_ENTER ; -/* * ftrace_disabled is set when an anomaly is discovered. * ftrace_disabled is much stronger than ftrace_enabled.
@@ -1576,15 +1573,9 @@ int register_ftrace_function(struct ftrace_ops *ops)mutex_lock (&ftrace_sysctl_lock); -if (ftrace_tracing_type ==FTRACE_TYPE_RETURN ) { - ret = -EBUSY ; - goto out; - } - ret =__register_ftrace_function (ops);ftrace_startup (0 ); -out:mutex_unlock (&ftrace_sysctl_lock);return ret; }
@@ -1731,23 +1722,16 @@ int register_ftrace_graph(trace_func_graph_ret_t retfunc,mutex_lock (&ftrace_sysctl_lock); -/* -* Don't launch return tracing if normal function -* tracing is already running. -*/ -if (ftrace_trace_function != ftrace_stub) { - ret = -EBUSY ; - goto out; - }atomic_inc (&ftrace_graph_active); ret =start_graph_tracing ();if (ret) {atomic_dec (&ftrace_graph_active); goto out; } - ftrace_tracing_type =FTRACE_TYPE_RETURN ; + ftrace_graph_return = retfunc; ftrace_graph_entry = entryfunc; +ftrace_startup (FTRACE_START_FUNC_RET ); out:
@@ -1763,8 +1747,6 @@ void unregister_ftrace_graph(void) ftrace_graph_return = (trace_func_graph_ret_t )ftrace_stub; ftrace_graph_entry = (trace_func_graph_ent_t )ftrace_stub;ftrace_shutdown (FTRACE_STOP_FUNC_RET ); -/* Restore normal tracing type */ - ftrace_tracing_type =FTRACE_TYPE_ENTER ;mutex_unlock (&ftrace_sysctl_lock); }
Signature · 3 lines
1.5.6.5 --
Steven Rostedt
maintainerPATCH 4/5 ftrace: add thread comm to function graph tracer
In reply to Steven RostedtFrom: Steven Rostedt <[email protected]>
Impact: enhancement to function graph tracer
Export the trace_find_cmdline so the function graph tracer can use it to print the comms of the threads.
--- kernel/trace/trace.c | 2 +- kernel/trace/trace.h | 1 + kernel/trace/trace_functions_graph.c | 21 ++++++++++++++++----- 3 files changed, 18 insertions(+), 6 deletions(-)
kernel/trace/trace.c +1 -1
@@ -804,7 +804,7 @@ static void trace_save_cmdline(struct task_struct *tsk)spin_unlock (&trace_cmdline_lock); } -static char *trace_find_cmdline (int pid) +char *trace_find_cmdline (int pid) {char *cmdline ="<...>" ;unsigned map;
kernel/trace/trace.h +1 -0
@@ -450,6 +450,7 @@ struct tracer_switch_ops { struct tracer_switch_ops *next; }; +char *trace_find_cmdline (int pid);#endif /* CONFIG_CONTEXT_SWITCH_TRACER */ #ifdef CONFIG_DYNAMIC_FTRACE
kernel/trace/trace_functions_graph.c +16 -5
@@ -32,29 +32,40 @@ static pid_t last_pid = -1;static int graph_trace_init (struct trace_array *tr) { -int cpu; +int cpu, ret; +for_each_online_cpu (cpu)tracing_reset (tr, cpu); -return register_ftrace_graph (&trace_graph_return, + ret =register_ftrace_graph (&trace_graph_return, &trace_graph_entry); +if (ret) +return ret; +tracing_start_cmdline_record (); + +return 0 ; }static void graph_trace_reset (struct trace_array *tr) { -unregister_ftrace_graph (); +tracing_stop_cmdline_record (); +unregister_ftrace_graph (); }/* If the pid changed since the last trace, output this event */ static int verif_pid (struct trace_seq *s,pid_t pid) { +char *comm; +if (last_pid !=-1 && last_pid == pid)return 1 ; last_pid = pid; -return trace_seq_printf (s,"\n------------8<---------- thread %d" + comm =trace_find_cmdline (pid); + +return trace_seq_printf (s,"\n------------8<---------- thread %s-%d" " ------------8<----------\n\n" , - pid); + comm, pid); } static enum print_line_t
Signature · 3 lines
1.5.6.5 --
Re: [PATCH 4/5] ftrace: add thread comm to function graph tracer
Quoted 18 lines · On Wed, 26 Nov 2008 00:16:26 -0500 Steven Rostedt <[email protected]> wrote:
From: Steven Rostedt <[email protected]>
Impact: enhancement to function graph tracer
Export the
trace_find_cmdlineso the function graph tracer can use it to print the comms of the threads.kernel/trace/trace_functions_graph.c +3
-1-static char *trace_find_cmdline (int pid) +char *trace_find_cmdline (int pid) static int verif_pid(struct trace_seq *s, pid_t pid) { +char *comm; +if (last_pid !=-1 && last_pid == pid)return 1 ;
kernel/trace/trace_functions_graph.c static pid_t last_pid = -1; · line 69
Quoted 10 lines
kernel/trace/trace_functions_graph.c +4
-2last_pid = pid; -return trace_seq_printf (s,"\n------------8<---------- thread %d" + comm =trace_find_cmdline (pid); + +return trace_seq_printf (s,"\n------------8<---------- thread %s-%d" " ------------8<----------\n\n" , - pid); + comm, pid); }
This code gets its int's and pid_t's all mixed up. It's a bit cosmetic, but nice to get it right for readability's sake.
Steven Rostedt
maintainerRe: [PATCH 4/5] ftrace: add thread comm to function graph tracer
Quoted 21 lines · Andrew Morton
Quoted 19 lines · On Wed, 26 Nov 2008 00:16:26 -0500 Steven Rostedt <[email protected]> wrote:
From: Steven Rostedt <[email protected]>
Impact: enhancement to function graph tracer
Export the
trace_find_cmdlineso the function graph tracer can use it to print the comms of the threads.kernel/trace/trace_functions_graph.c +3
-1-static char *trace_find_cmdline (int pid) +char *trace_find_cmdline (int pid) static int verif_pid(struct trace_seq *s, pid_t pid) { +char *comm; +if (last_pid !=-1 && last_pid == pid)return 1 ;
Quoted 12 lines
kernel/trace/trace_functions_graph.c static pid_t last_pid = -1; · line 69
Quoted 9 lines
kernel/trace/trace_functions_graph.c +4
-2last_pid = pid; -return trace_seq_printf (s,"\n------------8<---------- thread %d" + comm =trace_find_cmdline (pid); + +return trace_seq_printf (s,"\n------------8<---------- thread %s-%d" " ------------8<----------\n\n" , - pid); + comm, pid); }This code gets its int's and pid_t's all mixed up. It's a bit cosmetic, but nice to get it right for readability's sake.
Hmm, I think I would like to keep all pids as ints. Perhaps because we do not have namespaces here ;-)
I'm totally confused by what to do. I'll have to think about it.
-- Steve
Re: [PATCH 4/5] ftrace: add thread comm to function graph tracer
Quoted 37 lines · On Wed, 26 Nov 2008 11:48:44 -0500 (EST) Steven Rostedt <[email protected]>…
Quoted 21 lines · Andrew Morton
Quoted 19 lines · On Wed, 26 Nov 2008 00:16:26 -0500 Steven Rostedt <[email protected]> wrote:
From: Steven Rostedt <[email protected]>
Impact: enhancement to function graph tracer
Export the
trace_find_cmdlineso the function graph tracer can use it to print the comms of the threads.kernel/trace/trace_functions_graph.c +3
-1-static char *trace_find_cmdline (int pid) +char *trace_find_cmdline (int pid) static int verif_pid(struct trace_seq *s, pid_t pid) { +char *comm; +if (last_pid !=-1 && last_pid == pid)return 1 ;Quoted 12 lines
kernel/trace/trace_functions_graph.c static pid_t last_pid = -1; · line 69
Quoted 9 lines
kernel/trace/trace_functions_graph.c +4
-2last_pid = pid; -return trace_seq_printf (s,"\n------------8<---------- thread %d" + comm =trace_find_cmdline (pid); + +return trace_seq_printf (s,"\n------------8<---------- thread %s-%d" " ------------8<----------\n\n" , - pid); + comm, pid); }This code gets its int's and pid_t's all mixed up. It's a bit cosmetic, but nice to get it right for readability's sake.
Quoted 4 lines
Hmm, I think I would like to keep all pids as ints. Perhaps because we do not have namespaces here ;-)
I'm totally confused by what to do. I'll have to think about it.
Well. Any variable which contains a process ID should have type pid_t.
It gets more complicated in places where these values are passed to and from userspace (including via printk!). It would be defensive (but unnecessary) to cast the pid_t's to and from known-size types at the kernel boundaries.
Steven Rostedt
maintainerPATCH 5/5 ftrace: add cpu annotation for function graph tracer
In reply to Steven RostedtFrom: Steven Rostedt <[email protected]>
Impact: enhancement for function graph tracer
When run on a SMP box, the function graph tracer is confusing because it shows the different CPUS as changes in the trace.
This patch adds the annotation of 'CPU[###]' where ### is a three digit number. The output will look similar to this:
CPU[001] dput() {
CPU[000] } 726
CPU[001] } 487
CPU[000] do_softirq() {
CPU[001] } 2221
CPU[000] __do_softirq() {
CPU[000] __local_bh_disable() {
CPU[001] unroll_tree_refs() {
CPU[000] } 569
CPU[001] } 501
CPU[000] rcu_process_callbacks() {
CPU[001] kfree() {
What makes this nice is that now you can grep the file and produce readable format for a particular CPU.
# cat /debug/tracing/trace > /tmp/trace # grep '^CPU\[000\]' /tmp/trace > /tmp/trace0 # grep '^CPU\[001\]' /tmp/trace > /tmp/trace1
Will give you:
# head /tmp/trace0
CPU[000] ------------8<---------- thread sshd-3899 ------------8<----------
CPU[000] inotify_dentry_parent_queue_event() {
CPU[000] } 2531
CPU[000] inotify_inode_queue_event() {
CPU[000] } 505
CPU[000] } 69626
CPU[000] } 73089
CPU[000] audit_syscall_exit() {
CPU[000] path_put() {
CPU[000] dput() { # head /tmp/trace1
CPU[001] ------------8<---------- thread pcscd-3446 ------------8<----------
CPU[001] } 4186
CPU[001] dput() {
CPU[001] } 543
CPU[001] vfs_permission() {
CPU[001] inode_permission() {
CPU[001] shmem_permission() {
CPU[001] generic_permission() {
CPU[001] } 501
CPU[001] } 2205--- kernel/trace/trace_functions_graph.c | 34 ++++++++++++++++++++++------------ 1 files changed, 22 insertions(+), 12 deletions(-)
kernel/trace/trace_functions_graph.c +22 -12
@@ -28,7 +28,7 @@ static struct tracer_flags tracer_flags = { };/* pid on the last trace processed */ -static pid_t last_pid =-1 ; +static pid_t last_pid[NR_CPUS ] = { [0 ...NR_CPUS -1 ] =-1 };static int graph_trace_init (struct trace_array *tr) {
@@ -53,29 +53,34 @@ static void graph_trace_reset(struct trace_array *tr) }/* If the pid changed since the last trace, output this event */ -static int verif_pid (struct trace_seq *s,pid_t pid) +static int verif_pid (struct trace_seq *s,pid_t pid,int cpu) {char *comm; -if (last_pid !=-1 && last_pid == pid) +if (last_pid[cpu] !=-1 && last_pid[cpu] == pid)return 1 ; - last_pid = pid; + last_pid[cpu] = pid; comm =trace_find_cmdline (pid); -return trace_seq_printf (s,"\n------------8<---------- thread %s-%d" +return trace_seq_printf (s,"\nCPU[%03d]" +" ------------8<---------- thread %s-%d" " ------------8<----------\n\n" , - comm, pid); + cpu, comm, pid); }static enum print_line_t print_graph_entry (struct ftrace_graph_ent *call,struct trace_seq *s, -struct trace_entry *ent) +struct trace_entry *ent,int cpu) {int i;int ret; -if (!verif_pid (s, ent->pid)) +if (!verif_pid (s, ent->pid, cpu)) +return TRACE_TYPE_PARTIAL_LINE ; + + ret =trace_seq_printf (s,"CPU[%03d] " , cpu); +if (!ret)return TRACE_TYPE_PARTIAL_LINE ;for (i =0 ; i < call->depth *TRACE_GRAPH_INDENT ; i++) {
@@ -96,12 +101,16 @@ print_graph_entry(struct ftrace_graph_ent *call, struct trace_seq *s,static enum print_line_t print_graph_return (struct ftrace_graph_ret *trace,struct trace_seq *s, -struct trace_entry *ent) +struct trace_entry *ent,int cpu) {int i;int ret; -if (!verif_pid (s, ent->pid)) +if (!verif_pid (s, ent->pid, cpu)) +return TRACE_TYPE_PARTIAL_LINE ; + + ret =trace_seq_printf (s,"CPU[%03d] " , cpu); +if (!ret)return TRACE_TYPE_PARTIAL_LINE ;for (i =0 ; i < trace->depth *TRACE_GRAPH_INDENT ; i++) {
@@ -137,12 +146,13 @@ print_graph_function(struct trace_iterator *iter)case TRACE_GRAPH_ENT : {struct ftrace_graph_ent_entry *field;trace_assign_type (field, entry); -return print_graph_entry (&field->graph_ent, s, entry); +return print_graph_entry (&field->graph_ent, s, entry, + iter->cpu); }case TRACE_GRAPH_RET : {struct ftrace_graph_ret_entry *field;trace_assign_type (field, entry); -return print_graph_return (&field->ret, s, entry); +return print_graph_return (&field->ret, s, entry, iter->cpu); }default :return TRACE_TYPE_UNHANDLED ;
Signature · 3 lines
1.5.6.5 --
Re: [PATCH 5/5] ftrace: add cpu annotation for function graph tracer
Quoted 2 lines · On Wed, 26 Nov 2008 00:16:27 -0500 Steven Rostedt <[email protected]> wrote:
This patch adds the annotation of 'CPU[###]' where ### is a three digit number
Is this a plot to get yourself sent a 1024-way test box?
Re: [PATCH 5/5] ftrace: add cpu annotation for function graph tracer
Quoted 6 lines · Andrew Morton
Quoted 2 lines · On Wed, 26 Nov 2008 00:16:27 -0500 Steven Rostedt <[email protected]> wrote:
This patch adds the annotation of 'CPU[###]' where ### is a three digit number
Is this a plot to get yourself sent a 1024-way test box?
So you are suggesting to change it to 4 digit printout?
(Is this a plot to get yourself snt a 16384-way test box? ;-)
Ingo
Re: [PATCH 5/5] ftrace: add cpu annotation for function graph tracer
Quoted 11 lines · On Wed, 26 Nov 2008 06:47:28 +0100 Ingo Molnar <[email protected]> wrote:
Quoted 6 lines · Andrew Morton
Quoted 2 lines · On Wed, 26 Nov 2008 00:16:27 -0500 Steven Rostedt <[email protected]> wrote:
This patch adds the annotation of 'CPU[###]' where ### is a three digit number
Is this a plot to get yourself sent a 1024-way test box?
So you are suggesting to change it to 4 digit printout?
Not really. I somehow doubt that it's economic to do your software tuning on the 1024-way ;)
Quoted 1 line
(Is this a plot to get yourself snt a 16384-way test box? ;-)
I'd settle for 1.6384 testers.
Steven Rostedt
maintainerRe: [PATCH 5/5] ftrace: add cpu annotation for function graph tracer
In reply to Andrew MortonQuoted 6 lines · Andrew Morton
Quoted 2 lines · On Wed, 26 Nov 2008 00:16:27 -0500 Steven Rostedt <[email protected]> wrote:
This patch adds the annotation of 'CPU[###]' where ### is a three digit number
Is this a plot to get yourself sent a 1024-way test box?
Have an extra one?
;)
-- Steve