[PATCH 0/7] perf tools: Fix cpu/thread map handling v3

[PATCH 0/7] perf tools: Fix cpu/thread map handling v3

[Namhyung Kim]

Hi, Arnaldo

This is my third iteration on the series and some patches in the previous set
got merged to your tree. You can see the discussion on the previous spin at
links below: [1], [2]

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. 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 acme/perf/core: 9389a46043c8 ("perf session:
Fail on processing event with unknown size").

 * Changes from v2
  - perf target errno uses negative number not to clash with standard errno.
    (thanks to Arnaldo)
  - Make return value of success 0.
  - Fix perf top not to break with this change.
  - Add Reviewed-by's from David Ahern.

 * 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
[2] https://lkml.org/lkml/2012/4/26/6


Namhyung Kim (7):
  perf top: Defaults to system_wide target
  perf evlist: Fix creation of cpu map
  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/builtin-record.c |   24 ++++++---
 tools/perf/builtin-stat.c   |   34 +++++--------
 tools/perf/builtin-top.c    |   25 +++++++--
 tools/perf/util/debug.c     |    1 +
 tools/perf/util/evlist.c    |    9 ++--
 tools/perf/util/evsel.c     |    8 +--
 tools/perf/util/target.c    |  119 +++++++++++++++++++++++++++++++++++++++----
 tools/perf/util/target.h    |   48 ++++++++++++++++-
 tools/perf/util/usage.c     |   31 -----------
 tools/perf/util/util.h      |    3 --
 10 files changed, 215 insertions(+), 87 deletions(-)

-- 
1.7.10.1

[PATCH 1/7] perf top: Defaults to system_wide target

[Namhyung Kim]

The default behaviour of perf to is monitoring events
system-wide, so make it explicit.

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

diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 2a0ec09b9b77..3ffb2b697968 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1152,6 +1152,9 @@ int cmd_top(int argc, const char **argv, const char *prefix __used)
 	struct perf_evsel *pos;
 	int status = -ENOMEM;
 	struct perf_top top = {
+		.target		     = {
+			.system_wide = true,
+		},
 		.count_filter	     = 5,
 		.delay_secs	     = 2,
 		.freq		     = 1000, /* 1 KHz */
-- 
1.7.10.1

[PATCH 2/7] 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>
Reviewed-by: David Ahern <dsahern@gmail.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 30328623cae6..183b199b0d09 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -609,8 +609,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.1

[tip:perf/core] perf evlist: Fix creation of cpu map

[tip-bot for Namhyung Kim]

Commit-ID:  55261f46702cec96911a81aacfb3cba13434d304
Gitweb:     http://git.kernel.org/tip/55261f46702cec96911a81aacfb3cba13434d304
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Mon, 7 May 2012 14:08:59 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 7 May 2012 16:46:21 -0300

perf evlist: Fix creation of cpu map

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>
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/1336367344-28071-3-git-send-email-namhyung.kim@lge.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/evlist.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 3032862..183b199 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -609,8 +609,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);

[PATCH 3/7] 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>
Reviewed-by: David Ahern <dsahern@gmail.com>
Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
---
 tools/perf/util/target.c |   33 ++++++++++++++++++++++-----------
 tools/perf/util/target.h |   25 ++++++++++++++++++++++++-
 2 files changed, 46 insertions(+), 12 deletions(-)

diff --git a/tools/perf/util/target.c b/tools/perf/util/target.c
index 3fadf85dd7e3..5c59dcfc8f8d 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_ERRNO_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_ERRNO_TARGET__SUCCESS)
+			ret = PERF_ERRNO_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_ERRNO_TARGET__SUCCESS)
+			ret = PERF_ERRNO_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_ERRNO_TARGET__SUCCESS)
+			ret = PERF_ERRNO_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_ERRNO_TARGET__SUCCESS)
+			ret = PERF_ERRNO_TARGET__PID_OVERRIDE_SYSTEM;
 	}
+
+	/* UID and SYSTEM are mutually exclusive */
+	if (target->uid_str && target->system_wide) {
+		target->system_wide = false;
+		if (ret == PERF_ERRNO_TARGET__SUCCESS)
+			ret = PERF_ERRNO_TARGET__UID_OVERRIDE_SYSTEM;
+	}
+
+	return ret;
 }
diff --git a/tools/perf/util/target.h b/tools/perf/util/target.h
index 218291f921ed..eb0d2101154a 100644
--- a/tools/perf/util/target.h
+++ b/tools/perf/util/target.h
@@ -13,6 +13,29 @@ struct perf_target {
 	bool	     system_wide;
 };
 
-void perf_target__validate(struct perf_target *target);
+enum perf_target_errno {
+	PERF_ERRNO_TARGET__SUCCESS		= 0,
+
+	/*
+	 * Choose an arbitrary negative big number not to clash with standard
+	 * errno since SUS requires the errno has distinct positive values.
+	 * See 'Issue 6' in the link below.
+	 *
+	 * http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/errno.h.html
+	 */
+	__PERF_ERRNO_TARGET__START		= -10000,
+
+
+	/* for perf_target__validate() */
+	PERF_ERRNO_TARGET__PID_OVERRIDE_CPU	= __PERF_ERRNO_TARGET__START,
+	PERF_ERRNO_TARGET__PID_OVERRIDE_UID,
+	PERF_ERRNO_TARGET__UID_OVERRIDE_CPU,
+	PERF_ERRNO_TARGET__PID_OVERRIDE_SYSTEM,
+	PERF_ERRNO_TARGET__UID_OVERRIDE_SYSTEM,
+
+	__PERF_ERRNO_TARGET__END,
+};
+
+enum perf_target_errno perf_target__validate(struct perf_target *target);
 
 #endif /* _PERF_TARGET_H */
-- 
1.7.10.1

[tip:perf/core] perf target: Introduce perf_target_errno

[tip-bot for Namhyung Kim]

Commit-ID:  60bbddaaa33865633efa2800702e3b02495a0e94
Gitweb:     http://git.kernel.org/tip/60bbddaaa33865633efa2800702e3b02495a0e94
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Mon, 7 May 2012 14:09:00 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 7 May 2012 16:46:35 -0300

perf target: Introduce perf_target_errno

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.

Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
Suggested-by: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
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/1336367344-28071-4-git-send-email-namhyung.kim@lge.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/target.c |   33 ++++++++++++++++++++++-----------
 tools/perf/util/target.h |   25 ++++++++++++++++++++++++-
 2 files changed, 46 insertions(+), 12 deletions(-)

diff --git a/tools/perf/util/target.c b/tools/perf/util/target.c
index 3fadf85..5c59dcf 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_ERRNO_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_ERRNO_TARGET__SUCCESS)
+			ret = PERF_ERRNO_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_ERRNO_TARGET__SUCCESS)
+			ret = PERF_ERRNO_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_ERRNO_TARGET__SUCCESS)
+			ret = PERF_ERRNO_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_ERRNO_TARGET__SUCCESS)
+			ret = PERF_ERRNO_TARGET__PID_OVERRIDE_SYSTEM;
 	}
+
+	/* UID and SYSTEM are mutually exclusive */
+	if (target->uid_str && target->system_wide) {
+		target->system_wide = false;
+		if (ret == PERF_ERRNO_TARGET__SUCCESS)
+			ret = PERF_ERRNO_TARGET__UID_OVERRIDE_SYSTEM;
+	}
+
+	return ret;
 }
diff --git a/tools/perf/util/target.h b/tools/perf/util/target.h
index 218291f..eb0d210 100644
--- a/tools/perf/util/target.h
+++ b/tools/perf/util/target.h
@@ -13,6 +13,29 @@ struct perf_target {
 	bool	     system_wide;
 };
 
-void perf_target__validate(struct perf_target *target);
+enum perf_target_errno {
+	PERF_ERRNO_TARGET__SUCCESS		= 0,
+
+	/*
+	 * Choose an arbitrary negative big number not to clash with standard
+	 * errno since SUS requires the errno has distinct positive values.
+	 * See 'Issue 6' in the link below.
+	 *
+	 * http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/errno.h.html
+	 */
+	__PERF_ERRNO_TARGET__START		= -10000,
+
+
+	/* for perf_target__validate() */
+	PERF_ERRNO_TARGET__PID_OVERRIDE_CPU	= __PERF_ERRNO_TARGET__START,
+	PERF_ERRNO_TARGET__PID_OVERRIDE_UID,
+	PERF_ERRNO_TARGET__UID_OVERRIDE_CPU,
+	PERF_ERRNO_TARGET__PID_OVERRIDE_SYSTEM,
+	PERF_ERRNO_TARGET__UID_OVERRIDE_SYSTEM,
+
+	__PERF_ERRNO_TARGET__END,
+};
+
+enum perf_target_errno perf_target__validate(struct perf_target *target);
 
 #endif /* _PERF_TARGET_H */

[PATCH 4/7] 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>
Reviewed-by: David Ahern <dsahern@gmail.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..d26a279796d9 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) < 0)
 		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 3ffb2b697968..59883c8c5817 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1257,8 +1257,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) < 0)
 		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 5c59dcfc8f8d..02a6bedb69a3 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_ERRNO_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_ERRNO_TARGET__INVALID_UID;
+
+		getpwuid_r(uid, &pwd, buf, sizeof(buf), &result);
+
+		if (result == NULL)
+			return PERF_ERRNO_TARGET__USER_NOT_FOUND;
+	}
+
+	target->uid = result->pw_uid;
+	return PERF_ERRNO_TARGET__SUCCESS;
+}
diff --git a/tools/perf/util/target.h b/tools/perf/util/target.h
index eb0d2101154a..d4aabdaba42a 100644
--- a/tools/perf/util/target.h
+++ b/tools/perf/util/target.h
@@ -33,9 +33,14 @@ enum perf_target_errno {
 	PERF_ERRNO_TARGET__PID_OVERRIDE_SYSTEM,
 	PERF_ERRNO_TARGET__UID_OVERRIDE_SYSTEM,
 
+	/* for perf_target__parse_uid() */
+	PERF_ERRNO_TARGET__INVALID_UID,
+	PERF_ERRNO_TARGET__USER_NOT_FOUND,
+
 	__PERF_ERRNO_TARGET__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.1

[tip:perf/core] perf target: Introduce perf_target__parse_uid()

[tip-bot for Namhyung Kim]

Commit-ID:  dfe78adaaca90417ece98edbd3eb1c9661334406
Gitweb:     http://git.kernel.org/tip/dfe78adaaca90417ece98edbd3eb1c9661334406
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Mon, 7 May 2012 14:09:01 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 7 May 2012 16:46:48 -0300

perf target: Introduce perf_target__parse_uid()

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>
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/1336367344-28071-5-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-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 d165909..d26a279 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) < 0)
 		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 e40f86e..c9137ba 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) < 0)
 		goto out_delete_evlist;
 
 	if (top.target.tid == 0 && top.target.pid == 0 &&
diff --git a/tools/perf/util/target.c b/tools/perf/util/target.c
index 5c59dcf..02a6bed 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_ERRNO_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_ERRNO_TARGET__INVALID_UID;
+
+		getpwuid_r(uid, &pwd, buf, sizeof(buf), &result);
+
+		if (result == NULL)
+			return PERF_ERRNO_TARGET__USER_NOT_FOUND;
+	}
+
+	target->uid = result->pw_uid;
+	return PERF_ERRNO_TARGET__SUCCESS;
+}
diff --git a/tools/perf/util/target.h b/tools/perf/util/target.h
index eb0d210..d4aabda 100644
--- a/tools/perf/util/target.h
+++ b/tools/perf/util/target.h
@@ -33,9 +33,14 @@ enum perf_target_errno {
 	PERF_ERRNO_TARGET__PID_OVERRIDE_SYSTEM,
 	PERF_ERRNO_TARGET__UID_OVERRIDE_SYSTEM,
 
+	/* for perf_target__parse_uid() */
+	PERF_ERRNO_TARGET__INVALID_UID,
+	PERF_ERRNO_TARGET__USER_NOT_FOUND,
+
 	__PERF_ERRNO_TARGET__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 e851abc..4007aca 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 52be74c..27a11a7 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)
 

[PATCH 5/7] 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>
Reviewed-by: David Ahern <dsahern@gmail.com>
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    |   51 +++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/target.h    |    3 +++
 5 files changed, 87 insertions(+), 5 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index d26a279796d9..211d28f01eff 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_ERRNO_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_ERRNO_TARGET__SUCCESS) {
+		int saved_errno = errno;
 
-	if (perf_target__parse_uid(&rec->opts.target) < 0)
+		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 59883c8c5817..a2518fc0626a 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 = {
 		.target		     = {
 			.system_wide = true,
@@ -1255,10 +1256,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_ERRNO_TARGET__SUCCESS) {
+		perf_target__strerror(&top.target, status, errbuf, BUFSIZ);
+		ui__warning("%s", errbuf);
+	}
+
+	status = perf_target__parse_uid(&top.target);
+	if (status != PERF_ERRNO_TARGET__SUCCESS) {
+		int saved_errno = errno;
 
-	if (perf_target__parse_uid(&top.target) < 0)
+		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 02a6bedb69a3..1064d5b148ad 100644
--- a/tools/perf/util/target.c
+++ b/tools/perf/util/target.c
@@ -10,6 +10,7 @@
 #include "debug.h"
 
 #include <pwd.h>
+#include <string.h>
 
 
 enum perf_target_errno perf_target__validate(struct perf_target *target)
@@ -89,3 +90,53 @@ enum perf_target_errno perf_target__parse_uid(struct perf_target *target)
 	target->uid = result->pw_uid;
 	return PERF_ERRNO_TARGET__SUCCESS;
 }
+
+/*
+ * This must have a same ordering as the enum perf_target_errno.
+ */
+static const char *perf_target__error_str[] = {
+	"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)
+{
+	int idx;
+	const char *msg;
+
+	if (errnum >= 0) {
+		strerror_r(errnum, buf, buflen);
+		return 0;
+	}
+
+	if (errnum <  __PERF_ERRNO_TARGET__START ||
+	    errnum >= __PERF_ERRNO_TARGET__END)
+		return -1;
+
+	idx = errnum - __PERF_ERRNO_TARGET__START;
+	msg = perf_target__error_str[idx];
+
+	switch (errnum) {
+	case PERF_ERRNO_TARGET__PID_OVERRIDE_CPU
+	 ... PERF_ERRNO_TARGET__UID_OVERRIDE_SYSTEM:
+		snprintf(buf, buflen, "%s", msg);
+		break;
+
+	case PERF_ERRNO_TARGET__INVALID_UID:
+	case PERF_ERRNO_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 d4aabdaba42a..6fcd01c440a9 100644
--- a/tools/perf/util/target.h
+++ b/tools/perf/util/target.h
@@ -43,4 +43,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.1

Re: [PATCH 5/7] perf tools: Introduce perf_target__strerror()

[Arnaldo Carvalho de Melo]

Em Mon, May 07, 2012 at 02:09:02PM +0900, Namhyung Kim escreveu:
> 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.

This has a problem: why should I be warned when I do, as root:

# perf top -u acme

It warns me that "UID switch overriding SYSTEM", yeah, right that is
what I asked, no need to tell me that :-)

So what is missing in this case is that top starts with system_wide set,
not from the command line.

I.e. if I had asked for:

# perf top --system_wide --user acme

Ok, the warning would make sense.

So just special case this in the top case, I think, like the patch
below, ack?

I'll fold this into this patch and add a [ committer note: ... ]

Ack?

- Arnaldo

diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index a2518fc..780791f 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1257,7 +1257,16 @@ int cmd_top(int argc, const char **argv, const char *prefix __used)
 	setup_browser(false);
 
 	status = perf_target__validate(&top.target);
-	if (status != PERF_ERRNO_TARGET__SUCCESS) {
+	if (status != PERF_ERRNO_TARGET__SUCCESS &&
+	    status != PERF_ERRNO_TARGET__PID_OVERRIDE_SYSTEM &&
+	    status != PERF_ERRNO_TARGET__UID_OVERRIDE_SYSTEM) {
+		/*
+ 		 * Top doesn't provide an explicit way to ask for system wide
+ 		 * profiling, it is the default.
+ 		 *
+ 		 * So we shouldn't warn the user when he/she asks for --pid or
+ 		 * --uid.
+ 		 */
 		perf_target__strerror(&top.target, status, errbuf, BUFSIZ);
 		ui__warning("%s", errbuf);
 	}

Re: [PATCH 5/7] perf tools: Introduce perf_target__strerror()

[David Ahern]

On 5/7/12 11:29 AM, Arnaldo Carvalho de Melo wrote:
> Em Mon, May 07, 2012 at 02:09:02PM +0900, Namhyung Kim escreveu:
>> 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.
>
> This has a problem: why should I be warned when I do, as root:
>
> # perf top -u acme
>
> It warns me that "UID switch overriding SYSTEM", yeah, right that is
> what I asked, no need to tell me that :-)
>
> So what is missing in this case is that top starts with system_wide set,
> not from the command line.
>
> I.e. if I had asked for:
>
> # perf top --system_wide --user acme
>
> Ok, the warning would make sense.
>
> So just special case this in the top case, I think, like the patch
> below, ack?
>
> I'll fold this into this patch and add a [ committer note: ... ]
>
> Ack?
>
> - Arnaldo
>
> diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> index a2518fc..780791f 100644
> --- a/tools/perf/builtin-top.c
> +++ b/tools/perf/builtin-top.c
> @@ -1257,7 +1257,16 @@ int cmd_top(int argc, const char **argv, const char *prefix __used)
>   	setup_browser(false);
>
>   	status = perf_target__validate(&top.target);
> -	if (status != PERF_ERRNO_TARGET__SUCCESS) {
> +	if (status != PERF_ERRNO_TARGET__SUCCESS&&
> +	    status != PERF_ERRNO_TARGET__PID_OVERRIDE_SYSTEM&&
> +	    status != PERF_ERRNO_TARGET__UID_OVERRIDE_SYSTEM) {
> +		/*
> + 		 * Top doesn't provide an explicit way to ask for system wide
> + 		 * profiling, it is the default.
> + 		 *
> + 		 * So we shouldn't warn the user when he/she asks for --pid or
> + 		 * --uid.
> + 		 */
>   		perf_target__strerror(&top.target, status, errbuf, BUFSIZ);
>   		ui__warning("%s", errbuf);
>   	}

Seems like we should avoid the propagation of the errnos beyond the 
depths of the library part. For perf-top why not default to nothing set 
and if the user does not request one, set system wide. i.e, following 
Patch 6 in this set do:

if (perf_target__none(&top.target)
     top.target.system_wide = true;

David

Re: [PATCH 5/7] perf tools: Introduce perf_target__strerror()

[Namhyung Kim]

Hi,

On Mon, 07 May 2012 11:43:09 -0600, David Ahern wrote:
> On 5/7/12 11:29 AM, Arnaldo Carvalho de Melo wrote:
>> Em Mon, May 07, 2012 at 02:09:02PM +0900, Namhyung Kim escreveu:
>>> 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.
>>
>> This has a problem: why should I be warned when I do, as root:
>>
>> # perf top -u acme
>>
>> It warns me that "UID switch overriding SYSTEM", yeah, right that is
>> what I asked, no need to tell me that :-)
>>
>> So what is missing in this case is that top starts with system_wide set,
>> not from the command line.
>>
> Seems like we should avoid the propagation of the errnos beyond the
> depths of the library part. For perf-top why not default to nothing
> set and if the user does not request one, set system wide. i.e,
> following Patch 6 in this set do:
>
> if (perf_target__none(&top.target)
>     top.target.system_wide = true;
>

Thanks for catching this (and sorry for not testing thoroughly).

Yes, this would make more sense if it goes to the patch 1. Now I see
that Arnaldo fixed it on patch 1 but forgot to update on patch 6 :).

Thanks,
Namhyung

Re: [PATCH 5/7] perf tools: Introduce perf_target__strerror()

[Arnaldo Carvalho de Melo]

Em Tue, May 08, 2012 at 10:57:12AM +0900, Namhyung Kim escreveu:
> On Mon, 07 May 2012 11:43:09 -0600, David Ahern wrote:
> > On 5/7/12 11:29 AM, Arnaldo Carvalho de Melo wrote:
> >> So what is missing in this case is that top starts with system_wide set,
> >> not from the command line.
> >>
> > Seems like we should avoid the propagation of the errnos beyond the
> > depths of the library part. For perf-top why not default to nothing
> > set and if the user does not request one, set system wide. i.e,
> > following Patch 6 in this set do:
> >
> > if (perf_target__none(&top.target)
> >     top.target.system_wide = true;
> >
> 
> Thanks for catching this (and sorry for not testing thoroughly).
> 
> Yes, this would make more sense if it goes to the patch 1. Now I see
> that Arnaldo fixed it on patch 1 but forgot to update on patch 6 :).

Oh, I see, adding a follow up patch using the __none method, my case
wasn't checking cpu_list, so this is not just a simple helper
replacement but a bug fix.

- Arnaldo

[tip:perf/core] perf tools: Introduce perf_target__strerror()

[tip-bot for Namhyung Kim]

Commit-ID:  16ad2ffb822cd28e2330284a60fdfec8bb90bbb0
Gitweb:     http://git.kernel.org/tip/16ad2ffb822cd28e2330284a60fdfec8bb90bbb0
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Mon, 7 May 2012 14:09:02 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 7 May 2012 17:30:21 -0300

perf tools: Introduce perf_target__strerror()

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.

Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
Suggested-by: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
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/1336367344-28071-6-git-send-email-namhyung.kim@lge.com
[ committer note: No need to use PERF_ERRNO_TARGET__SUCCESS, use shorter idiom ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-record.c |   18 +++++++++++++-
 tools/perf/builtin-top.c    |   19 +++++++++++++--
 tools/perf/util/debug.c     |    1 +
 tools/perf/util/target.c    |   51 +++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/target.h    |    3 ++
 5 files changed, 87 insertions(+), 5 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index d26a279..c8bf6ea 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__strerror(&rec->opts.target, err, errbuf, BUFSIZ);
+		ui__warning("%s", errbuf);
+	}
+
+	err = perf_target__parse_uid(&rec->opts.target);
+	if (err) {
+		int saved_errno = errno;
 
-	if (perf_target__parse_uid(&rec->opts.target) < 0)
+		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 c9137ba..7ba0f03 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__strerror(&top.target, status, errbuf, BUFSIZ);
+		ui__warning("%s", errbuf);
+	}
+
+	status = perf_target__parse_uid(&top.target);
+	if (status) {
+		int saved_errno = errno;
 
-	if (perf_target__parse_uid(&top.target) < 0)
+		perf_target__strerror(&top.target, status, errbuf, BUFSIZ);
+		ui__warning("%s", errbuf);
+
+		status = -saved_errno;
 		goto out_delete_evlist;
+	}
 
 	if (top.target.tid == 0 && top.target.pid == 0 &&
 	    top.target.uid_str == NULL)
diff --git a/tools/perf/util/debug.c b/tools/perf/util/debug.c
index 26817da..efb1fce 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 02a6bed..1064d5b 100644
--- a/tools/perf/util/target.c
+++ b/tools/perf/util/target.c
@@ -10,6 +10,7 @@
 #include "debug.h"
 
 #include <pwd.h>
+#include <string.h>
 
 
 enum perf_target_errno perf_target__validate(struct perf_target *target)
@@ -89,3 +90,53 @@ enum perf_target_errno perf_target__parse_uid(struct perf_target *target)
 	target->uid = result->pw_uid;
 	return PERF_ERRNO_TARGET__SUCCESS;
 }
+
+/*
+ * This must have a same ordering as the enum perf_target_errno.
+ */
+static const char *perf_target__error_str[] = {
+	"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)
+{
+	int idx;
+	const char *msg;
+
+	if (errnum >= 0) {
+		strerror_r(errnum, buf, buflen);
+		return 0;
+	}
+
+	if (errnum <  __PERF_ERRNO_TARGET__START ||
+	    errnum >= __PERF_ERRNO_TARGET__END)
+		return -1;
+
+	idx = errnum - __PERF_ERRNO_TARGET__START;
+	msg = perf_target__error_str[idx];
+
+	switch (errnum) {
+	case PERF_ERRNO_TARGET__PID_OVERRIDE_CPU
+	 ... PERF_ERRNO_TARGET__UID_OVERRIDE_SYSTEM:
+		snprintf(buf, buflen, "%s", msg);
+		break;
+
+	case PERF_ERRNO_TARGET__INVALID_UID:
+	case PERF_ERRNO_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 d4aabda..6fcd01c 100644
--- a/tools/perf/util/target.h
+++ b/tools/perf/util/target.h
@@ -43,4 +43,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 */

[PATCH 6/7] 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.

Reviewed-by: David Ahern <dsahern@gmail.com>
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 211d28f01eff..fa1774413224 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 a2518fc0626a..52e75e76686c 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 6fcd01c440a9..127cff3f8ced 100644
--- a/tools/perf/util/target.h
+++ b/tools/perf/util/target.h
@@ -46,4 +46,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.1

[tip:perf/core] perf target: Consolidate target task/cpu checking

[tip-bot for Namhyung Kim]

Commit-ID:  d67356e7f80f5c2ef487bedc11a91d5fe18c5a15
Gitweb:     http://git.kernel.org/tip/d67356e7f80f5c2ef487bedc11a91d5fe18c5a15
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Mon, 7 May 2012 14:09:03 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 7 May 2012 17:52:05 -0300

perf target: Consolidate target task/cpu checking

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>
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/1336367344-28071-7-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-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 c8bf6ea..42e2414 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 bb77232..d9ff246 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 7ba0f03..e4ca827 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 183b199..1201daf 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 bb785a0..21eaab2 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 6fcd01c..127cff3 100644
--- a/tools/perf/util/target.h
+++ b/tools/perf/util/target.h
@@ -46,4 +46,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 */

[PATCH 7/7] 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.

Reviewed-by: David Ahern <dsahern@gmail.com>
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.1

[tip:perf/core] perf stat: Use perf_evlist__create_maps

[tip-bot for Namhyung Kim]

Commit-ID:  77a6f014e9ae330c747c66bebfddf29abf9b89e9
Gitweb:     http://git.kernel.org/tip/77a6f014e9ae330c747c66bebfddf29abf9b89e9
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Mon, 7 May 2012 14:09:04 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 7 May 2012 17:52:22 -0300

perf stat: Use perf_evlist__create_maps

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>
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/1336367344-28071-8-git-send-email-namhyung.kim@lge.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-stat.c |   22 ++++++++--------------
 1 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index d9ff246..e720ba7 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;
 	}