linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 00/15] perf: ftrace enhancement
@ 2020-06-27 13:36 Changbin Du
  2020-06-27 13:36 ` [PATCH v2 01/15] perf ftrace: select function/function_graph tracer automatically Changbin Du
                   ` (14 more replies)
  0 siblings, 15 replies; 35+ messages in thread
From: Changbin Du @ 2020-06-27 13:36 UTC (permalink / raw)
  To: Jiri Olsa, Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Ingo Molnar, Namhyung Kim, Steven Rostedt,
	linux-kernel, Changbin Du

The perf has basic kernel ftrace support but lack support of most tracing
options. This serias is target to enhance the perf ftrace functionality so
that we can make full use of kernel ftrace with perf.

In general, this serias be cataloged into two main changes:
  1) Improve usability of existing functions. For example, we don't need to type
     extra option to select the tracer.
  2) Add new options to support all other ftrace functions.

Here is a glance of all ftrace functions with this serias:

$ sudo perf ftrace -h
 Usage: perf ftrace [<options>] [<command>]
    or: perf ftrace [<options>] -- <command> [<options>]

    -a, --all-cpus        system-wide collection from all CPUs
    -C, --cpu <cpu>       list of cpus to monitor
    -d, --delay <n>       ms to wait before starting tracing after program start
    -D, --graph-depth <n>
                          Max depth for function graph tracer
    -F, --funcs           Show available functions to filter
    -G, --graph-funcs <func>
                          trace given functions using function_graph tracer
    -g, --nograph-funcs <func>
                          Set nograph filter on given functions
    -m, --buffer-size <n>
                          size of per cpu buffer in kb
    -N, --notrace-funcs <func>
                          do not trace given functions
    -p, --pid <pid>       trace on existing process id
    -t, --tid <tid>       trace on existing thread id (exclusive to --pid)
    -T, --trace-funcs <func>
                          trace given functions using function tracer
    -t, --tracer <tracer>
                          tracer to use: function or function_graph (This option is deprecated)
    -v, --verbose         be more verbose
        --func-call-graph
                          show kernel stack trace for function tracer
        --func-irq-info   display irq info for function tracer
        --graph-noirqs    ignore functions that happen inside interrupt for function_graph tracer
        --graph-nosleep-time
                          measure on-CPU time only for function_graph tracer
        --graph-thresh <n>
                          only show functions of which the duration is greater than <n>µs
        --graph-verbose   show process names, PIDs, timestamps for function_graph tracer
        --inherit         trace children processes

v2:
  o patches for option '-u/--userstacktrace' and '--no-pager' are dropped.
  o update all related perf documentation.
  o rename some options. Now all funcgraph tracer options are prefixed with
    '--graph-', while all function tracer options are prefixed with '--func-'.
  o mark old options deprecated instead of removing them.

Changbin Du (15):
  perf ftrace: select function/function_graph tracer automatically
  perf ftrace: add option '-F/--funcs' to list available functions
  perf ftrace: add option -t/--tid to filter by thread id
  perf ftrace: add option -d/--delay to delay tracing
  perf ftrace: factor out function write_tracing_file_int()
  perf ftrace: add option '-m/--buffer-size' to set per-cpu buffer size
  perf ftrace: show trace column header
  perf ftrace: add option '--inherit' to trace children processes
  perf ftrace: add support for tracing option 'func_stack_trace'
  perf ftrace: add support for trace option sleep-time
  perf ftrace: add support for trace option funcgraph-irqs
  perf ftrace: add support for tracing option 'irq-info'
  perf ftrace: add option '--graph-verbose' to show more info for graph
    tracer
  perf ftrace: add support for trace option tracing_thresh
  perf ftrace: add change log

 tools/perf/Documentation/perf-config.txt |   5 -
 tools/perf/Documentation/perf-ftrace.txt |  39 ++-
 tools/perf/builtin-ftrace.c              | 295 +++++++++++++++++++++--
 3 files changed, 319 insertions(+), 20 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH v2 01/15] perf ftrace: select function/function_graph tracer automatically
  2020-06-27 13:36 [PATCH v2 00/15] perf: ftrace enhancement Changbin Du
@ 2020-06-27 13:36 ` Changbin Du
  2020-07-03  5:47   ` Namhyung Kim
  2020-06-27 13:36 ` [PATCH v2 02/15] perf ftrace: add option '-F/--funcs' to list available functions Changbin Du
                   ` (13 subsequent siblings)
  14 siblings, 1 reply; 35+ messages in thread
From: Changbin Du @ 2020-06-27 13:36 UTC (permalink / raw)
  To: Jiri Olsa, Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Ingo Molnar, Namhyung Kim, Steven Rostedt,
	linux-kernel, Changbin Du

The '-g/-G' options have already implied function_graph tracer should be
used instead of function tracer. So the extra option '--tracer' can be
killed.

This patch changes the behavior as below:
  - By default, function tracer is used.
  - If '-g' or '-G' option is on, then function_graph tracer is used.
  - The perf configuration item 'ftrace.tracer' is marked as deprecated.
  - The option '--tracer' is marked as deprecated.
  - The default filter for -G/-T is to trace all functions.

Here are some examples.

This will start tracing all functions using function tracer:
  $ sudo perf ftrace

This will trace all functions using function graph tracer:
  $ sudo perf ftrace -G

This will trace function vfs_read using function graph tracer:
  $ sudo perf ftrace -G vfs_read

Signed-off-by: Changbin Du <changbin.du@gmail.com>
---
 tools/perf/Documentation/perf-config.txt |  5 -----
 tools/perf/Documentation/perf-ftrace.txt |  2 +-
 tools/perf/builtin-ftrace.c              | 19 ++++++++++++-------
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/tools/perf/Documentation/perf-config.txt b/tools/perf/Documentation/perf-config.txt
index c7d3df5798e2..a25fee7de3b2 100644
--- a/tools/perf/Documentation/perf-config.txt
+++ b/tools/perf/Documentation/perf-config.txt
@@ -612,11 +612,6 @@ trace.*::
 		"libbeauty", the default, to use the same argument beautifiers used in the
 		strace-like sys_enter+sys_exit lines.
 
-ftrace.*::
-	ftrace.tracer::
-		Can be used to select the default tracer. Possible values are
-		'function' and 'function_graph'.
-
 llvm.*::
 	llvm.clang-path::
 		Path to clang. If omit, search it from $PATH.
diff --git a/tools/perf/Documentation/perf-ftrace.txt b/tools/perf/Documentation/perf-ftrace.txt
index b80c84307dc9..952e46669168 100644
--- a/tools/perf/Documentation/perf-ftrace.txt
+++ b/tools/perf/Documentation/perf-ftrace.txt
@@ -24,7 +24,7 @@ OPTIONS
 
 -t::
 --tracer=::
-	Tracer to use: function_graph or function.
+	Tracer to use: function_graph or function. This option is deprecated.
 
 -v::
 --verbose=::
diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
index 2bfc1b0db536..c5718503eded 100644
--- a/tools/perf/builtin-ftrace.c
+++ b/tools/perf/builtin-ftrace.c
@@ -27,7 +27,6 @@
 #include "util/cap.h"
 #include "util/config.h"
 
-#define DEFAULT_TRACER  "function_graph"
 
 struct perf_ftrace {
 	struct evlist		*evlist;
@@ -419,6 +418,7 @@ static int perf_ftrace_config(const char *var, const char *value, void *cb)
 	if (strcmp(var, "ftrace.tracer"))
 		return -1;
 
+	pr_warning("Configuration ftrace.tracer is deprecated\n");
 	if (!strcmp(value, "function_graph") ||
 	    !strcmp(value, "function")) {
 		ftrace->tracer = value;
@@ -459,7 +459,7 @@ int cmd_ftrace(int argc, const char **argv)
 {
 	int ret;
 	struct perf_ftrace ftrace = {
-		.tracer = DEFAULT_TRACER,
+		.tracer = "function",
 		.target = { .uid = UINT_MAX, },
 	};
 	const char * const ftrace_usage[] = {
@@ -469,7 +469,7 @@ int cmd_ftrace(int argc, const char **argv)
 	};
 	const struct option ftrace_options[] = {
 	OPT_STRING('t', "tracer", &ftrace.tracer, "tracer",
-		   "tracer to use: function_graph(default) or function"),
+		   "tracer to use: function or function_graph (This option is deprecated)"),
 	OPT_STRING('p', "pid", &ftrace.target.pid, "pid",
 		   "trace on existing process id"),
 	OPT_INCR('v', "verbose", &verbose,
@@ -478,12 +478,14 @@ int cmd_ftrace(int argc, const char **argv)
 		    "system-wide collection from all CPUs"),
 	OPT_STRING('C', "cpu", &ftrace.target.cpu_list, "cpu",
 		    "list of cpus to monitor"),
-	OPT_CALLBACK('T', "trace-funcs", &ftrace.filters, "func",
-		     "trace given functions only", parse_filter_func),
+	OPT_CALLBACK_DEFAULT('T', "trace-funcs", &ftrace.filters, "func",
+			     "trace given functions using function tracer",
+			     parse_filter_func, "*"),
 	OPT_CALLBACK('N', "notrace-funcs", &ftrace.notrace, "func",
 		     "do not trace given functions", parse_filter_func),
-	OPT_CALLBACK('G', "graph-funcs", &ftrace.graph_funcs, "func",
-		     "Set graph filter on given functions", parse_filter_func),
+	OPT_CALLBACK_DEFAULT('G', "graph-funcs", &ftrace.graph_funcs, "func",
+			     "trace given functions using function_graph tracer",
+			     parse_filter_func, "*"),
 	OPT_CALLBACK('g', "nograph-funcs", &ftrace.nograph_funcs, "func",
 		     "Set nograph filter on given functions", parse_filter_func),
 	OPT_INTEGER('D', "graph-depth", &ftrace.graph_depth,
@@ -505,6 +507,9 @@ int cmd_ftrace(int argc, const char **argv)
 	if (!argc && target__none(&ftrace.target))
 		ftrace.target.system_wide = true;
 
+	if (!list_empty(&ftrace.graph_funcs) || !list_empty(&ftrace.nograph_funcs))
+		ftrace.tracer = "function_graph";
+
 	ret = target__validate(&ftrace.target);
 	if (ret) {
 		char errbuf[512];
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v2 02/15] perf ftrace: add option '-F/--funcs' to list available functions
  2020-06-27 13:36 [PATCH v2 00/15] perf: ftrace enhancement Changbin Du
  2020-06-27 13:36 ` [PATCH v2 01/15] perf ftrace: select function/function_graph tracer automatically Changbin Du
@ 2020-06-27 13:36 ` Changbin Du
  2020-07-03  5:58   ` Namhyung Kim
  2020-06-27 13:36 ` [PATCH v2 03/15] perf ftrace: add option -t/--tid to filter by thread id Changbin Du
                   ` (12 subsequent siblings)
  14 siblings, 1 reply; 35+ messages in thread
From: Changbin Du @ 2020-06-27 13:36 UTC (permalink / raw)
  To: Jiri Olsa, Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Ingo Molnar, Namhyung Kim, Steven Rostedt,
	linux-kernel, Changbin Du

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 c5718503eded..e793118e83a9 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)
+{
+	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,
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v2 03/15] perf ftrace: add option -t/--tid to filter by thread id
  2020-06-27 13:36 [PATCH v2 00/15] perf: ftrace enhancement Changbin Du
  2020-06-27 13:36 ` [PATCH v2 01/15] perf ftrace: select function/function_graph tracer automatically Changbin Du
  2020-06-27 13:36 ` [PATCH v2 02/15] perf ftrace: add option '-F/--funcs' to list available functions Changbin Du
@ 2020-06-27 13:36 ` Changbin Du
  2020-06-27 13:36 ` [PATCH v2 04/15] perf ftrace: add option -d/--delay to delay tracing Changbin Du
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 35+ messages in thread
From: Changbin Du @ 2020-06-27 13:36 UTC (permalink / raw)
  To: Jiri Olsa, Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Ingo Molnar, Namhyung Kim, Steven Rostedt,
	linux-kernel, Changbin Du

This allows us to trace single thread instead of the whole process.

Signed-off-by: Changbin Du <changbin.du@gmail.com>
---
 tools/perf/Documentation/perf-ftrace.txt | 4 ++++
 tools/perf/builtin-ftrace.c              | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/tools/perf/Documentation/perf-ftrace.txt b/tools/perf/Documentation/perf-ftrace.txt
index d79560dea19f..e204bf6d50d8 100644
--- a/tools/perf/Documentation/perf-ftrace.txt
+++ b/tools/perf/Documentation/perf-ftrace.txt
@@ -38,6 +38,10 @@ OPTIONS
 --pid=::
 	Trace on existing process id (comma separated list).
 
+-t::
+--tid=::
+	Trace on existing thread id (comma separated list).
+
 -a::
 --all-cpus::
 	Force system-wide collection.  Scripts run without a <command>
diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
index e793118e83a9..b427ab3977ad 100644
--- a/tools/perf/builtin-ftrace.c
+++ b/tools/perf/builtin-ftrace.c
@@ -515,6 +515,8 @@ int cmd_ftrace(int argc, const char **argv)
 		    "Show available functions to filter"),
 	OPT_STRING('p', "pid", &ftrace.target.pid, "pid",
 		   "trace on existing process id"),
+	OPT_STRING('t', "tid", &ftrace.target.tid, "tid",
+		   "trace on existing thread id (exclusive to --pid)"),
 	OPT_INCR('v', "verbose", &verbose,
 		 "be more verbose"),
 	OPT_BOOLEAN('a', "all-cpus", &ftrace.target.system_wide,
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v2 04/15] perf ftrace: add option -d/--delay to delay tracing
  2020-06-27 13:36 [PATCH v2 00/15] perf: ftrace enhancement Changbin Du
                   ` (2 preceding siblings ...)
  2020-06-27 13:36 ` [PATCH v2 03/15] perf ftrace: add option -t/--tid to filter by thread id Changbin Du
@ 2020-06-27 13:36 ` Changbin Du
  2020-06-27 13:36 ` [PATCH v2 05/15] perf ftrace: factor out function write_tracing_file_int() Changbin Du
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 35+ messages in thread
From: Changbin Du @ 2020-06-27 13:36 UTC (permalink / raw)
  To: Jiri Olsa, Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Ingo Molnar, Namhyung Kim, Steven Rostedt,
	linux-kernel, Changbin Du

This adds an option '-d/--delay' to allow us to start tracing some
times later after workload is launched.

Signed-off-by: Changbin Du <changbin.du@gmail.com>
---
 tools/perf/Documentation/perf-ftrace.txt |  4 ++++
 tools/perf/builtin-ftrace.c              | 19 ++++++++++++++++---
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/tools/perf/Documentation/perf-ftrace.txt b/tools/perf/Documentation/perf-ftrace.txt
index e204bf6d50d8..fd1776deebd7 100644
--- a/tools/perf/Documentation/perf-ftrace.txt
+++ b/tools/perf/Documentation/perf-ftrace.txt
@@ -42,6 +42,10 @@ OPTIONS
 --tid=::
 	Trace on existing thread id (comma separated list).
 
+-d::
+--delay::
+	Time (ms) to wait before starting tracing after program start.
+
 -a::
 --all-cpus::
 	Force system-wide collection.  Scripts run without a <command>
diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
index b427ab3977ad..dceae70c3a22 100644
--- a/tools/perf/builtin-ftrace.c
+++ b/tools/perf/builtin-ftrace.c
@@ -38,6 +38,7 @@ struct perf_ftrace {
 	struct list_head	graph_funcs;
 	struct list_head	nograph_funcs;
 	int			graph_depth;
+	unsigned		initial_delay;
 };
 
 struct filter_entry {
@@ -402,13 +403,23 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
 	fcntl(trace_fd, F_SETFL, O_NONBLOCK);
 	pollfd.fd = trace_fd;
 
-	if (write_tracing_file("tracing_on", "1") < 0) {
-		pr_err("can't enable tracing\n");
-		goto out_close_fd;
+	if (!ftrace->initial_delay) {
+		if (write_tracing_file("tracing_on", "1") < 0) {
+			pr_err("can't enable tracing\n");
+			goto out_close_fd;
+		}
 	}
 
 	perf_evlist__start_workload(ftrace->evlist);
 
+	if (ftrace->initial_delay) {
+		usleep(ftrace->initial_delay * 1000);
+		if (write_tracing_file("tracing_on", "1") < 0) {
+			pr_err("can't enable tracing\n");
+			goto out_close_fd;
+		}
+	}
+
 	while (!done) {
 		if (poll(&pollfd, 1, -1) < 0)
 			break;
@@ -535,6 +546,8 @@ int cmd_ftrace(int argc, const char **argv)
 		     "Set nograph filter on given functions", parse_filter_func),
 	OPT_INTEGER('D', "graph-depth", &ftrace.graph_depth,
 		    "Max depth for function graph tracer"),
+	OPT_UINTEGER('d', "delay", &ftrace.initial_delay,
+		     "ms to wait before starting tracing after program start"),
 	OPT_END()
 	};
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v2 05/15] perf ftrace: factor out function write_tracing_file_int()
  2020-06-27 13:36 [PATCH v2 00/15] perf: ftrace enhancement Changbin Du
                   ` (3 preceding siblings ...)
  2020-06-27 13:36 ` [PATCH v2 04/15] perf ftrace: add option -d/--delay to delay tracing Changbin Du
@ 2020-06-27 13:36 ` Changbin Du
  2020-07-07 16:09   ` Steven Rostedt
  2020-07-07 16:24   ` Arnaldo Carvalho de Melo
  2020-06-27 13:36 ` [PATCH v2 06/15] perf ftrace: add option '-m/--buffer-size' to set per-cpu buffer size Changbin Du
                   ` (9 subsequent siblings)
  14 siblings, 2 replies; 35+ messages in thread
From: Changbin Du @ 2020-06-27 13:36 UTC (permalink / raw)
  To: Jiri Olsa, Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Ingo Molnar, Namhyung Kim, Steven Rostedt,
	linux-kernel, Changbin Du

We will reuse this function later.

Signed-off-by: Changbin Du <changbin.du@gmail.com>
---
 tools/perf/builtin-ftrace.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
index dceae70c3a22..003efa756322 100644
--- a/tools/perf/builtin-ftrace.c
+++ b/tools/perf/builtin-ftrace.c
@@ -166,6 +166,17 @@ static int read_tracing_file_to_stdout(const char *name)
 	return ret;
 }
 
+static int write_tracing_file_int(const char *name, int value)
+{
+	char buf[16];
+
+	snprintf(buf, sizeof(buf), "%d", value);
+	if (write_tracing_file(name, buf) < 0)
+		return -1;
+
+	return 0;
+}
+
 static int reset_tracing_cpu(void);
 static void reset_tracing_filters(void);
 
@@ -296,8 +307,6 @@ static void reset_tracing_filters(void)
 
 static int set_tracing_depth(struct perf_ftrace *ftrace)
 {
-	char buf[16];
-
 	if (ftrace->graph_depth == 0)
 		return 0;
 
@@ -306,9 +315,7 @@ static int set_tracing_depth(struct perf_ftrace *ftrace)
 		return -1;
 	}
 
-	snprintf(buf, sizeof(buf), "%d", ftrace->graph_depth);
-
-	if (write_tracing_file("max_graph_depth", buf) < 0)
+	if (write_tracing_file_int("max_graph_depth", ftrace->graph_depth) < 0)
 		return -1;
 
 	return 0;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v2 06/15] perf ftrace: add option '-m/--buffer-size' to set per-cpu buffer size
  2020-06-27 13:36 [PATCH v2 00/15] perf: ftrace enhancement Changbin Du
                   ` (4 preceding siblings ...)
  2020-06-27 13:36 ` [PATCH v2 05/15] perf ftrace: factor out function write_tracing_file_int() Changbin Du
@ 2020-06-27 13:36 ` Changbin Du
  2020-07-03  6:17   ` Namhyung Kim
  2020-06-27 13:36 ` [PATCH v2 07/15] perf ftrace: show trace column header Changbin Du
                   ` (8 subsequent siblings)
  14 siblings, 1 reply; 35+ messages in thread
From: Changbin Du @ 2020-06-27 13:36 UTC (permalink / raw)
  To: Jiri Olsa, Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Ingo Molnar, Namhyung Kim, Steven Rostedt,
	linux-kernel, Changbin Du

This adds an option '-m/--buffer-size' to allow us set the size of per-cpu
tracing buffer.

Signed-off-by: Changbin Du <changbin.du@gmail.com>
---
 tools/perf/Documentation/perf-ftrace.txt |  4 ++++
 tools/perf/builtin-ftrace.c              | 22 ++++++++++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/tools/perf/Documentation/perf-ftrace.txt b/tools/perf/Documentation/perf-ftrace.txt
index fd1776deebd7..a18d640e90bb 100644
--- a/tools/perf/Documentation/perf-ftrace.txt
+++ b/tools/perf/Documentation/perf-ftrace.txt
@@ -60,6 +60,10 @@ OPTIONS
 	Ranges of CPUs are specified with -: 0-2.
 	Default is to trace on all online CPUs.
 
+-m::
+--buffer-size::
+	Set the size of per-cpu tracing buffer in KB.
+
 -T::
 --trace-funcs=::
 	Only trace functions given by the argument.  Multiple functions
diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
index 003efa756322..e45496012611 100644
--- a/tools/perf/builtin-ftrace.c
+++ b/tools/perf/builtin-ftrace.c
@@ -39,6 +39,7 @@ struct perf_ftrace {
 	struct list_head	nograph_funcs;
 	int			graph_depth;
 	unsigned		initial_delay;
+	unsigned		buffer_size_kb;
 };
 
 struct filter_entry {
@@ -321,6 +322,20 @@ static int set_tracing_depth(struct perf_ftrace *ftrace)
 	return 0;
 }
 
+static int set_tracing_buffer_size_kb(struct perf_ftrace *ftrace)
+{
+	int ret;
+
+	if (ftrace->buffer_size_kb == 0)
+		return 0;
+
+	ret = write_tracing_file_int("buffer_size_kb", ftrace->buffer_size_kb);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
 static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
 {
 	char *trace_file;
@@ -385,6 +400,11 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
 		goto out_reset;
 	}
 
+	if (set_tracing_buffer_size_kb(ftrace) < 0) {
+		pr_err("failed to set tracing per-cpu buffer size\n");
+		goto out_reset;
+	}
+
 	if (write_tracing_file("current_tracer", ftrace->tracer) < 0) {
 		pr_err("failed to set current_tracer to %s\n", ftrace->tracer);
 		goto out_reset;
@@ -555,6 +575,8 @@ int cmd_ftrace(int argc, const char **argv)
 		    "Max depth for function graph tracer"),
 	OPT_UINTEGER('d', "delay", &ftrace.initial_delay,
 		     "ms to wait before starting tracing after program start"),
+	OPT_UINTEGER('m', "buffer-size", &ftrace.buffer_size_kb,
+		     "size of per cpu buffer in kb"),
 	OPT_END()
 	};
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v2 07/15] perf ftrace: show trace column header
  2020-06-27 13:36 [PATCH v2 00/15] perf: ftrace enhancement Changbin Du
                   ` (5 preceding siblings ...)
  2020-06-27 13:36 ` [PATCH v2 06/15] perf ftrace: add option '-m/--buffer-size' to set per-cpu buffer size Changbin Du
@ 2020-06-27 13:36 ` Changbin Du
  2020-07-03  6:20   ` Namhyung Kim
  2020-06-27 13:36 ` [PATCH v2 08/15] perf ftrace: add option '--inherit' to trace children processes Changbin Du
                   ` (7 subsequent siblings)
  14 siblings, 1 reply; 35+ messages in thread
From: Changbin Du @ 2020-06-27 13:36 UTC (permalink / raw)
  To: Jiri Olsa, Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Ingo Molnar, Namhyung Kim, Steven Rostedt,
	linux-kernel, Changbin Du

This makes perf-ftrace display column header before printing trace.

$ sudo perf ftrace
\# tracer: function
\#
\# entries-in-buffer/entries-written: 0/0   #P:8
\#
\#           TASK-PID     CPU#   TIMESTAMP  FUNCTION
\#              | |         |       |         |
           <...>-9246  [006]  10726.262760: mutex_unlock <-rb_simple_write
           <...>-9246  [006]  10726.262764: __fsnotify_parent <-vfs_write
           <...>-9246  [006]  10726.262765: fsnotify <-vfs_write
           <...>-9246  [006]  10726.262766: __sb_end_write <-vfs_write
           <...>-9246  [006]  10726.262767: fpregs_assert_state_consistent <-do_syscall_64

Signed-off-by: Changbin Du <changbin.du@gmail.com>
---
 tools/perf/builtin-ftrace.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
index e45496012611..686d744d5025 100644
--- a/tools/perf/builtin-ftrace.c
+++ b/tools/perf/builtin-ftrace.c
@@ -430,6 +430,9 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
 	fcntl(trace_fd, F_SETFL, O_NONBLOCK);
 	pollfd.fd = trace_fd;
 
+	/* display column headers */
+	read_tracing_file_to_stdout("trace");
+
 	if (!ftrace->initial_delay) {
 		if (write_tracing_file("tracing_on", "1") < 0) {
 			pr_err("can't enable tracing\n");
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v2 08/15] perf ftrace: add option '--inherit' to trace children processes
  2020-06-27 13:36 [PATCH v2 00/15] perf: ftrace enhancement Changbin Du
                   ` (6 preceding siblings ...)
  2020-06-27 13:36 ` [PATCH v2 07/15] perf ftrace: show trace column header Changbin Du
@ 2020-06-27 13:36 ` Changbin Du
  2020-06-27 13:36 ` [PATCH v2 09/15] perf ftrace: add support for tracing option 'func_stack_trace' Changbin Du
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 35+ messages in thread
From: Changbin Du @ 2020-06-27 13:36 UTC (permalink / raw)
  To: Jiri Olsa, Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Ingo Molnar, Namhyung Kim, Steven Rostedt,
	linux-kernel, Changbin Du

This adds an option '--inherit' to allow us trace children
processes spawned by our target.

Signed-off-by: Changbin Du <changbin.du@gmail.com>

---
v2: option name '--trace-children' -> '--inherit'.
---
 tools/perf/Documentation/perf-ftrace.txt |  3 ++
 tools/perf/builtin-ftrace.c              | 38 ++++++++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/tools/perf/Documentation/perf-ftrace.txt b/tools/perf/Documentation/perf-ftrace.txt
index a18d640e90bb..66876e79cc24 100644
--- a/tools/perf/Documentation/perf-ftrace.txt
+++ b/tools/perf/Documentation/perf-ftrace.txt
@@ -64,6 +64,9 @@ OPTIONS
 --buffer-size::
 	Set the size of per-cpu tracing buffer in KB.
 
+--inherit::
+	Trace children processes spawned by our target.
+
 -T::
 --trace-funcs=::
 	Only trace functions given by the argument.  Multiple functions
diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
index 686d744d5025..b2651b9bd6d0 100644
--- a/tools/perf/builtin-ftrace.c
+++ b/tools/perf/builtin-ftrace.c
@@ -40,6 +40,7 @@ struct perf_ftrace {
 	int			graph_depth;
 	unsigned		initial_delay;
 	unsigned		buffer_size_kb;
+	bool			inherit;
 };
 
 struct filter_entry {
@@ -178,9 +179,27 @@ static int write_tracing_file_int(const char *name, int value)
 	return 0;
 }
 
+static int write_tracing_option_file(const char *name, const char *val)
+{
+	char *file;
+	int ret;
+
+	if (asprintf(&file, "options/%s", name) < 0)
+		return -1;
+
+	ret = __write_tracing_file(file, val, false);
+	free(file);
+	return ret;
+}
+
 static int reset_tracing_cpu(void);
 static void reset_tracing_filters(void);
 
+static void reset_tracing_options(struct perf_ftrace *ftrace __maybe_unused)
+{
+	write_tracing_option_file("function-fork", "0");
+}
+
 static int reset_tracing_files(struct perf_ftrace *ftrace __maybe_unused)
 {
 	if (write_tracing_file("tracing_on", "0") < 0)
@@ -199,6 +218,7 @@ static int reset_tracing_files(struct perf_ftrace *ftrace __maybe_unused)
 		return -1;
 
 	reset_tracing_filters();
+	reset_tracing_options(ftrace);
 	return 0;
 }
 
@@ -336,6 +356,17 @@ static int set_tracing_buffer_size_kb(struct perf_ftrace *ftrace)
 	return 0;
 }
 
+static int set_tracing_trace_inherit(struct perf_ftrace *ftrace)
+{
+	if (!ftrace->inherit)
+		return 0;
+
+	if (write_tracing_option_file("function-fork", "1") < 0)
+		return -1;
+
+	return 0;
+}
+
 static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
 {
 	char *trace_file;
@@ -405,6 +436,11 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
 		goto out_reset;
 	}
 
+	if (set_tracing_trace_inherit(ftrace) < 0) {
+		pr_err("failed to set tracing option function-fork\n");
+		goto out_reset;
+	}
+
 	if (write_tracing_file("current_tracer", ftrace->tracer) < 0) {
 		pr_err("failed to set current_tracer to %s\n", ftrace->tracer);
 		goto out_reset;
@@ -580,6 +616,8 @@ int cmd_ftrace(int argc, const char **argv)
 		     "ms to wait before starting tracing after program start"),
 	OPT_UINTEGER('m', "buffer-size", &ftrace.buffer_size_kb,
 		     "size of per cpu buffer in kb"),
+	OPT_BOOLEAN(0, "inherit", &ftrace.inherit,
+		    "trace children processes"),
 	OPT_END()
 	};
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v2 09/15] perf ftrace: add support for tracing option 'func_stack_trace'
  2020-06-27 13:36 [PATCH v2 00/15] perf: ftrace enhancement Changbin Du
                   ` (7 preceding siblings ...)
  2020-06-27 13:36 ` [PATCH v2 08/15] perf ftrace: add option '--inherit' to trace children processes Changbin Du
@ 2020-06-27 13:36 ` Changbin Du
  2020-07-03  6:30   ` Namhyung Kim
  2020-06-27 13:36 ` [PATCH v2 10/15] perf ftrace: add support for trace option sleep-time Changbin Du
                   ` (5 subsequent siblings)
  14 siblings, 1 reply; 35+ messages in thread
From: Changbin Du @ 2020-06-27 13:36 UTC (permalink / raw)
  To: Jiri Olsa, Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Ingo Molnar, Namhyung Kim, Steven Rostedt,
	linux-kernel, Changbin Du

This adds support to display call trace for function tracer. To do this,
just specify a '--func-call-graph' option.

$ sudo perf ftrace -T vfs_read --func-call-graph
 iio-sensor-prox-855   [003]   6168.369657: vfs_read <-ksys_read
 iio-sensor-prox-855   [003]   6168.369677: <stack trace>
 => vfs_read
 => ksys_read
 => __x64_sys_read
 => do_syscall_64
 => entry_SYSCALL_64_after_hwframe
 ...

Signed-off-by: Changbin Du <changbin.du@gmail.com>

---
v2: option name '-s' -> '--func-call-graph'
---
 tools/perf/Documentation/perf-ftrace.txt |  3 +++
 tools/perf/builtin-ftrace.c              | 20 ++++++++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/tools/perf/Documentation/perf-ftrace.txt b/tools/perf/Documentation/perf-ftrace.txt
index 66876e79cc24..94cdb2ebdf34 100644
--- a/tools/perf/Documentation/perf-ftrace.txt
+++ b/tools/perf/Documentation/perf-ftrace.txt
@@ -81,6 +81,9 @@ OPTIONS
 	(or glob patterns).  It will be passed to 'set_ftrace_notrace'
 	in tracefs.
 
+--func-call-graph::
+	Display kernel stack trace for function tracer.
+
 -G::
 --graph-funcs=::
 	Set graph filter on the given function (or a glob pattern).
diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
index b2651b9bd6d0..bbfa7973b4fa 100644
--- a/tools/perf/builtin-ftrace.c
+++ b/tools/perf/builtin-ftrace.c
@@ -41,6 +41,7 @@ struct perf_ftrace {
 	unsigned		initial_delay;
 	unsigned		buffer_size_kb;
 	bool			inherit;
+	bool			func_stack_trace;
 };
 
 struct filter_entry {
@@ -198,6 +199,7 @@ static void reset_tracing_filters(void);
 static void reset_tracing_options(struct perf_ftrace *ftrace __maybe_unused)
 {
 	write_tracing_option_file("function-fork", "0");
+	write_tracing_option_file("func_stack_trace", "0");
 }
 
 static int reset_tracing_files(struct perf_ftrace *ftrace __maybe_unused)
@@ -274,6 +276,17 @@ static int set_tracing_cpu(struct perf_ftrace *ftrace)
 	return set_tracing_cpumask(cpumap);
 }
 
+static int set_tracing_func_stack_trace(struct perf_ftrace *ftrace)
+{
+	if (!ftrace->func_stack_trace)
+		return 0;
+
+	if (write_tracing_option_file("func_stack_trace", "1") < 0)
+		return -1;
+
+	return 0;
+}
+
 static int reset_tracing_cpu(void)
 {
 	struct perf_cpu_map *cpumap = perf_cpu_map__new(NULL);
@@ -421,6 +434,11 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
 		goto out_reset;
 	}
 
+	if (set_tracing_func_stack_trace(ftrace) < 0) {
+		pr_err("failed to set tracing option func_stack_trace\n");
+		goto out_reset;
+	}
+
 	if (set_tracing_filters(ftrace) < 0) {
 		pr_err("failed to set tracing filters\n");
 		goto out_reset;
@@ -605,6 +623,8 @@ int cmd_ftrace(int argc, const char **argv)
 			     parse_filter_func, "*"),
 	OPT_CALLBACK('N', "notrace-funcs", &ftrace.notrace, "func",
 		     "do not trace given functions", parse_filter_func),
+	OPT_BOOLEAN(0, "func-call-graph", &ftrace.func_stack_trace,
+		    "show kernel stack trace for function tracer"),
 	OPT_CALLBACK_DEFAULT('G', "graph-funcs", &ftrace.graph_funcs, "func",
 			     "trace given functions using function_graph tracer",
 			     parse_filter_func, "*"),
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v2 10/15] perf ftrace: add support for trace option sleep-time
  2020-06-27 13:36 [PATCH v2 00/15] perf: ftrace enhancement Changbin Du
                   ` (8 preceding siblings ...)
  2020-06-27 13:36 ` [PATCH v2 09/15] perf ftrace: add support for tracing option 'func_stack_trace' Changbin Du
@ 2020-06-27 13:36 ` Changbin Du
  2020-07-03  6:40   ` Namhyung Kim
  2020-06-27 13:36 ` [PATCH v2 11/15] perf ftrace: add support for trace option funcgraph-irqs Changbin Du
                   ` (4 subsequent siblings)
  14 siblings, 1 reply; 35+ messages in thread
From: Changbin Du @ 2020-06-27 13:36 UTC (permalink / raw)
  To: Jiri Olsa, Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Ingo Molnar, Namhyung Kim, Steven Rostedt,
	linux-kernel, Changbin Du

This adds an option '--graph-nosleep-time' which allow us only to measure
on-CPU time. This option is function_graph tracer only.

Signed-off-by: Changbin Du <changbin.du@gmail.com>

---
v2: option name '--nosleep-time' -> '--graph-nosleep-time'.
---
 tools/perf/Documentation/perf-ftrace.txt |  3 +++
 tools/perf/builtin-ftrace.c              | 20 ++++++++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/tools/perf/Documentation/perf-ftrace.txt b/tools/perf/Documentation/perf-ftrace.txt
index 94cdb2ebdf34..a3000436f80b 100644
--- a/tools/perf/Documentation/perf-ftrace.txt
+++ b/tools/perf/Documentation/perf-ftrace.txt
@@ -104,6 +104,9 @@ OPTIONS
 --graph-depth=::
 	Set max depth for function graph tracer to follow
 
+--graph-nosleep-time::
+	Measure on-CPU time only for function_graph tracer.
+
 SEE ALSO
 --------
 linkperf:perf-record[1], linkperf:perf-trace[1]
diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
index bbfa7973b4fa..eba125a60820 100644
--- a/tools/perf/builtin-ftrace.c
+++ b/tools/perf/builtin-ftrace.c
@@ -42,6 +42,7 @@ struct perf_ftrace {
 	unsigned		buffer_size_kb;
 	bool			inherit;
 	bool			func_stack_trace;
+	bool			graph_nosleep_time;
 };
 
 struct filter_entry {
@@ -200,6 +201,7 @@ static void reset_tracing_options(struct perf_ftrace *ftrace __maybe_unused)
 {
 	write_tracing_option_file("function-fork", "0");
 	write_tracing_option_file("func_stack_trace", "0");
+	write_tracing_option_file("sleep-time", "1");
 }
 
 static int reset_tracing_files(struct perf_ftrace *ftrace __maybe_unused)
@@ -380,6 +382,17 @@ static int set_tracing_trace_inherit(struct perf_ftrace *ftrace)
 	return 0;
 }
 
+static int set_tracing_sleep_time(struct perf_ftrace *ftrace)
+{
+	if (!ftrace->graph_nosleep_time)
+		return 0;
+
+	if (write_tracing_option_file("sleep-time", "0") < 0)
+		return -1;
+
+	return 0;
+}
+
 static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
 {
 	char *trace_file;
@@ -459,6 +472,11 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
 		goto out_reset;
 	}
 
+	if (set_tracing_sleep_time(ftrace) < 0) {
+		pr_err("failed to set tracing option sleep-time\n");
+		goto out_reset;
+	}
+
 	if (write_tracing_file("current_tracer", ftrace->tracer) < 0) {
 		pr_err("failed to set current_tracer to %s\n", ftrace->tracer);
 		goto out_reset;
@@ -638,6 +656,8 @@ int cmd_ftrace(int argc, const char **argv)
 		     "size of per cpu buffer in kb"),
 	OPT_BOOLEAN(0, "inherit", &ftrace.inherit,
 		    "trace children processes"),
+	OPT_BOOLEAN(0, "graph-nosleep-time", &ftrace.graph_nosleep_time,
+		    "measure on-CPU time only for function_graph tracer"),
 	OPT_END()
 	};
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v2 11/15] perf ftrace: add support for trace option funcgraph-irqs
  2020-06-27 13:36 [PATCH v2 00/15] perf: ftrace enhancement Changbin Du
                   ` (9 preceding siblings ...)
  2020-06-27 13:36 ` [PATCH v2 10/15] perf ftrace: add support for trace option sleep-time Changbin Du
@ 2020-06-27 13:36 ` Changbin Du
  2020-07-03  6:43   ` Namhyung Kim
  2020-06-27 13:36 ` [PATCH v2 12/15] perf ftrace: add support for tracing option 'irq-info' Changbin Du
                   ` (3 subsequent siblings)
  14 siblings, 1 reply; 35+ messages in thread
From: Changbin Du @ 2020-06-27 13:36 UTC (permalink / raw)
  To: Jiri Olsa, Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Ingo Molnar, Namhyung Kim, Steven Rostedt,
	linux-kernel, Changbin Du

This adds an option '--graph-noirqs' to filter out functions executed
in irq context.

Signed-off-by: Changbin Du <changbin.du@gmail.com>

---
v2: option name '--nofuncgraph-irqs' -> '--graph-noirqs'.
---
 tools/perf/Documentation/perf-ftrace.txt |  3 +++
 tools/perf/builtin-ftrace.c              | 20 ++++++++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/tools/perf/Documentation/perf-ftrace.txt b/tools/perf/Documentation/perf-ftrace.txt
index a3000436f80b..b616e05d5156 100644
--- a/tools/perf/Documentation/perf-ftrace.txt
+++ b/tools/perf/Documentation/perf-ftrace.txt
@@ -107,6 +107,9 @@ OPTIONS
 --graph-nosleep-time::
 	Measure on-CPU time only for function_graph tracer.
 
+--graph-noirqs::
+	Ignore functions that happen inside interrupt for function_graph tracer.
+
 SEE ALSO
 --------
 linkperf:perf-record[1], linkperf:perf-trace[1]
diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
index eba125a60820..876c8e800425 100644
--- a/tools/perf/builtin-ftrace.c
+++ b/tools/perf/builtin-ftrace.c
@@ -43,6 +43,7 @@ struct perf_ftrace {
 	bool			inherit;
 	bool			func_stack_trace;
 	bool			graph_nosleep_time;
+	bool			graph_noirqs;
 };
 
 struct filter_entry {
@@ -202,6 +203,7 @@ static void reset_tracing_options(struct perf_ftrace *ftrace __maybe_unused)
 	write_tracing_option_file("function-fork", "0");
 	write_tracing_option_file("func_stack_trace", "0");
 	write_tracing_option_file("sleep-time", "1");
+	write_tracing_option_file("funcgraph-irqs", "1");
 }
 
 static int reset_tracing_files(struct perf_ftrace *ftrace __maybe_unused)
@@ -393,6 +395,17 @@ static int set_tracing_sleep_time(struct perf_ftrace *ftrace)
 	return 0;
 }
 
+static int set_tracing_funcgraph_irqs(struct perf_ftrace *ftrace)
+{
+	if (!ftrace->graph_noirqs)
+		return 0;
+
+	if (write_tracing_option_file("funcgraph-irqs", "0") < 0)
+		return -1;
+
+	return 0;
+}
+
 static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
 {
 	char *trace_file;
@@ -477,6 +490,11 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
 		goto out_reset;
 	}
 
+	if (set_tracing_funcgraph_irqs(ftrace) < 0) {
+		pr_err("failed to set tracing option funcgraph-irqs\n");
+		goto out_reset;
+	}
+
 	if (write_tracing_file("current_tracer", ftrace->tracer) < 0) {
 		pr_err("failed to set current_tracer to %s\n", ftrace->tracer);
 		goto out_reset;
@@ -658,6 +676,8 @@ int cmd_ftrace(int argc, const char **argv)
 		    "trace children processes"),
 	OPT_BOOLEAN(0, "graph-nosleep-time", &ftrace.graph_nosleep_time,
 		    "measure on-CPU time only for function_graph tracer"),
+	OPT_BOOLEAN(0, "graph-noirqs", &ftrace.graph_noirqs,
+		    "ignore functions that happen inside interrupt for function_graph tracer"),
 	OPT_END()
 	};
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v2 12/15] perf ftrace: add support for tracing option 'irq-info'
  2020-06-27 13:36 [PATCH v2 00/15] perf: ftrace enhancement Changbin Du
                   ` (10 preceding siblings ...)
  2020-06-27 13:36 ` [PATCH v2 11/15] perf ftrace: add support for trace option funcgraph-irqs Changbin Du
@ 2020-06-27 13:36 ` Changbin Du
  2020-06-27 13:36 ` [PATCH v2 13/15] perf ftrace: add option '--graph-verbose' to show more info for graph tracer Changbin Du
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 35+ messages in thread
From: Changbin Du @ 2020-06-27 13:36 UTC (permalink / raw)
  To: Jiri Olsa, Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Ingo Molnar, Namhyung Kim, Steven Rostedt,
	linux-kernel, Changbin Du

This adds support to display irq context info for function tracer. To do
this, just specify a '--func-irq-info' option.

Signed-off-by: Changbin Du <changbin.du@gmail.com>
---
 tools/perf/Documentation/perf-ftrace.txt |  3 +++
 tools/perf/builtin-ftrace.c              | 20 ++++++++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/tools/perf/Documentation/perf-ftrace.txt b/tools/perf/Documentation/perf-ftrace.txt
index b616e05d5156..3c9eb044b7eb 100644
--- a/tools/perf/Documentation/perf-ftrace.txt
+++ b/tools/perf/Documentation/perf-ftrace.txt
@@ -84,6 +84,9 @@ OPTIONS
 --func-call-graph::
 	Display kernel stack trace for function tracer.
 
+--func-irq-info::
+	Display irq context info for function tracer.
+
 -G::
 --graph-funcs=::
 	Set graph filter on the given function (or a glob pattern).
diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
index 876c8e800425..0a1e481b66a0 100644
--- a/tools/perf/builtin-ftrace.c
+++ b/tools/perf/builtin-ftrace.c
@@ -42,6 +42,7 @@ struct perf_ftrace {
 	unsigned		buffer_size_kb;
 	bool			inherit;
 	bool			func_stack_trace;
+	bool			func_irq_info;
 	bool			graph_nosleep_time;
 	bool			graph_noirqs;
 };
@@ -204,6 +205,7 @@ static void reset_tracing_options(struct perf_ftrace *ftrace __maybe_unused)
 	write_tracing_option_file("func_stack_trace", "0");
 	write_tracing_option_file("sleep-time", "1");
 	write_tracing_option_file("funcgraph-irqs", "1");
+	write_tracing_option_file("irq-info", "0");
 }
 
 static int reset_tracing_files(struct perf_ftrace *ftrace __maybe_unused)
@@ -291,6 +293,17 @@ static int set_tracing_func_stack_trace(struct perf_ftrace *ftrace)
 	return 0;
 }
 
+static int set_tracing_func_irqinfo(struct perf_ftrace *ftrace)
+{
+	if (!ftrace->func_irq_info)
+		return 0;
+
+	if (write_tracing_option_file("irq-info", "1") < 0)
+		return -1;
+
+	return 0;
+}
+
 static int reset_tracing_cpu(void)
 {
 	struct perf_cpu_map *cpumap = perf_cpu_map__new(NULL);
@@ -465,6 +478,11 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
 		goto out_reset;
 	}
 
+	if (set_tracing_func_irqinfo(ftrace) < 0) {
+		pr_err("failed to set tracing option irq-info\n");
+		goto out_reset;
+	}
+
 	if (set_tracing_filters(ftrace) < 0) {
 		pr_err("failed to set tracing filters\n");
 		goto out_reset;
@@ -661,6 +679,8 @@ int cmd_ftrace(int argc, const char **argv)
 		     "do not trace given functions", parse_filter_func),
 	OPT_BOOLEAN(0, "func-call-graph", &ftrace.func_stack_trace,
 		    "show kernel stack trace for function tracer"),
+	OPT_BOOLEAN(0, "func-irq-info", &ftrace.func_irq_info,
+		    "display irq info for function tracer"),
 	OPT_CALLBACK_DEFAULT('G', "graph-funcs", &ftrace.graph_funcs, "func",
 			     "trace given functions using function_graph tracer",
 			     parse_filter_func, "*"),
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v2 13/15] perf ftrace: add option '--graph-verbose' to show more info for graph tracer
  2020-06-27 13:36 [PATCH v2 00/15] perf: ftrace enhancement Changbin Du
                   ` (11 preceding siblings ...)
  2020-06-27 13:36 ` [PATCH v2 12/15] perf ftrace: add support for tracing option 'irq-info' Changbin Du
@ 2020-06-27 13:36 ` Changbin Du
  2020-06-27 13:36 ` [PATCH v2 14/15] perf ftrace: add support for trace option tracing_thresh Changbin Du
  2020-06-27 13:36 ` [PATCH v2 15/15] perf ftrace: add change log Changbin Du
  14 siblings, 0 replies; 35+ messages in thread
From: Changbin Du @ 2020-06-27 13:36 UTC (permalink / raw)
  To: Jiri Olsa, Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Ingo Molnar, Namhyung Kim, Steven Rostedt,
	linux-kernel, Changbin Du

Sometimes we want ftrace display more and longer information about the trace.

$ sudo perf ftrace -G
 2)   0.979 us    |  mutex_unlock();
 2)   1.540 us    |  __fsnotify_parent();
 2)   0.433 us    |  fsnotify();

$ sudo perf ftrace -G --graph-verbose
14160.770883 |   0)  <...>-47814   |  .... |   1.289 us    |  mutex_unlock();
14160.770886 |   0)  <...>-47814   |  .... |   1.624 us    |  __fsnotify_parent();
14160.770887 |   0)  <...>-47814   |  .... |   0.636 us    |  fsnotify();
14160.770888 |   0)  <...>-47814   |  .... |   0.328 us    |  __sb_end_write();
14160.770888 |   0)  <...>-47814   |  d... |   0.430 us    |  fpregs_assert_state_consistent();
14160.770889 |   0)  <...>-47814   |  d... |               |  do_syscall_64() {
14160.770889 |   0)  <...>-47814   |  .... |               |    __x64_sys_close() {

Signed-off-by: Changbin Du <changbin.du@gmail.com>
---
 tools/perf/Documentation/perf-ftrace.txt |  3 +++
 tools/perf/builtin-ftrace.c              | 28 ++++++++++++++++++++++++
 2 files changed, 31 insertions(+)

diff --git a/tools/perf/Documentation/perf-ftrace.txt b/tools/perf/Documentation/perf-ftrace.txt
index 3c9eb044b7eb..cc770fc0a1e8 100644
--- a/tools/perf/Documentation/perf-ftrace.txt
+++ b/tools/perf/Documentation/perf-ftrace.txt
@@ -113,6 +113,9 @@ OPTIONS
 --graph-noirqs::
 	Ignore functions that happen inside interrupt for function_graph tracer.
 
+--graph-verbose::
+	Show process names, PIDs, timestamps for function_graph tracer.
+
 SEE ALSO
 --------
 linkperf:perf-record[1], linkperf:perf-trace[1]
diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
index 0a1e481b66a0..e037fe7abd47 100644
--- a/tools/perf/builtin-ftrace.c
+++ b/tools/perf/builtin-ftrace.c
@@ -45,6 +45,7 @@ struct perf_ftrace {
 	bool			func_irq_info;
 	bool			graph_nosleep_time;
 	bool			graph_noirqs;
+	bool			graph_verbose;
 };
 
 struct filter_entry {
@@ -205,6 +206,9 @@ static void reset_tracing_options(struct perf_ftrace *ftrace __maybe_unused)
 	write_tracing_option_file("func_stack_trace", "0");
 	write_tracing_option_file("sleep-time", "1");
 	write_tracing_option_file("funcgraph-irqs", "1");
+	write_tracing_option_file("funcgraph-proc", "0");
+	write_tracing_option_file("funcgraph-abstime", "0");
+	write_tracing_option_file("latency-format", "0");
 	write_tracing_option_file("irq-info", "0");
 }
 
@@ -419,6 +423,23 @@ static int set_tracing_funcgraph_irqs(struct perf_ftrace *ftrace)
 	return 0;
 }
 
+static int set_tracing_funcgraph_verbose(struct perf_ftrace *ftrace)
+{
+	if (!ftrace->graph_verbose)
+		return 0;
+
+	if (write_tracing_option_file("funcgraph-proc", "1") < 0)
+		return -1;
+
+	if (write_tracing_option_file("funcgraph-abstime", "1") < 0)
+		return -1;
+
+	if (write_tracing_option_file("latency-format", "1") < 0)
+		return -1;
+
+	return 0;
+}
+
 static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
 {
 	char *trace_file;
@@ -513,6 +534,11 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
 		goto out_reset;
 	}
 
+	if (set_tracing_funcgraph_verbose(ftrace) < 0) {
+		pr_err("failed to set tracing option funcgraph-proc/funcgraph-abstime\n");
+		goto out_reset;
+	}
+
 	if (write_tracing_file("current_tracer", ftrace->tracer) < 0) {
 		pr_err("failed to set current_tracer to %s\n", ftrace->tracer);
 		goto out_reset;
@@ -698,6 +724,8 @@ int cmd_ftrace(int argc, const char **argv)
 		    "measure on-CPU time only for function_graph tracer"),
 	OPT_BOOLEAN(0, "graph-noirqs", &ftrace.graph_noirqs,
 		    "ignore functions that happen inside interrupt for function_graph tracer"),
+	OPT_BOOLEAN(0, "graph-verbose", &ftrace.graph_verbose,
+		    "show process names, PIDs, timestamps for function_graph tracer"),
 	OPT_END()
 	};
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v2 14/15] perf ftrace: add support for trace option tracing_thresh
  2020-06-27 13:36 [PATCH v2 00/15] perf: ftrace enhancement Changbin Du
                   ` (12 preceding siblings ...)
  2020-06-27 13:36 ` [PATCH v2 13/15] perf ftrace: add option '--graph-verbose' to show more info for graph tracer Changbin Du
@ 2020-06-27 13:36 ` Changbin Du
  2020-06-27 13:36 ` [PATCH v2 15/15] perf ftrace: add change log Changbin Du
  14 siblings, 0 replies; 35+ messages in thread
From: Changbin Du @ 2020-06-27 13:36 UTC (permalink / raw)
  To: Jiri Olsa, Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Ingo Molnar, Namhyung Kim, Steven Rostedt,
	linux-kernel, Changbin Du

This adds an option '--graph-thresh' to setup trace duration threshold
for funcgraph tracer.

$ sudo ./perf ftrace -G --graph-thresh 100
 3) ! 184.060 us  |    } /* schedule */
 3) ! 185.600 us  |  } /* exit_to_usermode_loop */
 2) ! 225.989 us  |    } /* schedule_idle */
 2) # 4140.051 us |  } /* do_idle */

Signed-off-by: Changbin Du <changbin.du@gmail.com>
---
 tools/perf/Documentation/perf-ftrace.txt |  3 +++
 tools/perf/builtin-ftrace.c              | 25 ++++++++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/tools/perf/Documentation/perf-ftrace.txt b/tools/perf/Documentation/perf-ftrace.txt
index cc770fc0a1e8..64843d0cdc45 100644
--- a/tools/perf/Documentation/perf-ftrace.txt
+++ b/tools/perf/Documentation/perf-ftrace.txt
@@ -116,6 +116,9 @@ OPTIONS
 --graph-verbose::
 	Show process names, PIDs, timestamps for function_graph tracer.
 
+--graph-thresh::
+	Setup trace duration threshold for funcgraph tracer.
+
 SEE ALSO
 --------
 linkperf:perf-record[1], linkperf:perf-trace[1]
diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
index e037fe7abd47..0f8716ea0da4 100644
--- a/tools/perf/builtin-ftrace.c
+++ b/tools/perf/builtin-ftrace.c
@@ -46,6 +46,7 @@ struct perf_ftrace {
 	bool			graph_nosleep_time;
 	bool			graph_noirqs;
 	bool			graph_verbose;
+	unsigned		graph_thresh;
 };
 
 struct filter_entry {
@@ -229,6 +230,9 @@ static int reset_tracing_files(struct perf_ftrace *ftrace __maybe_unused)
 	if (write_tracing_file("max_graph_depth", "0") < 0)
 		return -1;
 
+	if (write_tracing_file("tracing_thresh", "0") < 0)
+		return -1;
+
 	reset_tracing_filters();
 	reset_tracing_options(ftrace);
 	return 0;
@@ -440,6 +444,20 @@ static int set_tracing_funcgraph_verbose(struct perf_ftrace *ftrace)
 	return 0;
 }
 
+static int set_tracing_thresh(struct perf_ftrace *ftrace)
+{
+	int ret;
+
+	if (ftrace->graph_thresh == 0)
+		return 0;
+
+	ret = write_tracing_file_int("tracing_thresh", ftrace->graph_thresh);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
 static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
 {
 	char *trace_file;
@@ -539,6 +557,11 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
 		goto out_reset;
 	}
 
+	if (set_tracing_thresh(ftrace) < 0) {
+		pr_err("failed to set tracing thresh\n");
+		goto out_reset;
+	}
+
 	if (write_tracing_file("current_tracer", ftrace->tracer) < 0) {
 		pr_err("failed to set current_tracer to %s\n", ftrace->tracer);
 		goto out_reset;
@@ -726,6 +749,8 @@ int cmd_ftrace(int argc, const char **argv)
 		    "ignore functions that happen inside interrupt for function_graph tracer"),
 	OPT_BOOLEAN(0, "graph-verbose", &ftrace.graph_verbose,
 		    "show process names, PIDs, timestamps for function_graph tracer"),
+	OPT_UINTEGER(0, "graph-thresh", &ftrace.graph_thresh,
+		     "only show functions of which the duration is greater than <n>µs"),
 	OPT_END()
 	};
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v2 15/15] perf ftrace: add change log
  2020-06-27 13:36 [PATCH v2 00/15] perf: ftrace enhancement Changbin Du
                   ` (13 preceding siblings ...)
  2020-06-27 13:36 ` [PATCH v2 14/15] perf ftrace: add support for trace option tracing_thresh Changbin Du
@ 2020-06-27 13:36 ` Changbin Du
  14 siblings, 0 replies; 35+ messages in thread
From: Changbin Du @ 2020-06-27 13:36 UTC (permalink / raw)
  To: Jiri Olsa, Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Ingo Molnar, Namhyung Kim, Steven Rostedt,
	linux-kernel, Changbin Du

Add a change log after previous enhancements.

Signed-off-by: Changbin Du <changbin.du@gmail.com>
---
 tools/perf/builtin-ftrace.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
index 0f8716ea0da4..3d73809150c8 100644
--- a/tools/perf/builtin-ftrace.c
+++ b/tools/perf/builtin-ftrace.c
@@ -3,6 +3,7 @@
  * builtin-ftrace.c
  *
  * Copyright (c) 2013  LG Electronics,  Namhyung Kim <namhyung@kernel.org>
+ * Copyright (c) 2020  Changbin Du <changbin.du@gmail.com>, significant enhancement.
  */
 
 #include "builtin.h"
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 01/15] perf ftrace: select function/function_graph tracer automatically
  2020-06-27 13:36 ` [PATCH v2 01/15] perf ftrace: select function/function_graph tracer automatically Changbin Du
@ 2020-07-03  5:47   ` Namhyung Kim
  2020-07-07 15:00     ` Changbin Du
  0 siblings, 1 reply; 35+ messages in thread
From: Namhyung Kim @ 2020-07-03  5:47 UTC (permalink / raw)
  To: Changbin Du
  Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
	Steven Rostedt, linux-kernel

Hello,

On Sat, Jun 27, 2020 at 10:37 PM Changbin Du <changbin.du@gmail.com> wrote:
>
> The '-g/-G' options have already implied function_graph tracer should be
> used instead of function tracer. So the extra option '--tracer' can be
> killed.
>
> This patch changes the behavior as below:
>   - By default, function tracer is used.
>   - If '-g' or '-G' option is on, then function_graph tracer is used.
>   - The perf configuration item 'ftrace.tracer' is marked as deprecated.
>   - The option '--tracer' is marked as deprecated.
>   - The default filter for -G/-T is to trace all functions.
>
> Here are some examples.
>
> This will start tracing all functions using function tracer:
>   $ sudo perf ftrace
>
> This will trace all functions using function graph tracer:
>   $ sudo perf ftrace -G
>
> This will trace function vfs_read using function graph tracer:
>   $ sudo perf ftrace -G vfs_read

As we support running a new task on the command line, it might
confuse users whether it's an argument of -G option or a task to run.
One can use -- to separate them but it's easy to miss..

Thanks
Namhyung


>
> Signed-off-by: Changbin Du <changbin.du@gmail.com>
> ---
>  tools/perf/Documentation/perf-config.txt |  5 -----
>  tools/perf/Documentation/perf-ftrace.txt |  2 +-
>  tools/perf/builtin-ftrace.c              | 19 ++++++++++++-------
>  3 files changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/tools/perf/Documentation/perf-config.txt b/tools/perf/Documentation/perf-config.txt
> index c7d3df5798e2..a25fee7de3b2 100644
> --- a/tools/perf/Documentation/perf-config.txt
> +++ b/tools/perf/Documentation/perf-config.txt
> @@ -612,11 +612,6 @@ trace.*::
>                 "libbeauty", the default, to use the same argument beautifiers used in the
>                 strace-like sys_enter+sys_exit lines.
>
> -ftrace.*::
> -       ftrace.tracer::
> -               Can be used to select the default tracer. Possible values are
> -               'function' and 'function_graph'.
> -
>  llvm.*::
>         llvm.clang-path::
>                 Path to clang. If omit, search it from $PATH.
> diff --git a/tools/perf/Documentation/perf-ftrace.txt b/tools/perf/Documentation/perf-ftrace.txt
> index b80c84307dc9..952e46669168 100644
> --- a/tools/perf/Documentation/perf-ftrace.txt
> +++ b/tools/perf/Documentation/perf-ftrace.txt
> @@ -24,7 +24,7 @@ OPTIONS
>
>  -t::
>  --tracer=::
> -       Tracer to use: function_graph or function.
> +       Tracer to use: function_graph or function. This option is deprecated.
>
>  -v::
>  --verbose=::
> diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
> index 2bfc1b0db536..c5718503eded 100644
> --- a/tools/perf/builtin-ftrace.c
> +++ b/tools/perf/builtin-ftrace.c
> @@ -27,7 +27,6 @@
>  #include "util/cap.h"
>  #include "util/config.h"
>
> -#define DEFAULT_TRACER  "function_graph"
>
>  struct perf_ftrace {
>         struct evlist           *evlist;
> @@ -419,6 +418,7 @@ static int perf_ftrace_config(const char *var, const char *value, void *cb)
>         if (strcmp(var, "ftrace.tracer"))
>                 return -1;
>
> +       pr_warning("Configuration ftrace.tracer is deprecated\n");
>         if (!strcmp(value, "function_graph") ||
>             !strcmp(value, "function")) {
>                 ftrace->tracer = value;
> @@ -459,7 +459,7 @@ int cmd_ftrace(int argc, const char **argv)
>  {
>         int ret;
>         struct perf_ftrace ftrace = {
> -               .tracer = DEFAULT_TRACER,
> +               .tracer = "function",
>                 .target = { .uid = UINT_MAX, },
>         };
>         const char * const ftrace_usage[] = {
> @@ -469,7 +469,7 @@ int cmd_ftrace(int argc, const char **argv)
>         };
>         const struct option ftrace_options[] = {
>         OPT_STRING('t', "tracer", &ftrace.tracer, "tracer",
> -                  "tracer to use: function_graph(default) or function"),
> +                  "tracer to use: function or function_graph (This option is deprecated)"),
>         OPT_STRING('p', "pid", &ftrace.target.pid, "pid",
>                    "trace on existing process id"),
>         OPT_INCR('v', "verbose", &verbose,
> @@ -478,12 +478,14 @@ int cmd_ftrace(int argc, const char **argv)
>                     "system-wide collection from all CPUs"),
>         OPT_STRING('C', "cpu", &ftrace.target.cpu_list, "cpu",
>                     "list of cpus to monitor"),
> -       OPT_CALLBACK('T', "trace-funcs", &ftrace.filters, "func",
> -                    "trace given functions only", parse_filter_func),
> +       OPT_CALLBACK_DEFAULT('T', "trace-funcs", &ftrace.filters, "func",
> +                            "trace given functions using function tracer",
> +                            parse_filter_func, "*"),
>         OPT_CALLBACK('N', "notrace-funcs", &ftrace.notrace, "func",
>                      "do not trace given functions", parse_filter_func),
> -       OPT_CALLBACK('G', "graph-funcs", &ftrace.graph_funcs, "func",
> -                    "Set graph filter on given functions", parse_filter_func),
> +       OPT_CALLBACK_DEFAULT('G', "graph-funcs", &ftrace.graph_funcs, "func",
> +                            "trace given functions using function_graph tracer",
> +                            parse_filter_func, "*"),
>         OPT_CALLBACK('g', "nograph-funcs", &ftrace.nograph_funcs, "func",
>                      "Set nograph filter on given functions", parse_filter_func),
>         OPT_INTEGER('D', "graph-depth", &ftrace.graph_depth,
> @@ -505,6 +507,9 @@ int cmd_ftrace(int argc, const char **argv)
>         if (!argc && target__none(&ftrace.target))
>                 ftrace.target.system_wide = true;
>
> +       if (!list_empty(&ftrace.graph_funcs) || !list_empty(&ftrace.nograph_funcs))
> +               ftrace.tracer = "function_graph";
> +
>         ret = target__validate(&ftrace.target);
>         if (ret) {
>                 char errbuf[512];
> --
> 2.25.1
>

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 02/15] perf ftrace: add option '-F/--funcs' to list available functions
  2020-06-27 13:36 ` [PATCH v2 02/15] perf ftrace: add option '-F/--funcs' to list available functions Changbin Du
@ 2020-07-03  5:58   ` Namhyung Kim
  2020-07-07 15:15     ` Changbin Du
  0 siblings, 1 reply; 35+ messages in thread
From: Namhyung Kim @ 2020-07-03  5:58 UTC (permalink / raw)
  To: Changbin Du
  Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
	Steven Rostedt, linux-kernel

On Sat, Jun 27, 2020 at 10:37 PM 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 c5718503eded..e793118e83a9 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)
> +{
> +       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;

It seems the return value cannot be 0?

Thanks
Namhyung

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

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 06/15] perf ftrace: add option '-m/--buffer-size' to set per-cpu buffer size
  2020-06-27 13:36 ` [PATCH v2 06/15] perf ftrace: add option '-m/--buffer-size' to set per-cpu buffer size Changbin Du
@ 2020-07-03  6:17   ` Namhyung Kim
  0 siblings, 0 replies; 35+ messages in thread
From: Namhyung Kim @ 2020-07-03  6:17 UTC (permalink / raw)
  To: Changbin Du
  Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
	Steven Rostedt, linux-kernel

On Sat, Jun 27, 2020 at 10:38 PM Changbin Du <changbin.du@gmail.com> wrote:
>
> This adds an option '-m/--buffer-size' to allow us set the size of per-cpu
> tracing buffer.
>
> Signed-off-by: Changbin Du <changbin.du@gmail.com>
> ---
[SNIP]
> @@ -555,6 +575,8 @@ int cmd_ftrace(int argc, const char **argv)
>                     "Max depth for function graph tracer"),
>         OPT_UINTEGER('d', "delay", &ftrace.initial_delay,
>                      "ms to wait before starting tracing after program start"),
> +       OPT_UINTEGER('m', "buffer-size", &ftrace.buffer_size_kb,
> +                    "size of per cpu buffer in kb"),

It'd be nice if we support units as a suffix (like perf record). e.g. -m 4M

Thanks
Namhyung

>         OPT_END()
>         };
>
> --
> 2.25.1
>

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 07/15] perf ftrace: show trace column header
  2020-06-27 13:36 ` [PATCH v2 07/15] perf ftrace: show trace column header Changbin Du
@ 2020-07-03  6:20   ` Namhyung Kim
  2020-07-03 12:35     ` Arnaldo Carvalho de Melo
  2020-07-07 14:06     ` Changbin Du
  0 siblings, 2 replies; 35+ messages in thread
From: Namhyung Kim @ 2020-07-03  6:20 UTC (permalink / raw)
  To: Changbin Du
  Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
	Steven Rostedt, linux-kernel

On Sat, Jun 27, 2020 at 10:38 PM Changbin Du <changbin.du@gmail.com> wrote:
>
> This makes perf-ftrace display column header before printing trace.
>
> $ sudo perf ftrace
> \# tracer: function
> \#
> \# entries-in-buffer/entries-written: 0/0   #P:8
> \#
> \#           TASK-PID     CPU#   TIMESTAMP  FUNCTION
> \#              | |         |       |         |
>            <...>-9246  [006]  10726.262760: mutex_unlock <-rb_simple_write
>            <...>-9246  [006]  10726.262764: __fsnotify_parent <-vfs_write
>            <...>-9246  [006]  10726.262765: fsnotify <-vfs_write
>            <...>-9246  [006]  10726.262766: __sb_end_write <-vfs_write
>            <...>-9246  [006]  10726.262767: fpregs_assert_state_consistent <-do_syscall_64

You'd better indent the example output by 2 spaces to prevent
the # signs commented out.

Thanks
Namhyung


>
> Signed-off-by: Changbin Du <changbin.du@gmail.com>
> ---
>  tools/perf/builtin-ftrace.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
> index e45496012611..686d744d5025 100644
> --- a/tools/perf/builtin-ftrace.c
> +++ b/tools/perf/builtin-ftrace.c
> @@ -430,6 +430,9 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
>         fcntl(trace_fd, F_SETFL, O_NONBLOCK);
>         pollfd.fd = trace_fd;
>
> +       /* display column headers */
> +       read_tracing_file_to_stdout("trace");
> +
>         if (!ftrace->initial_delay) {
>                 if (write_tracing_file("tracing_on", "1") < 0) {
>                         pr_err("can't enable tracing\n");
> --
> 2.25.1
>

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 09/15] perf ftrace: add support for tracing option 'func_stack_trace'
  2020-06-27 13:36 ` [PATCH v2 09/15] perf ftrace: add support for tracing option 'func_stack_trace' Changbin Du
@ 2020-07-03  6:30   ` Namhyung Kim
  2020-07-07 15:19     ` Changbin Du
  0 siblings, 1 reply; 35+ messages in thread
From: Namhyung Kim @ 2020-07-03  6:30 UTC (permalink / raw)
  To: Changbin Du
  Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
	Steven Rostedt, linux-kernel

On Sat, Jun 27, 2020 at 10:38 PM Changbin Du <changbin.du@gmail.com> wrote:
>
> This adds support to display call trace for function tracer. To do this,
> just specify a '--func-call-graph' option.

What if it's used with -G option?  Also it might be used only with
the -T option..  How about showing a warning if it's missing.. ?

Thanks
Namhyung

>
> $ sudo perf ftrace -T vfs_read --func-call-graph
>  iio-sensor-prox-855   [003]   6168.369657: vfs_read <-ksys_read
>  iio-sensor-prox-855   [003]   6168.369677: <stack trace>
>  => vfs_read
>  => ksys_read
>  => __x64_sys_read
>  => do_syscall_64
>  => entry_SYSCALL_64_after_hwframe
>  ...
>
> Signed-off-by: Changbin Du <changbin.du@gmail.com>

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 10/15] perf ftrace: add support for trace option sleep-time
  2020-06-27 13:36 ` [PATCH v2 10/15] perf ftrace: add support for trace option sleep-time Changbin Du
@ 2020-07-03  6:40   ` Namhyung Kim
  2020-07-07 15:35     ` Changbin Du
  0 siblings, 1 reply; 35+ messages in thread
From: Namhyung Kim @ 2020-07-03  6:40 UTC (permalink / raw)
  To: Changbin Du
  Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
	Steven Rostedt, linux-kernel

On Sat, Jun 27, 2020 at 10:38 PM Changbin Du <changbin.du@gmail.com> wrote:
>
> This adds an option '--graph-nosleep-time' which allow us only to measure
> on-CPU time. This option is function_graph tracer only.

I'd suggest --graph-sleep-time instead and set it by default.
Then we can have --no-graph-sleep-time as well.

By the way,  didn't we agree to have something like --graph-opts instead?

Thanks
Namhyung

>
> Signed-off-by: Changbin Du <changbin.du@gmail.com>
>

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 11/15] perf ftrace: add support for trace option funcgraph-irqs
  2020-06-27 13:36 ` [PATCH v2 11/15] perf ftrace: add support for trace option funcgraph-irqs Changbin Du
@ 2020-07-03  6:43   ` Namhyung Kim
  0 siblings, 0 replies; 35+ messages in thread
From: Namhyung Kim @ 2020-07-03  6:43 UTC (permalink / raw)
  To: Changbin Du
  Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
	Steven Rostedt, linux-kernel

On Sat, Jun 27, 2020 at 10:38 PM Changbin Du <changbin.du@gmail.com> wrote:
>
> This adds an option '--graph-noirqs' to filter out functions executed
> in irq context.

Ditto.

>
> Signed-off-by: Changbin Du <changbin.du@gmail.com>
>
> ---
> v2: option name '--nofuncgraph-irqs' -> '--graph-noirqs'.
> ---
>  tools/perf/Documentation/perf-ftrace.txt |  3 +++
>  tools/perf/builtin-ftrace.c              | 20 ++++++++++++++++++++
>  2 files changed, 23 insertions(+)
>
> diff --git a/tools/perf/Documentation/perf-ftrace.txt b/tools/perf/Documentation/perf-ftrace.txt
> index a3000436f80b..b616e05d5156 100644
> --- a/tools/perf/Documentation/perf-ftrace.txt
> +++ b/tools/perf/Documentation/perf-ftrace.txt
> @@ -107,6 +107,9 @@ OPTIONS
>  --graph-nosleep-time::
>         Measure on-CPU time only for function_graph tracer.
>
> +--graph-noirqs::
> +       Ignore functions that happen inside interrupt for function_graph tracer.
> +
>  SEE ALSO
>  --------
>  linkperf:perf-record[1], linkperf:perf-trace[1]
> diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
> index eba125a60820..876c8e800425 100644
> --- a/tools/perf/builtin-ftrace.c
> +++ b/tools/perf/builtin-ftrace.c
> @@ -43,6 +43,7 @@ struct perf_ftrace {
>         bool                    inherit;
>         bool                    func_stack_trace;
>         bool                    graph_nosleep_time;
> +       bool                    graph_noirqs;
>  };
>
>  struct filter_entry {
> @@ -202,6 +203,7 @@ static void reset_tracing_options(struct perf_ftrace *ftrace __maybe_unused)
>         write_tracing_option_file("function-fork", "0");
>         write_tracing_option_file("func_stack_trace", "0");
>         write_tracing_option_file("sleep-time", "1");
> +       write_tracing_option_file("funcgraph-irqs", "1");
>  }
>
>  static int reset_tracing_files(struct perf_ftrace *ftrace __maybe_unused)
> @@ -393,6 +395,17 @@ static int set_tracing_sleep_time(struct perf_ftrace *ftrace)
>         return 0;
>  }
>
> +static int set_tracing_funcgraph_irqs(struct perf_ftrace *ftrace)
> +{
> +       if (!ftrace->graph_noirqs)
> +               return 0;
> +
> +       if (write_tracing_option_file("funcgraph-irqs", "0") < 0)
> +               return -1;
> +
> +       return 0;
> +}
> +
>  static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
>  {
>         char *trace_file;
> @@ -477,6 +490,11 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
>                 goto out_reset;
>         }
>
> +       if (set_tracing_funcgraph_irqs(ftrace) < 0) {
> +               pr_err("failed to set tracing option funcgraph-irqs\n");
> +               goto out_reset;
> +       }

Why not add set_tracing_options() which calls these individual functions
to handle option processing altogether?  It seems consistent to its
'reset' counterpart.

Thanks
Namhyung


> +
>         if (write_tracing_file("current_tracer", ftrace->tracer) < 0) {
>                 pr_err("failed to set current_tracer to %s\n", ftrace->tracer);
>                 goto out_reset;
> @@ -658,6 +676,8 @@ int cmd_ftrace(int argc, const char **argv)
>                     "trace children processes"),
>         OPT_BOOLEAN(0, "graph-nosleep-time", &ftrace.graph_nosleep_time,
>                     "measure on-CPU time only for function_graph tracer"),
> +       OPT_BOOLEAN(0, "graph-noirqs", &ftrace.graph_noirqs,
> +                   "ignore functions that happen inside interrupt for function_graph tracer"),
>         OPT_END()
>         };
>
> --
> 2.25.1
>

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 07/15] perf ftrace: show trace column header
  2020-07-03  6:20   ` Namhyung Kim
@ 2020-07-03 12:35     ` Arnaldo Carvalho de Melo
  2020-07-07 14:06     ` Changbin Du
  1 sibling, 0 replies; 35+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-07-03 12:35 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Changbin Du, Jiri Olsa, Peter Zijlstra, Ingo Molnar,
	Steven Rostedt, linux-kernel

Em Fri, Jul 03, 2020 at 03:20:15PM +0900, Namhyung Kim escreveu:
> On Sat, Jun 27, 2020 at 10:38 PM Changbin Du <changbin.du@gmail.com> wrote:
> >
> > This makes perf-ftrace display column header before printing trace.
> >
> > $ sudo perf ftrace
> > \# tracer: function
> > \#
> > \# entries-in-buffer/entries-written: 0/0   #P:8
> > \#
> > \#           TASK-PID     CPU#   TIMESTAMP  FUNCTION
> > \#              | |         |       |         |
> >            <...>-9246  [006]  10726.262760: mutex_unlock <-rb_simple_write
> >            <...>-9246  [006]  10726.262764: __fsnotify_parent <-vfs_write
> >            <...>-9246  [006]  10726.262765: fsnotify <-vfs_write
> >            <...>-9246  [006]  10726.262766: __sb_end_write <-vfs_write
> >            <...>-9246  [006]  10726.262767: fpregs_assert_state_consistent <-do_syscall_64
> 
> You'd better indent the example output by 2 spaces to prevent
> the # signs commented out.

Yes, all examples need to be two spaces to the right, to avoid things
like that.

- Arnaldo
 
> Thanks
> Namhyung
> 
> 
> >
> > Signed-off-by: Changbin Du <changbin.du@gmail.com>
> > ---
> >  tools/perf/builtin-ftrace.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
> > index e45496012611..686d744d5025 100644
> > --- a/tools/perf/builtin-ftrace.c
> > +++ b/tools/perf/builtin-ftrace.c
> > @@ -430,6 +430,9 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
> >         fcntl(trace_fd, F_SETFL, O_NONBLOCK);
> >         pollfd.fd = trace_fd;
> >
> > +       /* display column headers */
> > +       read_tracing_file_to_stdout("trace");
> > +
> >         if (!ftrace->initial_delay) {
> >                 if (write_tracing_file("tracing_on", "1") < 0) {
> >                         pr_err("can't enable tracing\n");
> > --
> > 2.25.1
> >

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 07/15] perf ftrace: show trace column header
  2020-07-03  6:20   ` Namhyung Kim
  2020-07-03 12:35     ` Arnaldo Carvalho de Melo
@ 2020-07-07 14:06     ` Changbin Du
  1 sibling, 0 replies; 35+ messages in thread
From: Changbin Du @ 2020-07-07 14:06 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Changbin Du, Jiri Olsa, Arnaldo Carvalho de Melo, Peter Zijlstra,
	Ingo Molnar, Steven Rostedt, linux-kernel

On Fri, Jul 03, 2020 at 03:20:15PM +0900, Namhyung Kim wrote:
> On Sat, Jun 27, 2020 at 10:38 PM Changbin Du <changbin.du@gmail.com> wrote:
> >
> > This makes perf-ftrace display column header before printing trace.
> >
> > $ sudo perf ftrace
> > \# tracer: function
> > \#
> > \# entries-in-buffer/entries-written: 0/0   #P:8
> > \#
> > \#           TASK-PID     CPU#   TIMESTAMP  FUNCTION
> > \#              | |         |       |         |
> >            <...>-9246  [006]  10726.262760: mutex_unlock <-rb_simple_write
> >            <...>-9246  [006]  10726.262764: __fsnotify_parent <-vfs_write
> >            <...>-9246  [006]  10726.262765: fsnotify <-vfs_write
> >            <...>-9246  [006]  10726.262766: __sb_end_write <-vfs_write
> >            <...>-9246  [006]  10726.262767: fpregs_assert_state_consistent <-do_syscall_64
> 
> You'd better indent the example output by 2 spaces to prevent
> the # signs commented out.
>
okay. I even didn't know this recipe befoe so I add a '\' :).  Thanks.

> Thanks
> Namhyung
> 
> 
> >
> > Signed-off-by: Changbin Du <changbin.du@gmail.com>
> > ---
> >  tools/perf/builtin-ftrace.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
> > index e45496012611..686d744d5025 100644
> > --- a/tools/perf/builtin-ftrace.c
> > +++ b/tools/perf/builtin-ftrace.c
> > @@ -430,6 +430,9 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
> >         fcntl(trace_fd, F_SETFL, O_NONBLOCK);
> >         pollfd.fd = trace_fd;
> >
> > +       /* display column headers */
> > +       read_tracing_file_to_stdout("trace");
> > +
> >         if (!ftrace->initial_delay) {
> >                 if (write_tracing_file("tracing_on", "1") < 0) {
> >                         pr_err("can't enable tracing\n");
> > --
> > 2.25.1
> >

-- 
Cheers,
Changbin Du

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 01/15] perf ftrace: select function/function_graph tracer automatically
  2020-07-03  5:47   ` Namhyung Kim
@ 2020-07-07 15:00     ` Changbin Du
  0 siblings, 0 replies; 35+ messages in thread
From: Changbin Du @ 2020-07-07 15:00 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Changbin Du, Jiri Olsa, Arnaldo Carvalho de Melo, Peter Zijlstra,
	Ingo Molnar, Steven Rostedt, linux-kernel

On Fri, Jul 03, 2020 at 02:47:09PM +0900, Namhyung Kim wrote:
> Hello,
> 
> On Sat, Jun 27, 2020 at 10:37 PM Changbin Du <changbin.du@gmail.com> wrote:
> >
> > The '-g/-G' options have already implied function_graph tracer should be
> > used instead of function tracer. So the extra option '--tracer' can be
> > killed.
> >
> > This patch changes the behavior as below:
> >   - By default, function tracer is used.
> >   - If '-g' or '-G' option is on, then function_graph tracer is used.
> >   - The perf configuration item 'ftrace.tracer' is marked as deprecated.
> >   - The option '--tracer' is marked as deprecated.
> >   - The default filter for -G/-T is to trace all functions.
> >
> > Here are some examples.
> >
> > This will start tracing all functions using function tracer:
> >   $ sudo perf ftrace
> >
> > This will trace all functions using function graph tracer:
> >   $ sudo perf ftrace -G
> >
> > This will trace function vfs_read using function graph tracer:
> >   $ sudo perf ftrace -G vfs_read
> 
> As we support running a new task on the command line, it might
> confuse users whether it's an argument of -G option or a task to run.
> One can use -- to separate them but it's easy to miss..
>
yes, it is a bit confusing. How about remove the default value '*'?

> Thanks
> Namhyung
> 
> 
> >
> > Signed-off-by: Changbin Du <changbin.du@gmail.com>
> > ---
> >  tools/perf/Documentation/perf-config.txt |  5 -----
> >  tools/perf/Documentation/perf-ftrace.txt |  2 +-
> >  tools/perf/builtin-ftrace.c              | 19 ++++++++++++-------
> >  3 files changed, 13 insertions(+), 13 deletions(-)
> >
> > diff --git a/tools/perf/Documentation/perf-config.txt b/tools/perf/Documentation/perf-config.txt
> > index c7d3df5798e2..a25fee7de3b2 100644
> > --- a/tools/perf/Documentation/perf-config.txt
> > +++ b/tools/perf/Documentation/perf-config.txt
> > @@ -612,11 +612,6 @@ trace.*::
> >                 "libbeauty", the default, to use the same argument beautifiers used in the
> >                 strace-like sys_enter+sys_exit lines.
> >
> > -ftrace.*::
> > -       ftrace.tracer::
> > -               Can be used to select the default tracer. Possible values are
> > -               'function' and 'function_graph'.
> > -
> >  llvm.*::
> >         llvm.clang-path::
> >                 Path to clang. If omit, search it from $PATH.
> > diff --git a/tools/perf/Documentation/perf-ftrace.txt b/tools/perf/Documentation/perf-ftrace.txt
> > index b80c84307dc9..952e46669168 100644
> > --- a/tools/perf/Documentation/perf-ftrace.txt
> > +++ b/tools/perf/Documentation/perf-ftrace.txt
> > @@ -24,7 +24,7 @@ OPTIONS
> >
> >  -t::
> >  --tracer=::
> > -       Tracer to use: function_graph or function.
> > +       Tracer to use: function_graph or function. This option is deprecated.
> >
> >  -v::
> >  --verbose=::
> > diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
> > index 2bfc1b0db536..c5718503eded 100644
> > --- a/tools/perf/builtin-ftrace.c
> > +++ b/tools/perf/builtin-ftrace.c
> > @@ -27,7 +27,6 @@
> >  #include "util/cap.h"
> >  #include "util/config.h"
> >
> > -#define DEFAULT_TRACER  "function_graph"
> >
> >  struct perf_ftrace {
> >         struct evlist           *evlist;
> > @@ -419,6 +418,7 @@ static int perf_ftrace_config(const char *var, const char *value, void *cb)
> >         if (strcmp(var, "ftrace.tracer"))
> >                 return -1;
> >
> > +       pr_warning("Configuration ftrace.tracer is deprecated\n");
> >         if (!strcmp(value, "function_graph") ||
> >             !strcmp(value, "function")) {
> >                 ftrace->tracer = value;
> > @@ -459,7 +459,7 @@ int cmd_ftrace(int argc, const char **argv)
> >  {
> >         int ret;
> >         struct perf_ftrace ftrace = {
> > -               .tracer = DEFAULT_TRACER,
> > +               .tracer = "function",
> >                 .target = { .uid = UINT_MAX, },
> >         };
> >         const char * const ftrace_usage[] = {
> > @@ -469,7 +469,7 @@ int cmd_ftrace(int argc, const char **argv)
> >         };
> >         const struct option ftrace_options[] = {
> >         OPT_STRING('t', "tracer", &ftrace.tracer, "tracer",
> > -                  "tracer to use: function_graph(default) or function"),
> > +                  "tracer to use: function or function_graph (This option is deprecated)"),
> >         OPT_STRING('p', "pid", &ftrace.target.pid, "pid",
> >                    "trace on existing process id"),
> >         OPT_INCR('v', "verbose", &verbose,
> > @@ -478,12 +478,14 @@ int cmd_ftrace(int argc, const char **argv)
> >                     "system-wide collection from all CPUs"),
> >         OPT_STRING('C', "cpu", &ftrace.target.cpu_list, "cpu",
> >                     "list of cpus to monitor"),
> > -       OPT_CALLBACK('T', "trace-funcs", &ftrace.filters, "func",
> > -                    "trace given functions only", parse_filter_func),
> > +       OPT_CALLBACK_DEFAULT('T', "trace-funcs", &ftrace.filters, "func",
> > +                            "trace given functions using function tracer",
> > +                            parse_filter_func, "*"),
> >         OPT_CALLBACK('N', "notrace-funcs", &ftrace.notrace, "func",
> >                      "do not trace given functions", parse_filter_func),
> > -       OPT_CALLBACK('G', "graph-funcs", &ftrace.graph_funcs, "func",
> > -                    "Set graph filter on given functions", parse_filter_func),
> > +       OPT_CALLBACK_DEFAULT('G', "graph-funcs", &ftrace.graph_funcs, "func",
> > +                            "trace given functions using function_graph tracer",
> > +                            parse_filter_func, "*"),
> >         OPT_CALLBACK('g', "nograph-funcs", &ftrace.nograph_funcs, "func",
> >                      "Set nograph filter on given functions", parse_filter_func),
> >         OPT_INTEGER('D', "graph-depth", &ftrace.graph_depth,
> > @@ -505,6 +507,9 @@ int cmd_ftrace(int argc, const char **argv)
> >         if (!argc && target__none(&ftrace.target))
> >                 ftrace.target.system_wide = true;
> >
> > +       if (!list_empty(&ftrace.graph_funcs) || !list_empty(&ftrace.nograph_funcs))
> > +               ftrace.tracer = "function_graph";
> > +
> >         ret = target__validate(&ftrace.target);
> >         if (ret) {
> >                 char errbuf[512];
> > --
> > 2.25.1
> >

-- 
Cheers,
Changbin Du

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 02/15] perf ftrace: add option '-F/--funcs' to list available functions
  2020-07-03  5:58   ` Namhyung Kim
@ 2020-07-07 15:15     ` Changbin Du
  0 siblings, 0 replies; 35+ messages in thread
From: Changbin Du @ 2020-07-07 15:15 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Changbin Du, Jiri Olsa, Arnaldo Carvalho de Melo, Peter Zijlstra,
	Ingo Molnar, Steven Rostedt, linux-kernel

On Fri, Jul 03, 2020 at 02:58:19PM +0900, Namhyung Kim wrote:
> On Sat, Jun 27, 2020 at 10:37 PM 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 c5718503eded..e793118e83a9 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)
> > +{
> > +       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;
> 
> It seems the return value cannot be 0?
>
If all above success, the return value is 0.

> Thanks
> Namhyung
> 
> > +
> > +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,
> > --
> > 2.25.1
> >

-- 
Cheers,
Changbin Du

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 09/15] perf ftrace: add support for tracing option 'func_stack_trace'
  2020-07-03  6:30   ` Namhyung Kim
@ 2020-07-07 15:19     ` Changbin Du
  0 siblings, 0 replies; 35+ messages in thread
From: Changbin Du @ 2020-07-07 15:19 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Changbin Du, Jiri Olsa, Arnaldo Carvalho de Melo, Peter Zijlstra,
	Ingo Molnar, Steven Rostedt, linux-kernel

On Fri, Jul 03, 2020 at 03:30:32PM +0900, Namhyung Kim wrote:
> On Sat, Jun 27, 2020 at 10:38 PM Changbin Du <changbin.du@gmail.com> wrote:
> >
> > This adds support to display call trace for function tracer. To do this,
> > just specify a '--func-call-graph' option.
> 
> What if it's used with -G option?  Also it might be used only with
> the -T option..  How about showing a warning if it's missing.. ?
>
All '--func-xxx' options are function tracer only. The warning message is a
good idea.

> Thanks
> Namhyung
> 
> >
> > $ sudo perf ftrace -T vfs_read --func-call-graph
> >  iio-sensor-prox-855   [003]   6168.369657: vfs_read <-ksys_read
> >  iio-sensor-prox-855   [003]   6168.369677: <stack trace>
> >  => vfs_read
> >  => ksys_read
> >  => __x64_sys_read
> >  => do_syscall_64
> >  => entry_SYSCALL_64_after_hwframe
> >  ...
> >
> > Signed-off-by: Changbin Du <changbin.du@gmail.com>

-- 
Cheers,
Changbin Du

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 10/15] perf ftrace: add support for trace option sleep-time
  2020-07-03  6:40   ` Namhyung Kim
@ 2020-07-07 15:35     ` Changbin Du
  2020-07-07 16:09       ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 35+ messages in thread
From: Changbin Du @ 2020-07-07 15:35 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Changbin Du, Jiri Olsa, Arnaldo Carvalho de Melo, Peter Zijlstra,
	Ingo Molnar, Steven Rostedt, linux-kernel

On Fri, Jul 03, 2020 at 03:40:59PM +0900, Namhyung Kim wrote:
> On Sat, Jun 27, 2020 at 10:38 PM Changbin Du <changbin.du@gmail.com> wrote:
> >
> > This adds an option '--graph-nosleep-time' which allow us only to measure
> > on-CPU time. This option is function_graph tracer only.
> 
> I'd suggest --graph-sleep-time instead and set it by default.
> Then we can have --no-graph-sleep-time as well.
>
If so can we make --graph-sleep-time as default. Seems there is no something
like OPT_BOOLEAN_DEFAULT().

> By the way,  didn't we agree to have something like --graph-opts instead?
>
As I mentioned in previous version, --graph-opts make the parsing a little
complex.
	--graph-opts depth=<n>,nosleep_time,noirqs,no_tail,verbose

While the exsiting style is '--graph-depth <n>'. The cons is we must type more
characters. The pros is don't need to reimplement parsing and align with old
behaviour.
	--graph-depth <n>
	--graph-nosleep-time
	--graph-noirqs
	--graph-notail
	--graph-verbose

> Thanks
> Namhyung
> 
> >
> > Signed-off-by: Changbin Du <changbin.du@gmail.com>
> >

-- 
Cheers,
Changbin Du

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 05/15] perf ftrace: factor out function write_tracing_file_int()
  2020-06-27 13:36 ` [PATCH v2 05/15] perf ftrace: factor out function write_tracing_file_int() Changbin Du
@ 2020-07-07 16:09   ` Steven Rostedt
  2020-07-07 16:10     ` Steven Rostedt
  2020-07-08 12:53     ` Changbin Du
  2020-07-07 16:24   ` Arnaldo Carvalho de Melo
  1 sibling, 2 replies; 35+ messages in thread
From: Steven Rostedt @ 2020-07-07 16:09 UTC (permalink / raw)
  To: Changbin Du
  Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
	Namhyung Kim, linux-kernel

On Sat, 27 Jun 2020 21:36:44 +0800
Changbin Du <changbin.du@gmail.com> wrote:

> We will reuse this function later.
> 

BTW, trace-cmd.git now has a libtracefs.so library, which I'm hoping
within a month to have as a stand alone (probably along with
libtraceevent and even a libtracecmd).

  https://git.kernel.org/pub/scm/utils/trace-cmd/trace-cmd.git/tree/lib/tracefs

This is a library made to interact with the tracefs directory to remove
reimplementing it all over the place.

-- Steve



> Signed-off-by: Changbin Du <changbin.du@gmail.com>
> ---
>  tools/perf/builtin-ftrace.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
> index dceae70c3a22..003efa756322 100644
> --- a/tools/perf/builtin-ftrace.c
> +++ b/tools/perf/builtin-ftrace.c
> @@ -166,6 +166,17 @@ static int read_tracing_file_to_stdout(const char *name)
>  	return ret;
>  }
>  
> +static int write_tracing_file_int(const char *name, int value)
> +{
> +	char buf[16];
> +
> +	snprintf(buf, sizeof(buf), "%d", value);
> +	if (write_tracing_file(name, buf) < 0)
> +		return -1;
> +
> +	return 0;
> +}
> +
>  static int reset_tracing_cpu(void);
>  static void reset_tracing_filters(void);
>  
> @@ -296,8 +307,6 @@ static void reset_tracing_filters(void)
>  
>  static int set_tracing_depth(struct perf_ftrace *ftrace)
>  {
> -	char buf[16];
> -
>  	if (ftrace->graph_depth == 0)
>  		return 0;
>  
> @@ -306,9 +315,7 @@ static int set_tracing_depth(struct perf_ftrace *ftrace)
>  		return -1;
>  	}
>  
> -	snprintf(buf, sizeof(buf), "%d", ftrace->graph_depth);
> -
> -	if (write_tracing_file("max_graph_depth", buf) < 0)
> +	if (write_tracing_file_int("max_graph_depth", ftrace->graph_depth) < 0)
>  		return -1;
>  
>  	return 0;


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 10/15] perf ftrace: add support for trace option sleep-time
  2020-07-07 15:35     ` Changbin Du
@ 2020-07-07 16:09       ` Arnaldo Carvalho de Melo
  2020-07-08 12:59         ` Changbin Du
  0 siblings, 1 reply; 35+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-07-07 16:09 UTC (permalink / raw)
  To: Changbin Du
  Cc: Namhyung Kim, Jiri Olsa, Peter Zijlstra, Ingo Molnar,
	Steven Rostedt, linux-kernel

Em Tue, Jul 07, 2020 at 11:35:14PM +0800, Changbin Du escreveu:
> On Fri, Jul 03, 2020 at 03:40:59PM +0900, Namhyung Kim wrote:
> > On Sat, Jun 27, 2020 at 10:38 PM Changbin Du <changbin.du@gmail.com> wrote:
> > > This adds an option '--graph-nosleep-time' which allow us only to measure
> > > on-CPU time. This option is function_graph tracer only.

> > I'd suggest --graph-sleep-time instead and set it by default.
> > Then we can have --no-graph-sleep-time as well.

> If so can we make --graph-sleep-time as default. Seems there is no something
> like OPT_BOOLEAN_DEFAULT().

> > By the way,  didn't we agree to have something like --graph-opts instead?

> As I mentioned in previous version, --graph-opts make the parsing a little
> complex.
> 	--graph-opts depth=<n>,nosleep_time,noirqs,no_tail,verbose

> While the exsiting style is '--graph-depth <n>'. The cons is we must type more
> characters. The pros is don't need to reimplement parsing and align with old
> behaviour.
> 	--graph-depth <n>
> 	--graph-nosleep-time
> 	--graph-noirqs
> 	--graph-notail
> 	--graph-verbose

This achieves the same result, yes, but it is more cumbersome, to use
all we would have to do:

    perf ftrace --graph-depth 10 --graph-nosleep-time --graph-noirqs --graph-notail --graph-verbose ...

When we could have a more compact:

    perf ftrace --graph-opts depth=10,nosleep-time,noirqs,notail,verbose ...

I.e., instead of a series of:

        OPT_INTEGER(0, "graph-depth", &ftrace.graph_depth, "Max depth for function graph tracer"),
	OPT_BOOLEAN(0, "sleep-time", &ftrace.sleep_time, "explanation"),
	OPT_BOOLEAN(0, "irqs" &ftrace.irqs, "explanation"),
	OPT_BOOLEAN(0, "tail" &ftrace.tail, "explanation"),
	OPT_BOOLEAN(0, "verbose" &ftrace.verbose, "explanation"),

We'd have a:

	OPT_CALLBACK(0, "graph-opts", &ftrace, "options", "graph options", parse_graph_opts),

And parse_graph_opts() would just have a simple strchr(opt, ',') + if
strcmp()/else loop, i.e., it would be easier for users, slightly more
difficult for perf developers :-)

- Arnaldo

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 05/15] perf ftrace: factor out function write_tracing_file_int()
  2020-07-07 16:09   ` Steven Rostedt
@ 2020-07-07 16:10     ` Steven Rostedt
  2020-07-08 12:53     ` Changbin Du
  1 sibling, 0 replies; 35+ messages in thread
From: Steven Rostedt @ 2020-07-07 16:10 UTC (permalink / raw)
  To: Changbin Du
  Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
	Namhyung Kim, linux-kernel

On Tue, 7 Jul 2020 12:09:23 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Sat, 27 Jun 2020 21:36:44 +0800
> Changbin Du <changbin.du@gmail.com> wrote:
> 
> > We will reuse this function later.
> >   
> 
> BTW, trace-cmd.git now has a libtracefs.so library, which I'm hoping
> within a month to have as a stand alone (probably along with
> libtraceevent and even a libtracecmd).
> 
>   https://git.kernel.org/pub/scm/utils/trace-cmd/trace-cmd.git/tree/lib/tracefs

And the API is defined in this header:

 https://git.kernel.org/pub/scm/utils/trace-cmd/trace-cmd.git/tree/include/tracefs/tracefs.h

-- Steve

> 
> This is a library made to interact with the tracefs directory to remove
> reimplementing it all over the place.

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 05/15] perf ftrace: factor out function write_tracing_file_int()
  2020-06-27 13:36 ` [PATCH v2 05/15] perf ftrace: factor out function write_tracing_file_int() Changbin Du
  2020-07-07 16:09   ` Steven Rostedt
@ 2020-07-07 16:24   ` Arnaldo Carvalho de Melo
  1 sibling, 0 replies; 35+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-07-07 16:24 UTC (permalink / raw)
  To: Changbin Du
  Cc: Jiri Olsa, Peter Zijlstra, Ingo Molnar, Namhyung Kim,
	Steven Rostedt, linux-kernel

Em Sat, Jun 27, 2020 at 09:36:44PM +0800, Changbin Du escreveu:
> We will reuse this function later.
> 
> Signed-off-by: Changbin Du <changbin.du@gmail.com>
> ---
>  tools/perf/builtin-ftrace.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
> index dceae70c3a22..003efa756322 100644
> --- a/tools/perf/builtin-ftrace.c
> +++ b/tools/perf/builtin-ftrace.c
> @@ -166,6 +166,17 @@ static int read_tracing_file_to_stdout(const char *name)
>  	return ret;
>  }
>  
> +static int write_tracing_file_int(const char *name, int value)
> +{
> +	char buf[16];
> +
> +	snprintf(buf, sizeof(buf), "%d", value);
> +	if (write_tracing_file(name, buf) < 0)
> +		return -1;
> +
> +	return 0;
> +}
> +
>  static int reset_tracing_cpu(void);
>  static void reset_tracing_filters(void);
>  
> @@ -296,8 +307,6 @@ static void reset_tracing_filters(void)
>  
>  static int set_tracing_depth(struct perf_ftrace *ftrace)
>  {
> -	char buf[16];
> -
>  	if (ftrace->graph_depth == 0)
>  		return 0;
>  
> @@ -306,9 +315,7 @@ static int set_tracing_depth(struct perf_ftrace *ftrace)
>  		return -1;
>  	}
>  
> -	snprintf(buf, sizeof(buf), "%d", ftrace->graph_depth);
> -
> -	if (write_tracing_file("max_graph_depth", buf) < 0)
> +	if (write_tracing_file_int("max_graph_depth", ftrace->graph_depth) < 0)
>  		return -1;

We've been consolidating these sysfs, tracefs, debugfs, hugetlbfs, etc
in tools/lib/fs/, where we have things like:

[acme@quaco perf]$ grep "int sysfs__" tools/lib/api/fs/fs.c 
static int sysfs__read_ull_base(const char *entry,
int sysfs__read_xll(const char *entry, unsigned long long *value)
int sysfs__read_ull(const char *entry, unsigned long long *value)
int sysfs__read_int(const char *entry, int *value)
int sysfs__read_str(const char *entry, char **buf, size_t *sizep)
int sysfs__read_bool(const char *entry, bool *value)
int sysfs__write_int(const char *entry, int value)
[acme@quaco perf]$ grep "int debugfs__" tools/lib/api/fs/fs.c 
[acme@quaco perf]$ grep "int sysctl__" tools/lib/api/fs/fs.c 
int sysctl__read_int(const char *sysctl, int *value)
[acme@quaco perf]$

Please take a look at tools/lib/api/fs/fs.h it already has the functions
to find the mount points, etc, so adding those there since you're
touchign this area now seems best.

- Arnaldo

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 05/15] perf ftrace: factor out function write_tracing_file_int()
  2020-07-07 16:09   ` Steven Rostedt
  2020-07-07 16:10     ` Steven Rostedt
@ 2020-07-08 12:53     ` Changbin Du
  1 sibling, 0 replies; 35+ messages in thread
From: Changbin Du @ 2020-07-08 12:53 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Changbin Du, Jiri Olsa, Arnaldo Carvalho de Melo, Peter Zijlstra,
	Ingo Molnar, Namhyung Kim, linux-kernel

On Tue, Jul 07, 2020 at 12:09:23PM -0400, Steven Rostedt wrote:
> On Sat, 27 Jun 2020 21:36:44 +0800
> Changbin Du <changbin.du@gmail.com> wrote:
> 
> > We will reuse this function later.
> > 
> 
> BTW, trace-cmd.git now has a libtracefs.so library, which I'm hoping
> within a month to have as a stand alone (probably along with
> libtraceevent and even a libtracecmd).
> 
>   https://git.kernel.org/pub/scm/utils/trace-cmd/trace-cmd.git/tree/lib/tracefs
> 
> This is a library made to interact with the tracefs directory to remove
> reimplementing it all over the place.
>
Nice! This will make implementing a tracing tool much simpler. Look forward
to seeing it appears in most Linux distributions.

> -- Steve
> 
> 
> 
> > Signed-off-by: Changbin Du <changbin.du@gmail.com>
> > ---
> >  tools/perf/builtin-ftrace.c | 17 ++++++++++++-----
> >  1 file changed, 12 insertions(+), 5 deletions(-)
> > 
> > diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
> > index dceae70c3a22..003efa756322 100644
> > --- a/tools/perf/builtin-ftrace.c
> > +++ b/tools/perf/builtin-ftrace.c
> > @@ -166,6 +166,17 @@ static int read_tracing_file_to_stdout(const char *name)
> >  	return ret;
> >  }
> >  
> > +static int write_tracing_file_int(const char *name, int value)
> > +{
> > +	char buf[16];
> > +
> > +	snprintf(buf, sizeof(buf), "%d", value);
> > +	if (write_tracing_file(name, buf) < 0)
> > +		return -1;
> > +
> > +	return 0;
> > +}
> > +
> >  static int reset_tracing_cpu(void);
> >  static void reset_tracing_filters(void);
> >  
> > @@ -296,8 +307,6 @@ static void reset_tracing_filters(void)
> >  
> >  static int set_tracing_depth(struct perf_ftrace *ftrace)
> >  {
> > -	char buf[16];
> > -
> >  	if (ftrace->graph_depth == 0)
> >  		return 0;
> >  
> > @@ -306,9 +315,7 @@ static int set_tracing_depth(struct perf_ftrace *ftrace)
> >  		return -1;
> >  	}
> >  
> > -	snprintf(buf, sizeof(buf), "%d", ftrace->graph_depth);
> > -
> > -	if (write_tracing_file("max_graph_depth", buf) < 0)
> > +	if (write_tracing_file_int("max_graph_depth", ftrace->graph_depth) < 0)
> >  		return -1;
> >  
> >  	return 0;
> 

-- 
Cheers,
Changbin Du

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 10/15] perf ftrace: add support for trace option sleep-time
  2020-07-07 16:09       ` Arnaldo Carvalho de Melo
@ 2020-07-08 12:59         ` Changbin Du
  0 siblings, 0 replies; 35+ messages in thread
From: Changbin Du @ 2020-07-08 12:59 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Changbin Du, Namhyung Kim, Jiri Olsa, Peter Zijlstra,
	Ingo Molnar, Steven Rostedt, linux-kernel

On Tue, Jul 07, 2020 at 01:09:43PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Jul 07, 2020 at 11:35:14PM +0800, Changbin Du escreveu:
> > On Fri, Jul 03, 2020 at 03:40:59PM +0900, Namhyung Kim wrote:
> > > On Sat, Jun 27, 2020 at 10:38 PM Changbin Du <changbin.du@gmail.com> wrote:
> > > > This adds an option '--graph-nosleep-time' which allow us only to measure
> > > > on-CPU time. This option is function_graph tracer only.
> 
> > > I'd suggest --graph-sleep-time instead and set it by default.
> > > Then we can have --no-graph-sleep-time as well.
> 
> > If so can we make --graph-sleep-time as default. Seems there is no something
> > like OPT_BOOLEAN_DEFAULT().
> 
> > > By the way,  didn't we agree to have something like --graph-opts instead?
> 
> > As I mentioned in previous version, --graph-opts make the parsing a little
> > complex.
> > 	--graph-opts depth=<n>,nosleep_time,noirqs,no_tail,verbose
> 
> > While the exsiting style is '--graph-depth <n>'. The cons is we must type more
> > characters. The pros is don't need to reimplement parsing and align with old
> > behaviour.
> > 	--graph-depth <n>
> > 	--graph-nosleep-time
> > 	--graph-noirqs
> > 	--graph-notail
> > 	--graph-verbose
> 
> This achieves the same result, yes, but it is more cumbersome, to use
> all we would have to do:
> 
>     perf ftrace --graph-depth 10 --graph-nosleep-time --graph-noirqs --graph-notail --graph-verbose ...
> 
> When we could have a more compact:
> 
>     perf ftrace --graph-opts depth=10,nosleep-time,noirqs,notail,verbose ...
> 
> I.e., instead of a series of:
> 
>         OPT_INTEGER(0, "graph-depth", &ftrace.graph_depth, "Max depth for function graph tracer"),
> 	OPT_BOOLEAN(0, "sleep-time", &ftrace.sleep_time, "explanation"),
> 	OPT_BOOLEAN(0, "irqs" &ftrace.irqs, "explanation"),
> 	OPT_BOOLEAN(0, "tail" &ftrace.tail, "explanation"),
> 	OPT_BOOLEAN(0, "verbose" &ftrace.verbose, "explanation"),
> 
> We'd have a:
> 
> 	OPT_CALLBACK(0, "graph-opts", &ftrace, "options", "graph options", parse_graph_opts),
> 
> And parse_graph_opts() would just have a simple strchr(opt, ',') + if
> strcmp()/else loop, i.e., it would be easier for users, slightly more
> difficult for perf developers :-)
>
And we also have to parse key-value pairs for 'graph-depth' and move option
'--graph-depth' to 'graph-opts'.

There is an existing similr option '--debug'. It can be factored out as a
general interface.
	--debug --debug verbose=2,stderr

> - Arnaldo

-- 
Cheers,
Changbin Du

^ permalink raw reply	[flat|nested] 35+ messages in thread

end of thread, other threads:[~2020-07-08 12:59 UTC | newest]

Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-27 13:36 [PATCH v2 00/15] perf: ftrace enhancement Changbin Du
2020-06-27 13:36 ` [PATCH v2 01/15] perf ftrace: select function/function_graph tracer automatically Changbin Du
2020-07-03  5:47   ` Namhyung Kim
2020-07-07 15:00     ` Changbin Du
2020-06-27 13:36 ` [PATCH v2 02/15] perf ftrace: add option '-F/--funcs' to list available functions Changbin Du
2020-07-03  5:58   ` Namhyung Kim
2020-07-07 15:15     ` Changbin Du
2020-06-27 13:36 ` [PATCH v2 03/15] perf ftrace: add option -t/--tid to filter by thread id Changbin Du
2020-06-27 13:36 ` [PATCH v2 04/15] perf ftrace: add option -d/--delay to delay tracing Changbin Du
2020-06-27 13:36 ` [PATCH v2 05/15] perf ftrace: factor out function write_tracing_file_int() Changbin Du
2020-07-07 16:09   ` Steven Rostedt
2020-07-07 16:10     ` Steven Rostedt
2020-07-08 12:53     ` Changbin Du
2020-07-07 16:24   ` Arnaldo Carvalho de Melo
2020-06-27 13:36 ` [PATCH v2 06/15] perf ftrace: add option '-m/--buffer-size' to set per-cpu buffer size Changbin Du
2020-07-03  6:17   ` Namhyung Kim
2020-06-27 13:36 ` [PATCH v2 07/15] perf ftrace: show trace column header Changbin Du
2020-07-03  6:20   ` Namhyung Kim
2020-07-03 12:35     ` Arnaldo Carvalho de Melo
2020-07-07 14:06     ` Changbin Du
2020-06-27 13:36 ` [PATCH v2 08/15] perf ftrace: add option '--inherit' to trace children processes Changbin Du
2020-06-27 13:36 ` [PATCH v2 09/15] perf ftrace: add support for tracing option 'func_stack_trace' Changbin Du
2020-07-03  6:30   ` Namhyung Kim
2020-07-07 15:19     ` Changbin Du
2020-06-27 13:36 ` [PATCH v2 10/15] perf ftrace: add support for trace option sleep-time Changbin Du
2020-07-03  6:40   ` Namhyung Kim
2020-07-07 15:35     ` Changbin Du
2020-07-07 16:09       ` Arnaldo Carvalho de Melo
2020-07-08 12:59         ` Changbin Du
2020-06-27 13:36 ` [PATCH v2 11/15] perf ftrace: add support for trace option funcgraph-irqs Changbin Du
2020-07-03  6:43   ` Namhyung Kim
2020-06-27 13:36 ` [PATCH v2 12/15] perf ftrace: add support for tracing option 'irq-info' Changbin Du
2020-06-27 13:36 ` [PATCH v2 13/15] perf ftrace: add option '--graph-verbose' to show more info for graph tracer Changbin Du
2020-06-27 13:36 ` [PATCH v2 14/15] perf ftrace: add support for trace option tracing_thresh Changbin Du
2020-06-27 13:36 ` [PATCH v2 15/15] perf ftrace: add change log 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).