* [PATCH 1/7] perf top: Defaults to system_wide target
2012-05-07 5:08 [PATCH 0/7] perf tools: Fix cpu/thread map handling v3 Namhyung Kim
@ 2012-05-07 5:08 ` Namhyung Kim
2012-05-07 5:08 ` [PATCH 2/7] perf evlist: Fix creation of cpu map Namhyung Kim
` (5 subsequent siblings)
6 siblings, 0 replies; 18+ messages in thread
From: Namhyung Kim @ 2012-05-07 5:08 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML,
David Ahern
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
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/7] perf evlist: Fix creation of cpu map
2012-05-07 5:08 [PATCH 0/7] perf tools: Fix cpu/thread map handling v3 Namhyung Kim
2012-05-07 5:08 ` [PATCH 1/7] perf top: Defaults to system_wide target Namhyung Kim
@ 2012-05-07 5:08 ` Namhyung Kim
2012-05-11 6:39 ` [tip:perf/core] " tip-bot for Namhyung Kim
2012-05-07 5:09 ` [PATCH 3/7] perf target: Introduce perf_target_errno Namhyung Kim
` (4 subsequent siblings)
6 siblings, 1 reply; 18+ messages in thread
From: Namhyung Kim @ 2012-05-07 5:08 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML,
David Ahern
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
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [tip:perf/core] perf evlist: Fix creation of cpu map
2012-05-07 5:08 ` [PATCH 2/7] perf evlist: Fix creation of cpu map Namhyung Kim
@ 2012-05-11 6:39 ` tip-bot for Namhyung Kim
0 siblings, 0 replies; 18+ messages in thread
From: tip-bot for Namhyung Kim @ 2012-05-11 6:39 UTC (permalink / raw)
To: linux-tip-commits
Cc: acme, linux-kernel, paulus, mingo, hpa, mingo, a.p.zijlstra,
namhyung.kim, namhyung, dsahern, tglx
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);
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 3/7] perf target: Introduce perf_target_errno
2012-05-07 5:08 [PATCH 0/7] perf tools: Fix cpu/thread map handling v3 Namhyung Kim
2012-05-07 5:08 ` [PATCH 1/7] perf top: Defaults to system_wide target Namhyung Kim
2012-05-07 5:08 ` [PATCH 2/7] perf evlist: Fix creation of cpu map Namhyung Kim
@ 2012-05-07 5:09 ` Namhyung Kim
2012-05-11 6:40 ` [tip:perf/core] " tip-bot for Namhyung Kim
2012-05-07 5:09 ` [PATCH 4/7] perf target: Introduce perf_target__parse_uid() Namhyung Kim
` (3 subsequent siblings)
6 siblings, 1 reply; 18+ messages in thread
From: Namhyung Kim @ 2012-05-07 5:09 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML,
David Ahern
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
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [tip:perf/core] perf target: Introduce perf_target_errno
2012-05-07 5:09 ` [PATCH 3/7] perf target: Introduce perf_target_errno Namhyung Kim
@ 2012-05-11 6:40 ` tip-bot for Namhyung Kim
0 siblings, 0 replies; 18+ messages in thread
From: tip-bot for Namhyung Kim @ 2012-05-11 6:40 UTC (permalink / raw)
To: linux-tip-commits
Cc: acme, linux-kernel, paulus, mingo, hpa, mingo, a.p.zijlstra,
acme, namhyung.kim, namhyung, dsahern, tglx
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 */
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 4/7] perf target: Introduce perf_target__parse_uid()
2012-05-07 5:08 [PATCH 0/7] perf tools: Fix cpu/thread map handling v3 Namhyung Kim
` (2 preceding siblings ...)
2012-05-07 5:09 ` [PATCH 3/7] perf target: Introduce perf_target_errno Namhyung Kim
@ 2012-05-07 5:09 ` Namhyung Kim
2012-05-11 6:41 ` [tip:perf/core] " tip-bot for Namhyung Kim
2012-05-07 5:09 ` [PATCH 5/7] perf tools: Introduce perf_target__strerror() Namhyung Kim
` (2 subsequent siblings)
6 siblings, 1 reply; 18+ messages in thread
From: Namhyung Kim @ 2012-05-07 5:09 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML,
David Ahern
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
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [tip:perf/core] perf target: Introduce perf_target__parse_uid()
2012-05-07 5:09 ` [PATCH 4/7] perf target: Introduce perf_target__parse_uid() Namhyung Kim
@ 2012-05-11 6:41 ` tip-bot for Namhyung Kim
0 siblings, 0 replies; 18+ messages in thread
From: tip-bot for Namhyung Kim @ 2012-05-11 6:41 UTC (permalink / raw)
To: linux-tip-commits
Cc: acme, linux-kernel, paulus, mingo, hpa, mingo, a.p.zijlstra,
namhyung.kim, namhyung, dsahern, tglx
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)
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 5/7] perf tools: Introduce perf_target__strerror()
2012-05-07 5:08 [PATCH 0/7] perf tools: Fix cpu/thread map handling v3 Namhyung Kim
` (3 preceding siblings ...)
2012-05-07 5:09 ` [PATCH 4/7] perf target: Introduce perf_target__parse_uid() Namhyung Kim
@ 2012-05-07 5:09 ` Namhyung Kim
2012-05-07 17:29 ` Arnaldo Carvalho de Melo
2012-05-11 6:42 ` [tip:perf/core] " tip-bot for Namhyung Kim
2012-05-07 5:09 ` [PATCH 6/7] perf target: Consolidate target task/cpu checking Namhyung Kim
2012-05-07 5:09 ` [PATCH 7/7] perf stat: Use perf_evlist__create_maps Namhyung Kim
6 siblings, 2 replies; 18+ messages in thread
From: Namhyung Kim @ 2012-05-07 5:09 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML,
David Ahern
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
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 5/7] perf tools: Introduce perf_target__strerror()
2012-05-07 5:09 ` [PATCH 5/7] perf tools: Introduce perf_target__strerror() Namhyung Kim
@ 2012-05-07 17:29 ` Arnaldo Carvalho de Melo
2012-05-07 17:43 ` David Ahern
2012-05-11 6:42 ` [tip:perf/core] " tip-bot for Namhyung Kim
1 sibling, 1 reply; 18+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-05-07 17:29 UTC (permalink / raw)
To: Namhyung Kim
Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML,
David Ahern
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);
}
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 5/7] perf tools: Introduce perf_target__strerror()
2012-05-07 17:29 ` Arnaldo Carvalho de Melo
@ 2012-05-07 17:43 ` David Ahern
2012-05-08 1:57 ` Namhyung Kim
0 siblings, 1 reply; 18+ messages in thread
From: David Ahern @ 2012-05-07 17:43 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Namhyung Kim, Peter Zijlstra, Paul Mackerras, Ingo Molnar,
Namhyung Kim, LKML
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
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 5/7] perf tools: Introduce perf_target__strerror()
2012-05-07 17:43 ` David Ahern
@ 2012-05-08 1:57 ` Namhyung Kim
2012-05-08 13:17 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 18+ messages in thread
From: Namhyung Kim @ 2012-05-08 1:57 UTC (permalink / raw)
To: David Ahern
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Paul Mackerras,
Ingo Molnar, LKML
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
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 5/7] perf tools: Introduce perf_target__strerror()
2012-05-08 1:57 ` Namhyung Kim
@ 2012-05-08 13:17 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 18+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-05-08 13:17 UTC (permalink / raw)
To: Namhyung Kim
Cc: David Ahern, Peter Zijlstra, Paul Mackerras, Ingo Molnar, LKML
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
^ permalink raw reply [flat|nested] 18+ messages in thread
* [tip:perf/core] perf tools: Introduce perf_target__strerror()
2012-05-07 5:09 ` [PATCH 5/7] perf tools: Introduce perf_target__strerror() Namhyung Kim
2012-05-07 17:29 ` Arnaldo Carvalho de Melo
@ 2012-05-11 6:42 ` tip-bot for Namhyung Kim
1 sibling, 0 replies; 18+ messages in thread
From: tip-bot for Namhyung Kim @ 2012-05-11 6:42 UTC (permalink / raw)
To: linux-tip-commits
Cc: acme, linux-kernel, paulus, mingo, hpa, mingo, a.p.zijlstra,
acme, namhyung.kim, namhyung, dsahern, tglx
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 */
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 6/7] perf target: Consolidate target task/cpu checking
2012-05-07 5:08 [PATCH 0/7] perf tools: Fix cpu/thread map handling v3 Namhyung Kim
` (4 preceding siblings ...)
2012-05-07 5:09 ` [PATCH 5/7] perf tools: Introduce perf_target__strerror() Namhyung Kim
@ 2012-05-07 5:09 ` Namhyung Kim
2012-05-11 6:42 ` [tip:perf/core] " tip-bot for Namhyung Kim
2012-05-07 5:09 ` [PATCH 7/7] perf stat: Use perf_evlist__create_maps Namhyung Kim
6 siblings, 1 reply; 18+ messages in thread
From: Namhyung Kim @ 2012-05-07 5:09 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML,
David Ahern
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
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [tip:perf/core] perf target: Consolidate target task/cpu checking
2012-05-07 5:09 ` [PATCH 6/7] perf target: Consolidate target task/cpu checking Namhyung Kim
@ 2012-05-11 6:42 ` tip-bot for Namhyung Kim
0 siblings, 0 replies; 18+ messages in thread
From: tip-bot for Namhyung Kim @ 2012-05-11 6:42 UTC (permalink / raw)
To: linux-tip-commits
Cc: acme, linux-kernel, paulus, mingo, hpa, mingo, a.p.zijlstra,
namhyung.kim, namhyung, dsahern, tglx
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 */
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 7/7] perf stat: Use perf_evlist__create_maps
2012-05-07 5:08 [PATCH 0/7] perf tools: Fix cpu/thread map handling v3 Namhyung Kim
` (5 preceding siblings ...)
2012-05-07 5:09 ` [PATCH 6/7] perf target: Consolidate target task/cpu checking Namhyung Kim
@ 2012-05-07 5:09 ` Namhyung Kim
2012-05-11 6:43 ` [tip:perf/core] " tip-bot for Namhyung Kim
6 siblings, 1 reply; 18+ messages in thread
From: Namhyung Kim @ 2012-05-07 5:09 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML,
David Ahern
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
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [tip:perf/core] perf stat: Use perf_evlist__create_maps
2012-05-07 5:09 ` [PATCH 7/7] perf stat: Use perf_evlist__create_maps Namhyung Kim
@ 2012-05-11 6:43 ` tip-bot for Namhyung Kim
0 siblings, 0 replies; 18+ messages in thread
From: tip-bot for Namhyung Kim @ 2012-05-11 6:43 UTC (permalink / raw)
To: linux-tip-commits
Cc: acme, linux-kernel, paulus, mingo, hpa, mingo, a.p.zijlstra,
namhyung.kim, namhyung, dsahern, tglx
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;
}
^ permalink raw reply related [flat|nested] 18+ messages in thread