* Re: [PATCH v5 02/17] perf ftrace: add option '-F/--funcs' to list available functions
2020-07-17 15:05 ` [PATCH v5 02/17] perf ftrace: add option '-F/--funcs' to list available functions Steven Rostedt
@ 2020-07-17 16:21 ` Arnaldo Carvalho de Melo
2020-07-17 16:27 ` Steven Rostedt
2020-07-18 7:06 ` Changbin Du
1 sibling, 1 reply; 5+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-07-17 16:21 UTC (permalink / raw)
To: Steven Rostedt
Cc: Changbin Du, Jiri Olsa, Peter Zijlstra, Ingo Molnar,
Namhyung Kim, linux-kernel, linux-trace-devel
Em Fri, Jul 17, 2020 at 11:05:04AM -0400, Steven Rostedt escreveu:
> On Sat, 11 Jul 2020 20:40:20 +0800
> Changbin Du <changbin.du@gmail.com> wrote:
>
> > This adds an option '-F/--funcs' to list all available functions to trace,
> > which is read from tracing file 'available_filter_functions'.
> >
> > $ sudo ./perf ftrace -F | head
> > trace_initcall_finish_cb
> > initcall_blacklisted
> > do_one_initcall
> > do_one_initcall
> > trace_initcall_start_cb
> > run_init_process
> > try_to_run_init_process
> > match_dev_by_label
> > match_dev_by_uuid
> > rootfs_init_fs_context
> >
> > Signed-off-by: Changbin Du <changbin.du@gmail.com>
> >
> > ---
> > v2: option name '-l/--list-functions' -> '-F/--funcs'
> > ---
> > tools/perf/Documentation/perf-ftrace.txt | 4 +++
> > tools/perf/builtin-ftrace.c | 43 ++++++++++++++++++++++++
> > 2 files changed, 47 insertions(+)
> >
> > diff --git a/tools/perf/Documentation/perf-ftrace.txt b/tools/perf/Documentation/perf-ftrace.txt
> > index 952e46669168..d79560dea19f 100644
> > --- a/tools/perf/Documentation/perf-ftrace.txt
> > +++ b/tools/perf/Documentation/perf-ftrace.txt
> > @@ -30,6 +30,10 @@ OPTIONS
> > --verbose=::
> > Verbosity level.
> >
> > +-F::
> > +--funcs::
> > + List all available functions to trace.
> > +
> > -p::
> > --pid=::
> > Trace on existing process id (comma separated list).
> > diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
> > index 5f53da87040d..244cc8e6bd60 100644
> > --- a/tools/perf/builtin-ftrace.c
> > +++ b/tools/perf/builtin-ftrace.c
> > @@ -32,6 +32,7 @@ struct perf_ftrace {
> > struct evlist *evlist;
> > struct target target;
> > const char *tracer;
> > + bool list_avail_functions;
> > struct list_head filters;
> > struct list_head notrace;
> > struct list_head graph_funcs;
> > @@ -127,6 +128,43 @@ static int append_tracing_file(const char *name, const char *val)
> > return __write_tracing_file(name, val, true);
> > }
> > +static int read_tracing_file_to_stdout(const char *name)
> > +{
> All this is looking like its duplicating code that we are working on
> for libtracefs.
> Would you like to start contributing to that, and when we get the
> libtracefs.so packed in distributions, we can easily create the
> perf ftrace without having to rewrite the wheel 10 times?
Or we can use as soon as it is available, not preventing 'perf ftrace'
from having to wait for libtracefs.so?
Duplication is normal at some point, Changbin is moving 'perf ftrace'
forward, and has been doing this thru several patch series revisions, if
we continue putting new requirements, it gets tiresome at some point :-\
- Arnaldo
> -- Steve
>
>
> > + char buf[4096];
> > + char *file;
> > + int fd;
> > + int ret = -1;
> > +
> > + file = get_tracing_file(name);
> > + if (!file) {
> > + pr_debug("cannot get tracing file: %s\n", name);
> > + return -1;
> > + }
> > +
> > + fd = open(file, O_RDONLY);
> > + if (fd < 0) {
> > + pr_debug("cannot open tracing file: %s: %s\n",
> > + name, str_error_r(errno, buf, sizeof(buf)));
> > + goto out;
> > + }
> > +
> > + /* read contents to stdout */
> > + while (true) {
> > + int n = read(fd, buf, sizeof(buf));
> > + if (n <= 0)
> > + goto out_close;
> > + if (fwrite(buf, n, 1, stdout) != 1)
> > + goto out_close;
> > + }
> > + ret = 0;
> > +
> > +out_close:
> > + close(fd);
> > +out:
> > + put_tracing_file(file);
> > + return ret;
> > +}
> > +
> > static int reset_tracing_cpu(void);
> > static void reset_tracing_filters(void);
> >
> > @@ -301,6 +339,9 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
> > signal(SIGCHLD, sig_handler);
> > signal(SIGPIPE, sig_handler);
> >
> > + if (ftrace->list_avail_functions)
> > + return read_tracing_file_to_stdout("available_filter_functions");
> > +
> > if (reset_tracing_files(ftrace) < 0) {
> > pr_err("failed to reset ftrace\n");
> > goto out;
> > @@ -470,6 +511,8 @@ int cmd_ftrace(int argc, const char **argv)
> > const struct option ftrace_options[] = {
> > OPT_STRING('t', "tracer", &ftrace.tracer, "tracer",
> > "tracer to use: function or function_graph (This option is deprecated)"),
> > + OPT_BOOLEAN('F', "funcs", &ftrace.list_avail_functions,
> > + "Show available functions to filter"),
> > OPT_STRING('p', "pid", &ftrace.target.pid, "pid",
> > "trace on existing process id"),
> > OPT_INCR('v', "verbose", &verbose,
>
--
- Arnaldo
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v5 02/17] perf ftrace: add option '-F/--funcs' to list available functions
2020-07-17 15:05 ` [PATCH v5 02/17] perf ftrace: add option '-F/--funcs' to list available functions Steven Rostedt
2020-07-17 16:21 ` Arnaldo Carvalho de Melo
@ 2020-07-18 7:06 ` Changbin Du
1 sibling, 0 replies; 5+ messages in thread
From: Changbin Du @ 2020-07-18 7:06 UTC (permalink / raw)
To: Steven Rostedt
Cc: Changbin Du, Jiri Olsa, Arnaldo Carvalho de Melo, Peter Zijlstra,
Ingo Molnar, Namhyung Kim, linux-kernel, linux-trace-devel
On Fri, Jul 17, 2020 at 11:05:04AM -0400, Steven Rostedt wrote:
> On Sat, 11 Jul 2020 20:40:20 +0800
> Changbin Du <changbin.du@gmail.com> wrote:
>
> > This adds an option '-F/--funcs' to list all available functions to trace,
> > which is read from tracing file 'available_filter_functions'.
> >
> > $ sudo ./perf ftrace -F | head
> > trace_initcall_finish_cb
> > initcall_blacklisted
> > do_one_initcall
> > do_one_initcall
> > trace_initcall_start_cb
> > run_init_process
> > try_to_run_init_process
> > match_dev_by_label
> > match_dev_by_uuid
> > rootfs_init_fs_context
> >
> > Signed-off-by: Changbin Du <changbin.du@gmail.com>
> >
> > ---
> > v2: option name '-l/--list-functions' -> '-F/--funcs'
> > ---
> > tools/perf/Documentation/perf-ftrace.txt | 4 +++
> > tools/perf/builtin-ftrace.c | 43 ++++++++++++++++++++++++
> > 2 files changed, 47 insertions(+)
> >
> > diff --git a/tools/perf/Documentation/perf-ftrace.txt b/tools/perf/Documentation/perf-ftrace.txt
> > index 952e46669168..d79560dea19f 100644
> > --- a/tools/perf/Documentation/perf-ftrace.txt
> > +++ b/tools/perf/Documentation/perf-ftrace.txt
> > @@ -30,6 +30,10 @@ OPTIONS
> > --verbose=::
> > Verbosity level.
> >
> > +-F::
> > +--funcs::
> > + List all available functions to trace.
> > +
> > -p::
> > --pid=::
> > Trace on existing process id (comma separated list).
> > diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
> > index 5f53da87040d..244cc8e6bd60 100644
> > --- a/tools/perf/builtin-ftrace.c
> > +++ b/tools/perf/builtin-ftrace.c
> > @@ -32,6 +32,7 @@ struct perf_ftrace {
> > struct evlist *evlist;
> > struct target target;
> > const char *tracer;
> > + bool list_avail_functions;
> > struct list_head filters;
> > struct list_head notrace;
> > struct list_head graph_funcs;
> > @@ -127,6 +128,43 @@ static int append_tracing_file(const char *name, const char *val)
> > return __write_tracing_file(name, val, true);
> > }
> >
> > +static int read_tracing_file_to_stdout(const char *name)
> > +{
>
> All this is looking like its duplicating code that we are working on
> for libtracefs.
>
> Would you like to start contributing to that, and when we get the
> libtracefs.so packed in distributions, we can easily create the
> perf ftrace without having to rewrite the wheel 10 times?
>
> -- Steve
Yes, I'd like to join the development of libtracefs. But honestly speaking,
I am not sure whether I can get enough time on it since I have to take care
of my little baby :).
I have the same opinion with Arnaldo that we can merge what we'v already
done here. Then consider to migrate to libtracefs instead before adding
more new features.
--
Cheers,
Changbin Du
^ permalink raw reply [flat|nested] 5+ messages in thread