[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c62985530910120347v2d0d30f1o4d7f7c62969f9b28@mail.gmail.com>
Date: Mon, 12 Oct 2009 12:47:18 +0200
From: Frédéric Weisbecker <fweisbec@...il.com>
To: Stefan Assmann <sassmann@...hat.com>
Cc: rostedt@...dmis.org,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Ingo Molnar <mingo@...e.hu>
Subject: Re: [PATCH] ftrace: add kernel command line graph function filtering
Le 12 octobre 2009 12:05, Stefan Assmann <sassmann@...hat.com> a écrit :
> Thanks for reviewing Frédéric!
> I hope the following is what you had in mind.
>
> Stefan
>
> From edeb75ce9c9a35d5905d45954f66a0e05d5fe8ab Mon Sep 17 00:00:00 2001
> From: Stefan Assmann <sassmann@...hat.com>
> Date: Mon, 12 Oct 2009 09:11:47 +0200
> Subject: [PATCH] ftrace: add kernel command line graph function filtering
>
> Add a command line parameter to allow limiting the top level callers functions
> traced by the function graph tracer at boot up.
>
> This patch adds the following command line option:
> ftrace_graph_filter=function-list
>
> Where function-list is a comma separated list of functions to filter.
>
> Signed-off-by: Stefan Assmann <sassmann@...hat.com>
> ---
> Documentation/kernel-parameters.txt | 9 ++++++++-
> kernel/trace/ftrace.c | 35 +++++++++++++++++++++++++++++++++++
> kernel/trace/trace.c | 2 +-
> kernel/trace/trace.h | 1 +
> 4 files changed, 45 insertions(+), 2 deletions(-)
Yup, looks good to me. I'm going to queue it.
Just some minor neats I'll change, see below:
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index 9107b38..539a053 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -771,7 +771,14 @@ and is between 256 and 4096 characters. It is defined in the file
> tracer at boot up. function-list is a comma separated
> list of functions. This list can be changed at run
> time by the set_ftrace_filter file in the debugfs
> - tracing directory.
> + tracing directory.
> +
> + ftrace_graph_filter=[function-list]
> + [FTRACE] Limit the top level callers functions traced
> + by the function graph tracer at boot up.
> + function-list is a comma separated list of functions
> + that can be changed at run time by the
> + set_graph_function file in the debugfs tracing directory.
>
> ftrace_notrace=[function-list]
> [FTRACE] Do not trace the functions specified in
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 3724756..ee9c509 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -78,6 +78,10 @@ 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;
>
> +#ifdef CONFIG_FUNCTION_GRAPH_TRACER
> +static int ftrace_set_func(unsigned long *array, int *idx, char *buffer);
> +#endif
I'll try to move most of this new code in the bottom of ftrace.c, where most of
the ftrace related function graph tracer features are implemented.
Hopefully that could avoid us this ifdef'd declaration and all of the new others
by putting them in the already ifdef'd section of function graph helpers.
And also it's better to group the function graph helpers together.
> static void ftrace_list_func(unsigned long ip, unsigned long parent_ip)
> {
> struct ftrace_ops *op = ftrace_list;
> @@ -2302,6 +2306,7 @@ void ftrace_set_notrace(unsigned char *buf, int len, int reset)
> #define FTRACE_FILTER_SIZE COMMAND_LINE_SIZE
> static char ftrace_notrace_buf[FTRACE_FILTER_SIZE] __initdata;
> static char ftrace_filter_buf[FTRACE_FILTER_SIZE] __initdata;
> +static char ftrace_graph_buf[FTRACE_FILTER_SIZE] __initdata;
>
> static int __init set_ftrace_notrace(char *str)
> {
> @@ -2317,6 +2322,32 @@ static int __init set_ftrace_filter(char *str)
> }
> __setup("ftrace_filter=", set_ftrace_filter);
>
> +#ifdef CONFIG_FUNCTION_GRAPH_TRACER
> +static int __init set_graph_function(char *str)
> +{
> + strncpy(ftrace_graph_buf, str, FTRACE_FILTER_SIZE);
> + return 1;
> +}
> +__setup("ftrace_graph_filter=", set_graph_function);
> +
> +static void __init set_ftrace_early_graph(char *buf)
> +{
> + int ret;
> + char *func;
> +
> + set_ftrace("function_graph");
> + while (buf) {
> + func = strsep(&buf, ",");
> + /* we allow only one expression at a time */
> + ret = ftrace_set_func(ftrace_graph_funcs, &ftrace_graph_count,
> + func);
> + if (ret)
> + printk(KERN_DEBUG "ftrace: function %s not "
> + "traceable\n", func);
> + }
> +}
> +#endif /* CONFIG_FUNCTION_GRAPH_TRACER */
>
> static void __init set_ftrace_early_filter(char *buf, int enable)
> {
> char *func;
> @@ -2333,6 +2364,10 @@ static void __init set_ftrace_early_filters(void)
> set_ftrace_early_filter(ftrace_filter_buf, 1);
> if (ftrace_notrace_buf[0])
> set_ftrace_early_filter(ftrace_notrace_buf, 0);
> +#ifdef CONFIG_FUNCTION_GRAPH_TRACER
> + if (ftrace_graph_buf[0])
> + set_ftrace_early_graph(ftrace_graph_buf);
> +#endif /* CONFIG_FUNCTION_GRAPH_TRACER */
> }
>
> static int
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 4506826..811c918 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -129,7 +129,7 @@ static int tracing_set_tracer(const char *buf);
> static char bootup_tracer_buf[MAX_TRACER_SIZE] __initdata;
> static char *default_bootup_tracer;
>
> -static int __init set_ftrace(char *str)
> +int __init set_ftrace(char *str)
I'm also going to rename this into set_early_ftrace() :-)
Thanks!
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists