[PATCH 1/4] perf ftrace: Add support for --pid option

[PATCH 1/4] perf ftrace: Add support for --pid option

[Namhyung Kim]

The -p (--pid) option enables to trace existing process by its pid.

Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/Documentation/perf-ftrace.txt |  4 ++
 tools/perf/builtin-ftrace.c              | 91 ++++++++++++++++++++++----------
 2 files changed, 68 insertions(+), 27 deletions(-)

diff --git a/tools/perf/Documentation/perf-ftrace.txt b/tools/perf/Documentation/perf-ftrace.txt
index 2d96de6132a9..2d39397f3f30 100644
--- a/tools/perf/Documentation/perf-ftrace.txt
+++ b/tools/perf/Documentation/perf-ftrace.txt
@@ -30,6 +30,10 @@ OPTIONS
 --verbose=::
         Verbosity level.
 
+-p::
+--pid=::
+	Trace on existing process id (comma separated list).
+
 
 SEE ALSO
 --------
diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
index c3e643666c72..85eee9c444ae 100644
--- a/tools/perf/builtin-ftrace.c
+++ b/tools/perf/builtin-ftrace.c
@@ -11,6 +11,7 @@
 
 #include <unistd.h>
 #include <signal.h>
+#include <fcntl.h>
 
 #include "debug.h"
 #include <subcmd/parse-options.h>
@@ -50,11 +51,12 @@ static void ftrace__workload_exec_failed_signal(int signo __maybe_unused,
 	done = true;
 }
 
-static int write_tracing_file(const char *name, const char *val)
+static int __write_tracing_file(const char *name, const char *val, bool append)
 {
 	char *file;
 	int fd, ret = -1;
 	ssize_t size = strlen(val);
+	int flags = O_WRONLY;
 
 	file = get_tracing_file(name);
 	if (!file) {
@@ -62,7 +64,12 @@ static int write_tracing_file(const char *name, const char *val)
 		return -1;
 	}
 
-	fd = open(file, O_WRONLY);
+	if (append)
+		flags |= O_APPEND;
+	else
+		flags |= O_TRUNC;
+
+	fd = open(file, flags);
 	if (fd < 0) {
 		pr_debug("cannot open tracing file: %s\n", name);
 		goto out;
@@ -79,6 +86,16 @@ static int write_tracing_file(const char *name, const char *val)
 	return ret;
 }
 
+static int write_tracing_file(const char *name, const char *val)
+{
+	return __write_tracing_file(name, val, false);
+}
+
+static int append_tracing_file(const char *name, const char *val)
+{
+	return __write_tracing_file(name, val, true);
+}
+
 static int reset_tracing_files(struct perf_ftrace *ftrace __maybe_unused)
 {
 	if (write_tracing_file("tracing_on", "0") < 0)
@@ -93,11 +110,27 @@ static int reset_tracing_files(struct perf_ftrace *ftrace __maybe_unused)
 	return 0;
 }
 
+static int set_tracing_pid(struct perf_ftrace *ftrace)
+{
+	int i;
+	char buf[16];
+
+	if (target__has_cpu(&ftrace->target))
+		return 0;
+
+	for (i = 0; i < thread_map__nr(ftrace->evlist->threads); i++) {
+		scnprintf(buf, sizeof(buf), "%d",
+			  ftrace->evlist->threads->map[i]);
+		if (append_tracing_file("set_ftrace_pid", buf) < 0)
+			return -1;
+	}
+	return 0;
+}
+
 static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
 {
 	char *trace_file;
 	int trace_fd;
-	char *trace_pid;
 	char buf[4096];
 	struct pollfd pollfd = {
 		.events = POLLIN,
@@ -108,42 +141,37 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
 		return -1;
 	}
 
-	if (argc < 1)
-		return -1;
-
 	signal(SIGINT, sig_handler);
 	signal(SIGUSR1, sig_handler);
 	signal(SIGCHLD, sig_handler);
 
-	reset_tracing_files(ftrace);
+	if (reset_tracing_files(ftrace) < 0)
+		goto out;
 
 	/* reset ftrace buffer */
 	if (write_tracing_file("trace", "0") < 0)
 		goto out;
 
-	if (perf_evlist__prepare_workload(ftrace->evlist, &ftrace->target,
-					  argv, false, ftrace__workload_exec_failed_signal) < 0)
-		goto out;
-
-	if (write_tracing_file("current_tracer", ftrace->tracer) < 0) {
-		pr_err("failed to set current_tracer to %s\n", ftrace->tracer);
+	if (argc && perf_evlist__prepare_workload(ftrace->evlist,
+				&ftrace->target, argv, false,
+				ftrace__workload_exec_failed_signal) < 0) {
 		goto out;
 	}
 
-	if (asprintf(&trace_pid, "%d", thread_map__pid(ftrace->evlist->threads, 0)) < 0) {
-		pr_err("failed to allocate pid string\n");
-		goto out;
+	if (set_tracing_pid(ftrace) < 0) {
+		pr_err("failed to set ftrace pid\n");
+		goto out_reset;
 	}
 
-	if (write_tracing_file("set_ftrace_pid", trace_pid) < 0) {
-		pr_err("failed to set pid: %s\n", trace_pid);
-		goto out_free_pid;
+	if (write_tracing_file("current_tracer", ftrace->tracer) < 0) {
+		pr_err("failed to set current_tracer to %s\n", ftrace->tracer);
+		goto out_reset;
 	}
 
 	trace_file = get_tracing_file("trace_pipe");
 	if (!trace_file) {
 		pr_err("failed to open trace_pipe\n");
-		goto out_free_pid;
+		goto out_reset;
 	}
 
 	trace_fd = open(trace_file, O_RDONLY);
@@ -152,7 +180,7 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
 
 	if (trace_fd < 0) {
 		pr_err("failed to open trace_pipe\n");
-		goto out_free_pid;
+		goto out_reset;
 	}
 
 	fcntl(trace_fd, F_SETFL, O_NONBLOCK);
@@ -191,11 +219,9 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
 
 out_close_fd:
 	close(trace_fd);
-out_free_pid:
-	free(trace_pid);
-out:
+out_reset:
 	reset_tracing_files(ftrace);
-
+out:
 	return done ? 0 : -1;
 }
 
@@ -227,13 +253,15 @@ int cmd_ftrace(int argc, const char **argv, const char *prefix __maybe_unused)
 		.target = { .uid = UINT_MAX, },
 	};
 	const char * const ftrace_usage[] = {
-		"perf ftrace [<options>] <command>",
+		"perf ftrace [<options>] [<command>]",
 		"perf ftrace [<options>] -- <command> [<options>]",
 		NULL
 	};
 	const struct option ftrace_options[] = {
 	OPT_STRING('t', "tracer", &ftrace.tracer, "tracer",
 		   "tracer to use: function_graph(default) or function"),
+	OPT_STRING('p', "pid", &ftrace.target.pid, "pid",
+		   "trace on existing process id"),
 	OPT_INCR('v', "verbose", &verbose,
 		 "be more verbose"),
 	OPT_END()
@@ -245,9 +273,18 @@ int cmd_ftrace(int argc, const char **argv, const char *prefix __maybe_unused)
 
 	argc = parse_options(argc, argv, ftrace_options, ftrace_usage,
 			    PARSE_OPT_STOP_AT_NON_OPTION);
-	if (!argc)
+	if (!argc && target__none(&ftrace.target))
 		usage_with_options(ftrace_usage, ftrace_options);
 
+	ret = target__validate(&ftrace.target);
+	if (ret) {
+		char errbuf[512];
+
+		target__strerror(&ftrace.target, ret, errbuf, 512);
+		pr_err("%s\n", errbuf);
+		return -EINVAL;
+	}
+
 	ftrace.evlist = perf_evlist__new();
 	if (ftrace.evlist == NULL)
 		return -ENOMEM;
-- 
2.11.1

[PATCH 2/4] perf tools: Introduce cpu_map__snprint_mask()

[Namhyung Kim]

The cpu_map__snprint_mask() is to generate string representation of
cpumask bitmap.  For cpu 0 to 11, it'll return "fff".

Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/cpumap.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/cpumap.h |  1 +
 2 files changed, 47 insertions(+)

diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
index 8c7504939113..08540ab2a891 100644
--- a/tools/perf/util/cpumap.c
+++ b/tools/perf/util/cpumap.c
@@ -673,3 +673,49 @@ size_t cpu_map__snprint(struct cpu_map *map, char *buf, size_t size)
 	pr_debug("cpumask list: %s\n", buf);
 	return ret;
 }
+
+static char hex_char(char val)
+{
+	if (0 <= val && val <= 9)
+		return val + '0';
+	if (10 <= val && val < 16)
+		return val - 10 + 'a';
+	return '?';
+}
+
+size_t cpu_map__snprint_mask(struct cpu_map *map, char *buf, size_t size)
+{
+	int i, cpu;
+	char *ptr = buf;
+	unsigned char *bitmap;
+	int last_cpu = cpu_map__cpu(map, map->nr - 1);
+
+	bitmap = zalloc((last_cpu + 7) / 8);
+	if (bitmap == NULL) {
+		buf[0] = '\0';
+		return 0;
+	}
+
+	for (i = 0; i < map->nr; i++) {
+		cpu = cpu_map__cpu(map, i);
+		bitmap[cpu / 8] |= 1 << (cpu % 8);
+	}
+
+	for (cpu = last_cpu / 4 * 4; cpu >= 0; cpu -= 4) {
+		unsigned char bits = bitmap[cpu / 8];
+
+		if (cpu % 8)
+			bits >>= 4;
+		else
+			bits &= 0xf;
+
+		*ptr++ = hex_char(bits);
+		if ((cpu % 32) == 0 && cpu > 0)
+			*ptr++ = ',';
+	}
+	*ptr = '\0';
+	free(bitmap);
+
+	buf[size - 1] = '\0';
+	return ptr - buf;
+}
diff --git a/tools/perf/util/cpumap.h b/tools/perf/util/cpumap.h
index 1a0549af8f5c..4d231c6eac0c 100644
--- a/tools/perf/util/cpumap.h
+++ b/tools/perf/util/cpumap.h
@@ -20,6 +20,7 @@ struct cpu_map *cpu_map__dummy_new(void);
 struct cpu_map *cpu_map__new_data(struct cpu_map_data *data);
 struct cpu_map *cpu_map__read(FILE *file);
 size_t cpu_map__snprint(struct cpu_map *map, char *buf, size_t size);
+size_t cpu_map__snprint_mask(struct cpu_map *map, char *buf, size_t size);
 size_t cpu_map__fprintf(struct cpu_map *map, FILE *fp);
 int cpu_map__get_socket_id(int cpu);
 int cpu_map__get_socket(struct cpu_map *map, int idx, void *data);
-- 
2.11.1

Re: [PATCH 2/4] perf tools: Introduce cpu_map__snprint_mask()

[Arnaldo Carvalho de Melo]

Em Fri, Feb 24, 2017 at 10:12:49AM +0900, Namhyung Kim escreveu:
> The cpu_map__snprint_mask() is to generate string representation of
> cpumask bitmap.  For cpu 0 to 11, it'll return "fff".
> 
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/util/cpumap.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>  tools/perf/util/cpumap.h |  1 +
>  2 files changed, 47 insertions(+)
> 
> diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
> index 8c7504939113..08540ab2a891 100644
> --- a/tools/perf/util/cpumap.c
> +++ b/tools/perf/util/cpumap.c
> @@ -673,3 +673,49 @@ size_t cpu_map__snprint(struct cpu_map *map, char *buf, size_t size)
>  	pr_debug("cpumask list: %s\n", buf);
>  	return ret;
>  }
> +
> +static char hex_char(char val)

Why do you use 'char' above and...

> +{
> +	if (0 <= val && val <= 9)
> +		return val + '0';
> +	if (10 <= val && val < 16)
> +		return val - 10 + 'a';
> +	return '?';
> +}
> +size_t cpu_map__snprint_mask(struct cpu_map *map, char *buf, size_t size)
<SNIP>
> +	for (cpu = last_cpu / 4 * 4; cpu >= 0; cpu -= 4) {
> +		unsigned char bits = bitmap[cpu / 8];

'unsigned char' here?

Some compilers don't like it, for instance:

  19 fedora:24-x-ARC-uClibc: FAIL

  CC       /tmp/build/perf/util/cpumap.o
util/cpumap.c: In function 'hex_char':
util/cpumap.c:679:2: error: comparison is always true due to limited range of data type [-Werror=type-limits]
  if (0 <= val && val <= 9)
  ^
cc1: all warnings being treated as errors

And:

  10 debian:experimental-x-arm64: FAIL

  CC       /tmp/build/perf/util/cpumap.o
util/cpumap.c: In function 'hex_char':
util/cpumap.c:679:8: error: comparison is always true due to limited range of data type [-Werror=type-limits]
  if (0 <= val && val <= 9)
        ^~

Are you ok with the patch below?

diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
index 6ab8699f0233..405f56ad5c24 100644
--- a/tools/perf/util/cpumap.c
+++ b/tools/perf/util/cpumap.c
@@ -687,7 +687,7 @@ size_t cpu_map__snprint_mask(struct cpu_map *map, char *buf, size_t size)
 {
 	int i, cpu;
 	char *ptr = buf;
-	unsigned char *bitmap;
+	char *bitmap;
 	int last_cpu = cpu_map__cpu(map, map->nr - 1);
 
 	bitmap = zalloc((last_cpu + 7) / 8);
@@ -702,7 +702,7 @@ size_t cpu_map__snprint_mask(struct cpu_map *map, char *buf, size_t size)
 	}
 
 	for (cpu = last_cpu / 4 * 4; cpu >= 0; cpu -= 4) {
-		unsigned char bits = bitmap[cpu / 8];
+		char bits = bitmap[cpu / 8];
 
 		if (cpu % 8)
 			bits >>= 4;


> +		if (cpu % 8)
> +			bits >>= 4;
> +		else
> +			bits &= 0xf;
> +
> +		*ptr++ = hex_char(bits);
> +		if ((cpu % 32) == 0 && cpu > 0)
> +			*ptr++ = ',';
> +	}
> +	*ptr = '\0';
> +	free(bitmap);
> +
> +	buf[size - 1] = '\0';
> +	return ptr - buf;
> +}
> diff --git a/tools/perf/util/cpumap.h b/tools/perf/util/cpumap.h
> index 1a0549af8f5c..4d231c6eac0c 100644
> --- a/tools/perf/util/cpumap.h
> +++ b/tools/perf/util/cpumap.h
> @@ -20,6 +20,7 @@ struct cpu_map *cpu_map__dummy_new(void);
>  struct cpu_map *cpu_map__new_data(struct cpu_map_data *data);
>  struct cpu_map *cpu_map__read(FILE *file);
>  size_t cpu_map__snprint(struct cpu_map *map, char *buf, size_t size);
> +size_t cpu_map__snprint_mask(struct cpu_map *map, char *buf, size_t size);
>  size_t cpu_map__fprintf(struct cpu_map *map, FILE *fp);
>  int cpu_map__get_socket_id(int cpu);
>  int cpu_map__get_socket(struct cpu_map *map, int idx, void *data);
> -- 
> 2.11.1

Re: [PATCH 2/4] perf tools: Introduce cpu_map__snprint_mask()

[Namhyung Kim]

Hi Arnaldo,

On Fri, Feb 24, 2017 at 06:08:53PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, Feb 24, 2017 at 10:12:49AM +0900, Namhyung Kim escreveu:
> > The cpu_map__snprint_mask() is to generate string representation of
> > cpumask bitmap.  For cpu 0 to 11, it'll return "fff".
> > 
> > Cc: Steven Rostedt <rostedt@goodmis.org>
> > Cc: Frederic Weisbecker <fweisbec@gmail.com>
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> >  tools/perf/util/cpumap.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
> >  tools/perf/util/cpumap.h |  1 +
> >  2 files changed, 47 insertions(+)
> > 
> > diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
> > index 8c7504939113..08540ab2a891 100644
> > --- a/tools/perf/util/cpumap.c
> > +++ b/tools/perf/util/cpumap.c
> > @@ -673,3 +673,49 @@ size_t cpu_map__snprint(struct cpu_map *map, char *buf, size_t size)
> >  	pr_debug("cpumask list: %s\n", buf);
> >  	return ret;
> >  }
> > +
> > +static char hex_char(char val)
> 
> Why do you use 'char' above and...

My bad.  The argument should be unsigned.  The semantics is that it
converts a bitmap value into a character (string).

> 
> > +{
> > +	if (0 <= val && val <= 9)
> > +		return val + '0';
> > +	if (10 <= val && val < 16)
> > +		return val - 10 + 'a';
> > +	return '?';
> > +}
> > +size_t cpu_map__snprint_mask(struct cpu_map *map, char *buf, size_t size)
> <SNIP>
> > +	for (cpu = last_cpu / 4 * 4; cpu >= 0; cpu -= 4) {
> > +		unsigned char bits = bitmap[cpu / 8];
> 
> 'unsigned char' here?
> 
> Some compilers don't like it, for instance:
> 
>   19 fedora:24-x-ARC-uClibc: FAIL
> 
>   CC       /tmp/build/perf/util/cpumap.o
> util/cpumap.c: In function 'hex_char':
> util/cpumap.c:679:2: error: comparison is always true due to limited range of data type [-Werror=type-limits]
>   if (0 <= val && val <= 9)
>   ^
> cc1: all warnings being treated as errors
> 
> And:
> 
>   10 debian:experimental-x-arm64: FAIL
> 
>   CC       /tmp/build/perf/util/cpumap.o
> util/cpumap.c: In function 'hex_char':
> util/cpumap.c:679:8: error: comparison is always true due to limited range of data type [-Werror=type-limits]
>   if (0 <= val && val <= 9)
>         ^~
> 
> Are you ok with the patch below?

I'd rather change hex_char() instead.  How about this?

diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
index 6ab8699f0233..061018b42393 100644
--- a/tools/perf/util/cpumap.c
+++ b/tools/perf/util/cpumap.c
@@ -674,11 +674,11 @@ size_t cpu_map__snprint(struct cpu_map *map, char *buf, size_t size)
        return ret;
 }
 
-static char hex_char(char val)
+static char hex_char(unsigned char val)
 {
-       if (0 <= val && val <= 9)
+       if (val < 10)
                return val + '0';
-       if (10 <= val && val < 16)
+       if (val < 16)
                return val - 10 + 'a';
        return '?';
 }

Re: [PATCH 2/4] perf tools: Introduce cpu_map__snprint_mask()

[Namhyung Kim]

Hi Arnaldo,

On Sat, Feb 25, 2017 at 1:27 PM, Namhyung Kim <namhyung@kernel.org> wrote:
> Hi Arnaldo,
>
> On Fri, Feb 24, 2017 at 06:08:53PM -0300, Arnaldo Carvalho de Melo wrote:
>> Em Fri, Feb 24, 2017 at 10:12:49AM +0900, Namhyung Kim escreveu:
>> > The cpu_map__snprint_mask() is to generate string representation of
>> > cpumask bitmap.  For cpu 0 to 11, it'll return "fff".
>> >
>> > Cc: Steven Rostedt <rostedt@goodmis.org>
>> > Cc: Frederic Weisbecker <fweisbec@gmail.com>
>> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
>> > ---
>> >  tools/perf/util/cpumap.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>> >  tools/perf/util/cpumap.h |  1 +
>> >  2 files changed, 47 insertions(+)
>> >
>> > diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
>> > index 8c7504939113..08540ab2a891 100644
>> > --- a/tools/perf/util/cpumap.c
>> > +++ b/tools/perf/util/cpumap.c
>> > @@ -673,3 +673,49 @@ size_t cpu_map__snprint(struct cpu_map *map, char *buf, size_t size)
>> >     pr_debug("cpumask list: %s\n", buf);
>> >     return ret;
>> >  }
>> > +
>> > +static char hex_char(char val)
>>
>> Why do you use 'char' above and...
>
> My bad.  The argument should be unsigned.  The semantics is that it
> converts a bitmap value into a character (string).
>
>>
>> > +{
>> > +   if (0 <= val && val <= 9)
>> > +           return val + '0';
>> > +   if (10 <= val && val < 16)
>> > +           return val - 10 + 'a';
>> > +   return '?';
>> > +}
>> > +size_t cpu_map__snprint_mask(struct cpu_map *map, char *buf, size_t size)
>> <SNIP>
>> > +   for (cpu = last_cpu / 4 * 4; cpu >= 0; cpu -= 4) {
>> > +           unsigned char bits = bitmap[cpu / 8];
>>
>> 'unsigned char' here?
>>
>> Some compilers don't like it, for instance:
>>
>>   19 fedora:24-x-ARC-uClibc: FAIL
>>
>>   CC       /tmp/build/perf/util/cpumap.o
>> util/cpumap.c: In function 'hex_char':
>> util/cpumap.c:679:2: error: comparison is always true due to limited range of data type [-Werror=type-limits]
>>   if (0 <= val && val <= 9)
>>   ^
>> cc1: all warnings being treated as errors
>>
>> And:
>>
>>   10 debian:experimental-x-arm64: FAIL
>>
>>   CC       /tmp/build/perf/util/cpumap.o
>> util/cpumap.c: In function 'hex_char':
>> util/cpumap.c:679:8: error: comparison is always true due to limited range of data type [-Werror=type-limits]
>>   if (0 <= val && val <= 9)
>>         ^~
>>
>> Are you ok with the patch below?
>
> I'd rather change hex_char() instead.  How about this?
>
> diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
> index 6ab8699f0233..061018b42393 100644
> --- a/tools/perf/util/cpumap.c
> +++ b/tools/perf/util/cpumap.c
> @@ -674,11 +674,11 @@ size_t cpu_map__snprint(struct cpu_map *map, char *buf, size_t size)
>         return ret;
>  }
>
> -static char hex_char(char val)
> +static char hex_char(unsigned char val)
>  {
> -       if (0 <= val && val <= 9)
> +       if (val < 10)
>                 return val + '0';
> -       if (10 <= val && val < 16)
> +       if (val < 16)
>                 return val - 10 + 'a';
>         return '?';
>  }
>

Ping!

Thanks,
Namhyung

Re: [PATCH 2/4] perf tools: Introduce cpu_map__snprint_mask()

[Arnaldo Carvalho de Melo]

Em Sat, Feb 25, 2017 at 01:27:34PM +0900, Namhyung Kim escreveu:
> On Fri, Feb 24, 2017 at 06:08:53PM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Fri, Feb 24, 2017 at 10:12:49AM +0900, Namhyung Kim escreveu:
> > util/cpumap.c:679:8: error: comparison is always true due to limited range of data type [-Werror=type-limits]
> >   if (0 <= val && val <= 9)
> >         ^~
> > Are you ok with the patch below?
 
> I'd rather change hex_char() instead.  How about this?

Ok, applying by hand, somehow it didn't apply using git-am...

- Arnaldo

> +++ b/tools/perf/util/cpumap.c
> @@ -674,11 +674,11 @@ size_t cpu_map__snprint(struct cpu_map *map, char *buf, size_t size)
>         return ret;
>  }
>  
> -static char hex_char(char val)
> +static char hex_char(unsigned char val)
>  {
> -       if (0 <= val && val <= 9)
> +       if (val < 10)
>                 return val + '0';
> -       if (10 <= val && val < 16)
> +       if (val < 16)
>                 return val - 10 + 'a';
>         return '?';
>  }

[tip:perf/core] perf cpumap: Introduce cpu_map__snprint_mask()

[tip-bot for Namhyung Kim]

Commit-ID:  4400ac8a9a900318f8516dc0fb94075cb3fdb50d
Gitweb:     http://git.kernel.org/tip/4400ac8a9a900318f8516dc0fb94075cb3fdb50d
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Fri, 24 Feb 2017 10:12:49 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 3 Mar 2017 19:07:17 -0300

perf cpumap: Introduce cpu_map__snprint_mask()

The cpu_map__snprint_mask() generates a string representation of a
cpumask bitmap.  For cpu 0 to 11, it'll return "fff".

Committer notes:

Fix compiler warning on some toolchains:

    19 fedora:24-x-ARC-uClibc: FAIL

    CC       /tmp/build/perf/util/cpumap.o
  util/cpumap.c: In function 'hex_char':
  util/cpumap.c:679:2: error: comparison is always true due to limited range of data type [-Werror=type-limits]
    if (0 <= val && val <= 9)
    ^
  cc1: all warnings being treated as errors

Applying patch from Namhyung that makes function receive an 'unsigned
char', that is what the callers are passing to this function.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: kernel-team@lge.com
Link: http://lkml.kernel.org/r/20170224011251.14946-2-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/cpumap.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/cpumap.h |  1 +
 2 files changed, 47 insertions(+)

diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
index 39ad2ca..061018b 100644
--- a/tools/perf/util/cpumap.c
+++ b/tools/perf/util/cpumap.c
@@ -673,3 +673,49 @@ size_t cpu_map__snprint(struct cpu_map *map, char *buf, size_t size)
 	pr_debug("cpumask list: %s\n", buf);
 	return ret;
 }
+
+static char hex_char(unsigned char val)
+{
+	if (val < 10)
+		return val + '0';
+	if (val < 16)
+		return val - 10 + 'a';
+	return '?';
+}
+
+size_t cpu_map__snprint_mask(struct cpu_map *map, char *buf, size_t size)
+{
+	int i, cpu;
+	char *ptr = buf;
+	unsigned char *bitmap;
+	int last_cpu = cpu_map__cpu(map, map->nr - 1);
+
+	bitmap = zalloc((last_cpu + 7) / 8);
+	if (bitmap == NULL) {
+		buf[0] = '\0';
+		return 0;
+	}
+
+	for (i = 0; i < map->nr; i++) {
+		cpu = cpu_map__cpu(map, i);
+		bitmap[cpu / 8] |= 1 << (cpu % 8);
+	}
+
+	for (cpu = last_cpu / 4 * 4; cpu >= 0; cpu -= 4) {
+		unsigned char bits = bitmap[cpu / 8];
+
+		if (cpu % 8)
+			bits >>= 4;
+		else
+			bits &= 0xf;
+
+		*ptr++ = hex_char(bits);
+		if ((cpu % 32) == 0 && cpu > 0)
+			*ptr++ = ',';
+	}
+	*ptr = '\0';
+	free(bitmap);
+
+	buf[size - 1] = '\0';
+	return ptr - buf;
+}
diff --git a/tools/perf/util/cpumap.h b/tools/perf/util/cpumap.h
index e844916..6b8bff8 100644
--- a/tools/perf/util/cpumap.h
+++ b/tools/perf/util/cpumap.h
@@ -20,6 +20,7 @@ struct cpu_map *cpu_map__dummy_new(void);
 struct cpu_map *cpu_map__new_data(struct cpu_map_data *data);
 struct cpu_map *cpu_map__read(FILE *file);
 size_t cpu_map__snprint(struct cpu_map *map, char *buf, size_t size);
+size_t cpu_map__snprint_mask(struct cpu_map *map, char *buf, size_t size);
 size_t cpu_map__fprintf(struct cpu_map *map, FILE *fp);
 int cpu_map__get_socket_id(int cpu);
 int cpu_map__get_socket(struct cpu_map *map, int idx, void *data);

[PATCH 3/4] perf ftrace: Add support for -a and -C option

[Namhyung Kim]

The -a/--all-cpus and -C/--cpu option is for controlling tracing cpus.

Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/Documentation/perf-ftrace.txt | 14 ++++++++
 tools/perf/builtin-ftrace.c              | 60 ++++++++++++++++++++++++++++++++
 2 files changed, 74 insertions(+)

diff --git a/tools/perf/Documentation/perf-ftrace.txt b/tools/perf/Documentation/perf-ftrace.txt
index 2d39397f3f30..6e6a8b22c859 100644
--- a/tools/perf/Documentation/perf-ftrace.txt
+++ b/tools/perf/Documentation/perf-ftrace.txt
@@ -34,6 +34,20 @@ OPTIONS
 --pid=::
 	Trace on existing process id (comma separated list).
 
+-a::
+--all-cpus::
+	Force system-wide collection.  Scripts run without a <command>
+	normally use -a by default, while scripts run with a <command>
+	normally don't - this option allows the latter to be run in
+	system-wide mode.
+
+-C::
+--cpu=::
+	Only trace for the list of CPUs provided.  Multiple CPUs can
+	be provided as a comma separated list with no space like: 0,1.
+	Ranges of CPUs are specified with -: 0-2.
+	Default is to trace on all online CPUs.
+
 
 SEE ALSO
 --------
diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
index 85eee9c444ae..d5b566ed7178 100644
--- a/tools/perf/builtin-ftrace.c
+++ b/tools/perf/builtin-ftrace.c
@@ -17,6 +17,7 @@
 #include <subcmd/parse-options.h>
 #include "evlist.h"
 #include "target.h"
+#include "cpumap.h"
 #include "thread_map.h"
 #include "util/config.h"
 
@@ -96,6 +97,8 @@ static int append_tracing_file(const char *name, const char *val)
 	return __write_tracing_file(name, val, true);
 }
 
+static int reset_tracing_cpu(void);
+
 static int reset_tracing_files(struct perf_ftrace *ftrace __maybe_unused)
 {
 	if (write_tracing_file("tracing_on", "0") < 0)
@@ -107,6 +110,9 @@ static int reset_tracing_files(struct perf_ftrace *ftrace __maybe_unused)
 	if (write_tracing_file("set_ftrace_pid", " ") < 0)
 		return -1;
 
+	if (reset_tracing_cpu() < 0)
+		return -1;
+
 	return 0;
 }
 
@@ -127,6 +133,51 @@ static int set_tracing_pid(struct perf_ftrace *ftrace)
 	return 0;
 }
 
+static int set_tracing_cpumask(struct cpu_map *cpumap)
+{
+	char *cpumask;
+	size_t mask_size;
+	int ret;
+	int last_cpu;
+
+	last_cpu = cpu_map__cpu(cpumap, cpumap->nr - 1);
+	mask_size = (last_cpu + 3) / 4 + 1;
+	mask_size += last_cpu / 32; /* ',' is needed for every 32th cpus */
+
+	cpumask = malloc(mask_size);
+	if (cpumask == NULL) {
+		pr_debug("failed to allocate cpu mask\n");
+		return -1;
+	}
+
+	cpu_map__snprint_mask(cpumap, cpumask, mask_size);
+
+	ret = write_tracing_file("tracing_cpumask", cpumask);
+
+	free(cpumask);
+	return ret;
+}
+
+static int set_tracing_cpu(struct perf_ftrace *ftrace)
+{
+	struct cpu_map *cpumap = ftrace->evlist->cpus;
+
+	if (!target__has_cpu(&ftrace->target))
+		return 0;
+
+	return set_tracing_cpumask(cpumap);
+}
+
+static int reset_tracing_cpu(void)
+{
+	struct cpu_map *cpumap = cpu_map__new(NULL);
+	int ret;
+
+	ret = set_tracing_cpumask(cpumap);
+	cpu_map__put(cpumap);
+	return ret;
+}
+
 static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
 {
 	char *trace_file;
@@ -163,6 +214,11 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
 		goto out_reset;
 	}
 
+	if (set_tracing_cpu(ftrace) < 0) {
+		pr_err("failed to set tracing cpumask\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;
@@ -264,6 +320,10 @@ int cmd_ftrace(int argc, const char **argv, const char *prefix __maybe_unused)
 		   "trace on existing process id"),
 	OPT_INCR('v', "verbose", &verbose,
 		 "be more verbose"),
+	OPT_BOOLEAN('a', "all-cpus", &ftrace.target.system_wide,
+		    "system-wide collection from all CPUs"),
+	OPT_STRING('C', "cpu", &ftrace.target.cpu_list, "cpu",
+		    "list of cpus to monitor"),
 	OPT_END()
 	};
 
-- 
2.11.1

[tip:perf/core] perf ftrace: Add support for -a and -C option

[tip-bot for Namhyung Kim]

Commit-ID:  dc23103278c5ad53c177a25e209ef687e6d5d293
Gitweb:     http://git.kernel.org/tip/dc23103278c5ad53c177a25e209ef687e6d5d293
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Fri, 24 Feb 2017 10:12:50 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 3 Mar 2017 19:07:17 -0300

perf ftrace: Add support for -a and -C option

The -a/--all-cpus and -C/--cpu option is for controlling tracing cpus.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: kernel-team@lge.com
Link: http://lkml.kernel.org/r/20170224011251.14946-3-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Documentation/perf-ftrace.txt | 14 ++++++++
 tools/perf/builtin-ftrace.c              | 60 ++++++++++++++++++++++++++++++++
 2 files changed, 74 insertions(+)

diff --git a/tools/perf/Documentation/perf-ftrace.txt b/tools/perf/Documentation/perf-ftrace.txt
index 2d39397..6e6a8b2 100644
--- a/tools/perf/Documentation/perf-ftrace.txt
+++ b/tools/perf/Documentation/perf-ftrace.txt
@@ -34,6 +34,20 @@ OPTIONS
 --pid=::
 	Trace on existing process id (comma separated list).
 
+-a::
+--all-cpus::
+	Force system-wide collection.  Scripts run without a <command>
+	normally use -a by default, while scripts run with a <command>
+	normally don't - this option allows the latter to be run in
+	system-wide mode.
+
+-C::
+--cpu=::
+	Only trace for the list of CPUs provided.  Multiple CPUs can
+	be provided as a comma separated list with no space like: 0,1.
+	Ranges of CPUs are specified with -: 0-2.
+	Default is to trace on all online CPUs.
+
 
 SEE ALSO
 --------
diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
index 85eee9c..d5b566e 100644
--- a/tools/perf/builtin-ftrace.c
+++ b/tools/perf/builtin-ftrace.c
@@ -17,6 +17,7 @@
 #include <subcmd/parse-options.h>
 #include "evlist.h"
 #include "target.h"
+#include "cpumap.h"
 #include "thread_map.h"
 #include "util/config.h"
 
@@ -96,6 +97,8 @@ static int append_tracing_file(const char *name, const char *val)
 	return __write_tracing_file(name, val, true);
 }
 
+static int reset_tracing_cpu(void);
+
 static int reset_tracing_files(struct perf_ftrace *ftrace __maybe_unused)
 {
 	if (write_tracing_file("tracing_on", "0") < 0)
@@ -107,6 +110,9 @@ static int reset_tracing_files(struct perf_ftrace *ftrace __maybe_unused)
 	if (write_tracing_file("set_ftrace_pid", " ") < 0)
 		return -1;
 
+	if (reset_tracing_cpu() < 0)
+		return -1;
+
 	return 0;
 }
 
@@ -127,6 +133,51 @@ static int set_tracing_pid(struct perf_ftrace *ftrace)
 	return 0;
 }
 
+static int set_tracing_cpumask(struct cpu_map *cpumap)
+{
+	char *cpumask;
+	size_t mask_size;
+	int ret;
+	int last_cpu;
+
+	last_cpu = cpu_map__cpu(cpumap, cpumap->nr - 1);
+	mask_size = (last_cpu + 3) / 4 + 1;
+	mask_size += last_cpu / 32; /* ',' is needed for every 32th cpus */
+
+	cpumask = malloc(mask_size);
+	if (cpumask == NULL) {
+		pr_debug("failed to allocate cpu mask\n");
+		return -1;
+	}
+
+	cpu_map__snprint_mask(cpumap, cpumask, mask_size);
+
+	ret = write_tracing_file("tracing_cpumask", cpumask);
+
+	free(cpumask);
+	return ret;
+}
+
+static int set_tracing_cpu(struct perf_ftrace *ftrace)
+{
+	struct cpu_map *cpumap = ftrace->evlist->cpus;
+
+	if (!target__has_cpu(&ftrace->target))
+		return 0;
+
+	return set_tracing_cpumask(cpumap);
+}
+
+static int reset_tracing_cpu(void)
+{
+	struct cpu_map *cpumap = cpu_map__new(NULL);
+	int ret;
+
+	ret = set_tracing_cpumask(cpumap);
+	cpu_map__put(cpumap);
+	return ret;
+}
+
 static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
 {
 	char *trace_file;
@@ -163,6 +214,11 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
 		goto out_reset;
 	}
 
+	if (set_tracing_cpu(ftrace) < 0) {
+		pr_err("failed to set tracing cpumask\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;
@@ -264,6 +320,10 @@ int cmd_ftrace(int argc, const char **argv, const char *prefix __maybe_unused)
 		   "trace on existing process id"),
 	OPT_INCR('v', "verbose", &verbose,
 		 "be more verbose"),
+	OPT_BOOLEAN('a', "all-cpus", &ftrace.target.system_wide,
+		    "system-wide collection from all CPUs"),
+	OPT_STRING('C', "cpu", &ftrace.target.cpu_list, "cpu",
+		    "list of cpus to monitor"),
 	OPT_END()
 	};
 

[PATCH 4/4] perf ftrace: Use pager for displaying result

[Namhyung Kim]

It's convenient to use pager when seeing many lines of result.

Note that setup_pager() should be called after perf_evlist__
prepare_workload() since they can interfere each other regarding
shared stdio streams.

Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 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 d5b566ed7178..6087295f8827 100644
--- a/tools/perf/builtin-ftrace.c
+++ b/tools/perf/builtin-ftrace.c
@@ -195,6 +195,7 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
 	signal(SIGINT, sig_handler);
 	signal(SIGUSR1, sig_handler);
 	signal(SIGCHLD, sig_handler);
+	signal(SIGPIPE, sig_handler);
 
 	if (reset_tracing_files(ftrace) < 0)
 		goto out;
@@ -247,6 +248,8 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
 		goto out_close_fd;
 	}
 
+	setup_pager();
+
 	perf_evlist__start_workload(ftrace->evlist);
 
 	while (!done) {
-- 
2.11.1

[tip:perf/core] perf ftrace: Use pager for displaying result

[tip-bot for Namhyung Kim]

Commit-ID:  583359646fde8526ea9456618cc24dc359b34094
Gitweb:     http://git.kernel.org/tip/583359646fde8526ea9456618cc24dc359b34094
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Fri, 24 Feb 2017 10:12:51 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 3 Mar 2017 19:07:17 -0300

perf ftrace: Use pager for displaying result

It's convenient to use the pager when seeing many lines of result.

Note that setup_pager() should be called after perf_evlist__prepare_workload()
since they can interfere each other regarding shared stdio streams.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: kernel-team@lge.com
Link: http://lkml.kernel.org/r/20170224011251.14946-4-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.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 d5b566e..6087295 100644
--- a/tools/perf/builtin-ftrace.c
+++ b/tools/perf/builtin-ftrace.c
@@ -195,6 +195,7 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
 	signal(SIGINT, sig_handler);
 	signal(SIGUSR1, sig_handler);
 	signal(SIGCHLD, sig_handler);
+	signal(SIGPIPE, sig_handler);
 
 	if (reset_tracing_files(ftrace) < 0)
 		goto out;
@@ -247,6 +248,8 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
 		goto out_close_fd;
 	}
 
+	setup_pager();
+
 	perf_evlist__start_workload(ftrace->evlist);
 
 	while (!done) {

Re: [PATCH 1/4] perf ftrace: Add support for --pid option

[Arnaldo Carvalho de Melo]

Em Fri, Feb 24, 2017 at 10:12:48AM +0900, Namhyung Kim escreveu:
> The -p (--pid) option enables to trace existing process by its pid.

Thanks, tested and applied the series, adding committer notes with
examples to this one with --pid.

- Arnaldo

[tip:perf/core] perf ftrace: Add support for --pid option

[tip-bot for Namhyung Kim]

Commit-ID:  a9af6be5bc25214f7870fef2b6d3490fe8b87bf7
Gitweb:     http://git.kernel.org/tip/a9af6be5bc25214f7870fef2b6d3490fe8b87bf7
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Fri, 24 Feb 2017 10:12:48 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 3 Mar 2017 19:07:16 -0300

perf ftrace: Add support for --pid option

The -p (--pid) option enables to trace existing process by its pid.

Committer notes:

Testing it:

Using the function_graph tracer on a process that is just waiting for user
input and thus will make 'perf ftrace' sit there waiting for that, then press
any key on that mutt session and see what happens:

  # perf ftrace -t function_graph -p `pidof mutt` | head -40
  2)   1.038 us    |  switch_mm_irqs_off();
  ------------------------------------------
  2)    <idle>-0    =>   mutt-3595
  ------------------------------------------

  2)               |              finish_task_switch() {
  2)               |                smp_irq_work_interrupt() {
  2)               |                  irq_enter() {
  2)   0.180 us    |                    rcu_irq_enter();
  2)   1.248 us    |                  }
  2)               |                  __wake_up() {
  2)   0.126 us    |                    _raw_spin_lock_irqsave();
  2)               |                    __wake_up_common() {
  2)               |                      pollwake() {
  2)               |                        default_wake_function() {
  2)               |                          try_to_wake_up() {
  2)   0.662 us    |                            _raw_spin_lock_irqsave();
  2)               |                            select_task_rq_fair() {
  2)   1.719 us    |                              effective_load.isra.41();
  2)   1.343 us    |                              effective_load.isra.41();
  2)               |                              select_idle_sibling() {
  2)   0.331 us    |                                idle_cpu();
  2)   1.458 us    |                              }
  2)   8.350 us    |                            }
  2)   0.200 us    |                            _raw_spin_lock();
  2)               |                            ttwu_do_activate() {
  2)               |                              activate_task() {
  2)   0.136 us    |                                update_rq_clock.part.77();
  2)               |                                enqueue_task_fair() {
  2)               |                                  enqueue_entity() {
  2)   0.146 us    |                                    update_curr();
  2)   0.330 us    |                                    account_entity_enqueue();
  2)   0.280 us    |                                    update_cfs_shares();
  2)   0.321 us    |                                    place_entity();
  2)   0.206 us    |                                    __enqueue_entity();
  2)   6.926 us    |                                  }
  2)               |                                  enqueue_entity() {
  2)   0.105 us    |                                    update_curr();
  2)   0.175 us    |                                    account_entity_enqueue();
  2)   0.531 us    |                                    update_cfs_shares();
 #

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: kernel-team@lge.com
Link: http://lkml.kernel.org/r/20170224011251.14946-1-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Documentation/perf-ftrace.txt |  4 ++
 tools/perf/builtin-ftrace.c              | 91 ++++++++++++++++++++++----------
 2 files changed, 68 insertions(+), 27 deletions(-)

diff --git a/tools/perf/Documentation/perf-ftrace.txt b/tools/perf/Documentation/perf-ftrace.txt
index 2d96de6..2d39397 100644
--- a/tools/perf/Documentation/perf-ftrace.txt
+++ b/tools/perf/Documentation/perf-ftrace.txt
@@ -30,6 +30,10 @@ OPTIONS
 --verbose=::
         Verbosity level.
 
+-p::
+--pid=::
+	Trace on existing process id (comma separated list).
+
 
 SEE ALSO
 --------
diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
index c3e6436..85eee9c 100644
--- a/tools/perf/builtin-ftrace.c
+++ b/tools/perf/builtin-ftrace.c
@@ -11,6 +11,7 @@
 
 #include <unistd.h>
 #include <signal.h>
+#include <fcntl.h>
 
 #include "debug.h"
 #include <subcmd/parse-options.h>
@@ -50,11 +51,12 @@ static void ftrace__workload_exec_failed_signal(int signo __maybe_unused,
 	done = true;
 }
 
-static int write_tracing_file(const char *name, const char *val)
+static int __write_tracing_file(const char *name, const char *val, bool append)
 {
 	char *file;
 	int fd, ret = -1;
 	ssize_t size = strlen(val);
+	int flags = O_WRONLY;
 
 	file = get_tracing_file(name);
 	if (!file) {
@@ -62,7 +64,12 @@ static int write_tracing_file(const char *name, const char *val)
 		return -1;
 	}
 
-	fd = open(file, O_WRONLY);
+	if (append)
+		flags |= O_APPEND;
+	else
+		flags |= O_TRUNC;
+
+	fd = open(file, flags);
 	if (fd < 0) {
 		pr_debug("cannot open tracing file: %s\n", name);
 		goto out;
@@ -79,6 +86,16 @@ out:
 	return ret;
 }
 
+static int write_tracing_file(const char *name, const char *val)
+{
+	return __write_tracing_file(name, val, false);
+}
+
+static int append_tracing_file(const char *name, const char *val)
+{
+	return __write_tracing_file(name, val, true);
+}
+
 static int reset_tracing_files(struct perf_ftrace *ftrace __maybe_unused)
 {
 	if (write_tracing_file("tracing_on", "0") < 0)
@@ -93,11 +110,27 @@ static int reset_tracing_files(struct perf_ftrace *ftrace __maybe_unused)
 	return 0;
 }
 
+static int set_tracing_pid(struct perf_ftrace *ftrace)
+{
+	int i;
+	char buf[16];
+
+	if (target__has_cpu(&ftrace->target))
+		return 0;
+
+	for (i = 0; i < thread_map__nr(ftrace->evlist->threads); i++) {
+		scnprintf(buf, sizeof(buf), "%d",
+			  ftrace->evlist->threads->map[i]);
+		if (append_tracing_file("set_ftrace_pid", buf) < 0)
+			return -1;
+	}
+	return 0;
+}
+
 static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
 {
 	char *trace_file;
 	int trace_fd;
-	char *trace_pid;
 	char buf[4096];
 	struct pollfd pollfd = {
 		.events = POLLIN,
@@ -108,42 +141,37 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
 		return -1;
 	}
 
-	if (argc < 1)
-		return -1;
-
 	signal(SIGINT, sig_handler);
 	signal(SIGUSR1, sig_handler);
 	signal(SIGCHLD, sig_handler);
 
-	reset_tracing_files(ftrace);
+	if (reset_tracing_files(ftrace) < 0)
+		goto out;
 
 	/* reset ftrace buffer */
 	if (write_tracing_file("trace", "0") < 0)
 		goto out;
 
-	if (perf_evlist__prepare_workload(ftrace->evlist, &ftrace->target,
-					  argv, false, ftrace__workload_exec_failed_signal) < 0)
-		goto out;
-
-	if (write_tracing_file("current_tracer", ftrace->tracer) < 0) {
-		pr_err("failed to set current_tracer to %s\n", ftrace->tracer);
+	if (argc && perf_evlist__prepare_workload(ftrace->evlist,
+				&ftrace->target, argv, false,
+				ftrace__workload_exec_failed_signal) < 0) {
 		goto out;
 	}
 
-	if (asprintf(&trace_pid, "%d", thread_map__pid(ftrace->evlist->threads, 0)) < 0) {
-		pr_err("failed to allocate pid string\n");
-		goto out;
+	if (set_tracing_pid(ftrace) < 0) {
+		pr_err("failed to set ftrace pid\n");
+		goto out_reset;
 	}
 
-	if (write_tracing_file("set_ftrace_pid", trace_pid) < 0) {
-		pr_err("failed to set pid: %s\n", trace_pid);
-		goto out_free_pid;
+	if (write_tracing_file("current_tracer", ftrace->tracer) < 0) {
+		pr_err("failed to set current_tracer to %s\n", ftrace->tracer);
+		goto out_reset;
 	}
 
 	trace_file = get_tracing_file("trace_pipe");
 	if (!trace_file) {
 		pr_err("failed to open trace_pipe\n");
-		goto out_free_pid;
+		goto out_reset;
 	}
 
 	trace_fd = open(trace_file, O_RDONLY);
@@ -152,7 +180,7 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
 
 	if (trace_fd < 0) {
 		pr_err("failed to open trace_pipe\n");
-		goto out_free_pid;
+		goto out_reset;
 	}
 
 	fcntl(trace_fd, F_SETFL, O_NONBLOCK);
@@ -191,11 +219,9 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
 
 out_close_fd:
 	close(trace_fd);
-out_free_pid:
-	free(trace_pid);
-out:
+out_reset:
 	reset_tracing_files(ftrace);
-
+out:
 	return done ? 0 : -1;
 }
 
@@ -227,13 +253,15 @@ int cmd_ftrace(int argc, const char **argv, const char *prefix __maybe_unused)
 		.target = { .uid = UINT_MAX, },
 	};
 	const char * const ftrace_usage[] = {
-		"perf ftrace [<options>] <command>",
+		"perf ftrace [<options>] [<command>]",
 		"perf ftrace [<options>] -- <command> [<options>]",
 		NULL
 	};
 	const struct option ftrace_options[] = {
 	OPT_STRING('t', "tracer", &ftrace.tracer, "tracer",
 		   "tracer to use: function_graph(default) or function"),
+	OPT_STRING('p', "pid", &ftrace.target.pid, "pid",
+		   "trace on existing process id"),
 	OPT_INCR('v', "verbose", &verbose,
 		 "be more verbose"),
 	OPT_END()
@@ -245,9 +273,18 @@ int cmd_ftrace(int argc, const char **argv, const char *prefix __maybe_unused)
 
 	argc = parse_options(argc, argv, ftrace_options, ftrace_usage,
 			    PARSE_OPT_STOP_AT_NON_OPTION);
-	if (!argc)
+	if (!argc && target__none(&ftrace.target))
 		usage_with_options(ftrace_usage, ftrace_options);
 
+	ret = target__validate(&ftrace.target);
+	if (ret) {
+		char errbuf[512];
+
+		target__strerror(&ftrace.target, ret, errbuf, 512);
+		pr_err("%s\n", errbuf);
+		return -EINVAL;
+	}
+
 	ftrace.evlist = perf_evlist__new();
 	if (ftrace.evlist == NULL)
 		return -ENOMEM;