linux-trace-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH v5 02/17] perf ftrace: add option '-F/--funcs' to list available functions
       [not found] ` <20200711124035.6513-3-changbin.du@gmail.com>
@ 2020-07-17 15:05   ` Steven Rostedt
  2020-07-17 16:21     ` Arnaldo Carvalho de Melo
  2020-07-18  7:06     ` Changbin Du
  0 siblings, 2 replies; 5+ messages in thread
From: Steven Rostedt @ 2020-07-17 15:05 UTC (permalink / raw)
  To: Changbin Du
  Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
	Namhyung Kim, linux-kernel, linux-trace-devel

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


> +	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,


^ 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-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 16:21     ` Arnaldo Carvalho de Melo
@ 2020-07-17 16:27       ` Steven Rostedt
  2020-07-17 16:37         ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 5+ messages in thread
From: Steven Rostedt @ 2020-07-17 16:27 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Changbin Du, Jiri Olsa, Peter Zijlstra, Ingo Molnar,
	Namhyung Kim, linux-kernel, linux-trace-devel

On Fri, 17 Jul 2020 13:21:16 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

>  
> > 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 :-\

We're finally at the point to move libtracefs.so and libtraceevent.so
into their own repository.

My fear is that the two will become incompatible, and forked forever.

-- Steve

^ 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 16:27       ` Steven Rostedt
@ 2020-07-17 16:37         ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 5+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-07-17 16:37 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 12:27:40PM -0400, Steven Rostedt escreveu:
> On Fri, 17 Jul 2020 13:21:16 -0300 Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > > 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 :-\
 
> We're finally at the point to move libtracefs.so and libtraceevent.so
> into their own repository.
 
> My fear is that the two will become incompatible, and forked forever.

I don't share this fear, and since libtracefs is not generally
available, this will make perf progress to be slowed down, so its better
to merge what he has so far, after some review issues that surfaced are
solved, and when a better way of achieving that is available, consider
using it.

- 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

end of thread, other threads:[~2020-07-18  7:06 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20200711124035.6513-1-changbin.du@gmail.com>
     [not found] ` <20200711124035.6513-3-changbin.du@gmail.com>
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-17 16:37         ` Arnaldo Carvalho de Melo
2020-07-18  7:06     ` Changbin Du

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).