lkml.org 
[lkml]   [2008]   [Nov]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/5] ftrace: add function tracing to single thread
On Wed, 26 Nov 2008 00:16:23 -0500 Steven Rostedt <rostedt@goodmis.org> wrote:

> From: Steven Rostedt <srostedt@redhat.com>
>
> 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.
>
> ...
>
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -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)
> {
> @@ -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.

> +#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.

>
> ...
>
> +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?

> + 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))
> + return -EINVAL;
> +
> + if (copy_from_user(&buf, ubuf, cnt))
> + return -EFAULT;
> +
> + buf[cnt] = 0;

Might be able to use strncpy_from_user() here?

> + 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:
> + mutex_unlock(&ftrace_start_lock);
> +
> + return cnt;
> +}
> +
> +static struct file_operations ftrace_pid_fops = {

const, please.

> + .read = ftrace_pid_read,
> + .write = ftrace_pid_write,
> +};
> +



\
 
 \ /
  Last update: 2008-11-26 06:33    [W:0.124 / U:0.304 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site