[RFC PATCHSET 00/13] perf tools: Fix cpu/thread map handling v2

[RFC PATCHSET 00/13] perf tools: Fix cpu/thread map handling v2

[Namhyung Kim]

Hi,

This is a second iteration of my previous series [1] but group event handling
part already got mainlined separately.

The current behaviour of perf tools dealing with PID/TID, UID and CPU has
some implications and I think there're a few bugs - For example,
'perf record sleep 1' will create multiple events as many as number of online
cpus on the system. I don't think it's intended. This indeed makes perf test
fails on validation of PERF_RECORD_* event and perf_sample fields on my 6-core
(12-thread) system like this:

  namhyung@sejong:perf$ ./perf test -v 7
   7: Validate PERF_RECORD_* events & perf_sample fields:
  --- start ---
  perf_evlist__mmap: Operation not permitted
  ---- end ----
  Validate PERF_RECORD_* events & perf_sample fields: FAILED!

It's because perf_evlist__create_maps() created 12 cpu maps when no target PID,
TID, UID and CPU list is given (it's same as 'perf record sleep 1'), and then
perf_evlist__mmap() tried to mmap 256 pages for each cpu map so it hit a mlock
limit for a process. After this patch set was applied, the problem was gone.

During the cleanup I found some combinations of PID/TID, UID and CPU are not
allowed and have some implications. They need to be fixed and warned to user
explicitly IMHO, if needed. I think we have following implicit rules:

 * If -p option is given, -t option would be ignored.
 * If -p or -t option is given, -u, -C and/or -a options would be ignored.
 * If -u option is given (w/o -p or -t), -C and/or -a options would be ignored.

A subtle case is when -C option is given without -a option. I think it should
be treated as a system-wide mode as if -a option is given. Also if *NO* option
is given (like above examples) it should be treated as a task mode, not a
system-wide mode.

To make libperf more generic library, perf_target code use its own error code
and perf_target__strerror() as Arnaldo suggested. Although I tried to address
all of concerns he raised, I'm not 100% sure this is in the shape he wanted
to see finally. Comments on this area would be appreciated especially :).

Once it's settled down, perf_evlist__create_maps() and its related functions
can be converted to use perf_target_errno incrementally IMHO.

This series is based on latest tip/perf/core: 3dbe927b1edd ("Merge tag
'v3.4-rc4' into perf/core").

 * Changes from v1
  - Drop group handling patches since it's mainlined independently.
  - Rename 'struct perf_maps_ops' to 'struct perf_target' as Arnaldo suggested.
  - Introduce perf_target_strerror() for better error handling as Arnaldo
    suggested.
  - Add perf_target__parse_uid() function to replace parse_target_uid().
  - Not break python/twatch.py any more :).

Any comments are welcome, thanks.
Namhyung

[1] https://lkml.org/lkml/2012/2/13/57 - sorry, it wasn't threaded properly :(


Namhyung Kim (13):
  perf tools: Introduce struct perf_target
  perf stat: Convert to struct perf_target
  perf top: Convert to struct perf_target
  perf tools: Introduce perf_target__validate() helper
  perf tools: Make perf_evlist__create_maps() take struct perf_target
  perf tools: Check more combinations of PID/TID, UID and CPU switches
  perf evlist: Fix creation of cpu map
  perf target: Split out perf_target handling code
  perf target: Introduce perf_target_errno
  perf target: Introduce perf_target__parse_uid()
  perf tools: Introduce perf_target__strerror()
  perf target: Consolidate target task/cpu checking
  perf stat: Use perf_evlist__create_maps

 tools/perf/Makefile         |    2 +
 tools/perf/builtin-record.c |   48 ++++++++-------
 tools/perf/builtin-stat.c   |   58 ++++++++----------
 tools/perf/builtin-test.c   |    6 +-
 tools/perf/builtin-top.c    |   46 +++++++-------
 tools/perf/perf.h           |    8 +--
 tools/perf/util/debug.c     |    1 +
 tools/perf/util/evlist.c    |   16 ++---
 tools/perf/util/evlist.h    |    4 +-
 tools/perf/util/evsel.c     |    9 +--
 tools/perf/util/target.c    |  140 +++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/target.h    |   58 ++++++++++++++++++
 tools/perf/util/top.c       |   19 +++---
 tools/perf/util/top.h       |    6 +-
 tools/perf/util/usage.c     |   38 ------------
 tools/perf/util/util.h      |    3 -
 16 files changed, 311 insertions(+), 151 deletions(-)
 create mode 100644 tools/perf/util/target.c
 create mode 100644 tools/perf/util/target.h

-- 
1.7.10

[PATCH 01/13] perf tools: Introduce struct perf_target

[Namhyung Kim]

The perf_target struct will be used for taking care of cpu/thread
maps based on user's input. Since it is used on various subcommands
it'd better factoring it out.

Thanks to Arnaldo for suggesting the better name.

Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
---
 tools/perf/builtin-record.c |   41 ++++++++++++++++++++++-------------------
 tools/perf/builtin-test.c   |    5 +++--
 tools/perf/perf.h           |   15 ++++++++++-----
 tools/perf/util/evlist.c    |    2 +-
 tools/perf/util/evsel.c     |   10 +++++-----
 5 files changed, 41 insertions(+), 32 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 10b1f1f25ed7..4dcf27057bd2 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -44,7 +44,6 @@ struct perf_record {
 	struct perf_evlist	*evlist;
 	struct perf_session	*session;
 	const char		*progname;
-	const char		*uid_str;
 	int			output;
 	unsigned int		page_size;
 	int			realtime_prio;
@@ -218,7 +217,7 @@ try_again:
 			if (err == EPERM || err == EACCES) {
 				ui__error_paranoid();
 				exit(EXIT_FAILURE);
-			} else if (err ==  ENODEV && opts->cpu_list) {
+			} else if (err ==  ENODEV && opts->target.cpu_list) {
 				die("No such device - did you specify"
 					" an out-of-range profile CPU?\n");
 			} else if (err == EINVAL) {
@@ -578,7 +577,7 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
 		perf_session__process_machines(session, tool,
 					       perf_event__synthesize_guest_os);
 
-	if (!opts->system_wide)
+	if (!opts->target.system_wide)
 		perf_event__synthesize_thread_map(tool, evsel_list->threads,
 						  process_synthesized_event,
 						  machine);
@@ -765,9 +764,9 @@ const struct option record_options[] = {
 		     parse_events_option),
 	OPT_CALLBACK(0, "filter", &record.evlist, "filter",
 		     "event filter", parse_filter),
-	OPT_STRING('p', "pid", &record.opts.target_pid, "pid",
+	OPT_STRING('p', "pid", &record.opts.target.pid, "pid",
 		    "record events on existing process id"),
-	OPT_STRING('t', "tid", &record.opts.target_tid, "tid",
+	OPT_STRING('t', "tid", &record.opts.target.tid, "tid",
 		    "record events on existing thread id"),
 	OPT_INTEGER('r', "realtime", &record.realtime_prio,
 		    "collect data with this RT SCHED_FIFO priority"),
@@ -775,11 +774,11 @@ const struct option record_options[] = {
 		    "collect data without buffering"),
 	OPT_BOOLEAN('R', "raw-samples", &record.opts.raw_samples,
 		    "collect raw sample records from all opened counters"),
-	OPT_BOOLEAN('a', "all-cpus", &record.opts.system_wide,
+	OPT_BOOLEAN('a', "all-cpus", &record.opts.target.system_wide,
 			    "system-wide collection from all CPUs"),
 	OPT_BOOLEAN('A', "append", &record.append_file,
 			    "append to the output file to do incremental profiling"),
-	OPT_STRING('C', "cpu", &record.opts.cpu_list, "cpu",
+	OPT_STRING('C', "cpu", &record.opts.target.cpu_list, "cpu",
 		    "list of cpus to monitor"),
 	OPT_BOOLEAN('f', "force", &record.force,
 			"overwrite existing data file (deprecated)"),
@@ -813,7 +812,8 @@ const struct option record_options[] = {
 	OPT_CALLBACK('G', "cgroup", &record.evlist, "name",
 		     "monitor event in cgroup name only",
 		     parse_cgroups),
-	OPT_STRING('u', "uid", &record.uid_str, "user", "user to profile"),
+	OPT_STRING('u', "uid", &record.opts.target.uid_str, "user",
+		   "user to profile"),
 
 	OPT_CALLBACK_NOOPT('b', "branch-any", &record.opts.branch_stack,
 		     "branch any", "sample any taken branches",
@@ -842,8 +842,9 @@ int cmd_record(int argc, const char **argv, const char *prefix __used)
 
 	argc = parse_options(argc, argv, record_options, record_usage,
 			    PARSE_OPT_STOP_AT_NON_OPTION);
-	if (!argc && !rec->opts.target_pid && !rec->opts.target_tid &&
-		!rec->opts.system_wide && !rec->opts.cpu_list && !rec->uid_str)
+	if (!argc && !rec->opts.target.pid && !rec->opts.target.tid &&
+	    !rec->opts.target.system_wide && !rec->opts.target.cpu_list &&
+	    !rec->opts.target.uid_str)
 		usage_with_options(record_usage, record_options);
 
 	if (rec->force && rec->append_file) {
@@ -856,7 +857,7 @@ int cmd_record(int argc, const char **argv, const char *prefix __used)
 		rec->write_mode = WRITE_FORCE;
 	}
 
-	if (nr_cgroups && !rec->opts.system_wide) {
+	if (nr_cgroups && !rec->opts.target.system_wide) {
 		fprintf(stderr, "cgroup monitoring only available in"
 			" system-wide mode\n");
 		usage_with_options(record_usage, record_options);
@@ -883,17 +884,19 @@ int cmd_record(int argc, const char **argv, const char *prefix __used)
 		goto out_symbol_exit;
 	}
 
-	rec->opts.uid = parse_target_uid(rec->uid_str, rec->opts.target_tid,
-					 rec->opts.target_pid);
-	if (rec->uid_str != NULL && rec->opts.uid == UINT_MAX - 1)
+	rec->opts.target.uid = parse_target_uid(rec->opts.target.uid_str,
+						rec->opts.target.tid,
+						rec->opts.target.pid);
+	if (rec->opts.target.uid_str != NULL &&
+	    rec->opts.target.uid == UINT_MAX - 1)
 		goto out_free_fd;
 
-	if (rec->opts.target_pid)
-		rec->opts.target_tid = rec->opts.target_pid;
+	if (rec->opts.target.pid)
+		rec->opts.target.tid = rec->opts.target.pid;
 
-	if (perf_evlist__create_maps(evsel_list, rec->opts.target_pid,
-				     rec->opts.target_tid, rec->opts.uid,
-				     rec->opts.cpu_list) < 0)
+	if (perf_evlist__create_maps(evsel_list, rec->opts.target.pid,
+				     rec->opts.target.tid, rec->opts.target.uid,
+				     rec->opts.target.cpu_list) < 0)
 		usage_with_options(record_usage, record_options);
 
 	list_for_each_entry(pos, &evsel_list->entries, node) {
diff --git a/tools/perf/builtin-test.c b/tools/perf/builtin-test.c
index 1c5b9801ac61..e9b256920da0 100644
--- a/tools/perf/builtin-test.c
+++ b/tools/perf/builtin-test.c
@@ -1207,8 +1207,9 @@ static int test__PERF_RECORD(void)
 	 * perf_evlist__prepare_workload we'll fill in the only thread
 	 * we're monitoring, the one forked there.
 	 */
-	err = perf_evlist__create_maps(evlist, opts.target_pid,
-				       opts.target_tid, UINT_MAX, opts.cpu_list);
+	err = perf_evlist__create_maps(evlist, opts.target.pid,
+				       opts.target.tid, UINT_MAX,
+				       opts.target.cpu_list);
 	if (err < 0) {
 		pr_debug("Not enough memory to create thread/cpu maps\n");
 		goto out_delete_evlist;
diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index 89e3355ab173..7e226c0e0e31 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -207,10 +207,17 @@ extern const char perf_version_string[];
 
 void pthread__unblock_sigwinch(void);
 
-struct perf_record_opts {
-	const char   *target_pid;
-	const char   *target_tid;
+struct perf_target {
+	const char   *pid;
+	const char   *tid;
+	const char   *cpu_list;
+	const char   *uid_str;
 	uid_t	     uid;
+	bool	     system_wide;
+};
+
+struct perf_record_opts {
+	struct perf_target target;
 	bool	     call_graph;
 	bool	     group;
 	bool	     inherit_stat;
@@ -223,7 +230,6 @@ struct perf_record_opts {
 	bool	     sample_time;
 	bool	     sample_id_all_missing;
 	bool	     exclude_guest_missing;
-	bool	     system_wide;
 	bool	     period;
 	unsigned int freq;
 	unsigned int mmap_pages;
@@ -231,7 +237,6 @@ struct perf_record_opts {
 	int	     branch_stack;
 	u64	     default_interval;
 	u64	     user_interval;
-	const char   *cpu_list;
 };
 
 #endif
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 1986d8051bd1..7080901a2717 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -827,7 +827,7 @@ int perf_evlist__prepare_workload(struct perf_evlist *evlist,
 		exit(-1);
 	}
 
-	if (!opts->system_wide && !opts->target_tid && !opts->target_pid)
+	if (!opts->target.system_wide && !opts->target.tid && !opts->target.pid)
 		evlist->threads->map[0] = evlist->workload.pid;
 
 	close(child_ready_pipe[1]);
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 8c13dbcb84b9..d90598edcf1d 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -106,15 +106,15 @@ void perf_evsel__config(struct perf_evsel *evsel, struct perf_record_opts *opts,
 	if (opts->call_graph)
 		attr->sample_type	|= PERF_SAMPLE_CALLCHAIN;
 
-	if (opts->system_wide)
+	if (opts->target.system_wide)
 		attr->sample_type	|= PERF_SAMPLE_CPU;
 
 	if (opts->period)
 		attr->sample_type	|= PERF_SAMPLE_PERIOD;
 
 	if (!opts->sample_id_all_missing &&
-	    (opts->sample_time || opts->system_wide ||
-	     !opts->no_inherit || opts->cpu_list))
+	    (opts->sample_time || opts->target.system_wide ||
+	     !opts->no_inherit || opts->target.cpu_list))
 		attr->sample_type	|= PERF_SAMPLE_TIME;
 
 	if (opts->raw_samples) {
@@ -135,8 +135,8 @@ void perf_evsel__config(struct perf_evsel *evsel, struct perf_record_opts *opts,
 	attr->mmap = track;
 	attr->comm = track;
 
-	if (!opts->target_pid && !opts->target_tid && !opts->system_wide &&
-	    (!opts->group || evsel == first)) {
+	if (!opts->target.pid && !opts->target.tid &&
+	    !opts->target.system_wide && (!opts->group || evsel == first)) {
 		attr->disabled = 1;
 		attr->enable_on_exec = 1;
 	}
-- 
1.7.10

[PATCH 02/13] perf stat: Convert to struct perf_target

[Namhyung Kim]

Use struct perf_target as it is introduced by previous patch.
This is a preparation of further changes.

Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
---
 tools/perf/builtin-stat.c |   47 +++++++++++++++++++++------------------------
 1 file changed, 22 insertions(+), 25 deletions(-)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index dde9e17c018b..1ca767d906ef 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -175,22 +175,19 @@ static struct perf_event_attr very_very_detailed_attrs[] = {
 
 static struct perf_evlist	*evsel_list;
 
-static bool			system_wide			=  false;
-static int			run_idx				=  0;
+static struct perf_target	target;
 
+static int			run_idx				=  0;
 static int			run_count			=  1;
 static bool			no_inherit			= false;
 static bool			scale				=  true;
 static bool			no_aggr				= false;
-static const char		*target_pid;
-static const char		*target_tid;
 static pid_t			child_pid			= -1;
 static bool			null_run			=  false;
 static int			detailed_run			=  0;
 static bool			sync_run			=  false;
 static bool			big_num				=  true;
 static int			big_num_opt			=  -1;
-static const char		*cpu_list;
 static const char		*csv_sep			= NULL;
 static bool			csv_output			= false;
 static bool			group				= false;
@@ -293,10 +290,10 @@ static int create_perf_stat_counter(struct perf_evsel *evsel,
 
 	attr->inherit = !no_inherit;
 
-	if (system_wide)
+	if (target.system_wide)
 		return perf_evsel__open_per_cpu(evsel, evsel_list->cpus,
 						group, group_fd);
-	if (!target_pid && !target_tid && (!group || evsel == first)) {
+	if (!target.pid && !target.tid && (!group || evsel == first)) {
 		attr->disabled = 1;
 		attr->enable_on_exec = 1;
 	}
@@ -446,7 +443,7 @@ static int run_perf_stat(int argc __used, const char **argv)
 			exit(-1);
 		}
 
-		if (!target_tid && !target_pid && !system_wide)
+		if (!target.tid && !target.pid && !target.system_wide)
 			evsel_list->threads->map[0] = child_pid;
 
 		/*
@@ -476,7 +473,7 @@ static int run_perf_stat(int argc __used, const char **argv)
 				error("You may not have permission to collect %sstats.\n"
 				      "\t Consider tweaking"
 				      " /proc/sys/kernel/perf_event_paranoid or running as root.",
-				      system_wide ? "system-wide " : "");
+				      target.system_wide ? "system-wide " : "");
 			} else {
 				error("open_counter returned with %d (%s). "
 				      "/bin/dmesg may provide additional information.\n",
@@ -968,14 +965,14 @@ static void print_stat(int argc, const char **argv)
 	if (!csv_output) {
 		fprintf(output, "\n");
 		fprintf(output, " Performance counter stats for ");
-		if (!target_pid && !target_tid) {
+		if (!target.pid && !target.tid) {
 			fprintf(output, "\'%s", argv[0]);
 			for (i = 1; i < argc; i++)
 				fprintf(output, " %s", argv[i]);
-		} else if (target_pid)
-			fprintf(output, "process id \'%s", target_pid);
+		} else if (target.pid)
+			fprintf(output, "process id \'%s", target.pid);
 		else
-			fprintf(output, "thread id \'%s", target_tid);
+			fprintf(output, "thread id \'%s", target.tid);
 
 		fprintf(output, "\'");
 		if (run_count > 1)
@@ -1049,11 +1046,11 @@ static const struct option options[] = {
 		     "event filter", parse_filter),
 	OPT_BOOLEAN('i', "no-inherit", &no_inherit,
 		    "child tasks do not inherit counters"),
-	OPT_STRING('p', "pid", &target_pid, "pid",
+	OPT_STRING('p', "pid", &target.pid, "pid",
 		   "stat events on existing process id"),
-	OPT_STRING('t', "tid", &target_tid, "tid",
+	OPT_STRING('t', "tid", &target.tid, "tid",
 		   "stat events on existing thread id"),
-	OPT_BOOLEAN('a', "all-cpus", &system_wide,
+	OPT_BOOLEAN('a', "all-cpus", &target.system_wide,
 		    "system-wide collection from all CPUs"),
 	OPT_BOOLEAN('g', "group", &group,
 		    "put the counters into a counter group"),
@@ -1072,7 +1069,7 @@ static const struct option options[] = {
 	OPT_CALLBACK_NOOPT('B', "big-num", NULL, NULL, 
 			   "print large numbers with thousands\' separators",
 			   stat__set_big_num),
-	OPT_STRING('C', "cpu", &cpu_list, "cpu",
+	OPT_STRING('C', "cpu", &target.cpu_list, "cpu",
 		    "list of cpus to monitor in system-wide"),
 	OPT_BOOLEAN('A', "no-aggr", &no_aggr,
 		    "disable CPU count aggregation"),
@@ -1190,13 +1187,13 @@ int cmd_stat(int argc, const char **argv, const char *prefix __used)
 	} else if (big_num_opt == 0) /* User passed --no-big-num */
 		big_num = false;
 
-	if (!argc && !target_pid && !target_tid)
+	if (!argc && !target.pid && !target.tid)
 		usage_with_options(stat_usage, options);
 	if (run_count <= 0)
 		usage_with_options(stat_usage, options);
 
 	/* no_aggr, cgroup are for system-wide only */
-	if ((no_aggr || nr_cgroups) && !system_wide) {
+	if ((no_aggr || nr_cgroups) && !target.system_wide) {
 		fprintf(stderr, "both cgroup and no-aggregation "
 			"modes only available in system-wide mode\n");
 
@@ -1206,18 +1203,18 @@ int cmd_stat(int argc, const char **argv, const char *prefix __used)
 	if (add_default_attributes())
 		goto out;
 
-	if (target_pid)
-		target_tid = target_pid;
+	if (target.pid)
+		target.tid = target.pid;
 
-	evsel_list->threads = thread_map__new_str(target_pid,
-						  target_tid, UINT_MAX);
+	evsel_list->threads = thread_map__new_str(target.pid,
+						  target.tid, UINT_MAX);
 	if (evsel_list->threads == NULL) {
 		pr_err("Problems finding threads of monitor\n");
 		usage_with_options(stat_usage, options);
 	}
 
-	if (system_wide)
-		evsel_list->cpus = cpu_map__new(cpu_list);
+	if (target.system_wide)
+		evsel_list->cpus = cpu_map__new(target.cpu_list);
 	else
 		evsel_list->cpus = cpu_map__dummy_new();
 
-- 
1.7.10

[PATCH 03/13] perf top: Convert to struct perf_target

[Namhyung Kim]

Use struct perf_target as it is introduced by previous patch.
This is a preparation of further changes.

Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
---
 tools/perf/builtin-top.c |   33 +++++++++++++++++----------------
 tools/perf/util/top.c    |   19 ++++++++++---------
 tools/perf/util/top.h    |    6 +-----
 3 files changed, 28 insertions(+), 30 deletions(-)

diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 8ef59f8262bb..2c1c207627b4 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -588,7 +588,7 @@ static void *display_thread_tui(void *arg)
 	 * via --uid.
 	 */
 	list_for_each_entry(pos, &top->evlist->entries, node)
-		pos->hists.uid_filter_str = top->uid_str;
+		pos->hists.uid_filter_str = top->target.uid_str;
 
 	perf_evlist__tui_browse_hists(top->evlist, help,
 				      perf_top__sort_new_samples,
@@ -1016,7 +1016,7 @@ static int __cmd_top(struct perf_top *top)
 	if (ret)
 		goto out_delete;
 
-	if (top->target_tid || top->uid != UINT_MAX)
+	if (top->target.tid || top->target.uid != UINT_MAX)
 		perf_event__synthesize_thread_map(&top->tool, top->evlist->threads,
 						  perf_event__process,
 						  &top->session->host_machine);
@@ -1154,7 +1154,6 @@ int cmd_top(int argc, const char **argv, const char *prefix __used)
 	struct perf_top top = {
 		.count_filter	     = 5,
 		.delay_secs	     = 2,
-		.uid		     = UINT_MAX,
 		.freq		     = 1000, /* 1 KHz */
 		.mmap_pages	     = 128,
 		.sym_pcnt_filter     = 5,
@@ -1166,13 +1165,13 @@ int cmd_top(int argc, const char **argv, const char *prefix __used)
 		     parse_events_option),
 	OPT_INTEGER('c', "count", &top.default_interval,
 		    "event period to sample"),
-	OPT_STRING('p', "pid", &top.target_pid, "pid",
+	OPT_STRING('p', "pid", &top.target.pid, "pid",
 		    "profile events on existing process id"),
-	OPT_STRING('t', "tid", &top.target_tid, "tid",
+	OPT_STRING('t', "tid", &top.target.tid, "tid",
 		    "profile events on existing thread id"),
-	OPT_BOOLEAN('a', "all-cpus", &top.system_wide,
+	OPT_BOOLEAN('a', "all-cpus", &top.target.system_wide,
 			    "system-wide collection from all CPUs"),
-	OPT_STRING('C', "cpu", &top.cpu_list, "cpu",
+	OPT_STRING('C', "cpu", &top.target.cpu_list, "cpu",
 		    "list of cpus to monitor"),
 	OPT_STRING('k', "vmlinux", &symbol_conf.vmlinux_name,
 		   "file", "vmlinux pathname"),
@@ -1227,7 +1226,7 @@ int cmd_top(int argc, const char **argv, const char *prefix __used)
 		    "Display raw encoding of assembly instructions (default)"),
 	OPT_STRING('M', "disassembler-style", &disassembler_style, "disassembler style",
 		   "Specify disassembler style (e.g. -M intel for intel syntax)"),
-	OPT_STRING('u', "uid", &top.uid_str, "user", "user to profile"),
+	OPT_STRING('u', "uid", &top.target.uid_str, "user", "user to profile"),
 	OPT_END()
 	};
 
@@ -1253,22 +1252,24 @@ int cmd_top(int argc, const char **argv, const char *prefix __used)
 
 	setup_browser(false);
 
-	top.uid = parse_target_uid(top.uid_str, top.target_tid, top.target_pid);
-	if (top.uid_str != NULL && top.uid == UINT_MAX - 1)
+	top.target.uid = parse_target_uid(top.target.uid_str, top.target.tid,
+					  top.target.pid);
+	if (top.target.uid_str != NULL && top.target.uid == UINT_MAX - 1)
 		goto out_delete_evlist;
 
 	/* CPU and PID are mutually exclusive */
-	if (top.target_tid && top.cpu_list) {
+	if (top.target.tid && top.target.cpu_list) {
 		printf("WARNING: PID switch overriding CPU\n");
 		sleep(1);
-		top.cpu_list = NULL;
+		top.target.cpu_list = NULL;
 	}
 
-	if (top.target_pid)
-		top.target_tid = top.target_pid;
+	if (top.target.pid)
+		top.target.tid = top.target.pid;
 
-	if (perf_evlist__create_maps(top.evlist, top.target_pid,
-				     top.target_tid, top.uid, top.cpu_list) < 0)
+	if (perf_evlist__create_maps(top.evlist, top.target.pid,
+				     top.target.tid, top.target.uid,
+				     top.target.cpu_list) < 0)
 		usage_with_options(top_usage, options);
 
 	if (!top.evlist->nr_entries &&
diff --git a/tools/perf/util/top.c b/tools/perf/util/top.c
index 09fe579ccafb..abe0e8e95068 100644
--- a/tools/perf/util/top.c
+++ b/tools/perf/util/top.c
@@ -69,23 +69,24 @@ size_t perf_top__header_snprintf(struct perf_top *top, char *bf, size_t size)
 
 	ret += SNPRINTF(bf + ret, size - ret, "], ");
 
-	if (top->target_pid)
+	if (top->target.pid)
 		ret += SNPRINTF(bf + ret, size - ret, " (target_pid: %s",
-				top->target_pid);
-	else if (top->target_tid)
+				top->target.pid);
+	else if (top->target.tid)
 		ret += SNPRINTF(bf + ret, size - ret, " (target_tid: %s",
-				top->target_tid);
-	else if (top->uid_str != NULL)
+				top->target.tid);
+	else if (top->target.uid_str != NULL)
 		ret += SNPRINTF(bf + ret, size - ret, " (uid: %s",
-				top->uid_str);
+				top->target.uid_str);
 	else
 		ret += SNPRINTF(bf + ret, size - ret, " (all");
 
-	if (top->cpu_list)
+	if (top->target.cpu_list)
 		ret += SNPRINTF(bf + ret, size - ret, ", CPU%s: %s)",
-				top->evlist->cpus->nr > 1 ? "s" : "", top->cpu_list);
+				top->evlist->cpus->nr > 1 ? "s" : "",
+				top->target.cpu_list);
 	else {
-		if (top->target_tid)
+		if (top->target.tid)
 			ret += SNPRINTF(bf + ret, size - ret, ")");
 		else
 			ret += SNPRINTF(bf + ret, size - ret, ", %d CPU%s)",
diff --git a/tools/perf/util/top.h b/tools/perf/util/top.h
index ce61cb2d1acf..33347ca89ee4 100644
--- a/tools/perf/util/top.h
+++ b/tools/perf/util/top.h
@@ -13,6 +13,7 @@ struct perf_session;
 struct perf_top {
 	struct perf_tool   tool;
 	struct perf_evlist *evlist;
+	struct perf_target target;
 	/*
 	 * Symbols will be added here in perf_event__process_sample and will
 	 * get out after decayed.
@@ -23,10 +24,7 @@ struct perf_top {
 	u64		   guest_us_samples, guest_kernel_samples;
 	int		   print_entries, count_filter, delay_secs;
 	int		   freq;
-	const char	   *target_pid, *target_tid;
-	uid_t		   uid;
 	bool		   hide_kernel_symbols, hide_user_symbols, zero;
-	bool		   system_wide;
 	bool		   use_tui, use_stdio;
 	bool		   sort_has_symbols;
 	bool		   dont_use_callchains;
@@ -37,7 +35,6 @@ struct perf_top {
 	bool		   sample_id_all_missing;
 	bool		   exclude_guest_missing;
 	bool		   dump_symtab;
-	const char	   *cpu_list;
 	struct hist_entry  *sym_filter_entry;
 	struct perf_evsel  *sym_evsel;
 	struct perf_session *session;
@@ -47,7 +44,6 @@ struct perf_top {
 	int		   realtime_prio;
 	int		   sym_pcnt_filter;
 	const char	   *sym_filter;
-	const char	   *uid_str;
 };
 
 size_t perf_top__header_snprintf(struct perf_top *top, char *bf, size_t size);
-- 
1.7.10

[PATCH 04/13] perf tools: Introduce perf_target__validate() helper

[Namhyung Kim]

The perf_target__validate function is used to check given PID/TID/UID/CPU
target options and warn if some combination is impossible. Also this can
make some arguments of parse_target_uid() function useless as it is checked
before the call via our new helper.

Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
---
 tools/perf/builtin-record.c |    9 +++------
 tools/perf/builtin-stat.c   |    3 +--
 tools/perf/builtin-top.c    |   15 +++------------
 tools/perf/util/usage.c     |   29 +++++++++++++++++++++--------
 tools/perf/util/util.h      |    4 +++-
 5 files changed, 31 insertions(+), 29 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 4dcf27057bd2..3596ccab6d3b 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -884,16 +884,13 @@ int cmd_record(int argc, const char **argv, const char *prefix __used)
 		goto out_symbol_exit;
 	}
 
-	rec->opts.target.uid = parse_target_uid(rec->opts.target.uid_str,
-						rec->opts.target.tid,
-						rec->opts.target.pid);
+	perf_target__validate(&rec->opts.target);
+
+	rec->opts.target.uid = parse_target_uid(rec->opts.target.uid_str);
 	if (rec->opts.target.uid_str != NULL &&
 	    rec->opts.target.uid == UINT_MAX - 1)
 		goto out_free_fd;
 
-	if (rec->opts.target.pid)
-		rec->opts.target.tid = rec->opts.target.pid;
-
 	if (perf_evlist__create_maps(evsel_list, rec->opts.target.pid,
 				     rec->opts.target.tid, rec->opts.target.uid,
 				     rec->opts.target.cpu_list) < 0)
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 1ca767d906ef..bb7723221c0d 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -1203,8 +1203,7 @@ int cmd_stat(int argc, const char **argv, const char *prefix __used)
 	if (add_default_attributes())
 		goto out;
 
-	if (target.pid)
-		target.tid = target.pid;
+	perf_target__validate(&target);
 
 	evsel_list->threads = thread_map__new_str(target.pid,
 						  target.tid, UINT_MAX);
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 2c1c207627b4..4f47952eddbd 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1252,21 +1252,12 @@ int cmd_top(int argc, const char **argv, const char *prefix __used)
 
 	setup_browser(false);
 
-	top.target.uid = parse_target_uid(top.target.uid_str, top.target.tid,
-					  top.target.pid);
+	perf_target__validate(&top.target);
+
+	top.target.uid = parse_target_uid(top.target.uid_str);
 	if (top.target.uid_str != NULL && top.target.uid == UINT_MAX - 1)
 		goto out_delete_evlist;
 
-	/* CPU and PID are mutually exclusive */
-	if (top.target.tid && top.target.cpu_list) {
-		printf("WARNING: PID switch overriding CPU\n");
-		sleep(1);
-		top.target.cpu_list = NULL;
-	}
-
-	if (top.target.pid)
-		top.target.tid = top.target.pid;
-
 	if (perf_evlist__create_maps(top.evlist, top.target.pid,
 				     top.target.tid, top.target.uid,
 				     top.target.cpu_list) < 0)
diff --git a/tools/perf/util/usage.c b/tools/perf/util/usage.c
index 52bb07c6442a..0a1a885a5914 100644
--- a/tools/perf/util/usage.c
+++ b/tools/perf/util/usage.c
@@ -83,7 +83,7 @@ void warning(const char *warn, ...)
 	va_end(params);
 }
 
-uid_t parse_target_uid(const char *str, const char *tid, const char *pid)
+uid_t parse_target_uid(const char *str)
 {
 	struct passwd pwd, *result;
 	char buf[1024];
@@ -91,13 +91,6 @@ uid_t parse_target_uid(const char *str, const char *tid, const char *pid)
 	if (str == NULL)
 		return UINT_MAX;
 
-	/* UID and PID are mutually exclusive */
-	if (tid || pid) {
-		ui__warning("PID/TID switch overriding UID\n");
-		sleep(1);
-		return UINT_MAX;
-	}
-
 	getpwnam_r(str, &pwd, buf, sizeof(buf), &result);
 
 	if (result == NULL) {
@@ -120,3 +113,23 @@ uid_t parse_target_uid(const char *str, const char *tid, const char *pid)
 
 	return result->pw_uid;
 }
+
+void perf_target__validate(struct perf_target *target)
+{
+	if (target->pid)
+		target->tid = target->pid;
+
+	/* CPU and PID are mutually exclusive */
+	if (target->tid && target->cpu_list) {
+		ui__warning("WARNING: PID switch overriding CPU\n");
+		sleep(1);
+		target->cpu_list = NULL;
+	}
+
+	/* UID and PID are mutually exclusive */
+	if (target->tid && target->uid_str) {
+		ui__warning("PID/TID switch overriding UID\n");
+		sleep(1);
+		target->uid_str = NULL;
+	}
+}
diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index 0f99f394d8e0..3f05d6264dab 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -246,10 +246,12 @@ unsigned long convert_unit(unsigned long value, char *unit);
 int readn(int fd, void *buf, size_t size);
 
 struct perf_event_attr;
+struct perf_target;
 
 void event_attr_init(struct perf_event_attr *attr);
 
-uid_t parse_target_uid(const char *str, const char *tid, const char *pid);
+uid_t parse_target_uid(const char *str);
+void perf_target__validate(struct perf_target *target);
 
 #define _STR(x) #x
 #define STR(x) _STR(x)
-- 
1.7.10

[PATCH 05/13] perf tools: Make perf_evlist__create_maps() take struct perf_target

[Namhyung Kim]

Now we have all information that needed to create cpu/thread maps
in struct perf_target, it'd be better using it as an argument.

Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
---
 tools/perf/builtin-record.c |    4 +---
 tools/perf/builtin-test.c   |    7 ++++---
 tools/perf/builtin-top.c    |    4 +---
 tools/perf/util/evlist.c    |   12 +++++++-----
 tools/perf/util/evlist.h    |    4 ++--
 5 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 3596ccab6d3b..d16590942cec 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -891,9 +891,7 @@ int cmd_record(int argc, const char **argv, const char *prefix __used)
 	    rec->opts.target.uid == UINT_MAX - 1)
 		goto out_free_fd;
 
-	if (perf_evlist__create_maps(evsel_list, rec->opts.target.pid,
-				     rec->opts.target.tid, rec->opts.target.uid,
-				     rec->opts.target.cpu_list) < 0)
+	if (perf_evlist__create_maps(evsel_list, &rec->opts.target) < 0)
 		usage_with_options(record_usage, record_options);
 
 	list_for_each_entry(pos, &evsel_list->entries, node) {
diff --git a/tools/perf/builtin-test.c b/tools/perf/builtin-test.c
index e9b256920da0..a281284ee2f5 100644
--- a/tools/perf/builtin-test.c
+++ b/tools/perf/builtin-test.c
@@ -1165,6 +1165,9 @@ realloc:
 static int test__PERF_RECORD(void)
 {
 	struct perf_record_opts opts = {
+		.target = {
+			.uid = UINT_MAX,
+		},
 		.no_delay   = true,
 		.freq	    = 10,
 		.mmap_pages = 256,
@@ -1207,9 +1210,7 @@ static int test__PERF_RECORD(void)
 	 * perf_evlist__prepare_workload we'll fill in the only thread
 	 * we're monitoring, the one forked there.
 	 */
-	err = perf_evlist__create_maps(evlist, opts.target.pid,
-				       opts.target.tid, UINT_MAX,
-				       opts.target.cpu_list);
+	err = perf_evlist__create_maps(evlist, &opts.target);
 	if (err < 0) {
 		pr_debug("Not enough memory to create thread/cpu maps\n");
 		goto out_delete_evlist;
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 4f47952eddbd..2a0ec09b9b77 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1258,9 +1258,7 @@ int cmd_top(int argc, const char **argv, const char *prefix __used)
 	if (top.target.uid_str != NULL && top.target.uid == UINT_MAX - 1)
 		goto out_delete_evlist;
 
-	if (perf_evlist__create_maps(top.evlist, top.target.pid,
-				     top.target.tid, top.target.uid,
-				     top.target.cpu_list) < 0)
+	if (perf_evlist__create_maps(top.evlist, &top.target) < 0)
 		usage_with_options(top_usage, options);
 
 	if (!top.evlist->nr_entries &&
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 7080901a2717..a43e2c56d1c6 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -599,18 +599,20 @@ int perf_evlist__mmap(struct perf_evlist *evlist, unsigned int pages,
 	return perf_evlist__mmap_per_cpu(evlist, prot, mask);
 }
 
-int perf_evlist__create_maps(struct perf_evlist *evlist, const char *target_pid,
-			     const char *target_tid, uid_t uid, const char *cpu_list)
+int perf_evlist__create_maps(struct perf_evlist *evlist,
+			     struct perf_target *target)
 {
-	evlist->threads = thread_map__new_str(target_pid, target_tid, uid);
+	evlist->threads = thread_map__new_str(target->pid, target->tid,
+					      target->uid);
 
 	if (evlist->threads == NULL)
 		return -1;
 
-	if (uid != UINT_MAX || (cpu_list == NULL && target_tid))
+	if (target->uid != UINT_MAX ||
+	    (target->cpu_list == NULL && target->tid))
 		evlist->cpus = cpu_map__dummy_new();
 	else
-		evlist->cpus = cpu_map__new(cpu_list);
+		evlist->cpus = cpu_map__new(target->cpu_list);
 
 	if (evlist->cpus == NULL)
 		goto out_delete_threads;
diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
index 21f1c9e57f13..58abb63ac13a 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -106,8 +106,8 @@ static inline void perf_evlist__set_maps(struct perf_evlist *evlist,
 	evlist->threads	= threads;
 }
 
-int perf_evlist__create_maps(struct perf_evlist *evlist, const char *target_pid,
-			     const char *tid, uid_t uid, const char *cpu_list);
+int perf_evlist__create_maps(struct perf_evlist *evlist,
+			     struct perf_target *target);
 void perf_evlist__delete_maps(struct perf_evlist *evlist);
 int perf_evlist__set_filters(struct perf_evlist *evlist);
 
-- 
1.7.10

[PATCH 06/13] perf tools: Check more combinations of PID/TID, UID and CPU switches

[Namhyung Kim]

There were some combinations of these switches that are not so
appropriate IMHO. Since there are implicit priorities between
them and they worked well anyway, but it ends up opening useless
duplicated events. For example, 'perf stat -t <pid> -a' will
open multiple events for the thread instead of one.

Add explicit checks and warn user in perf_target__validate().

Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
---
 tools/perf/util/usage.c |   14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/tools/perf/util/usage.c b/tools/perf/util/usage.c
index 0a1a885a5914..228f0a558872 100644
--- a/tools/perf/util/usage.c
+++ b/tools/perf/util/usage.c
@@ -132,4 +132,18 @@ void perf_target__validate(struct perf_target *target)
 		sleep(1);
 		target->uid_str = NULL;
 	}
+
+	/* UID and CPU are mutually exclusive */
+	if (target->uid_str && target->cpu_list) {
+		ui__warning("UID switch overriding CPU\n");
+		sleep(1);
+		target->cpu_list = NULL;
+	}
+
+	/* PID/UID and SYSTEM are mutually exclusive */
+	if ((target->tid || target->uid_str) && target->system_wide) {
+		ui__warning("PID/TID/UID switch overriding CPU\n");
+		sleep(1);
+		target->system_wide = false;
+	}
 }
-- 
1.7.10

[PATCH 07/13] perf evlist: Fix creation of cpu map

[Namhyung Kim]

Currently, 'perf record -- sleep 1' creates a cpu map for all online
cpus since it turns out calling cpu_map__new(NULL). Fix it. Also it
is guaranteed that cpu_list is NULL if PID/TID is given by calling
perf_target__validate(), so we can make the conditional bit simpler.

This also fixes perf test 7 (Validate) failure on my 6 core machine:

  $ cat /sys/devices/system/cpu/online 
  0-11
  $ ./perf test -v 7
   7: Validate PERF_RECORD_* events & perf_sample fields:
  --- start ---
  perf_evlist__mmap: Operation not permitted
  ---- end ----
  Validate PERF_RECORD_* events & perf_sample fields: FAILED!

Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
---
 tools/perf/util/evlist.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index a43e2c56d1c6..556d98af1a0b 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -608,8 +608,9 @@ int perf_evlist__create_maps(struct perf_evlist *evlist,
 	if (evlist->threads == NULL)
 		return -1;
 
-	if (target->uid != UINT_MAX ||
-	    (target->cpu_list == NULL && target->tid))
+	if (target->uid != UINT_MAX || target->tid)
+		evlist->cpus = cpu_map__dummy_new();
+	else if (!target->system_wide && target->cpu_list == NULL)
 		evlist->cpus = cpu_map__dummy_new();
 	else
 		evlist->cpus = cpu_map__new(target->cpu_list);
-- 
1.7.10

[PATCH 08/13] perf target: Split out perf_target handling code

[Namhyung Kim]

For further work on perf_target, it'd be better off splitting
the code into a separate file.

Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
---
 tools/perf/Makefile      |    2 ++
 tools/perf/perf.h        |    9 +--------
 tools/perf/util/evlist.c |    1 +
 tools/perf/util/evsel.c  |    1 +
 tools/perf/util/target.c |   45 +++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/target.h |   17 +++++++++++++++++
 tools/perf/util/usage.c  |   34 ----------------------------------
 tools/perf/util/util.h   |    2 --
 8 files changed, 67 insertions(+), 44 deletions(-)
 create mode 100644 tools/perf/util/target.c
 create mode 100644 tools/perf/util/target.h

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index e98e14c88532..4122a668952e 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -300,6 +300,7 @@ LIB_H += util/cpumap.h
 LIB_H += util/top.h
 LIB_H += $(ARCH_INCLUDE)
 LIB_H += util/cgroup.h
+LIB_H += util/target.h
 
 LIB_OBJS += $(OUTPUT)util/abspath.o
 LIB_OBJS += $(OUTPUT)util/alias.o
@@ -361,6 +362,7 @@ LIB_OBJS += $(OUTPUT)util/util.o
 LIB_OBJS += $(OUTPUT)util/xyarray.o
 LIB_OBJS += $(OUTPUT)util/cpumap.o
 LIB_OBJS += $(OUTPUT)util/cgroup.o
+LIB_OBJS += $(OUTPUT)util/target.o
 
 BUILTIN_OBJS += $(OUTPUT)builtin-annotate.o
 
diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index 7e226c0e0e31..14f1034f14f9 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -207,14 +207,7 @@ extern const char perf_version_string[];
 
 void pthread__unblock_sigwinch(void);
 
-struct perf_target {
-	const char   *pid;
-	const char   *tid;
-	const char   *cpu_list;
-	const char   *uid_str;
-	uid_t	     uid;
-	bool	     system_wide;
-};
+#include "util/target.h"
 
 struct perf_record_opts {
 	struct perf_target target;
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 556d98af1a0b..183b199b0d09 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -11,6 +11,7 @@
 #include <poll.h>
 #include "cpumap.h"
 #include "thread_map.h"
+#include "target.h"
 #include "evlist.h"
 #include "evsel.h"
 #include <unistd.h>
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index d90598edcf1d..bb785a098ced 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -14,6 +14,7 @@
 #include "util.h"
 #include "cpumap.h"
 #include "thread_map.h"
+#include "target.h"
 
 #define FD(e, x, y) (*(int *)xyarray__entry(e->fd, x, y))
 #define GROUP_FD(group_fd, cpu) (*(int *)xyarray__entry(group_fd, cpu, 0))
diff --git a/tools/perf/util/target.c b/tools/perf/util/target.c
new file mode 100644
index 000000000000..3fadf85dd7e3
--- /dev/null
+++ b/tools/perf/util/target.c
@@ -0,0 +1,45 @@
+/*
+ * Helper functions for handling target threads/cpus
+ *
+ * Copyright (C) 2012, LG Electronics, Namhyung Kim <namhyung.kim@lge.com>
+ *
+ * Released under the GPL v2.
+ */
+
+#include "target.h"
+#include "debug.h"
+
+
+void perf_target__validate(struct perf_target *target)
+{
+	if (target->pid)
+		target->tid = target->pid;
+
+	/* CPU and PID are mutually exclusive */
+	if (target->tid && target->cpu_list) {
+		ui__warning("WARNING: PID switch overriding CPU\n");
+		sleep(1);
+		target->cpu_list = NULL;
+	}
+
+	/* UID and PID are mutually exclusive */
+	if (target->tid && target->uid_str) {
+		ui__warning("PID/TID switch overriding UID\n");
+		sleep(1);
+		target->uid_str = NULL;
+	}
+
+	/* UID and CPU are mutually exclusive */
+	if (target->uid_str && target->cpu_list) {
+		ui__warning("UID switch overriding CPU\n");
+		sleep(1);
+		target->cpu_list = NULL;
+	}
+
+	/* PID/UID and SYSTEM are mutually exclusive */
+	if ((target->tid || target->uid_str) && target->system_wide) {
+		ui__warning("PID/TID/UID switch overriding CPU\n");
+		sleep(1);
+		target->system_wide = false;
+	}
+}
diff --git a/tools/perf/util/target.h b/tools/perf/util/target.h
new file mode 100644
index 000000000000..1348065ada5e
--- /dev/null
+++ b/tools/perf/util/target.h
@@ -0,0 +1,17 @@
+#ifndef _PERF_TARGET_H
+#define _PERF_TARGET_H
+
+#include "util.h"
+
+struct perf_target {
+	const char   *pid;
+	const char   *tid;
+	const char   *cpu_list;
+	const char   *uid_str;
+	uid_t	     uid;
+	bool	     system_wide;
+};
+
+void perf_target__validate(struct perf_target *target);
+
+#endif /* _PERF_TARGET_H */
diff --git a/tools/perf/util/usage.c b/tools/perf/util/usage.c
index 228f0a558872..e851abc22ccc 100644
--- a/tools/perf/util/usage.c
+++ b/tools/perf/util/usage.c
@@ -113,37 +113,3 @@ uid_t parse_target_uid(const char *str)
 
 	return result->pw_uid;
 }
-
-void perf_target__validate(struct perf_target *target)
-{
-	if (target->pid)
-		target->tid = target->pid;
-
-	/* CPU and PID are mutually exclusive */
-	if (target->tid && target->cpu_list) {
-		ui__warning("WARNING: PID switch overriding CPU\n");
-		sleep(1);
-		target->cpu_list = NULL;
-	}
-
-	/* UID and PID are mutually exclusive */
-	if (target->tid && target->uid_str) {
-		ui__warning("PID/TID switch overriding UID\n");
-		sleep(1);
-		target->uid_str = NULL;
-	}
-
-	/* UID and CPU are mutually exclusive */
-	if (target->uid_str && target->cpu_list) {
-		ui__warning("UID switch overriding CPU\n");
-		sleep(1);
-		target->cpu_list = NULL;
-	}
-
-	/* PID/UID and SYSTEM are mutually exclusive */
-	if ((target->tid || target->uid_str) && target->system_wide) {
-		ui__warning("PID/TID/UID switch overriding CPU\n");
-		sleep(1);
-		target->system_wide = false;
-	}
-}
diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index 3f05d6264dab..52be74c359d3 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -246,12 +246,10 @@ unsigned long convert_unit(unsigned long value, char *unit);
 int readn(int fd, void *buf, size_t size);
 
 struct perf_event_attr;
-struct perf_target;
 
 void event_attr_init(struct perf_event_attr *attr);
 
 uid_t parse_target_uid(const char *str);
-void perf_target__validate(struct perf_target *target);
 
 #define _STR(x) #x
 #define STR(x) _STR(x)
-- 
1.7.10

[PATCH 09/13] perf target: Introduce perf_target_errno

[Namhyung Kim]

The perf_target_errno enumerations are used to indicate
specific error cases on perf target operations. It'd
help libperf being a more generic library.

Suggested-by: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
---
 tools/perf/util/target.c |   33 ++++++++++++++++++++++-----------
 tools/perf/util/target.h |   20 +++++++++++++++++++-
 2 files changed, 41 insertions(+), 12 deletions(-)

diff --git a/tools/perf/util/target.c b/tools/perf/util/target.c
index 3fadf85dd7e3..04cc374b7c32 100644
--- a/tools/perf/util/target.c
+++ b/tools/perf/util/target.c
@@ -10,36 +10,47 @@
 #include "debug.h"
 
 
-void perf_target__validate(struct perf_target *target)
+enum perf_target_errno perf_target__validate(struct perf_target *target)
 {
+	enum perf_target_errno ret = PERF_TARGET__SUCCESS;
+
 	if (target->pid)
 		target->tid = target->pid;
 
 	/* CPU and PID are mutually exclusive */
 	if (target->tid && target->cpu_list) {
-		ui__warning("WARNING: PID switch overriding CPU\n");
-		sleep(1);
 		target->cpu_list = NULL;
+		if (ret == PERF_TARGET__SUCCESS)
+			ret = PERF_TARGET__PID_OVERRIDE_CPU;
 	}
 
 	/* UID and PID are mutually exclusive */
 	if (target->tid && target->uid_str) {
-		ui__warning("PID/TID switch overriding UID\n");
-		sleep(1);
 		target->uid_str = NULL;
+		if (ret == PERF_TARGET__SUCCESS)
+			ret = PERF_TARGET__PID_OVERRIDE_UID;
 	}
 
 	/* UID and CPU are mutually exclusive */
 	if (target->uid_str && target->cpu_list) {
-		ui__warning("UID switch overriding CPU\n");
-		sleep(1);
 		target->cpu_list = NULL;
+		if (ret == PERF_TARGET__SUCCESS)
+			ret = PERF_TARGET__UID_OVERRIDE_CPU;
 	}
 
-	/* PID/UID and SYSTEM are mutually exclusive */
-	if ((target->tid || target->uid_str) && target->system_wide) {
-		ui__warning("PID/TID/UID switch overriding CPU\n");
-		sleep(1);
+	/* PID and SYSTEM are mutually exclusive */
+	if (target->tid && target->system_wide) {
 		target->system_wide = false;
+		if (ret == PERF_TARGET__SUCCESS)
+			ret = PERF_TARGET__PID_OVERRIDE_SYSTEM;
 	}
+
+	/* UID and SYSTEM are mutually exclusive */
+	if (target->uid_str && target->system_wide) {
+		target->system_wide = false;
+		if (ret == PERF_TARGET__SUCCESS)
+			ret = PERF_TARGET__UID_OVERRIDE_SYSTEM;
+	}
+
+	return ret;
 }
diff --git a/tools/perf/util/target.h b/tools/perf/util/target.h
index 1348065ada5e..c3914c8a9890 100644
--- a/tools/perf/util/target.h
+++ b/tools/perf/util/target.h
@@ -12,6 +12,24 @@ struct perf_target {
 	bool	     system_wide;
 };
 
-void perf_target__validate(struct perf_target *target);
+enum perf_target_errno {
+	/*
+	 * XXX: Just choose an arbitrary big number standard errno can't have
+	 */
+	__PERF_TARGET__ERRNO_START		= 0x10000,
+
+	PERF_TARGET__SUCCESS			= __PERF_TARGET__ERRNO_START,
+
+	/* for perf_target__validate() */
+	PERF_TARGET__PID_OVERRIDE_CPU,
+	PERF_TARGET__PID_OVERRIDE_UID,
+	PERF_TARGET__UID_OVERRIDE_CPU,
+	PERF_TARGET__PID_OVERRIDE_SYSTEM,
+	PERF_TARGET__UID_OVERRIDE_SYSTEM,
+
+	__PERF_TARGET__ERRNO_END
+};
+
+enum perf_target_errno perf_target__validate(struct perf_target *target);
 
 #endif /* _PERF_TARGET_H */
-- 
1.7.10

[PATCH 10/13] perf target: Introduce perf_target__parse_uid()

[Namhyung Kim]

Add and use the modern perf_target__parse_uid() and get rid of
the old parse_target_uid().

Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
---
 tools/perf/builtin-record.c |    4 +---
 tools/perf/builtin-top.c    |    3 +--
 tools/perf/util/target.c    |   35 +++++++++++++++++++++++++++++++++++
 tools/perf/util/target.h    |    5 +++++
 tools/perf/util/usage.c     |   31 -------------------------------
 tools/perf/util/util.h      |    3 ---
 6 files changed, 42 insertions(+), 39 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index d16590942cec..e5d77452eef6 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -886,9 +886,7 @@ int cmd_record(int argc, const char **argv, const char *prefix __used)
 
 	perf_target__validate(&rec->opts.target);
 
-	rec->opts.target.uid = parse_target_uid(rec->opts.target.uid_str);
-	if (rec->opts.target.uid_str != NULL &&
-	    rec->opts.target.uid == UINT_MAX - 1)
+	if (perf_target__parse_uid(&rec->opts.target) != PERF_TARGET__SUCCESS)
 		goto out_free_fd;
 
 	if (perf_evlist__create_maps(evsel_list, &rec->opts.target) < 0)
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 2a0ec09b9b77..80fccc7bd1e0 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1254,8 +1254,7 @@ int cmd_top(int argc, const char **argv, const char *prefix __used)
 
 	perf_target__validate(&top.target);
 
-	top.target.uid = parse_target_uid(top.target.uid_str);
-	if (top.target.uid_str != NULL && top.target.uid == UINT_MAX - 1)
+	if (perf_target__parse_uid(&top.target) != PERF_TARGET__SUCCESS)
 		goto out_delete_evlist;
 
 	if (perf_evlist__create_maps(top.evlist, &top.target) < 0)
diff --git a/tools/perf/util/target.c b/tools/perf/util/target.c
index 04cc374b7c32..140adfdfd520 100644
--- a/tools/perf/util/target.c
+++ b/tools/perf/util/target.c
@@ -9,6 +9,8 @@
 #include "target.h"
 #include "debug.h"
 
+#include <pwd.h>
+
 
 enum perf_target_errno perf_target__validate(struct perf_target *target)
 {
@@ -54,3 +56,36 @@ enum perf_target_errno perf_target__validate(struct perf_target *target)
 
 	return ret;
 }
+
+enum perf_target_errno perf_target__parse_uid(struct perf_target *target)
+{
+	struct passwd pwd, *result;
+	char buf[1024];
+	const char *str = target->uid_str;
+
+	target->uid = UINT_MAX;
+	if (str == NULL)
+		return PERF_TARGET__SUCCESS;
+
+	/* Try user name first */
+	getpwnam_r(str, &pwd, buf, sizeof(buf), &result);
+
+	if (result == NULL) {
+		/*
+		 * The user name not found. Maybe it's a UID number.
+		 */
+		char *endptr;
+		int uid = strtol(str, &endptr, 10);
+
+		if (*endptr != '\0')
+			return PERF_TARGET__INVALID_UID;
+
+		getpwuid_r(uid, &pwd, buf, sizeof(buf), &result);
+
+		if (result == NULL)
+			return PERF_TARGET__USER_NOT_FOUND;
+	}
+
+	target->uid = result->pw_uid;
+	return PERF_TARGET__SUCCESS;
+}
diff --git a/tools/perf/util/target.h b/tools/perf/util/target.h
index c3914c8a9890..ec35fc043260 100644
--- a/tools/perf/util/target.h
+++ b/tools/perf/util/target.h
@@ -27,9 +27,14 @@ enum perf_target_errno {
 	PERF_TARGET__PID_OVERRIDE_SYSTEM,
 	PERF_TARGET__UID_OVERRIDE_SYSTEM,
 
+	/* for perf_target__parse_uid() */
+	PERF_TARGET__INVALID_UID,
+	PERF_TARGET__USER_NOT_FOUND,
+
 	__PERF_TARGET__ERRNO_END
 };
 
 enum perf_target_errno perf_target__validate(struct perf_target *target);
+enum perf_target_errno perf_target__parse_uid(struct perf_target *target);
 
 #endif /* _PERF_TARGET_H */
diff --git a/tools/perf/util/usage.c b/tools/perf/util/usage.c
index e851abc22ccc..4007aca8e0ca 100644
--- a/tools/perf/util/usage.c
+++ b/tools/perf/util/usage.c
@@ -82,34 +82,3 @@ void warning(const char *warn, ...)
 	warn_routine(warn, params);
 	va_end(params);
 }
-
-uid_t parse_target_uid(const char *str)
-{
-	struct passwd pwd, *result;
-	char buf[1024];
-
-	if (str == NULL)
-		return UINT_MAX;
-
-	getpwnam_r(str, &pwd, buf, sizeof(buf), &result);
-
-	if (result == NULL) {
-		char *endptr;
-		int uid = strtol(str, &endptr, 10);
-
-		if (*endptr != '\0') {
-			ui__error("Invalid user %s\n", str);
-			return UINT_MAX - 1;
-		}
-
-		getpwuid_r(uid, &pwd, buf, sizeof(buf), &result);
-
-		if (result == NULL) {
-			ui__error("Problems obtaining information for user %s\n",
-				  str);
-			return UINT_MAX - 1;
-		}
-	}
-
-	return result->pw_uid;
-}
diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index 52be74c359d3..27a11a78ad39 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -74,7 +74,6 @@
 #include <netinet/tcp.h>
 #include <arpa/inet.h>
 #include <netdb.h>
-#include <pwd.h>
 #include <inttypes.h>
 #include "../../../include/linux/magic.h"
 #include "types.h"
@@ -249,8 +248,6 @@ struct perf_event_attr;
 
 void event_attr_init(struct perf_event_attr *attr);
 
-uid_t parse_target_uid(const char *str);
-
 #define _STR(x) #x
 #define STR(x) _STR(x)
 
-- 
1.7.10

[PATCH 11/13] perf tools: Introduce perf_target__strerror()

[Namhyung Kim]

The perf_target__strerror() sets @buf to a string that
describes the (perf_target-specific) error condition
that is passed via @errnum.

This is similar to strerror_r() and does same thing if
@errnum has a standard errno value.

Suggested-by: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
---
 tools/perf/builtin-record.c |   18 ++++++++++++++--
 tools/perf/builtin-top.c    |   19 ++++++++++++++---
 tools/perf/util/debug.c     |    1 +
 tools/perf/util/target.c    |   49 +++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/target.h    |    3 +++
 5 files changed, 85 insertions(+), 5 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index e5d77452eef6..8caa84034a51 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -831,6 +831,7 @@ int cmd_record(int argc, const char **argv, const char *prefix __used)
 	struct perf_evsel *pos;
 	struct perf_evlist *evsel_list;
 	struct perf_record *rec = &record;
+	char errbuf[BUFSIZ];
 
 	perf_header__set_cmdline(argc, argv);
 
@@ -884,11 +885,24 @@ int cmd_record(int argc, const char **argv, const char *prefix __used)
 		goto out_symbol_exit;
 	}
 
-	perf_target__validate(&rec->opts.target);
+	err = perf_target__validate(&rec->opts.target);
+	if (err != PERF_TARGET__SUCCESS) {
+		perf_target__strerror(&rec->opts.target, err, errbuf, BUFSIZ);
+		ui__warning("%s", errbuf);
+	}
+
+	err = perf_target__parse_uid(&rec->opts.target);
+	if (err != PERF_TARGET__SUCCESS) {
+		int saved_errno = errno;
 
-	if (perf_target__parse_uid(&rec->opts.target) != PERF_TARGET__SUCCESS)
+		perf_target__strerror(&rec->opts.target, err, errbuf, BUFSIZ);
+		ui__warning("%s", errbuf);
+
+		err = -saved_errno;
 		goto out_free_fd;
+	}
 
+	err = -ENOMEM;
 	if (perf_evlist__create_maps(evsel_list, &rec->opts.target) < 0)
 		usage_with_options(record_usage, record_options);
 
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 80fccc7bd1e0..6f584179ecc1 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1150,7 +1150,8 @@ static const char * const top_usage[] = {
 int cmd_top(int argc, const char **argv, const char *prefix __used)
 {
 	struct perf_evsel *pos;
-	int status = -ENOMEM;
+	int status;
+	char errbuf[BUFSIZ];
 	struct perf_top top = {
 		.count_filter	     = 5,
 		.delay_secs	     = 2,
@@ -1252,10 +1253,22 @@ int cmd_top(int argc, const char **argv, const char *prefix __used)
 
 	setup_browser(false);
 
-	perf_target__validate(&top.target);
+	status = perf_target__validate(&top.target);
+	if (status != PERF_TARGET__SUCCESS) {
+		perf_target__strerror(&top.target, status, errbuf, BUFSIZ);
+		ui__warning("%s", errbuf);
+	}
+
+	status = perf_target__parse_uid(&top.target);
+	if (status != PERF_TARGET__SUCCESS) {
+		int saved_errno = errno;
 
-	if (perf_target__parse_uid(&top.target) != PERF_TARGET__SUCCESS)
+		perf_target__strerror(&top.target, status, errbuf, BUFSIZ);
+		ui__warning("%s", errbuf);
+
+		status = -saved_errno;
 		goto out_delete_evlist;
+	}
 
 	if (perf_evlist__create_maps(top.evlist, &top.target) < 0)
 		usage_with_options(top_usage, options);
diff --git a/tools/perf/util/debug.c b/tools/perf/util/debug.c
index 26817daa2961..efb1fce259a4 100644
--- a/tools/perf/util/debug.c
+++ b/tools/perf/util/debug.c
@@ -11,6 +11,7 @@
 #include "event.h"
 #include "debug.h"
 #include "util.h"
+#include "target.h"
 
 int verbose;
 bool dump_trace = false, quiet = false;
diff --git a/tools/perf/util/target.c b/tools/perf/util/target.c
index 140adfdfd520..196a24390bcf 100644
--- a/tools/perf/util/target.c
+++ b/tools/perf/util/target.c
@@ -89,3 +89,52 @@ enum perf_target_errno perf_target__parse_uid(struct perf_target *target)
 	target->uid = result->pw_uid;
 	return PERF_TARGET__SUCCESS;
 }
+
+/*
+ * This must have a same ordering as the enum perf_target_errno.
+ */
+static const char *perf_target__error_str[] = {
+	"Success",
+	"PID/TID switch overriding CPU",
+	"PID/TID switch overriding UID",
+	"UID switch overriding CPU",
+	"PID/TID switch overriding SYSTEM",
+	"UID switch overriding SYSTEM",
+	"Invalid User: %s",
+	"Problems obtaining information for user %s",
+};
+
+int perf_target__strerror(struct perf_target *target, int errnum,
+			  char *buf, size_t buflen)
+{
+	const char *msg;
+
+	if (errnum <  __PERF_TARGET__ERRNO_START) {
+		strerror_r(errnum, buf, buflen);
+		return 0;
+	}
+
+	if (errnum >= __PERF_TARGET__ERRNO_END)
+		return -1;
+
+	errnum -= __PERF_TARGET__ERRNO_START;
+	msg = perf_target__error_str[errnum];
+
+	switch (errnum) {
+	case PERF_TARGET__SUCCESS:
+	case PERF_TARGET__PID_OVERRIDE_CPU ... PERF_TARGET__UID_OVERRIDE_SYSTEM:
+		snprintf(buf, buflen, "%s", msg);
+		break;
+
+	case PERF_TARGET__INVALID_UID:
+	case PERF_TARGET__USER_NOT_FOUND:
+		snprintf(buf, buflen, msg, target->uid_str);
+		break;
+
+	default:
+		/* cannot reach here */
+		break;
+	}
+
+	return 0;
+}
diff --git a/tools/perf/util/target.h b/tools/perf/util/target.h
index ec35fc043260..fff28c18f401 100644
--- a/tools/perf/util/target.h
+++ b/tools/perf/util/target.h
@@ -37,4 +37,7 @@ enum perf_target_errno {
 enum perf_target_errno perf_target__validate(struct perf_target *target);
 enum perf_target_errno perf_target__parse_uid(struct perf_target *target);
 
+int perf_target__strerror(struct perf_target *target, int errnum, char *buf,
+			  size_t buflen);
+
 #endif /* _PERF_TARGET_H */
-- 
1.7.10

[PATCH 12/13] perf target: Consolidate target task/cpu checking

[Namhyung Kim]

There are places that check whether target task/cpu is given or not
and some of them didn't check newly introduced uid or cpu list. Add
and use three of helper functions to treat them properly.

Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
---
 tools/perf/builtin-record.c |    4 +---
 tools/perf/builtin-stat.c   |   12 ++++++------
 tools/perf/builtin-top.c    |    2 +-
 tools/perf/util/evlist.c    |   10 ++++------
 tools/perf/util/evsel.c     |    8 ++++----
 tools/perf/util/target.h    |   15 +++++++++++++++
 6 files changed, 31 insertions(+), 20 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 8caa84034a51..9c0fa105e063 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -843,9 +843,7 @@ int cmd_record(int argc, const char **argv, const char *prefix __used)
 
 	argc = parse_options(argc, argv, record_options, record_usage,
 			    PARSE_OPT_STOP_AT_NON_OPTION);
-	if (!argc && !rec->opts.target.pid && !rec->opts.target.tid &&
-	    !rec->opts.target.system_wide && !rec->opts.target.cpu_list &&
-	    !rec->opts.target.uid_str)
+	if (!argc && perf_target__none(&rec->opts.target))
 		usage_with_options(record_usage, record_options);
 
 	if (rec->force && rec->append_file) {
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index bb7723221c0d..d9ff24637eeb 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -290,10 +290,10 @@ static int create_perf_stat_counter(struct perf_evsel *evsel,
 
 	attr->inherit = !no_inherit;
 
-	if (target.system_wide)
+	if (!perf_target__no_cpu(&target))
 		return perf_evsel__open_per_cpu(evsel, evsel_list->cpus,
 						group, group_fd);
-	if (!target.pid && !target.tid && (!group || evsel == first)) {
+	if (perf_target__no_task(&target) && (!group || evsel == first)) {
 		attr->disabled = 1;
 		attr->enable_on_exec = 1;
 	}
@@ -443,7 +443,7 @@ static int run_perf_stat(int argc __used, const char **argv)
 			exit(-1);
 		}
 
-		if (!target.tid && !target.pid && !target.system_wide)
+		if (perf_target__none(&target))
 			evsel_list->threads->map[0] = child_pid;
 
 		/*
@@ -965,7 +965,7 @@ static void print_stat(int argc, const char **argv)
 	if (!csv_output) {
 		fprintf(output, "\n");
 		fprintf(output, " Performance counter stats for ");
-		if (!target.pid && !target.tid) {
+		if (perf_target__no_task(&target)) {
 			fprintf(output, "\'%s", argv[0]);
 			for (i = 1; i < argc; i++)
 				fprintf(output, " %s", argv[i]);
@@ -1187,13 +1187,13 @@ int cmd_stat(int argc, const char **argv, const char *prefix __used)
 	} else if (big_num_opt == 0) /* User passed --no-big-num */
 		big_num = false;
 
-	if (!argc && !target.pid && !target.tid)
+	if (!argc && perf_target__no_task(&target))
 		usage_with_options(stat_usage, options);
 	if (run_count <= 0)
 		usage_with_options(stat_usage, options);
 
 	/* no_aggr, cgroup are for system-wide only */
-	if ((no_aggr || nr_cgroups) && !target.system_wide) {
+	if ((no_aggr || nr_cgroups) && perf_target__no_cpu(&target)) {
 		fprintf(stderr, "both cgroup and no-aggregation "
 			"modes only available in system-wide mode\n");
 
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 6f584179ecc1..948eff6be59a 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1016,7 +1016,7 @@ static int __cmd_top(struct perf_top *top)
 	if (ret)
 		goto out_delete;
 
-	if (top->target.tid || top->target.uid != UINT_MAX)
+	if (!perf_target__no_task(&top->target))
 		perf_event__synthesize_thread_map(&top->tool, top->evlist->threads,
 						  perf_event__process,
 						  &top->session->host_machine);
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 183b199b0d09..1201daf71719 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -609,12 +609,10 @@ int perf_evlist__create_maps(struct perf_evlist *evlist,
 	if (evlist->threads == NULL)
 		return -1;
 
-	if (target->uid != UINT_MAX || target->tid)
-		evlist->cpus = cpu_map__dummy_new();
-	else if (!target->system_wide && target->cpu_list == NULL)
-		evlist->cpus = cpu_map__dummy_new();
-	else
+	if (!perf_target__no_cpu(target))
 		evlist->cpus = cpu_map__new(target->cpu_list);
+	else
+		evlist->cpus = cpu_map__dummy_new();
 
 	if (evlist->cpus == NULL)
 		goto out_delete_threads;
@@ -831,7 +829,7 @@ int perf_evlist__prepare_workload(struct perf_evlist *evlist,
 		exit(-1);
 	}
 
-	if (!opts->target.system_wide && !opts->target.tid && !opts->target.pid)
+	if (perf_target__none(&opts->target))
 		evlist->threads->map[0] = evlist->workload.pid;
 
 	close(child_ready_pipe[1]);
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index bb785a098ced..21eaab240396 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -114,8 +114,8 @@ void perf_evsel__config(struct perf_evsel *evsel, struct perf_record_opts *opts,
 		attr->sample_type	|= PERF_SAMPLE_PERIOD;
 
 	if (!opts->sample_id_all_missing &&
-	    (opts->sample_time || opts->target.system_wide ||
-	     !opts->no_inherit || opts->target.cpu_list))
+	    (opts->sample_time || !opts->no_inherit ||
+	     !perf_target__no_cpu(&opts->target)))
 		attr->sample_type	|= PERF_SAMPLE_TIME;
 
 	if (opts->raw_samples) {
@@ -136,8 +136,8 @@ void perf_evsel__config(struct perf_evsel *evsel, struct perf_record_opts *opts,
 	attr->mmap = track;
 	attr->comm = track;
 
-	if (!opts->target.pid && !opts->target.tid &&
-	    !opts->target.system_wide && (!opts->group || evsel == first)) {
+	if (perf_target__none(&opts->target) &&
+	    (!opts->group || evsel == first)) {
 		attr->disabled = 1;
 		attr->enable_on_exec = 1;
 	}
diff --git a/tools/perf/util/target.h b/tools/perf/util/target.h
index fff28c18f401..c9d7ee587fe2 100644
--- a/tools/perf/util/target.h
+++ b/tools/perf/util/target.h
@@ -40,4 +40,19 @@ enum perf_target_errno perf_target__parse_uid(struct perf_target *target);
 int perf_target__strerror(struct perf_target *target, int errnum, char *buf,
 			  size_t buflen);
 
+static inline bool perf_target__no_task(struct perf_target *target)
+{
+	return !target->pid && !target->tid && !target->uid_str;
+}
+
+static inline bool perf_target__no_cpu(struct perf_target *target)
+{
+	return !target->system_wide && !target->cpu_list;
+}
+
+static inline bool perf_target__none(struct perf_target *target)
+{
+	return perf_target__no_task(target) && perf_target__no_cpu(target);
+}
+
 #endif /* _PERF_TARGET_H */
-- 
1.7.10

[PATCH 13/13] perf stat: Use perf_evlist__create_maps

[Namhyung Kim]

Use same function with perf record and top to share the code
checks combinations of different switches.

Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
---
 tools/perf/builtin-stat.c |   22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index d9ff24637eeb..e720ba7b801e 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -175,7 +175,9 @@ static struct perf_event_attr very_very_detailed_attrs[] = {
 
 static struct perf_evlist	*evsel_list;
 
-static struct perf_target	target;
+static struct perf_target	target = {
+	.uid	= UINT_MAX,
+};
 
 static int			run_idx				=  0;
 static int			run_count			=  1;
@@ -1205,20 +1207,12 @@ int cmd_stat(int argc, const char **argv, const char *prefix __used)
 
 	perf_target__validate(&target);
 
-	evsel_list->threads = thread_map__new_str(target.pid,
-						  target.tid, UINT_MAX);
-	if (evsel_list->threads == NULL) {
-		pr_err("Problems finding threads of monitor\n");
-		usage_with_options(stat_usage, options);
-	}
-
-	if (target.system_wide)
-		evsel_list->cpus = cpu_map__new(target.cpu_list);
-	else
-		evsel_list->cpus = cpu_map__dummy_new();
+	if (perf_evlist__create_maps(evsel_list, &target) < 0) {
+		if (!perf_target__no_task(&target))
+			pr_err("Problems finding threads of monitor\n");
+		if (!perf_target__no_cpu(&target))
+			perror("failed to parse CPUs map");
 
-	if (evsel_list->cpus == NULL) {
-		perror("failed to parse CPUs map");
 		usage_with_options(stat_usage, options);
 		return -1;
 	}
-- 
1.7.10

Re: [PATCH 07/13] perf evlist: Fix creation of cpu map

[David Ahern]

On 4/25/12 11:15 PM, Namhyung Kim wrote:
> Currently, 'perf record -- sleep 1' creates a cpu map for all online
> cpus since it turns out calling cpu_map__new(NULL). Fix it. Also it
> is guaranteed that cpu_list is NULL if PID/TID is given by calling
> perf_target__validate(), so we can make the conditional bit simpler.
>
> This also fixes perf test 7 (Validate) failure on my 6 core machine:
>
>    $ cat /sys/devices/system/cpu/online
>    0-11
>    $ ./perf test -v 7
>     7: Validate PERF_RECORD_* events&  perf_sample fields:
>    --- start ---
>    perf_evlist__mmap: Operation not permitted
>    ---- end ----
>    Validate PERF_RECORD_* events&  perf_sample fields: FAILED!

Works fine for me with latest tip:
$ cat /sys/devices/system/cpu/online
0-15

$ /tmp/perf/perf test -v 7
  7: Validate PERF_RECORD_* events & perf_sample fields:
--- start ---
64740167922229 0 PERF_RECORD_SAMPLE
64740167926354 0 PERF_RECORD_SAMPLE
64740167928389 0 PERF_RECORD_SAMPLE
64740167930832 0 PERF_RECORD_SAMPLE
64740168404145 0 PERF_RECORD_COMM: sleep:16523
64740168424672 0 PERF_RECORD_MMAP 16523/16523: [0x400000(0x6000) @ 0]: 
/bin/sleep
64740168441676 0 PERF_RECORD_MMAP 16523/16523: [0x7f83de5bb000(0x224000) 
@ 0]: /lib64/ld-2.14.90.so
64740168458460 0 PERF_RECORD_MMAP 16523/16523: [0x7fff009ff000(0x1000) @ 
0x7fff009ff000]: [vdso]
64740168586358 0 PERF_RECORD_MMAP 16523/16523: [0x7f83de203000(0x3b8000) 
@ 0]: /lib64/libc-2.14.90.so
64741168625653 0 PERF_RECORD_EXIT(16523:16523):(16523:16523)
---- end ----
Validate PERF_RECORD_* events & perf_sample fields: Ok

Is the failure a by-product of the other patches in this set?

David

>
> Signed-off-by: Namhyung Kim<namhyung.kim@lge.com>
> ---
>   tools/perf/util/evlist.c |    5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index a43e2c56d1c6..556d98af1a0b 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -608,8 +608,9 @@ int perf_evlist__create_maps(struct perf_evlist *evlist,
>   	if (evlist->threads == NULL)
>   		return -1;
>
> -	if (target->uid != UINT_MAX ||
> -	    (target->cpu_list == NULL&&  target->tid))
> +	if (target->uid != UINT_MAX || target->tid)
> +		evlist->cpus = cpu_map__dummy_new();
> +	else if (!target->system_wide&&  target->cpu_list == NULL)
>   		evlist->cpus = cpu_map__dummy_new();
>   	else
>   		evlist->cpus = cpu_map__new(target->cpu_list);

Re: [PATCH 07/13] perf evlist: Fix creation of cpu map

[Namhyung Kim]

Hi,

2012-04-26 (Thu), 09:05 -0600, David Ahern wrote:
> On 4/25/12 11:15 PM, Namhyung Kim wrote:
> > Currently, 'perf record -- sleep 1' creates a cpu map for all online
> > cpus since it turns out calling cpu_map__new(NULL). Fix it. Also it
> > is guaranteed that cpu_list is NULL if PID/TID is given by calling
> > perf_target__validate(), so we can make the conditional bit simpler.
> >
> > This also fixes perf test 7 (Validate) failure on my 6 core machine:
> >
> >    $ cat /sys/devices/system/cpu/online
> >    0-11
> >    $ ./perf test -v 7
> >     7: Validate PERF_RECORD_* events&  perf_sample fields:
> >    --- start ---
> >    perf_evlist__mmap: Operation not permitted
> >    ---- end ----
> >    Validate PERF_RECORD_* events&  perf_sample fields: FAILED!
> 
> Works fine for me with latest tip:
> $ cat /sys/devices/system/cpu/online
> 0-15
> 
> $ /tmp/perf/perf test -v 7
>   7: Validate PERF_RECORD_* events & perf_sample fields:
> --- start ---
> 64740167922229 0 PERF_RECORD_SAMPLE
> 64740167926354 0 PERF_RECORD_SAMPLE
> 64740167928389 0 PERF_RECORD_SAMPLE
> 64740167930832 0 PERF_RECORD_SAMPLE
> 64740168404145 0 PERF_RECORD_COMM: sleep:16523
> 64740168424672 0 PERF_RECORD_MMAP 16523/16523: [0x400000(0x6000) @ 0]: 
> /bin/sleep
> 64740168441676 0 PERF_RECORD_MMAP 16523/16523: [0x7f83de5bb000(0x224000) 
> @ 0]: /lib64/ld-2.14.90.so
> 64740168458460 0 PERF_RECORD_MMAP 16523/16523: [0x7fff009ff000(0x1000) @ 
> 0x7fff009ff000]: [vdso]
> 64740168586358 0 PERF_RECORD_MMAP 16523/16523: [0x7f83de203000(0x3b8000) 
> @ 0]: /lib64/libc-2.14.90.so
> 64741168625653 0 PERF_RECORD_EXIT(16523:16523):(16523:16523)
> ---- end ----
> Validate PERF_RECORD_* events & perf_sample fields: Ok
> 
> Is the failure a by-product of the other patches in this set?
> 

Hmm.. No, I can reproduce it without any of this series. And now I think
that it is not related to the number of cpus. On my 4 core (no
hyperthreading) machine at home, the result was same.

BTW, did you change sysctl settings?

  $ cat /sys/devices/system/cpu/online 
  0-3
  $ grep . /proc/sys/kernel/perf_event_*
  /proc/sys/kernel/perf_event_max_sample_rate:100000
  /proc/sys/kernel/perf_event_mlock_kb:516
  /proc/sys/kernel/perf_event_paranoid:1
  $ ./perf test 7
   7: Validate PERF_RECORD_* events & perf_sample fields: FAILED!
  $ ./perf --version
  perf version 3.4.rc1


Thanks.

-- 
Regards,
Namhyung Kim

Re: [PATCH 07/13] perf evlist: Fix creation of cpu map

[David Ahern]

On 4/26/12 3:12 PM, Namhyung Kim wrote:
> Hmm.. No, I can reproduce it without any of this series. And now I think
> that it is not related to the number of cpus. On my 4 core (no
> hyperthreading) machine at home, the result was same.
>
> BTW, did you change sysctl settings?
>
>    $ cat /sys/devices/system/cpu/online
>    0-3
>    $ grep . /proc/sys/kernel/perf_event_*
>    /proc/sys/kernel/perf_event_max_sample_rate:100000
>    /proc/sys/kernel/perf_event_mlock_kb:516
>    /proc/sys/kernel/perf_event_paranoid:1

$ grep . /proc/sys/kernel/perf_event_*
/proc/sys/kernel/perf_event_max_sample_rate:100000
/proc/sys/kernel/perf_event_mlock_kb:516
/proc/sys/kernel/perf_event_paranoid:-1

That last one is the key. I have it set to not paranoid and usually run 
perf a non-root user.

>    $ ./perf test 7
>     7: Validate PERF_RECORD_* events&  perf_sample fields: FAILED!
>    $ ./perf --version
>    perf version 3.4.rc1

running 3.4-rc4 kernel + perf from tip/perf/core branch.

David

Re: [PATCH 07/13] perf evlist: Fix creation of cpu map

[Namhyung Kim]

Hi,

On Thu, 26 Apr 2012 15:22:55 -0600, David Ahern wrote:
> On 4/26/12 3:12 PM, Namhyung Kim wrote:
>> Hmm.. No, I can reproduce it without any of this series. And now I think
>> that it is not related to the number of cpus. On my 4 core (no
>> hyperthreading) machine at home, the result was same.
>>
>> BTW, did you change sysctl settings?
>>
>>    $ cat /sys/devices/system/cpu/online
>>    0-3
>>    $ grep . /proc/sys/kernel/perf_event_*
>>    /proc/sys/kernel/perf_event_max_sample_rate:100000
>>    /proc/sys/kernel/perf_event_mlock_kb:516
>>    /proc/sys/kernel/perf_event_paranoid:1
>
> $ grep . /proc/sys/kernel/perf_event_*
> /proc/sys/kernel/perf_event_max_sample_rate:100000
> /proc/sys/kernel/perf_event_mlock_kb:516
> /proc/sys/kernel/perf_event_paranoid:-1
>
> That last one is the key. I have it set to not paranoid and usually
> run perf a non-root user.
>

That's exactly what I want to see :). On perf_mmap() we have:

	if ((locked > lock_limit) && perf_paranoid_tracepoint_raw() &&
		!capable(CAP_IPC_LOCK)) {
		ret = -EPERM;
		goto unlock;
	}

So as long as you set perf_event_paranoid to -1 or run perf test as
root, you cannot see the failure.

Thanks,
Namhyung


>>    $ ./perf test 7
>>     7: Validate PERF_RECORD_* events&  perf_sample fields: FAILED!
>>    $ ./perf --version
>>    perf version 3.4.rc1
>
> running 3.4-rc4 kernel + perf from tip/perf/core branch.
>
> David

Re: [PATCH 07/13] perf evlist: Fix creation of cpu map

[Arnaldo Carvalho de Melo]

Em Fri, Apr 27, 2012 at 09:16:18AM +0900, Namhyung Kim escreveu:
> Hi,
> 
> On Thu, 26 Apr 2012 15:22:55 -0600, David Ahern wrote:
> > On 4/26/12 3:12 PM, Namhyung Kim wrote:
> >> Hmm.. No, I can reproduce it without any of this series. And now I think
> >> that it is not related to the number of cpus. On my 4 core (no
> >> hyperthreading) machine at home, the result was same.
> >>
> >> BTW, did you change sysctl settings?
> >>
> >>    $ cat /sys/devices/system/cpu/online
> >>    0-3
> >>    $ grep . /proc/sys/kernel/perf_event_*
> >>    /proc/sys/kernel/perf_event_max_sample_rate:100000
> >>    /proc/sys/kernel/perf_event_mlock_kb:516
> >>    /proc/sys/kernel/perf_event_paranoid:1
> >
> > $ grep . /proc/sys/kernel/perf_event_*
> > /proc/sys/kernel/perf_event_max_sample_rate:100000
> > /proc/sys/kernel/perf_event_mlock_kb:516
> > /proc/sys/kernel/perf_event_paranoid:-1
> >
> > That last one is the key. I have it set to not paranoid and usually
> > run perf a non-root user.
> >
> 
> That's exactly what I want to see :). On perf_mmap() we have:
> 
> 	if ((locked > lock_limit) && perf_paranoid_tracepoint_raw() &&
> 		!capable(CAP_IPC_LOCK)) {
> 		ret = -EPERM;
> 		goto unlock;
> 	}
> 
> So as long as you set perf_event_paranoid to -1 or run perf test as
> root, you cannot see the failure.

Ok, after this discussion, David, can I have your acked-by or tested-by?

- Arnaldo

Re: [PATCH 07/13] perf evlist: Fix creation of cpu map

[David Ahern]

On 4/27/12 9:08 AM, Arnaldo Carvalho de Melo wrote:
> Em Fri, Apr 27, 2012 at 09:16:18AM +0900, Namhyung Kim escreveu:
>> Hi,
>>
>> On Thu, 26 Apr 2012 15:22:55 -0600, David Ahern wrote:
>>> On 4/26/12 3:12 PM, Namhyung Kim wrote:
>>>> Hmm.. No, I can reproduce it without any of this series. And now I think
>>>> that it is not related to the number of cpus. On my 4 core (no
>>>> hyperthreading) machine at home, the result was same.
>>>>
>>>> BTW, did you change sysctl settings?
>>>>
>>>>     $ cat /sys/devices/system/cpu/online
>>>>     0-3
>>>>     $ grep . /proc/sys/kernel/perf_event_*
>>>>     /proc/sys/kernel/perf_event_max_sample_rate:100000
>>>>     /proc/sys/kernel/perf_event_mlock_kb:516
>>>>     /proc/sys/kernel/perf_event_paranoid:1
>>>
>>> $ grep . /proc/sys/kernel/perf_event_*
>>> /proc/sys/kernel/perf_event_max_sample_rate:100000
>>> /proc/sys/kernel/perf_event_mlock_kb:516
>>> /proc/sys/kernel/perf_event_paranoid:-1
>>>
>>> That last one is the key. I have it set to not paranoid and usually
>>> run perf a non-root user.
>>>
>>
>> That's exactly what I want to see :). On perf_mmap() we have:
>>
>> 	if ((locked>  lock_limit)&&  perf_paranoid_tracepoint_raw()&&
>> 		!capable(CAP_IPC_LOCK)) {
>> 		ret = -EPERM;
>> 		goto unlock;
>> 	}
>>
>> So as long as you set perf_event_paranoid to -1 or run perf test as
>> root, you cannot see the failure.
>
> Ok, after this discussion, David, can I have your acked-by or tested-by?
>
> - Arnaldo

I had reviewed the whole series yesterday, so you can add my

Reviewed-by: David Ahern <dsahern@gmail.com>

to the whole series if you want. I did not pull down the set and apply 
to test it, and this particular patch requires prior ones so I have not 
tested it.

David

Re: [PATCH 07/13] perf evlist: Fix creation of cpu map

[Arnaldo Carvalho de Melo]

Em Fri, Apr 27, 2012 at 09:32:09AM -0600, David Ahern escreveu:
> On 4/27/12 9:08 AM, Arnaldo Carvalho de Melo wrote:
> >Ok, after this discussion, David, can I have your acked-by or tested-by?
 
> I had reviewed the whole series yesterday, so you can add my
 
> Reviewed-by: David Ahern <dsahern@gmail.com>
 
> to the whole series if you want. I did not pull down the set and
> apply to test it, and this particular patch requires prior ones so I
> have not tested it.

Fair enough, I'll add the Reviewed-by tag.

I'm running 'perf test' after each patch. Doing it as root, but we need
to automate this to do it as root, user, and user with each of the
paranoid levels...

- Arnaldo

Re: [PATCH 07/13] perf evlist: Fix creation of cpu map

[David Ahern]

On 4/27/12 10:00 AM, Arnaldo Carvalho de Melo wrote:
> Em Fri, Apr 27, 2012 at 09:32:09AM -0600, David Ahern escreveu:
>> On 4/27/12 9:08 AM, Arnaldo Carvalho de Melo wrote:
>>> Ok, after this discussion, David, can I have your acked-by or tested-by?
>
>> I had reviewed the whole series yesterday, so you can add my
>
>> Reviewed-by: David Ahern<dsahern@gmail.com>
>
>> to the whole series if you want. I did not pull down the set and
>> apply to test it, and this particular patch requires prior ones so I
>> have not tested it.
>
> Fair enough, I'll add the Reviewed-by tag.
>
> I'm running 'perf test' after each patch. Doing it as root, but we need
> to automate this to do it as root, user, and user with each of the
> paranoid levels...
>
> - Arnaldo

Right - and that may include changing paranoid settings.

Also, the patchset is listed as RFC if that matters from running through 
all the formalities. One of the reasons I was not motivated to pull down 
the patches and apply/build/test.

David

Re: [PATCH 07/13] perf evlist: Fix creation of cpu map

[Arnaldo Carvalho de Melo]

Em Fri, Apr 27, 2012 at 10:05:06AM -0600, David Ahern escreveu:
> On 4/27/12 10:00 AM, Arnaldo Carvalho de Melo wrote:
> >Em Fri, Apr 27, 2012 at 09:32:09AM -0600, David Ahern escreveu:
> >>On 4/27/12 9:08 AM, Arnaldo Carvalho de Melo wrote:
> >>>Ok, after this discussion, David, can I have your acked-by or tested-by?
> >
> >>I had reviewed the whole series yesterday, so you can add my
> >
> >>Reviewed-by: David Ahern<dsahern@gmail.com>
> >
> >>to the whole series if you want. I did not pull down the set and
> >>apply to test it, and this particular patch requires prior ones so I
> >>have not tested it.
> >
> >Fair enough, I'll add the Reviewed-by tag.
> >
> >I'm running 'perf test' after each patch. Doing it as root, but we need
> >to automate this to do it as root, user, and user with each of the
> >paranoid levels...
> >
> >- Arnaldo
> 
> Right - and that may include changing paranoid settings.
> 
> Also, the patchset is listed as RFC if that matters from running
> through all the formalities. One of the reasons I was not motivated
> to pull down the patches and apply/build/test.

I'm ok with it, so I happily disregarded the RFC on it 8-)

And now that you reviewed it all too, even better.

Thanks,

- Arnaldo

Re: [PATCH 08/13] perf target: Split out perf_target handling code

[Arnaldo Carvalho de Melo]

Em Thu, Apr 26, 2012 at 02:15:22PM +0900, Namhyung Kim escreveu:
> For further work on perf_target, it'd be better off splitting
> the code into a separate file.
> 
> Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
> ---
>  tools/perf/Makefile      |    2 ++
>  tools/perf/perf.h        |    9 +--------
>  tools/perf/util/evlist.c |    1 +
>  tools/perf/util/evsel.c  |    1 +
>  tools/perf/util/target.c |   45 +++++++++++++++++++++++++++++++++++++++++++++
>  tools/perf/util/target.h |   17 +++++++++++++++++
>  tools/perf/util/usage.c  |   34 ----------------------------------
>  tools/perf/util/util.h   |    2 --
>  8 files changed, 67 insertions(+), 44 deletions(-)
>  create mode 100644 tools/perf/util/target.c
>  create mode 100644 tools/perf/util/target.h

Trying to fix this now...

[acme@sandy linux]$ make -C tools/perf/ O=/home/git/build/perf install
make: Entering directory `/home/git/linux/tools/perf'
    CC /home/git/build/perf/builtin-bench.o
    CC /home/git/build/perf/bench/sched-messaging.o
    CC /home/git/build/perf/bench/sched-pipe.o
    CC /home/git/build/perf/scripts/perl/Perf-Trace-Util/Context.o
In file included from
scripts/perl/Perf-Trace-Util/../../../util/target.h:4,
                 from scripts/perl/Perf-Trace-Util/../../../perf.h:210,
                 from Context.xs:25:
scripts/perl/Perf-Trace-Util/../../../util/util.h:44:1: error:
"HAS_BOOL" redefined
In file included from /usr/lib64/perl5/CORE/perl.h:2424,
                 from Context.xs:23:
/usr/lib64/perl5/CORE/handy.h:110:1: error: this is the location of the
previous definition
In file included from
scripts/perl/Perf-Trace-Util/../../../util/target.h:4,
                 from scripts/perl/Perf-Trace-Util/../../../perf.h:210,
                 from Context.xs:25:
scripts/perl/Perf-Trace-Util/../../../util/util.h:133: error:
conflicting types for ‘Perl_die_nocontext’
/usr/lib64/perl5/CORE/proto.h:331: note: previous declaration of
‘Perl_die_nocontext’ was here
make: *** [/home/git/build/perf/scripts/perl/Perf-Trace-Util/Context.o]
Error 1
make: Leaving directory `/home/git/linux/tools/perf'
[acme@sandy linux]$

Re: [PATCH 07/13] perf evlist: Fix creation of cpu map

[Arnaldo Carvalho de Melo]

Em Fri, Apr 27, 2012 at 09:16:18AM +0900, Namhyung Kim escreveu:
> On Thu, 26 Apr 2012 15:22:55 -0600, David Ahern wrote:
> > On 4/26/12 3:12 PM, Namhyung Kim wrote:
> >> Hmm.. No, I can reproduce it without any of this series. And now I think
> >> that it is not related to the number of cpus. On my 4 core (no
> >> hyperthreading) machine at home, the result was same.
> >>
> >> BTW, did you change sysctl settings?
> >>
> >>    $ cat /sys/devices/system/cpu/online
> >>    0-3
> >>    $ grep . /proc/sys/kernel/perf_event_*
> >>    /proc/sys/kernel/perf_event_max_sample_rate:100000
> >>    /proc/sys/kernel/perf_event_mlock_kb:516
> >>    /proc/sys/kernel/perf_event_paranoid:1
> >
> > $ grep . /proc/sys/kernel/perf_event_*
> > /proc/sys/kernel/perf_event_max_sample_rate:100000
> > /proc/sys/kernel/perf_event_mlock_kb:516
> > /proc/sys/kernel/perf_event_paranoid:-1
> >
> > That last one is the key. I have it set to not paranoid and usually
> > run perf a non-root user.
> >
> 
> That's exactly what I want to see :). On perf_mmap() we have:
> 
> 	if ((locked > lock_limit) && perf_paranoid_tracepoint_raw() &&
> 		!capable(CAP_IPC_LOCK)) {
> 		ret = -EPERM;
> 		goto unlock;
> 	}
> 
> So as long as you set perf_event_paranoid to -1 or run perf test as
> root, you cannot see the failure.

Ok, as root try 'perf top', here I get, with this patch:

[root@sandy ~]# perf top --stdio
Warning:
The sys_perf_event_open() syscall returned with 22 (Invalid argument).
/bin/dmesg may provide additional information.
No CONFIG_PERF_EVENTS=y kernel support configured?
[root@sandy ~]#

Skipping this one, will look again later.

- Arnaldo

Re: [PATCH 09/13] perf target: Introduce perf_target_errno

[Arnaldo Carvalho de Melo]

Em Thu, Apr 26, 2012 at 02:15:23PM +0900, Namhyung Kim escreveu:
> The perf_target_errno enumerations are used to indicate
> specific error cases on perf target operations. It'd
> help libperf being a more generic library.
> 
> Suggested-by: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
> Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
> +	/* UID and SYSTEM are mutually exclusive */
> +	if (target->uid_str && target->system_wide) {
> +		target->system_wide = false;
> +		if (ret == PERF_TARGET__SUCCESS)
> +			ret = PERF_TARGET__UID_OVERRIDE_SYSTEM;
> +	}
> +
> +	return ret;
>  }
> diff --git a/tools/perf/util/target.h b/tools/perf/util/target.h
> index 1348065ada5e..c3914c8a9890 100644
> --- a/tools/perf/util/target.h
> +++ b/tools/perf/util/target.h
> @@ -12,6 +12,24 @@ struct perf_target {
>  	bool	     system_wide;
>  };
>  
> -void perf_target__validate(struct perf_target *target);
> +enum perf_target_errno {
> +	/*
> +	 * XXX: Just choose an arbitrary big number standard errno can't have

Here I think its better for us to use _negative_ big numbers, because
according to:

http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/errno.h.html

<quote>
Issue 6

The following new requirements on POSIX implementations derive from
alignment with the Single UNIX Specification:

The majority of the error conditions previously marked as extensions are
now mandatory, except for the STREAMS-related error conditions.

Values for errno are now required to be distinct positive values rather
than non-zero values. This change is for alignment with the ISO/IEC
9899:1999 standard.
</quote>

So system errno range is all positive, since our error enumeration is a
superset of the system one, using negative values won't ever clash.

Also it would be better to have it as PERF_ERRNO__PID_OVERRIDE_CPU, etc.

Agreed?

Anybody else with reasons not to use this ernno range scheme?

Ingo?

- Arnaldo

> +	 */
> +	__PERF_TARGET__ERRNO_START		= 0x10000,
> +
> +	PERF_TARGET__SUCCESS			= __PERF_TARGET__ERRNO_START,
> +
> +	/* for perf_target__validate() */
> +	PERF_TARGET__PID_OVERRIDE_CPU,
> +	PERF_TARGET__PID_OVERRIDE_UID,
> +	PERF_TARGET__UID_OVERRIDE_CPU,
> +	PERF_TARGET__PID_OVERRIDE_SYSTEM,
> +	PERF_TARGET__UID_OVERRIDE_SYSTEM,
> +
> +	__PERF_TARGET__ERRNO_END
> +};
> +
> +enum perf_target_errno perf_target__validate(struct perf_target *target);
>  
>  #endif /* _PERF_TARGET_H */
> -- 
> 1.7.10

Re: [PATCH 08/13] perf target: Split out perf_target handling code

[Namhyung Kim]

2012-05-02 (수), 15:30 -0300, Arnaldo Carvalho de Melo:
> Em Thu, Apr 26, 2012 at 02:15:22PM +0900, Namhyung Kim escreveu:
> > For further work on perf_target, it'd be better off splitting
> > the code into a separate file.
> > 
> > Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
> > ---
> >  tools/perf/Makefile      |    2 ++
> >  tools/perf/perf.h        |    9 +--------
> >  tools/perf/util/evlist.c |    1 +
> >  tools/perf/util/evsel.c  |    1 +
> >  tools/perf/util/target.c |   45 +++++++++++++++++++++++++++++++++++++++++++++
> >  tools/perf/util/target.h |   17 +++++++++++++++++
> >  tools/perf/util/usage.c  |   34 ----------------------------------
> >  tools/perf/util/util.h   |    2 --
> >  8 files changed, 67 insertions(+), 44 deletions(-)
> >  create mode 100644 tools/perf/util/target.c
> >  create mode 100644 tools/perf/util/target.h
> 
> Trying to fix this now...
> 

Oops, sorry. I'll investigate it tomorrow.

Thanks,
Namhyung


> [acme@sandy linux]$ make -C tools/perf/ O=/home/git/build/perf install
> make: Entering directory `/home/git/linux/tools/perf'
>     CC /home/git/build/perf/builtin-bench.o
>     CC /home/git/build/perf/bench/sched-messaging.o
>     CC /home/git/build/perf/bench/sched-pipe.o
>     CC /home/git/build/perf/scripts/perl/Perf-Trace-Util/Context.o
> In file included from
> scripts/perl/Perf-Trace-Util/../../../util/target.h:4,
>                  from scripts/perl/Perf-Trace-Util/../../../perf.h:210,
>                  from Context.xs:25:
> scripts/perl/Perf-Trace-Util/../../../util/util.h:44:1: error:
> "HAS_BOOL" redefined
> In file included from /usr/lib64/perl5/CORE/perl.h:2424,
>                  from Context.xs:23:
> /usr/lib64/perl5/CORE/handy.h:110:1: error: this is the location of the
> previous definition
> In file included from
> scripts/perl/Perf-Trace-Util/../../../util/target.h:4,
>                  from scripts/perl/Perf-Trace-Util/../../../perf.h:210,
>                  from Context.xs:25:
> scripts/perl/Perf-Trace-Util/../../../util/util.h:133: error:
> conflicting types for ‘Perl_die_nocontext’
> /usr/lib64/perl5/CORE/proto.h:331: note: previous declaration of
> ‘Perl_die_nocontext’ was here
> make: *** [/home/git/build/perf/scripts/perl/Perf-Trace-Util/Context.o]
> Error 1
> make: Leaving directory `/home/git/linux/tools/perf'
> [acme@sandy linux]$


-- 
Regards,
Namhyung Kim

Re: [PATCH 09/13] perf target: Introduce perf_target_errno

[Namhyung Kim]

Hi,

2012-05-02 (수), 15:59 -0300, Arnaldo Carvalho de Melo:
> Em Thu, Apr 26, 2012 at 02:15:23PM +0900, Namhyung Kim escreveu:
> > The perf_target_errno enumerations are used to indicate
> > specific error cases on perf target operations. It'd
> > help libperf being a more generic library.
> > 
> > Suggested-by: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
> > Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
> > +	/* UID and SYSTEM are mutually exclusive */
> > +	if (target->uid_str && target->system_wide) {
> > +		target->system_wide = false;
> > +		if (ret == PERF_TARGET__SUCCESS)
> > +			ret = PERF_TARGET__UID_OVERRIDE_SYSTEM;
> > +	}
> > +
> > +	return ret;
> >  }
> > diff --git a/tools/perf/util/target.h b/tools/perf/util/target.h
> > index 1348065ada5e..c3914c8a9890 100644
> > --- a/tools/perf/util/target.h
> > +++ b/tools/perf/util/target.h
> > @@ -12,6 +12,24 @@ struct perf_target {
> >  	bool	     system_wide;
> >  };
> >  
> > -void perf_target__validate(struct perf_target *target);
> > +enum perf_target_errno {
> > +	/*
> > +	 * XXX: Just choose an arbitrary big number standard errno can't have
> 
> Here I think its better for us to use _negative_ big numbers, because
> according to:
> 
> http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/errno.h.html
> 
> <quote>
> Issue 6
> 
> The following new requirements on POSIX implementations derive from
> alignment with the Single UNIX Specification:
> 
> The majority of the error conditions previously marked as extensions are
> now mandatory, except for the STREAMS-related error conditions.
> 
> Values for errno are now required to be distinct positive values rather
> than non-zero values. This change is for alignment with the ISO/IEC
> 9899:1999 standard.
> </quote>
> 
> So system errno range is all positive, since our error enumeration is a
> superset of the system one, using negative values won't ever clash.
> 
> Also it would be better to have it as PERF_ERRNO__PID_OVERRIDE_CPU, etc.
> 
> Agreed?
> 

Agreed and thanks for the info. Will change the name of error constants
too.

Thanks,
Namhyung


> Anybody else with reasons not to use this ernno range scheme?
> 
> Ingo?
> 
> - Arnaldo
> 
> > +	 */
> > +	__PERF_TARGET__ERRNO_START		= 0x10000,
> > +
> > +	PERF_TARGET__SUCCESS			= __PERF_TARGET__ERRNO_START,
> > +
> > +	/* for perf_target__validate() */
> > +	PERF_TARGET__PID_OVERRIDE_CPU,
> > +	PERF_TARGET__PID_OVERRIDE_UID,
> > +	PERF_TARGET__UID_OVERRIDE_CPU,
> > +	PERF_TARGET__PID_OVERRIDE_SYSTEM,
> > +	PERF_TARGET__UID_OVERRIDE_SYSTEM,
> > +
> > +	__PERF_TARGET__ERRNO_END
> > +};
> > +
> > +enum perf_target_errno perf_target__validate(struct perf_target *target);
> >  
> >  #endif /* _PERF_TARGET_H */
> > -- 
> > 1.7.10


-- 
Regards,
Namhyung Kim

Re: [PATCH 08/13] perf target: Split out perf_target handling code

[Arnaldo Carvalho de Melo]

Em Thu, May 03, 2012 at 11:39:47PM +0900, Namhyung Kim escreveu:
> 2012-05-02 (수), 15:30 -0300, Arnaldo Carvalho de Melo:
> > Em Thu, Apr 26, 2012 at 02:15:22PM +0900, Namhyung Kim escreveu:
> > > For further work on perf_target, it'd be better off splitting
> > > the code into a separate file.
> > > 
> > > Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
> > > ---
> > >  tools/perf/Makefile      |    2 ++
> > >  tools/perf/perf.h        |    9 +--------
> > >  tools/perf/util/evlist.c |    1 +
> > >  tools/perf/util/evsel.c  |    1 +
> > >  tools/perf/util/target.c |   45 +++++++++++++++++++++++++++++++++++++++++++++
> > >  tools/perf/util/target.h |   17 +++++++++++++++++
> > >  tools/perf/util/usage.c  |   34 ----------------------------------
> > >  tools/perf/util/util.h   |    2 --
> > >  8 files changed, 67 insertions(+), 44 deletions(-)
> > >  create mode 100644 tools/perf/util/target.c
> > >  create mode 100644 tools/perf/util/target.h
> > 
> > Trying to fix this now...
> > 
> 
> Oops, sorry. I'll investigate it tomorrow.
> 

I fixed it, its on my latest perf/core pull req to Ingo.

- Arnaldo

Re: [PATCH 08/13] perf target: Split out perf_target handling code

[Namhyung Kim]

2012-05-03 (목), 12:27 -0300, Arnaldo Carvalho de Melo:
> Em Thu, May 03, 2012 at 11:39:47PM +0900, Namhyung Kim escreveu:
> > 2012-05-02 (수), 15:30 -0300, Arnaldo Carvalho de Melo:
> > > Em Thu, Apr 26, 2012 at 02:15:22PM +0900, Namhyung Kim escreveu:
> > > > For further work on perf_target, it'd be better off splitting
> > > > the code into a separate file.
> > > > 
> > > > Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
> > > > ---
> > > >  tools/perf/Makefile      |    2 ++
> > > >  tools/perf/perf.h        |    9 +--------
> > > >  tools/perf/util/evlist.c |    1 +
> > > >  tools/perf/util/evsel.c  |    1 +
> > > >  tools/perf/util/target.c |   45 +++++++++++++++++++++++++++++++++++++++++++++
> > > >  tools/perf/util/target.h |   17 +++++++++++++++++
> > > >  tools/perf/util/usage.c  |   34 ----------------------------------
> > > >  tools/perf/util/util.h   |    2 --
> > > >  8 files changed, 67 insertions(+), 44 deletions(-)
> > > >  create mode 100644 tools/perf/util/target.c
> > > >  create mode 100644 tools/perf/util/target.h
> > > 
> > > Trying to fix this now...
> > > 
> > 
> > Oops, sorry. I'll investigate it tomorrow.
> > 
> 
> I fixed it, its on my latest perf/core pull req to Ingo.
> 

Cool :). Thanks a lot, Arnaldo.

Namhyung

Re: [PATCH 09/13] perf target: Introduce perf_target_errno

[David Ahern]

On 5/2/12 12:59 PM, Arnaldo Carvalho de Melo wrote:
> Also it would be better to have it as PERF_ERRNO__PID_OVERRIDE_CPU, etc.

I thought you wanted subsystem based errno's (PERF_TARGET__XXXXX) versus 
one big set (PERF_ERRNO__XXXXX). Did you change your mind?

David

Re: [PATCH 09/13] perf target: Introduce perf_target_errno

[Arnaldo Carvalho de Melo]

Em Thu, May 03, 2012 at 02:34:20PM -0600, David Ahern escreveu:
> On 5/2/12 12:59 PM, Arnaldo Carvalho de Melo wrote:
> >Also it would be better to have it as PERF_ERRNO__PID_OVERRIDE_CPU, etc.
> 
> I thought you wanted subsystem based errno's (PERF_TARGET__XXXXX)
> versus one big set (PERF_ERRNO__XXXXX). Did you change your mind?

Oops, I didn't realize PID being the subsys, then yeah, that is ok.

But that would make it PERF_ERR__TARGET_, as PERF_TARGET__ doesn't
straight away brings back "error enumeration", at least for me :)

But this is getting overly long, ideas?

PERF_ we need, its libperf's "namespace", then ERRNO looks needed too,
heck, make it long:

PERF_ERRNO_TARGET__PID_OVERRIDE_CPU

After all most of the time this will just be inside the function setting
the error and the strerrno function that will convert this to an string,
right?

- Arnaldo

Re: [PATCH 09/13] perf target: Introduce perf_target_errno

[David Ahern]

On 5/3/12 2:42 PM, Arnaldo Carvalho de Melo wrote:
> Em Thu, May 03, 2012 at 02:34:20PM -0600, David Ahern escreveu:
>> On 5/2/12 12:59 PM, Arnaldo Carvalho de Melo wrote:
>>> Also it would be better to have it as PERF_ERRNO__PID_OVERRIDE_CPU, etc.
>>
>> I thought you wanted subsystem based errno's (PERF_TARGET__XXXXX)
>> versus one big set (PERF_ERRNO__XXXXX). Did you change your mind?
>
> Oops, I didn't realize PID being the subsys, then yeah, that is ok.
>
> But that would make it PERF_ERR__TARGET_, as PERF_TARGET__ doesn't
> straight away brings back "error enumeration", at least for me :)
>
> But this is getting overly long, ideas?
>
> PERF_ we need, its libperf's "namespace", then ERRNO looks needed too,
> heck, make it long:
>
> PERF_ERRNO_TARGET__PID_OVERRIDE_CPU
>
> After all most of the time this will just be inside the function setting
> the error and the strerrno function that will convert this to an string,
> right?

Agreed, the long macro name shouldn't be visible outside of the depths 
of the perf files that set it and convert it to a string.

David

Re: [PATCH 07/13] perf evlist: Fix creation of cpu map

[Namhyung Kim]

Hi,

On Wed, 2 May 2012 15:40:33 -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, Apr 27, 2012 at 09:16:18AM +0900, Namhyung Kim escreveu:
>> So as long as you set perf_event_paranoid to -1 or run perf test as
>> root, you cannot see the failure.
>
> Ok, as root try 'perf top', here I get, with this patch:
>
> [root@sandy ~]# perf top --stdio
> Warning:
> The sys_perf_event_open() syscall returned with 22 (Invalid argument).
> /bin/dmesg may provide additional information.
> No CONFIG_PERF_EVENTS=y kernel support configured?
> [root@sandy ~]#
>
> Skipping this one, will look again later.
>
> - Arnaldo

Sorry. It is because perf top depends on an undefined behaviour that
if neither target nor command line argument was specified it'd open
events for all online cpus. In contrast, perf record and perf stat will
show the help message in this case.

This patch makes it clear that we have to prepare a sane target
configuration before opening a perf event fd. So I think we should fix
perf top to set top.target.system_wide to true by default.

Thanks,
Namhyung

Re: [PATCH 07/13] perf evlist: Fix creation of cpu map

[Arnaldo Carvalho de Melo]

Em Fri, May 04, 2012 at 01:05:48PM +0900, Namhyung Kim escreveu:
> Hi,
> 
> On Wed, 2 May 2012 15:40:33 -0300, Arnaldo Carvalho de Melo wrote:
> > Em Fri, Apr 27, 2012 at 09:16:18AM +0900, Namhyung Kim escreveu:
> >> So as long as you set perf_event_paranoid to -1 or run perf test as
> >> root, you cannot see the failure.
> >
> > Ok, as root try 'perf top', here I get, with this patch:
> >
> > [root@sandy ~]# perf top --stdio
> > Warning:
> > The sys_perf_event_open() syscall returned with 22 (Invalid argument).
> > /bin/dmesg may provide additional information.
> > No CONFIG_PERF_EVENTS=y kernel support configured?
> > [root@sandy ~]#
> >
> > Skipping this one, will look again later.
> >
> > - Arnaldo
> 
> Sorry. It is because perf top depends on an undefined behaviour that
> if neither target nor command line argument was specified it'd open
> events for all online cpus. In contrast, perf record and perf stat will
> show the help message in this case.
> 
> This patch makes it clear that we have to prepare a sane target
> configuration before opening a perf event fd. So I think we should fix
> perf top to set top.target.system_wide to true by default.

So please provide a preparatory patch for top before this one, ok?

- Arnaldo

Re: [PATCH 07/13] perf evlist: Fix creation of cpu map

[Namhyung Kim]

Hi,

On Fri, 4 May 2012 10:37:06 -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, May 04, 2012 at 01:05:48PM +0900, Namhyung Kim escreveu:
>> On Wed, 2 May 2012 15:40:33 -0300, Arnaldo Carvalho de Melo wrote:
>> > Em Fri, Apr 27, 2012 at 09:16:18AM +0900, Namhyung Kim escreveu:
>> >> So as long as you set perf_event_paranoid to -1 or run perf test as
>> >> root, you cannot see the failure.
>> >
>> > Ok, as root try 'perf top', here I get, with this patch:
>> >
>> > [root@sandy ~]# perf top --stdio
>> > Warning:
>> > The sys_perf_event_open() syscall returned with 22 (Invalid argument).
>> > /bin/dmesg may provide additional information.
>> > No CONFIG_PERF_EVENTS=y kernel support configured?
>> > [root@sandy ~]#
>> >
>> > Skipping this one, will look again later.
>> >
>> Sorry. It is because perf top depends on an undefined behaviour that
>> if neither target nor command line argument was specified it'd open
>> events for all online cpus. In contrast, perf record and perf stat will
>> show the help message in this case.
>> 
>> This patch makes it clear that we have to prepare a sane target
>> configuration before opening a perf event fd. So I think we should fix
>> perf top to set top.target.system_wide to true by default.
>
> So please provide a preparatory patch for top before this one, ok?
>

Okay, will do.

Thanks,
Namhyung

[tip:perf/core] perf tools: Introduce struct perf_target

[tip-bot for Namhyung Kim]

Commit-ID:  bea0340582dc47b447a014f5bf9f460925afdaf4
Gitweb:     http://git.kernel.org/tip/bea0340582dc47b447a014f5bf9f460925afdaf4
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Thu, 26 Apr 2012 14:15:15 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 2 May 2012 15:17:58 -0300

perf tools: Introduce struct perf_target

The perf_target struct will be used for taking care of cpu/thread maps
based on user's input. Since it is used on various subcommands it'd
better factoring it out.

Thanks to Arnaldo for suggesting the better name.

Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
Reviewed-by: David Ahern <dsahern@gmail.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1335417327-11796-2-git-send-email-namhyung.kim@lge.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-record.c |   41 ++++++++++++++++++++++-------------------
 tools/perf/builtin-test.c   |    5 +++--
 tools/perf/perf.h           |   15 ++++++++++-----
 tools/perf/util/evlist.c    |    2 +-
 tools/perf/util/evsel.c     |   10 +++++-----
 5 files changed, 41 insertions(+), 32 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 10b1f1f..4dcf270 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -44,7 +44,6 @@ struct perf_record {
 	struct perf_evlist	*evlist;
 	struct perf_session	*session;
 	const char		*progname;
-	const char		*uid_str;
 	int			output;
 	unsigned int		page_size;
 	int			realtime_prio;
@@ -218,7 +217,7 @@ try_again:
 			if (err == EPERM || err == EACCES) {
 				ui__error_paranoid();
 				exit(EXIT_FAILURE);
-			} else if (err ==  ENODEV && opts->cpu_list) {
+			} else if (err ==  ENODEV && opts->target.cpu_list) {
 				die("No such device - did you specify"
 					" an out-of-range profile CPU?\n");
 			} else if (err == EINVAL) {
@@ -578,7 +577,7 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
 		perf_session__process_machines(session, tool,
 					       perf_event__synthesize_guest_os);
 
-	if (!opts->system_wide)
+	if (!opts->target.system_wide)
 		perf_event__synthesize_thread_map(tool, evsel_list->threads,
 						  process_synthesized_event,
 						  machine);
@@ -765,9 +764,9 @@ const struct option record_options[] = {
 		     parse_events_option),
 	OPT_CALLBACK(0, "filter", &record.evlist, "filter",
 		     "event filter", parse_filter),
-	OPT_STRING('p', "pid", &record.opts.target_pid, "pid",
+	OPT_STRING('p', "pid", &record.opts.target.pid, "pid",
 		    "record events on existing process id"),
-	OPT_STRING('t', "tid", &record.opts.target_tid, "tid",
+	OPT_STRING('t', "tid", &record.opts.target.tid, "tid",
 		    "record events on existing thread id"),
 	OPT_INTEGER('r', "realtime", &record.realtime_prio,
 		    "collect data with this RT SCHED_FIFO priority"),
@@ -775,11 +774,11 @@ const struct option record_options[] = {
 		    "collect data without buffering"),
 	OPT_BOOLEAN('R', "raw-samples", &record.opts.raw_samples,
 		    "collect raw sample records from all opened counters"),
-	OPT_BOOLEAN('a', "all-cpus", &record.opts.system_wide,
+	OPT_BOOLEAN('a', "all-cpus", &record.opts.target.system_wide,
 			    "system-wide collection from all CPUs"),
 	OPT_BOOLEAN('A', "append", &record.append_file,
 			    "append to the output file to do incremental profiling"),
-	OPT_STRING('C', "cpu", &record.opts.cpu_list, "cpu",
+	OPT_STRING('C', "cpu", &record.opts.target.cpu_list, "cpu",
 		    "list of cpus to monitor"),
 	OPT_BOOLEAN('f', "force", &record.force,
 			"overwrite existing data file (deprecated)"),
@@ -813,7 +812,8 @@ const struct option record_options[] = {
 	OPT_CALLBACK('G', "cgroup", &record.evlist, "name",
 		     "monitor event in cgroup name only",
 		     parse_cgroups),
-	OPT_STRING('u', "uid", &record.uid_str, "user", "user to profile"),
+	OPT_STRING('u', "uid", &record.opts.target.uid_str, "user",
+		   "user to profile"),
 
 	OPT_CALLBACK_NOOPT('b', "branch-any", &record.opts.branch_stack,
 		     "branch any", "sample any taken branches",
@@ -842,8 +842,9 @@ int cmd_record(int argc, const char **argv, const char *prefix __used)
 
 	argc = parse_options(argc, argv, record_options, record_usage,
 			    PARSE_OPT_STOP_AT_NON_OPTION);
-	if (!argc && !rec->opts.target_pid && !rec->opts.target_tid &&
-		!rec->opts.system_wide && !rec->opts.cpu_list && !rec->uid_str)
+	if (!argc && !rec->opts.target.pid && !rec->opts.target.tid &&
+	    !rec->opts.target.system_wide && !rec->opts.target.cpu_list &&
+	    !rec->opts.target.uid_str)
 		usage_with_options(record_usage, record_options);
 
 	if (rec->force && rec->append_file) {
@@ -856,7 +857,7 @@ int cmd_record(int argc, const char **argv, const char *prefix __used)
 		rec->write_mode = WRITE_FORCE;
 	}
 
-	if (nr_cgroups && !rec->opts.system_wide) {
+	if (nr_cgroups && !rec->opts.target.system_wide) {
 		fprintf(stderr, "cgroup monitoring only available in"
 			" system-wide mode\n");
 		usage_with_options(record_usage, record_options);
@@ -883,17 +884,19 @@ int cmd_record(int argc, const char **argv, const char *prefix __used)
 		goto out_symbol_exit;
 	}
 
-	rec->opts.uid = parse_target_uid(rec->uid_str, rec->opts.target_tid,
-					 rec->opts.target_pid);
-	if (rec->uid_str != NULL && rec->opts.uid == UINT_MAX - 1)
+	rec->opts.target.uid = parse_target_uid(rec->opts.target.uid_str,
+						rec->opts.target.tid,
+						rec->opts.target.pid);
+	if (rec->opts.target.uid_str != NULL &&
+	    rec->opts.target.uid == UINT_MAX - 1)
 		goto out_free_fd;
 
-	if (rec->opts.target_pid)
-		rec->opts.target_tid = rec->opts.target_pid;
+	if (rec->opts.target.pid)
+		rec->opts.target.tid = rec->opts.target.pid;
 
-	if (perf_evlist__create_maps(evsel_list, rec->opts.target_pid,
-				     rec->opts.target_tid, rec->opts.uid,
-				     rec->opts.cpu_list) < 0)
+	if (perf_evlist__create_maps(evsel_list, rec->opts.target.pid,
+				     rec->opts.target.tid, rec->opts.target.uid,
+				     rec->opts.target.cpu_list) < 0)
 		usage_with_options(record_usage, record_options);
 
 	list_for_each_entry(pos, &evsel_list->entries, node) {
diff --git a/tools/perf/builtin-test.c b/tools/perf/builtin-test.c
index 5502a4a..27882d8 100644
--- a/tools/perf/builtin-test.c
+++ b/tools/perf/builtin-test.c
@@ -1207,8 +1207,9 @@ static int test__PERF_RECORD(void)
 	 * perf_evlist__prepare_workload we'll fill in the only thread
 	 * we're monitoring, the one forked there.
 	 */
-	err = perf_evlist__create_maps(evlist, opts.target_pid,
-				       opts.target_tid, UINT_MAX, opts.cpu_list);
+	err = perf_evlist__create_maps(evlist, opts.target.pid,
+				       opts.target.tid, UINT_MAX,
+				       opts.target.cpu_list);
 	if (err < 0) {
 		pr_debug("Not enough memory to create thread/cpu maps\n");
 		goto out_delete_evlist;
diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index 89e3355..7e226c0 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -207,10 +207,17 @@ extern const char perf_version_string[];
 
 void pthread__unblock_sigwinch(void);
 
-struct perf_record_opts {
-	const char   *target_pid;
-	const char   *target_tid;
+struct perf_target {
+	const char   *pid;
+	const char   *tid;
+	const char   *cpu_list;
+	const char   *uid_str;
 	uid_t	     uid;
+	bool	     system_wide;
+};
+
+struct perf_record_opts {
+	struct perf_target target;
 	bool	     call_graph;
 	bool	     group;
 	bool	     inherit_stat;
@@ -223,7 +230,6 @@ struct perf_record_opts {
 	bool	     sample_time;
 	bool	     sample_id_all_missing;
 	bool	     exclude_guest_missing;
-	bool	     system_wide;
 	bool	     period;
 	unsigned int freq;
 	unsigned int mmap_pages;
@@ -231,7 +237,6 @@ struct perf_record_opts {
 	int	     branch_stack;
 	u64	     default_interval;
 	u64	     user_interval;
-	const char   *cpu_list;
 };
 
 #endif
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 1986d80..7080901 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -827,7 +827,7 @@ int perf_evlist__prepare_workload(struct perf_evlist *evlist,
 		exit(-1);
 	}
 
-	if (!opts->system_wide && !opts->target_tid && !opts->target_pid)
+	if (!opts->target.system_wide && !opts->target.tid && !opts->target.pid)
 		evlist->threads->map[0] = evlist->workload.pid;
 
 	close(child_ready_pipe[1]);
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 8c13dbc..d90598e 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -106,15 +106,15 @@ void perf_evsel__config(struct perf_evsel *evsel, struct perf_record_opts *opts,
 	if (opts->call_graph)
 		attr->sample_type	|= PERF_SAMPLE_CALLCHAIN;
 
-	if (opts->system_wide)
+	if (opts->target.system_wide)
 		attr->sample_type	|= PERF_SAMPLE_CPU;
 
 	if (opts->period)
 		attr->sample_type	|= PERF_SAMPLE_PERIOD;
 
 	if (!opts->sample_id_all_missing &&
-	    (opts->sample_time || opts->system_wide ||
-	     !opts->no_inherit || opts->cpu_list))
+	    (opts->sample_time || opts->target.system_wide ||
+	     !opts->no_inherit || opts->target.cpu_list))
 		attr->sample_type	|= PERF_SAMPLE_TIME;
 
 	if (opts->raw_samples) {
@@ -135,8 +135,8 @@ void perf_evsel__config(struct perf_evsel *evsel, struct perf_record_opts *opts,
 	attr->mmap = track;
 	attr->comm = track;
 
-	if (!opts->target_pid && !opts->target_tid && !opts->system_wide &&
-	    (!opts->group || evsel == first)) {
+	if (!opts->target.pid && !opts->target.tid &&
+	    !opts->target.system_wide && (!opts->group || evsel == first)) {
 		attr->disabled = 1;
 		attr->enable_on_exec = 1;
 	}

[tip:perf/core] perf stat: Convert to struct perf_target

[tip-bot for Namhyung Kim]

Commit-ID:  20f946b4a49dfd89c1c4ddeb55c0632893332674
Gitweb:     http://git.kernel.org/tip/20f946b4a49dfd89c1c4ddeb55c0632893332674
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Thu, 26 Apr 2012 14:15:16 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 2 May 2012 15:19:17 -0300

perf stat: Convert to struct perf_target

Use struct perf_target as it is introduced by previous patch.

This is a preparation of further changes.

Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
Reviewed-by: David Ahern <dsahern@gmail.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1335417327-11796-3-git-send-email-namhyung.kim@lge.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-stat.c |   47 +++++++++++++++++++++-----------------------
 1 files changed, 22 insertions(+), 25 deletions(-)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index dde9e17..1ca767d 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -175,22 +175,19 @@ static struct perf_event_attr very_very_detailed_attrs[] = {
 
 static struct perf_evlist	*evsel_list;
 
-static bool			system_wide			=  false;
-static int			run_idx				=  0;
+static struct perf_target	target;
 
+static int			run_idx				=  0;
 static int			run_count			=  1;
 static bool			no_inherit			= false;
 static bool			scale				=  true;
 static bool			no_aggr				= false;
-static const char		*target_pid;
-static const char		*target_tid;
 static pid_t			child_pid			= -1;
 static bool			null_run			=  false;
 static int			detailed_run			=  0;
 static bool			sync_run			=  false;
 static bool			big_num				=  true;
 static int			big_num_opt			=  -1;
-static const char		*cpu_list;
 static const char		*csv_sep			= NULL;
 static bool			csv_output			= false;
 static bool			group				= false;
@@ -293,10 +290,10 @@ static int create_perf_stat_counter(struct perf_evsel *evsel,
 
 	attr->inherit = !no_inherit;
 
-	if (system_wide)
+	if (target.system_wide)
 		return perf_evsel__open_per_cpu(evsel, evsel_list->cpus,
 						group, group_fd);
-	if (!target_pid && !target_tid && (!group || evsel == first)) {
+	if (!target.pid && !target.tid && (!group || evsel == first)) {
 		attr->disabled = 1;
 		attr->enable_on_exec = 1;
 	}
@@ -446,7 +443,7 @@ static int run_perf_stat(int argc __used, const char **argv)
 			exit(-1);
 		}
 
-		if (!target_tid && !target_pid && !system_wide)
+		if (!target.tid && !target.pid && !target.system_wide)
 			evsel_list->threads->map[0] = child_pid;
 
 		/*
@@ -476,7 +473,7 @@ static int run_perf_stat(int argc __used, const char **argv)
 				error("You may not have permission to collect %sstats.\n"
 				      "\t Consider tweaking"
 				      " /proc/sys/kernel/perf_event_paranoid or running as root.",
-				      system_wide ? "system-wide " : "");
+				      target.system_wide ? "system-wide " : "");
 			} else {
 				error("open_counter returned with %d (%s). "
 				      "/bin/dmesg may provide additional information.\n",
@@ -968,14 +965,14 @@ static void print_stat(int argc, const char **argv)
 	if (!csv_output) {
 		fprintf(output, "\n");
 		fprintf(output, " Performance counter stats for ");
-		if (!target_pid && !target_tid) {
+		if (!target.pid && !target.tid) {
 			fprintf(output, "\'%s", argv[0]);
 			for (i = 1; i < argc; i++)
 				fprintf(output, " %s", argv[i]);
-		} else if (target_pid)
-			fprintf(output, "process id \'%s", target_pid);
+		} else if (target.pid)
+			fprintf(output, "process id \'%s", target.pid);
 		else
-			fprintf(output, "thread id \'%s", target_tid);
+			fprintf(output, "thread id \'%s", target.tid);
 
 		fprintf(output, "\'");
 		if (run_count > 1)
@@ -1049,11 +1046,11 @@ static const struct option options[] = {
 		     "event filter", parse_filter),
 	OPT_BOOLEAN('i', "no-inherit", &no_inherit,
 		    "child tasks do not inherit counters"),
-	OPT_STRING('p', "pid", &target_pid, "pid",
+	OPT_STRING('p', "pid", &target.pid, "pid",
 		   "stat events on existing process id"),
-	OPT_STRING('t', "tid", &target_tid, "tid",
+	OPT_STRING('t', "tid", &target.tid, "tid",
 		   "stat events on existing thread id"),
-	OPT_BOOLEAN('a', "all-cpus", &system_wide,
+	OPT_BOOLEAN('a', "all-cpus", &target.system_wide,
 		    "system-wide collection from all CPUs"),
 	OPT_BOOLEAN('g', "group", &group,
 		    "put the counters into a counter group"),
@@ -1072,7 +1069,7 @@ static const struct option options[] = {
 	OPT_CALLBACK_NOOPT('B', "big-num", NULL, NULL, 
 			   "print large numbers with thousands\' separators",
 			   stat__set_big_num),
-	OPT_STRING('C', "cpu", &cpu_list, "cpu",
+	OPT_STRING('C', "cpu", &target.cpu_list, "cpu",
 		    "list of cpus to monitor in system-wide"),
 	OPT_BOOLEAN('A', "no-aggr", &no_aggr,
 		    "disable CPU count aggregation"),
@@ -1190,13 +1187,13 @@ int cmd_stat(int argc, const char **argv, const char *prefix __used)
 	} else if (big_num_opt == 0) /* User passed --no-big-num */
 		big_num = false;
 
-	if (!argc && !target_pid && !target_tid)
+	if (!argc && !target.pid && !target.tid)
 		usage_with_options(stat_usage, options);
 	if (run_count <= 0)
 		usage_with_options(stat_usage, options);
 
 	/* no_aggr, cgroup are for system-wide only */
-	if ((no_aggr || nr_cgroups) && !system_wide) {
+	if ((no_aggr || nr_cgroups) && !target.system_wide) {
 		fprintf(stderr, "both cgroup and no-aggregation "
 			"modes only available in system-wide mode\n");
 
@@ -1206,18 +1203,18 @@ int cmd_stat(int argc, const char **argv, const char *prefix __used)
 	if (add_default_attributes())
 		goto out;
 
-	if (target_pid)
-		target_tid = target_pid;
+	if (target.pid)
+		target.tid = target.pid;
 
-	evsel_list->threads = thread_map__new_str(target_pid,
-						  target_tid, UINT_MAX);
+	evsel_list->threads = thread_map__new_str(target.pid,
+						  target.tid, UINT_MAX);
 	if (evsel_list->threads == NULL) {
 		pr_err("Problems finding threads of monitor\n");
 		usage_with_options(stat_usage, options);
 	}
 
-	if (system_wide)
-		evsel_list->cpus = cpu_map__new(cpu_list);
+	if (target.system_wide)
+		evsel_list->cpus = cpu_map__new(target.cpu_list);
 	else
 		evsel_list->cpus = cpu_map__dummy_new();
 

[tip:perf/core] perf top: Convert to struct perf_target

[tip-bot for Namhyung Kim]

Commit-ID:  fe9d18a71d2018f8021fd2bd2aaf5137954ef839
Gitweb:     http://git.kernel.org/tip/fe9d18a71d2018f8021fd2bd2aaf5137954ef839
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Thu, 26 Apr 2012 14:15:17 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 2 May 2012 15:20:30 -0300

perf top: Convert to struct perf_target

Use struct perf_target as it is introduced by previous patch.

This is a preparation of further changes.

Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
Reviewed-by: David Ahern <dsahern@gmail.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1335417327-11796-4-git-send-email-namhyung.kim@lge.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-top.c |   33 +++++++++++++++++----------------
 tools/perf/util/top.c    |   19 ++++++++++---------
 tools/perf/util/top.h    |    6 +-----
 3 files changed, 28 insertions(+), 30 deletions(-)

diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 8ef59f8..2c1c207 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -588,7 +588,7 @@ static void *display_thread_tui(void *arg)
 	 * via --uid.
 	 */
 	list_for_each_entry(pos, &top->evlist->entries, node)
-		pos->hists.uid_filter_str = top->uid_str;
+		pos->hists.uid_filter_str = top->target.uid_str;
 
 	perf_evlist__tui_browse_hists(top->evlist, help,
 				      perf_top__sort_new_samples,
@@ -1016,7 +1016,7 @@ static int __cmd_top(struct perf_top *top)
 	if (ret)
 		goto out_delete;
 
-	if (top->target_tid || top->uid != UINT_MAX)
+	if (top->target.tid || top->target.uid != UINT_MAX)
 		perf_event__synthesize_thread_map(&top->tool, top->evlist->threads,
 						  perf_event__process,
 						  &top->session->host_machine);
@@ -1154,7 +1154,6 @@ int cmd_top(int argc, const char **argv, const char *prefix __used)
 	struct perf_top top = {
 		.count_filter	     = 5,
 		.delay_secs	     = 2,
-		.uid		     = UINT_MAX,
 		.freq		     = 1000, /* 1 KHz */
 		.mmap_pages	     = 128,
 		.sym_pcnt_filter     = 5,
@@ -1166,13 +1165,13 @@ int cmd_top(int argc, const char **argv, const char *prefix __used)
 		     parse_events_option),
 	OPT_INTEGER('c', "count", &top.default_interval,
 		    "event period to sample"),
-	OPT_STRING('p', "pid", &top.target_pid, "pid",
+	OPT_STRING('p', "pid", &top.target.pid, "pid",
 		    "profile events on existing process id"),
-	OPT_STRING('t', "tid", &top.target_tid, "tid",
+	OPT_STRING('t', "tid", &top.target.tid, "tid",
 		    "profile events on existing thread id"),
-	OPT_BOOLEAN('a', "all-cpus", &top.system_wide,
+	OPT_BOOLEAN('a', "all-cpus", &top.target.system_wide,
 			    "system-wide collection from all CPUs"),
-	OPT_STRING('C', "cpu", &top.cpu_list, "cpu",
+	OPT_STRING('C', "cpu", &top.target.cpu_list, "cpu",
 		    "list of cpus to monitor"),
 	OPT_STRING('k', "vmlinux", &symbol_conf.vmlinux_name,
 		   "file", "vmlinux pathname"),
@@ -1227,7 +1226,7 @@ int cmd_top(int argc, const char **argv, const char *prefix __used)
 		    "Display raw encoding of assembly instructions (default)"),
 	OPT_STRING('M', "disassembler-style", &disassembler_style, "disassembler style",
 		   "Specify disassembler style (e.g. -M intel for intel syntax)"),
-	OPT_STRING('u', "uid", &top.uid_str, "user", "user to profile"),
+	OPT_STRING('u', "uid", &top.target.uid_str, "user", "user to profile"),
 	OPT_END()
 	};
 
@@ -1253,22 +1252,24 @@ int cmd_top(int argc, const char **argv, const char *prefix __used)
 
 	setup_browser(false);
 
-	top.uid = parse_target_uid(top.uid_str, top.target_tid, top.target_pid);
-	if (top.uid_str != NULL && top.uid == UINT_MAX - 1)
+	top.target.uid = parse_target_uid(top.target.uid_str, top.target.tid,
+					  top.target.pid);
+	if (top.target.uid_str != NULL && top.target.uid == UINT_MAX - 1)
 		goto out_delete_evlist;
 
 	/* CPU and PID are mutually exclusive */
-	if (top.target_tid && top.cpu_list) {
+	if (top.target.tid && top.target.cpu_list) {
 		printf("WARNING: PID switch overriding CPU\n");
 		sleep(1);
-		top.cpu_list = NULL;
+		top.target.cpu_list = NULL;
 	}
 
-	if (top.target_pid)
-		top.target_tid = top.target_pid;
+	if (top.target.pid)
+		top.target.tid = top.target.pid;
 
-	if (perf_evlist__create_maps(top.evlist, top.target_pid,
-				     top.target_tid, top.uid, top.cpu_list) < 0)
+	if (perf_evlist__create_maps(top.evlist, top.target.pid,
+				     top.target.tid, top.target.uid,
+				     top.target.cpu_list) < 0)
 		usage_with_options(top_usage, options);
 
 	if (!top.evlist->nr_entries &&
diff --git a/tools/perf/util/top.c b/tools/perf/util/top.c
index 09fe579..abe0e8e 100644
--- a/tools/perf/util/top.c
+++ b/tools/perf/util/top.c
@@ -69,23 +69,24 @@ size_t perf_top__header_snprintf(struct perf_top *top, char *bf, size_t size)
 
 	ret += SNPRINTF(bf + ret, size - ret, "], ");
 
-	if (top->target_pid)
+	if (top->target.pid)
 		ret += SNPRINTF(bf + ret, size - ret, " (target_pid: %s",
-				top->target_pid);
-	else if (top->target_tid)
+				top->target.pid);
+	else if (top->target.tid)
 		ret += SNPRINTF(bf + ret, size - ret, " (target_tid: %s",
-				top->target_tid);
-	else if (top->uid_str != NULL)
+				top->target.tid);
+	else if (top->target.uid_str != NULL)
 		ret += SNPRINTF(bf + ret, size - ret, " (uid: %s",
-				top->uid_str);
+				top->target.uid_str);
 	else
 		ret += SNPRINTF(bf + ret, size - ret, " (all");
 
-	if (top->cpu_list)
+	if (top->target.cpu_list)
 		ret += SNPRINTF(bf + ret, size - ret, ", CPU%s: %s)",
-				top->evlist->cpus->nr > 1 ? "s" : "", top->cpu_list);
+				top->evlist->cpus->nr > 1 ? "s" : "",
+				top->target.cpu_list);
 	else {
-		if (top->target_tid)
+		if (top->target.tid)
 			ret += SNPRINTF(bf + ret, size - ret, ")");
 		else
 			ret += SNPRINTF(bf + ret, size - ret, ", %d CPU%s)",
diff --git a/tools/perf/util/top.h b/tools/perf/util/top.h
index ce61cb2..33347ca 100644
--- a/tools/perf/util/top.h
+++ b/tools/perf/util/top.h
@@ -13,6 +13,7 @@ struct perf_session;
 struct perf_top {
 	struct perf_tool   tool;
 	struct perf_evlist *evlist;
+	struct perf_target target;
 	/*
 	 * Symbols will be added here in perf_event__process_sample and will
 	 * get out after decayed.
@@ -23,10 +24,7 @@ struct perf_top {
 	u64		   guest_us_samples, guest_kernel_samples;
 	int		   print_entries, count_filter, delay_secs;
 	int		   freq;
-	const char	   *target_pid, *target_tid;
-	uid_t		   uid;
 	bool		   hide_kernel_symbols, hide_user_symbols, zero;
-	bool		   system_wide;
 	bool		   use_tui, use_stdio;
 	bool		   sort_has_symbols;
 	bool		   dont_use_callchains;
@@ -37,7 +35,6 @@ struct perf_top {
 	bool		   sample_id_all_missing;
 	bool		   exclude_guest_missing;
 	bool		   dump_symtab;
-	const char	   *cpu_list;
 	struct hist_entry  *sym_filter_entry;
 	struct perf_evsel  *sym_evsel;
 	struct perf_session *session;
@@ -47,7 +44,6 @@ struct perf_top {
 	int		   realtime_prio;
 	int		   sym_pcnt_filter;
 	const char	   *sym_filter;
-	const char	   *uid_str;
 };
 
 size_t perf_top__header_snprintf(struct perf_top *top, char *bf, size_t size);

[tip:perf/core] perf tools: Introduce perf_target__validate() helper

[tip-bot for Namhyung Kim]

Commit-ID:  4bd0f2d2c0cf14de9c84c2fe689120c6b0f667c8
Gitweb:     http://git.kernel.org/tip/4bd0f2d2c0cf14de9c84c2fe689120c6b0f667c8
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Thu, 26 Apr 2012 14:15:18 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 2 May 2012 15:22:08 -0300

perf tools: Introduce perf_target__validate() helper

The perf_target__validate function is used to check given PID/TID/UID/CPU
target options and warn if some combination is impossible. Also this can
make some arguments of parse_target_uid() function useless as it is checked
before the call via our new helper.

Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
Reviewed-by: David Ahern <dsahern@gmail.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1335417327-11796-5-git-send-email-namhyung.kim@lge.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-record.c |    9 +++------
 tools/perf/builtin-stat.c   |    3 +--
 tools/perf/builtin-top.c    |   15 +++------------
 tools/perf/util/usage.c     |   29 +++++++++++++++++++++--------
 tools/perf/util/util.h      |    4 +++-
 5 files changed, 31 insertions(+), 29 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 4dcf270..3596cca 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -884,16 +884,13 @@ int cmd_record(int argc, const char **argv, const char *prefix __used)
 		goto out_symbol_exit;
 	}
 
-	rec->opts.target.uid = parse_target_uid(rec->opts.target.uid_str,
-						rec->opts.target.tid,
-						rec->opts.target.pid);
+	perf_target__validate(&rec->opts.target);
+
+	rec->opts.target.uid = parse_target_uid(rec->opts.target.uid_str);
 	if (rec->opts.target.uid_str != NULL &&
 	    rec->opts.target.uid == UINT_MAX - 1)
 		goto out_free_fd;
 
-	if (rec->opts.target.pid)
-		rec->opts.target.tid = rec->opts.target.pid;
-
 	if (perf_evlist__create_maps(evsel_list, rec->opts.target.pid,
 				     rec->opts.target.tid, rec->opts.target.uid,
 				     rec->opts.target.cpu_list) < 0)
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 1ca767d..bb77232 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -1203,8 +1203,7 @@ int cmd_stat(int argc, const char **argv, const char *prefix __used)
 	if (add_default_attributes())
 		goto out;
 
-	if (target.pid)
-		target.tid = target.pid;
+	perf_target__validate(&target);
 
 	evsel_list->threads = thread_map__new_str(target.pid,
 						  target.tid, UINT_MAX);
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 2c1c207..4f47952 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1252,21 +1252,12 @@ int cmd_top(int argc, const char **argv, const char *prefix __used)
 
 	setup_browser(false);
 
-	top.target.uid = parse_target_uid(top.target.uid_str, top.target.tid,
-					  top.target.pid);
+	perf_target__validate(&top.target);
+
+	top.target.uid = parse_target_uid(top.target.uid_str);
 	if (top.target.uid_str != NULL && top.target.uid == UINT_MAX - 1)
 		goto out_delete_evlist;
 
-	/* CPU and PID are mutually exclusive */
-	if (top.target.tid && top.target.cpu_list) {
-		printf("WARNING: PID switch overriding CPU\n");
-		sleep(1);
-		top.target.cpu_list = NULL;
-	}
-
-	if (top.target.pid)
-		top.target.tid = top.target.pid;
-
 	if (perf_evlist__create_maps(top.evlist, top.target.pid,
 				     top.target.tid, top.target.uid,
 				     top.target.cpu_list) < 0)
diff --git a/tools/perf/util/usage.c b/tools/perf/util/usage.c
index 52bb07c..0a1a885 100644
--- a/tools/perf/util/usage.c
+++ b/tools/perf/util/usage.c
@@ -83,7 +83,7 @@ void warning(const char *warn, ...)
 	va_end(params);
 }
 
-uid_t parse_target_uid(const char *str, const char *tid, const char *pid)
+uid_t parse_target_uid(const char *str)
 {
 	struct passwd pwd, *result;
 	char buf[1024];
@@ -91,13 +91,6 @@ uid_t parse_target_uid(const char *str, const char *tid, const char *pid)
 	if (str == NULL)
 		return UINT_MAX;
 
-	/* UID and PID are mutually exclusive */
-	if (tid || pid) {
-		ui__warning("PID/TID switch overriding UID\n");
-		sleep(1);
-		return UINT_MAX;
-	}
-
 	getpwnam_r(str, &pwd, buf, sizeof(buf), &result);
 
 	if (result == NULL) {
@@ -120,3 +113,23 @@ uid_t parse_target_uid(const char *str, const char *tid, const char *pid)
 
 	return result->pw_uid;
 }
+
+void perf_target__validate(struct perf_target *target)
+{
+	if (target->pid)
+		target->tid = target->pid;
+
+	/* CPU and PID are mutually exclusive */
+	if (target->tid && target->cpu_list) {
+		ui__warning("WARNING: PID switch overriding CPU\n");
+		sleep(1);
+		target->cpu_list = NULL;
+	}
+
+	/* UID and PID are mutually exclusive */
+	if (target->tid && target->uid_str) {
+		ui__warning("PID/TID switch overriding UID\n");
+		sleep(1);
+		target->uid_str = NULL;
+	}
+}
diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index 0f99f39..3f05d62 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -246,10 +246,12 @@ unsigned long convert_unit(unsigned long value, char *unit);
 int readn(int fd, void *buf, size_t size);
 
 struct perf_event_attr;
+struct perf_target;
 
 void event_attr_init(struct perf_event_attr *attr);
 
-uid_t parse_target_uid(const char *str, const char *tid, const char *pid);
+uid_t parse_target_uid(const char *str);
+void perf_target__validate(struct perf_target *target);
 
 #define _STR(x) #x
 #define STR(x) _STR(x)

[tip:perf/core] perf evlist: Make create_maps() take struct perf_target

[tip-bot for Namhyung Kim]

Commit-ID:  b809ac100e2f12ebf1b58ff522dba15651a77d27
Gitweb:     http://git.kernel.org/tip/b809ac100e2f12ebf1b58ff522dba15651a77d27
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Thu, 26 Apr 2012 14:15:19 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 2 May 2012 15:23:11 -0300

perf evlist: Make create_maps() take struct perf_target

Now we have all information that needed to create cpu/thread maps in
struct perf_target, it'd be better using it as an argument.

Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
Reviewed-by: David Ahern <dsahern@gmail.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1335417327-11796-6-git-send-email-namhyung.kim@lge.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-record.c |    4 +---
 tools/perf/builtin-test.c   |    7 ++++---
 tools/perf/builtin-top.c    |    4 +---
 tools/perf/util/evlist.c    |   12 +++++++-----
 tools/perf/util/evlist.h    |    4 ++--
 5 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 3596cca..d165909 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -891,9 +891,7 @@ int cmd_record(int argc, const char **argv, const char *prefix __used)
 	    rec->opts.target.uid == UINT_MAX - 1)
 		goto out_free_fd;
 
-	if (perf_evlist__create_maps(evsel_list, rec->opts.target.pid,
-				     rec->opts.target.tid, rec->opts.target.uid,
-				     rec->opts.target.cpu_list) < 0)
+	if (perf_evlist__create_maps(evsel_list, &rec->opts.target) < 0)
 		usage_with_options(record_usage, record_options);
 
 	list_for_each_entry(pos, &evsel_list->entries, node) {
diff --git a/tools/perf/builtin-test.c b/tools/perf/builtin-test.c
index 27882d8..9d9abbb 100644
--- a/tools/perf/builtin-test.c
+++ b/tools/perf/builtin-test.c
@@ -1165,6 +1165,9 @@ realloc:
 static int test__PERF_RECORD(void)
 {
 	struct perf_record_opts opts = {
+		.target = {
+			.uid = UINT_MAX,
+		},
 		.no_delay   = true,
 		.freq	    = 10,
 		.mmap_pages = 256,
@@ -1207,9 +1210,7 @@ static int test__PERF_RECORD(void)
 	 * perf_evlist__prepare_workload we'll fill in the only thread
 	 * we're monitoring, the one forked there.
 	 */
-	err = perf_evlist__create_maps(evlist, opts.target.pid,
-				       opts.target.tid, UINT_MAX,
-				       opts.target.cpu_list);
+	err = perf_evlist__create_maps(evlist, &opts.target);
 	if (err < 0) {
 		pr_debug("Not enough memory to create thread/cpu maps\n");
 		goto out_delete_evlist;
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 4f47952..2a0ec09 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1258,9 +1258,7 @@ int cmd_top(int argc, const char **argv, const char *prefix __used)
 	if (top.target.uid_str != NULL && top.target.uid == UINT_MAX - 1)
 		goto out_delete_evlist;
 
-	if (perf_evlist__create_maps(top.evlist, top.target.pid,
-				     top.target.tid, top.target.uid,
-				     top.target.cpu_list) < 0)
+	if (perf_evlist__create_maps(top.evlist, &top.target) < 0)
 		usage_with_options(top_usage, options);
 
 	if (!top.evlist->nr_entries &&
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 7080901..a43e2c5 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -599,18 +599,20 @@ int perf_evlist__mmap(struct perf_evlist *evlist, unsigned int pages,
 	return perf_evlist__mmap_per_cpu(evlist, prot, mask);
 }
 
-int perf_evlist__create_maps(struct perf_evlist *evlist, const char *target_pid,
-			     const char *target_tid, uid_t uid, const char *cpu_list)
+int perf_evlist__create_maps(struct perf_evlist *evlist,
+			     struct perf_target *target)
 {
-	evlist->threads = thread_map__new_str(target_pid, target_tid, uid);
+	evlist->threads = thread_map__new_str(target->pid, target->tid,
+					      target->uid);
 
 	if (evlist->threads == NULL)
 		return -1;
 
-	if (uid != UINT_MAX || (cpu_list == NULL && target_tid))
+	if (target->uid != UINT_MAX ||
+	    (target->cpu_list == NULL && target->tid))
 		evlist->cpus = cpu_map__dummy_new();
 	else
-		evlist->cpus = cpu_map__new(cpu_list);
+		evlist->cpus = cpu_map__new(target->cpu_list);
 
 	if (evlist->cpus == NULL)
 		goto out_delete_threads;
diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
index 21f1c9e..58abb63 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -106,8 +106,8 @@ static inline void perf_evlist__set_maps(struct perf_evlist *evlist,
 	evlist->threads	= threads;
 }
 
-int perf_evlist__create_maps(struct perf_evlist *evlist, const char *target_pid,
-			     const char *tid, uid_t uid, const char *cpu_list);
+int perf_evlist__create_maps(struct perf_evlist *evlist,
+			     struct perf_target *target);
 void perf_evlist__delete_maps(struct perf_evlist *evlist);
 int perf_evlist__set_filters(struct perf_evlist *evlist);
 

[tip:perf/core] perf tools: Check more combinations of PID/TID, UID and CPU switches

[tip-bot for Namhyung Kim]

Commit-ID:  770a34a38b74982724dbb099225944b415f90281
Gitweb:     http://git.kernel.org/tip/770a34a38b74982724dbb099225944b415f90281
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Thu, 26 Apr 2012 14:15:20 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 2 May 2012 15:24:14 -0300

perf tools: Check more combinations of PID/TID, UID and CPU switches

There were some combinations of these switches that are not so
appropriate IMHO.

Since there are implicit priorities between them and they worked well
anyway, but it ends up opening useless duplicated events.

For example, 'perf stat -t <pid> -a' will open multiple events for the
thread instead of one.

Add explicit checks and warn user in perf_target__validate().

Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
Reviewed-by: David Ahern <dsahern@gmail.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1335417327-11796-7-git-send-email-namhyung.kim@lge.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/usage.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/tools/perf/util/usage.c b/tools/perf/util/usage.c
index 0a1a885..228f0a5 100644
--- a/tools/perf/util/usage.c
+++ b/tools/perf/util/usage.c
@@ -132,4 +132,18 @@ void perf_target__validate(struct perf_target *target)
 		sleep(1);
 		target->uid_str = NULL;
 	}
+
+	/* UID and CPU are mutually exclusive */
+	if (target->uid_str && target->cpu_list) {
+		ui__warning("UID switch overriding CPU\n");
+		sleep(1);
+		target->cpu_list = NULL;
+	}
+
+	/* PID/UID and SYSTEM are mutually exclusive */
+	if ((target->tid || target->uid_str) && target->system_wide) {
+		ui__warning("PID/TID/UID switch overriding CPU\n");
+		sleep(1);
+		target->system_wide = false;
+	}
 }

[tip:perf/core] perf target: Split out perf_target handling code

[tip-bot for Namhyung Kim]

Commit-ID:  12864b31583bcbd26789ebe68c612688f9ee2e30
Gitweb:     http://git.kernel.org/tip/12864b31583bcbd26789ebe68c612688f9ee2e30
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Thu, 26 Apr 2012 14:15:22 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 2 May 2012 15:41:11 -0300

perf target: Split out perf_target handling code

For further work on perf_target, it'd be better off splitting the code
into a separate file.

Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
Reviewed-by: David Ahern <dsahern@gmail.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1335417327-11796-9-git-send-email-namhyung.kim@lge.com
[ committer note: Fixed perl build by using stdbool and types.h in target.h ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Makefile      |    2 ++
 tools/perf/perf.h        |    9 +--------
 tools/perf/util/evlist.c |    1 +
 tools/perf/util/evsel.c  |    1 +
 tools/perf/util/target.c |   45 +++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/target.h |   18 ++++++++++++++++++
 tools/perf/util/usage.c  |   34 ----------------------------------
 tools/perf/util/util.h   |    2 --
 8 files changed, 68 insertions(+), 44 deletions(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index e98e14c..4122a66 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -300,6 +300,7 @@ LIB_H += util/cpumap.h
 LIB_H += util/top.h
 LIB_H += $(ARCH_INCLUDE)
 LIB_H += util/cgroup.h
+LIB_H += util/target.h
 
 LIB_OBJS += $(OUTPUT)util/abspath.o
 LIB_OBJS += $(OUTPUT)util/alias.o
@@ -361,6 +362,7 @@ LIB_OBJS += $(OUTPUT)util/util.o
 LIB_OBJS += $(OUTPUT)util/xyarray.o
 LIB_OBJS += $(OUTPUT)util/cpumap.o
 LIB_OBJS += $(OUTPUT)util/cgroup.o
+LIB_OBJS += $(OUTPUT)util/target.o
 
 BUILTIN_OBJS += $(OUTPUT)builtin-annotate.o
 
diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index 7e226c0..14f1034 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -207,14 +207,7 @@ extern const char perf_version_string[];
 
 void pthread__unblock_sigwinch(void);
 
-struct perf_target {
-	const char   *pid;
-	const char   *tid;
-	const char   *cpu_list;
-	const char   *uid_str;
-	uid_t	     uid;
-	bool	     system_wide;
-};
+#include "util/target.h"
 
 struct perf_record_opts {
 	struct perf_target target;
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index a43e2c5..3032862 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -11,6 +11,7 @@
 #include <poll.h>
 #include "cpumap.h"
 #include "thread_map.h"
+#include "target.h"
 #include "evlist.h"
 #include "evsel.h"
 #include <unistd.h>
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index d90598e..bb785a0 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -14,6 +14,7 @@
 #include "util.h"
 #include "cpumap.h"
 #include "thread_map.h"
+#include "target.h"
 
 #define FD(e, x, y) (*(int *)xyarray__entry(e->fd, x, y))
 #define GROUP_FD(group_fd, cpu) (*(int *)xyarray__entry(group_fd, cpu, 0))
diff --git a/tools/perf/util/target.c b/tools/perf/util/target.c
new file mode 100644
index 0000000..3fadf85
--- /dev/null
+++ b/tools/perf/util/target.c
@@ -0,0 +1,45 @@
+/*
+ * Helper functions for handling target threads/cpus
+ *
+ * Copyright (C) 2012, LG Electronics, Namhyung Kim <namhyung.kim@lge.com>
+ *
+ * Released under the GPL v2.
+ */
+
+#include "target.h"
+#include "debug.h"
+
+
+void perf_target__validate(struct perf_target *target)
+{
+	if (target->pid)
+		target->tid = target->pid;
+
+	/* CPU and PID are mutually exclusive */
+	if (target->tid && target->cpu_list) {
+		ui__warning("WARNING: PID switch overriding CPU\n");
+		sleep(1);
+		target->cpu_list = NULL;
+	}
+
+	/* UID and PID are mutually exclusive */
+	if (target->tid && target->uid_str) {
+		ui__warning("PID/TID switch overriding UID\n");
+		sleep(1);
+		target->uid_str = NULL;
+	}
+
+	/* UID and CPU are mutually exclusive */
+	if (target->uid_str && target->cpu_list) {
+		ui__warning("UID switch overriding CPU\n");
+		sleep(1);
+		target->cpu_list = NULL;
+	}
+
+	/* PID/UID and SYSTEM are mutually exclusive */
+	if ((target->tid || target->uid_str) && target->system_wide) {
+		ui__warning("PID/TID/UID switch overriding CPU\n");
+		sleep(1);
+		target->system_wide = false;
+	}
+}
diff --git a/tools/perf/util/target.h b/tools/perf/util/target.h
new file mode 100644
index 0000000..218291f
--- /dev/null
+++ b/tools/perf/util/target.h
@@ -0,0 +1,18 @@
+#ifndef _PERF_TARGET_H
+#define _PERF_TARGET_H
+
+#include <stdbool.h>
+#include <sys/types.h>
+
+struct perf_target {
+	const char   *pid;
+	const char   *tid;
+	const char   *cpu_list;
+	const char   *uid_str;
+	uid_t	     uid;
+	bool	     system_wide;
+};
+
+void perf_target__validate(struct perf_target *target);
+
+#endif /* _PERF_TARGET_H */
diff --git a/tools/perf/util/usage.c b/tools/perf/util/usage.c
index 228f0a5..e851abc 100644
--- a/tools/perf/util/usage.c
+++ b/tools/perf/util/usage.c
@@ -113,37 +113,3 @@ uid_t parse_target_uid(const char *str)
 
 	return result->pw_uid;
 }
-
-void perf_target__validate(struct perf_target *target)
-{
-	if (target->pid)
-		target->tid = target->pid;
-
-	/* CPU and PID are mutually exclusive */
-	if (target->tid && target->cpu_list) {
-		ui__warning("WARNING: PID switch overriding CPU\n");
-		sleep(1);
-		target->cpu_list = NULL;
-	}
-
-	/* UID and PID are mutually exclusive */
-	if (target->tid && target->uid_str) {
-		ui__warning("PID/TID switch overriding UID\n");
-		sleep(1);
-		target->uid_str = NULL;
-	}
-
-	/* UID and CPU are mutually exclusive */
-	if (target->uid_str && target->cpu_list) {
-		ui__warning("UID switch overriding CPU\n");
-		sleep(1);
-		target->cpu_list = NULL;
-	}
-
-	/* PID/UID and SYSTEM are mutually exclusive */
-	if ((target->tid || target->uid_str) && target->system_wide) {
-		ui__warning("PID/TID/UID switch overriding CPU\n");
-		sleep(1);
-		target->system_wide = false;
-	}
-}
diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index 3f05d62..52be74c 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -246,12 +246,10 @@ unsigned long convert_unit(unsigned long value, char *unit);
 int readn(int fd, void *buf, size_t size);
 
 struct perf_event_attr;
-struct perf_target;
 
 void event_attr_init(struct perf_event_attr *attr);
 
 uid_t parse_target_uid(const char *str);
-void perf_target__validate(struct perf_target *target);
 
 #define _STR(x) #x
 #define STR(x) _STR(x)