2020-06-27 13:37:47

by Changbin Du

[permalink] [raw]
Subject: [PATCH v2 00/15] perf: ftrace enhancement

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


2020-06-27 13:37:54

by Changbin Du

[permalink] [raw]
Subject: [PATCH v2 01/15] perf ftrace: select function/function_graph tracer automatically

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 <[email protected]>
---
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

2020-06-27 13:38:35

by Changbin Du

[permalink] [raw]
Subject: [PATCH v2 02/15] perf ftrace: add option '-F/--funcs' to list available functions

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 <[email protected]>

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

2020-06-27 13:38:52

by Changbin Du

[permalink] [raw]
Subject: [PATCH v2 04/15] perf ftrace: add option -d/--delay to delay tracing

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

Signed-off-by: Changbin Du <[email protected]>
---
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

2020-06-27 13:38:58

by Changbin Du

[permalink] [raw]
Subject: [PATCH v2 06/15] perf ftrace: add option '-m/--buffer-size' to set per-cpu buffer size

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

Signed-off-by: Changbin Du <[email protected]>
---
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

2020-06-27 13:39:11

by Changbin Du

[permalink] [raw]
Subject: [PATCH v2 05/15] perf ftrace: factor out function write_tracing_file_int()

We will reuse this function later.

Signed-off-by: Changbin Du <[email protected]>
---
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

2020-06-27 13:39:17

by Changbin Du

[permalink] [raw]
Subject: [PATCH v2 07/15] perf ftrace: show trace column header

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 <[email protected]>
---
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

2020-06-27 13:39:43

by Changbin Du

[permalink] [raw]
Subject: [PATCH v2 09/15] perf ftrace: add support for tracing option 'func_stack_trace'

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 <[email protected]>

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

2020-06-27 13:40:39

by Changbin Du

[permalink] [raw]
Subject: [PATCH v2 03/15] perf ftrace: add option -t/--tid to filter by thread id

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

Signed-off-by: Changbin Du <[email protected]>
---
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

2020-06-27 13:41:28

by Changbin Du

[permalink] [raw]
Subject: [PATCH v2 08/15] perf ftrace: add option '--inherit' to trace children processes

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

Signed-off-by: Changbin Du <[email protected]>

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

2020-06-27 13:41:41

by Changbin Du

[permalink] [raw]
Subject: [PATCH v2 10/15] perf ftrace: add support for trace option sleep-time

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 <[email protected]>

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

2020-06-27 13:41:41

by Changbin Du

[permalink] [raw]
Subject: [PATCH v2 11/15] perf ftrace: add support for trace option funcgraph-irqs

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

Signed-off-by: Changbin Du <[email protected]>

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

2020-06-27 13:41:47

by Changbin Du

[permalink] [raw]
Subject: [PATCH v2 12/15] perf ftrace: add support for tracing option 'irq-info'

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 <[email protected]>
---
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

2020-06-27 13:41:57

by Changbin Du

[permalink] [raw]
Subject: [PATCH v2 13/15] perf ftrace: add option '--graph-verbose' to show more info for graph tracer

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 <[email protected]>
---
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

2020-06-27 13:42:15

by Changbin Du

[permalink] [raw]
Subject: [PATCH v2 14/15] perf ftrace: add support for trace option tracing_thresh

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 <[email protected]>
---
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

2020-06-27 13:42:21

by Changbin Du

[permalink] [raw]
Subject: [PATCH v2 15/15] perf ftrace: add change log

Add a change log after previous enhancements.

Signed-off-by: Changbin Du <[email protected]>
---
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 <[email protected]>
+ * Copyright (c) 2020 Changbin Du <[email protected]>, significant enhancement.
*/

#include "builtin.h"
--
2.25.1

2020-07-03 05:48:56

by Namhyung Kim

[permalink] [raw]
Subject: Re: [PATCH v2 01/15] perf ftrace: select function/function_graph tracer automatically

Hello,

On Sat, Jun 27, 2020 at 10:37 PM Changbin Du <[email protected]> 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 <[email protected]>
> ---
> 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
>

2020-07-03 06:01:18

by Namhyung Kim

[permalink] [raw]
Subject: Re: [PATCH v2 02/15] perf ftrace: add option '-F/--funcs' to list available functions

On Sat, Jun 27, 2020 at 10:37 PM Changbin Du <[email protected]> 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 <[email protected]>
>
> ---
> 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
>

2020-07-03 06:17:52

by Namhyung Kim

[permalink] [raw]
Subject: Re: [PATCH v2 06/15] perf ftrace: add option '-m/--buffer-size' to set per-cpu buffer size

On Sat, Jun 27, 2020 at 10:38 PM Changbin Du <[email protected]> wrote:
>
> This adds an option '-m/--buffer-size' to allow us set the size of per-cpu
> tracing buffer.
>
> Signed-off-by: Changbin Du <[email protected]>
> ---
[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
>

2020-07-03 06:24:22

by Namhyung Kim

[permalink] [raw]
Subject: Re: [PATCH v2 07/15] perf ftrace: show trace column header

On Sat, Jun 27, 2020 at 10:38 PM Changbin Du <[email protected]> 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 <[email protected]>
> ---
> 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
>

2020-07-03 06:33:36

by Namhyung Kim

[permalink] [raw]
Subject: Re: [PATCH v2 09/15] perf ftrace: add support for tracing option 'func_stack_trace'

On Sat, Jun 27, 2020 at 10:38 PM Changbin Du <[email protected]> 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 <[email protected]>

2020-07-03 06:44:05

by Namhyung Kim

[permalink] [raw]
Subject: Re: [PATCH v2 10/15] perf ftrace: add support for trace option sleep-time

On Sat, Jun 27, 2020 at 10:38 PM Changbin Du <[email protected]> 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 <[email protected]>
>

2020-07-03 06:46:51

by Namhyung Kim

[permalink] [raw]
Subject: Re: [PATCH v2 11/15] perf ftrace: add support for trace option funcgraph-irqs

On Sat, Jun 27, 2020 at 10:38 PM Changbin Du <[email protected]> wrote:
>
> This adds an option '--graph-noirqs' to filter out functions executed
> in irq context.

Ditto.

>
> Signed-off-by: Changbin Du <[email protected]>
>
> ---
> 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
>

2020-07-03 12:39:33

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH v2 07/15] perf ftrace: show trace column header

Em Fri, Jul 03, 2020 at 03:20:15PM +0900, Namhyung Kim escreveu:
> On Sat, Jun 27, 2020 at 10:38 PM Changbin Du <[email protected]> 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 <[email protected]>
> > ---
> > 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

2020-07-07 14:07:02

by Changbin Du

[permalink] [raw]
Subject: Re: [PATCH v2 07/15] perf ftrace: show trace column header

On Fri, Jul 03, 2020 at 03:20:15PM +0900, Namhyung Kim wrote:
> On Sat, Jun 27, 2020 at 10:38 PM Changbin Du <[email protected]> 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 <[email protected]>
> > ---
> > 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

2020-07-07 15:04:30

by Changbin Du

[permalink] [raw]
Subject: Re: [PATCH v2 01/15] perf ftrace: select function/function_graph tracer automatically

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 <[email protected]> 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 <[email protected]>
> > ---
> > 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

2020-07-07 15:20:05

by Changbin Du

[permalink] [raw]
Subject: Re: [PATCH v2 02/15] perf ftrace: add option '-F/--funcs' to list available functions

On Fri, Jul 03, 2020 at 02:58:19PM +0900, Namhyung Kim wrote:
> On Sat, Jun 27, 2020 at 10:37 PM Changbin Du <[email protected]> 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 <[email protected]>
> >
> > ---
> > 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

2020-07-07 15:23:36

by Changbin Du

[permalink] [raw]
Subject: Re: [PATCH v2 09/15] perf ftrace: add support for tracing option 'func_stack_trace'

On Fri, Jul 03, 2020 at 03:30:32PM +0900, Namhyung Kim wrote:
> On Sat, Jun 27, 2020 at 10:38 PM Changbin Du <[email protected]> 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 <[email protected]>

--
Cheers,
Changbin Du

2020-07-07 15:37:55

by Changbin Du

[permalink] [raw]
Subject: Re: [PATCH v2 10/15] perf ftrace: add support for trace option sleep-time

On Fri, Jul 03, 2020 at 03:40:59PM +0900, Namhyung Kim wrote:
> On Sat, Jun 27, 2020 at 10:38 PM Changbin Du <[email protected]> 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 <[email protected]>
> >

--
Cheers,
Changbin Du

2020-07-07 16:12:03

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH v2 05/15] perf ftrace: factor out function write_tracing_file_int()

On Sat, 27 Jun 2020 21:36:44 +0800
Changbin Du <[email protected]> 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 <[email protected]>
> ---
> 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;

2020-07-07 16:12:32

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH v2 10/15] perf ftrace: add support for trace option sleep-time

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 <[email protected]> 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

2020-07-07 16:13:53

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH v2 05/15] perf ftrace: factor out function write_tracing_file_int()

On Tue, 7 Jul 2020 12:09:23 -0400
Steven Rostedt <[email protected]> wrote:

> On Sat, 27 Jun 2020 21:36:44 +0800
> Changbin Du <[email protected]> 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.

2020-07-07 16:24:46

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH v2 05/15] perf ftrace: factor out function write_tracing_file_int()

Em Sat, Jun 27, 2020 at 09:36:44PM +0800, Changbin Du escreveu:
> We will reuse this function later.
>
> Signed-off-by: Changbin Du <[email protected]>
> ---
> 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

2020-07-08 12:54:37

by Changbin Du

[permalink] [raw]
Subject: Re: [PATCH v2 05/15] perf ftrace: factor out function write_tracing_file_int()

On Tue, Jul 07, 2020 at 12:09:23PM -0400, Steven Rostedt wrote:
> On Sat, 27 Jun 2020 21:36:44 +0800
> Changbin Du <[email protected]> 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 <[email protected]>
> > ---
> > 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

2020-07-08 13:04:31

by Changbin Du

[permalink] [raw]
Subject: Re: [PATCH v2 10/15] perf ftrace: add support for trace option sleep-time

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 <[email protected]> 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