* [PATCH] perf tools: Allow multiple threads or processes in record, stat, top
@ 2012-02-08 16:32 David Ahern
2012-02-09 1:19 ` Namhyung Kim
` (2 more replies)
0 siblings, 3 replies; 22+ messages in thread
From: David Ahern @ 2012-02-08 16:32 UTC (permalink / raw)
To: acme, linux-kernel; +Cc: mingo, peterz, fweisbec, paulus, tglx, David Ahern
Allow a user to collect events for multiple threads or processes
using a comma separated list.
e.g., collect data on a VM and its vhost thread:
perf top -p 21483,21485
perf stat -p 21483,21485 -ddd
perf record -p 21483,21485
or monitoring vcpu threads
perf top -t 21488,21489
perf stat -t 21488,21489 -ddd
perf record -t 21488,21489
Signed-off-by: David Ahern <dsahern@gmail.com>
---
tools/perf/Documentation/perf-record.txt | 4 +-
tools/perf/Documentation/perf-stat.txt | 4 +-
tools/perf/Documentation/perf-top.txt | 4 +-
tools/perf/builtin-record.c | 10 +-
tools/perf/builtin-stat.c | 31 +++---
tools/perf/builtin-test.c | 2 -
tools/perf/builtin-top.c | 12 +--
tools/perf/perf.h | 4 +-
tools/perf/util/evlist.c | 10 +-
tools/perf/util/evlist.h | 4 +-
tools/perf/util/evsel.c | 2 +-
tools/perf/util/thread_map.c | 152 ++++++++++++++++++++++++++++++
tools/perf/util/thread_map.h | 4 +
tools/perf/util/top.c | 10 +-
tools/perf/util/top.h | 2 +-
tools/perf/util/usage.c | 6 +-
tools/perf/util/util.h | 2 +-
17 files changed, 207 insertions(+), 56 deletions(-)
diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index ff9a66e..a5766b4 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -52,11 +52,11 @@ OPTIONS
-p::
--pid=::
- Record events on existing process ID.
+ Record events on existing process ID (comma separated list).
-t::
--tid=::
- Record events on existing thread ID.
+ Record events on existing thread ID (comma separated list).
-u::
--uid=::
diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
index 8966b9a..2fa173b 100644
--- a/tools/perf/Documentation/perf-stat.txt
+++ b/tools/perf/Documentation/perf-stat.txt
@@ -35,11 +35,11 @@ OPTIONS
child tasks do not inherit counters
-p::
--pid=<pid>::
- stat events on existing process id
+ stat events on existing process id (comma separated list)
-t::
--tid=<tid>::
- stat events on existing thread id
+ stat events on existing thread id (comma separated list)
-a::
diff --git a/tools/perf/Documentation/perf-top.txt b/tools/perf/Documentation/perf-top.txt
index ab1454e..4a5680c 100644
--- a/tools/perf/Documentation/perf-top.txt
+++ b/tools/perf/Documentation/perf-top.txt
@@ -72,11 +72,11 @@ Default is to monitor all CPUS.
-p <pid>::
--pid=<pid>::
- Profile events on existing Process ID.
+ Profile events on existing Process ID (comma separated list).
-t <tid>::
--tid=<tid>::
- Profile events on existing thread ID.
+ Profile events on existing thread ID (comma separated list).
-u::
--uid=::
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index f8d9a54..3291c34 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -645,8 +645,6 @@ static const char * const record_usage[] = {
*/
static struct perf_record record = {
.opts = {
- .target_pid = -1,
- .target_tid = -1,
.mmap_pages = UINT_MAX,
.user_freq = UINT_MAX,
.user_interval = ULLONG_MAX,
@@ -670,9 +668,9 @@ const struct option record_options[] = {
parse_events_option),
OPT_CALLBACK(0, "filter", &record.evlist, "filter",
"event filter", parse_filter),
- OPT_INTEGER('p', "pid", &record.opts.target_pid,
+ OPT_STRING('p', "pid", &record.opts.target_pid, "pid",
"record events on existing process id"),
- OPT_INTEGER('t', "tid", &record.opts.target_tid,
+ OPT_STRING('t', "tid", &record.opts.target_tid, "tid",
"record events on existing thread id"),
OPT_INTEGER('r', "realtime", &record.realtime_prio,
"collect data with this RT SCHED_FIFO priority"),
@@ -739,7 +737,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 == -1 && rec->opts.target_tid == -1 &&
+ if (!argc && !rec->opts.target_pid && !rec->opts.target_tid &&
!rec->opts.system_wide && !rec->opts.cpu_list && !rec->uid_str)
usage_with_options(record_usage, record_options);
@@ -785,7 +783,7 @@ int cmd_record(int argc, const char **argv, const char *prefix __used)
if (rec->uid_str != NULL && rec->opts.uid == UINT_MAX - 1)
goto out_free_fd;
- if (rec->opts.target_pid != -1)
+ if (rec->opts.target_pid)
rec->opts.target_tid = rec->opts.target_pid;
if (perf_evlist__create_maps(evsel_list, rec->opts.target_pid,
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index d14b37a..ea40e4e 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -182,8 +182,8 @@ static int run_count = 1;
static bool no_inherit = false;
static bool scale = true;
static bool no_aggr = false;
-static pid_t target_pid = -1;
-static pid_t target_tid = -1;
+static const char *target_pid;
+static const char *target_tid;
static pid_t child_pid = -1;
static bool null_run = false;
static int detailed_run = 0;
@@ -296,7 +296,7 @@ static int create_perf_stat_counter(struct perf_evsel *evsel,
if (system_wide)
return perf_evsel__open_per_cpu(evsel, evsel_list->cpus,
group, group_fd);
- if (target_pid == -1 && target_tid == -1) {
+ if (!target_pid && !target_tid) {
attr->disabled = 1;
attr->enable_on_exec = 1;
}
@@ -446,7 +446,7 @@ static int run_perf_stat(int argc __used, const char **argv)
exit(-1);
}
- if (target_tid == -1 && target_pid == -1 && !system_wide)
+ if (!target_tid && !target_pid && !system_wide)
evsel_list->threads->map[0] = child_pid;
/*
@@ -968,14 +968,14 @@ static void print_stat(int argc, const char **argv)
if (!csv_output) {
fprintf(output, "\n");
fprintf(output, " Performance counter stats for ");
- if(target_pid == -1 && target_tid == -1) {
+ if (!target_pid && !target_tid) {
fprintf(output, "\'%s", argv[0]);
for (i = 1; i < argc; i++)
fprintf(output, " %s", argv[i]);
- } else if (target_pid != -1)
- fprintf(output, "process id \'%d", target_pid);
+ } else if (target_pid)
+ fprintf(output, "process id \'%s", target_pid);
else
- fprintf(output, "thread id \'%d", target_tid);
+ fprintf(output, "thread id \'%s", target_tid);
fprintf(output, "\'");
if (run_count > 1)
@@ -1049,10 +1049,10 @@ static const struct option options[] = {
"event filter", parse_filter),
OPT_BOOLEAN('i', "no-inherit", &no_inherit,
"child tasks do not inherit counters"),
- OPT_INTEGER('p', "pid", &target_pid,
- "stat events on existing process id"),
- OPT_INTEGER('t', "tid", &target_tid,
- "stat events on existing thread id"),
+ OPT_STRING('p', "pid", &target_pid, "pid",
+ "stat events on existing process id"),
+ OPT_STRING('t', "tid", &target_tid, "tid",
+ "stat events on existing thread id"),
OPT_BOOLEAN('a', "all-cpus", &system_wide,
"system-wide collection from all CPUs"),
OPT_BOOLEAN('g', "group", &group,
@@ -1190,7 +1190,7 @@ 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 == -1 && target_tid == -1)
+ if (!argc && !target_pid && !target_tid)
usage_with_options(stat_usage, options);
if (run_count <= 0)
usage_with_options(stat_usage, options);
@@ -1206,10 +1206,11 @@ int cmd_stat(int argc, const char **argv, const char *prefix __used)
if (add_default_attributes())
goto out;
- if (target_pid != -1)
+ if (target_pid)
target_tid = target_pid;
- evsel_list->threads = thread_map__new(target_pid, target_tid, UINT_MAX);
+ evsel_list->threads = thread_map__new_str(target_pid,
+ target_tid, UINT_MAX);
if (evsel_list->threads == NULL) {
pr_err("Problems finding threads of monitor\n");
usage_with_options(stat_usage, options);
diff --git a/tools/perf/builtin-test.c b/tools/perf/builtin-test.c
index 70c4eb2..0f15195 100644
--- a/tools/perf/builtin-test.c
+++ b/tools/perf/builtin-test.c
@@ -1010,8 +1010,6 @@ realloc:
static int test__PERF_RECORD(void)
{
struct perf_record_opts opts = {
- .target_pid = -1,
- .target_tid = -1,
.no_delay = true,
.freq = 10,
.mmap_pages = 256,
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index d869b21..94d55cb 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -965,7 +965,7 @@ static int __cmd_top(struct perf_top *top)
if (ret)
goto out_delete;
- if (top->target_tid != -1 || top->uid != UINT_MAX)
+ if (top->target_tid || top->uid != UINT_MAX)
perf_event__synthesize_thread_map(&top->tool, top->evlist->threads,
perf_event__process,
&top->session->host_machine);
@@ -1103,8 +1103,6 @@ int cmd_top(int argc, const char **argv, const char *prefix __used)
struct perf_top top = {
.count_filter = 5,
.delay_secs = 2,
- .target_pid = -1,
- .target_tid = -1,
.uid = UINT_MAX,
.freq = 1000, /* 1 KHz */
.sample_id_all_avail = true,
@@ -1118,9 +1116,9 @@ int cmd_top(int argc, const char **argv, const char *prefix __used)
parse_events_option),
OPT_INTEGER('c', "count", &top.default_interval,
"event period to sample"),
- OPT_INTEGER('p', "pid", &top.target_pid,
+ OPT_STRING('p', "pid", &top.target_pid, "pid",
"profile events on existing process id"),
- OPT_INTEGER('t', "tid", &top.target_tid,
+ OPT_STRING('t', "tid", &top.target_tid, "tid",
"profile events on existing thread id"),
OPT_BOOLEAN('a', "all-cpus", &top.system_wide,
"system-wide collection from all CPUs"),
@@ -1210,13 +1208,13 @@ int cmd_top(int argc, const char **argv, const char *prefix __used)
goto out_delete_evlist;
/* CPU and PID are mutually exclusive */
- if (top.target_tid > 0 && top.cpu_list) {
+ if (top.target_tid && top.cpu_list) {
printf("WARNING: PID switch overriding CPU\n");
sleep(1);
top.cpu_list = NULL;
}
- if (top.target_pid != -1)
+ if (top.target_pid)
top.target_tid = top.target_pid;
if (perf_evlist__create_maps(top.evlist, top.target_pid,
diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index 92af168..deb17db 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -186,8 +186,8 @@ extern const char perf_version_string[];
void pthread__unblock_sigwinch(void);
struct perf_record_opts {
- pid_t target_pid;
- pid_t target_tid;
+ const char *target_pid;
+ const char *target_tid;
uid_t uid;
bool call_graph;
bool group;
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index a57a8cf..5c61dc5 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -593,15 +593,15 @@ int perf_evlist__mmap(struct perf_evlist *evlist, unsigned int pages,
return perf_evlist__mmap_per_cpu(evlist, prot, mask);
}
-int perf_evlist__create_maps(struct perf_evlist *evlist, pid_t target_pid,
- pid_t target_tid, uid_t uid, const char *cpu_list)
+int perf_evlist__create_maps(struct perf_evlist *evlist, const char *target_pid,
+ const char *target_tid, uid_t uid, const char *cpu_list)
{
- evlist->threads = thread_map__new(target_pid, target_tid, uid);
+ evlist->threads = thread_map__new_str(target_pid, target_tid, uid);
if (evlist->threads == NULL)
return -1;
- if (uid != UINT_MAX || (cpu_list == NULL && target_tid != -1))
+ if (uid != UINT_MAX || (cpu_list == NULL && target_tid))
evlist->cpus = cpu_map__dummy_new();
else
evlist->cpus = cpu_map__new(cpu_list);
@@ -820,7 +820,7 @@ int perf_evlist__prepare_workload(struct perf_evlist *evlist,
exit(-1);
}
- if (!opts->system_wide && opts->target_tid == -1 && opts->target_pid == -1)
+ if (!opts->system_wide && !opts->target_tid && !opts->target_pid)
evlist->threads->map[0] = evlist->workload.pid;
close(child_ready_pipe[1]);
diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
index 1b4282b..21f1c9e 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -106,8 +106,8 @@ static inline void perf_evlist__set_maps(struct perf_evlist *evlist,
evlist->threads = threads;
}
-int perf_evlist__create_maps(struct perf_evlist *evlist, pid_t target_pid,
- pid_t tid, uid_t uid, const char *cpu_list);
+int perf_evlist__create_maps(struct perf_evlist *evlist, const char *target_pid,
+ const char *tid, uid_t uid, const char *cpu_list);
void perf_evlist__delete_maps(struct perf_evlist *evlist);
int perf_evlist__set_filters(struct perf_evlist *evlist);
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 9a11f9e..f910f50 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -130,7 +130,7 @@ void perf_evsel__config(struct perf_evsel *evsel, struct perf_record_opts *opts)
attr->mmap = track;
attr->comm = track;
- if (opts->target_pid == -1 && opts->target_tid == -1 && !opts->system_wide) {
+ if (!opts->target_pid && !opts->target_tid && !opts->system_wide) {
attr->disabled = 1;
attr->enable_on_exec = 1;
}
diff --git a/tools/perf/util/thread_map.c b/tools/perf/util/thread_map.c
index 3d4b6c5..e793c16 100644
--- a/tools/perf/util/thread_map.c
+++ b/tools/perf/util/thread_map.c
@@ -6,7 +6,10 @@
#include <sys/types.h>
#include <sys/stat.h>
#include <unistd.h>
+#include <ctype.h>
+#include <string.h>
#include "thread_map.h"
+#include "debug.h"
/* Skip "." and ".." directories */
static int filter(const struct dirent *dir)
@@ -152,6 +155,155 @@ struct thread_map *thread_map__new(pid_t pid, pid_t tid, uid_t uid)
return thread_map__new_by_tid(tid);
}
+
+static pid_t get_next_task_id(char **str)
+{
+ char *p, *pend;
+ pid_t ret = -1;
+ bool adv_pend = false;
+
+ pend = p = *str;
+
+ /* empty strings */
+ if (*pend == '\0')
+ return -1;
+
+ /* only expecting digits separated by commas */
+ while (isdigit(*pend))
+ ++pend;
+
+ if (*pend == ',') {
+ *pend = '\0';
+ adv_pend = true;
+ /* catch strings ending in a comma - like '1234,' */
+ if (*(pend+1) == '\0') {
+ ui__error("task id strings should not end in a comma\n");
+ goto out;
+ }
+ }
+
+ /* only convert if all characters are valid */
+ if (*pend == '\0') {
+ ret = atoi(p);
+ if (adv_pend)
+ pend++;
+ *str = pend;
+ }
+
+out:
+ return ret;
+}
+
+static struct thread_map *thread_map__new_by_pid_str(const char *pid_str)
+{
+ struct thread_map *threads = NULL;
+ char name[256];
+ int items, total_tasks = 0;
+ struct dirent **namelist = NULL;
+ int i, j = 0;
+ char *ostr, *str;
+ pid_t pid;
+
+ ostr = strdup(pid_str);
+ if (!ostr)
+ return NULL;
+ str = ostr;
+
+ while (*str) {
+ pid = get_next_task_id(&str);
+ if (pid < 0) {
+ free(threads);
+ threads = NULL;
+ break;
+ }
+
+ sprintf(name, "/proc/%d/task", pid);
+ items = scandir(name, &namelist, filter, NULL);
+ if (items <= 0)
+ break;
+
+ total_tasks += items;
+ if (threads)
+ threads = realloc(threads,
+ sizeof(*threads) + sizeof(pid_t)*total_tasks);
+ else
+ threads = malloc(sizeof(*threads) + sizeof(pid_t)*items);
+
+ if (threads) {
+ for (i = 0; i < items; i++)
+ threads->map[j++] = atoi(namelist[i]->d_name);
+ threads->nr = total_tasks;
+ }
+
+ for (i = 0; i < items; i++)
+ free(namelist[i]);
+ free(namelist);
+
+ if (!threads)
+ break;
+ }
+
+ free(ostr);
+
+ return threads;
+}
+
+static struct thread_map *thread_map__new_by_tid_str(const char *tid_str)
+{
+ struct thread_map *threads = NULL;
+ char *str;
+ int ntasks = 0;
+ pid_t tid;
+
+ /* perf-stat expects threads to be generated even if tid not given */
+ if (!tid_str) {
+ threads = malloc(sizeof(*threads) + sizeof(pid_t));
+ threads->map[1] = -1;
+ threads->nr = 1;
+ return threads;
+ }
+
+ str = strdup(tid_str);
+ if (!str)
+ return NULL;
+
+ while (*str) {
+ tid = get_next_task_id(&str);
+ if (tid < 0) {
+ free(threads);
+ threads = NULL;
+ break;
+ }
+
+ ntasks++;
+ if (threads)
+ threads = realloc(threads,
+ sizeof(*threads) + sizeof(pid_t) * ntasks);
+ else
+ threads = malloc(sizeof(*threads) + sizeof(pid_t));
+
+ if (threads == NULL)
+ break;
+
+ threads->map[ntasks-1] = tid;
+ threads->nr = ntasks;
+ }
+
+ return threads;
+}
+
+struct thread_map *thread_map__new_str(const char *pid, const char *tid,
+ uid_t uid)
+{
+ if (pid)
+ return thread_map__new_by_pid_str(pid);
+
+ if (!tid && uid != UINT_MAX)
+ return thread_map__new_by_uid(uid);
+
+ return thread_map__new_by_tid_str(tid);
+}
+
void thread_map__delete(struct thread_map *threads)
{
free(threads);
diff --git a/tools/perf/util/thread_map.h b/tools/perf/util/thread_map.h
index c75ddba..7da80f1 100644
--- a/tools/perf/util/thread_map.h
+++ b/tools/perf/util/thread_map.h
@@ -13,6 +13,10 @@ struct thread_map *thread_map__new_by_pid(pid_t pid);
struct thread_map *thread_map__new_by_tid(pid_t tid);
struct thread_map *thread_map__new_by_uid(uid_t uid);
struct thread_map *thread_map__new(pid_t pid, pid_t tid, uid_t uid);
+
+struct thread_map *thread_map__new_str(const char *pid,
+ const char *tid, uid_t uid);
+
void thread_map__delete(struct thread_map *threads);
size_t thread_map__fprintf(struct thread_map *threads, FILE *fp);
diff --git a/tools/perf/util/top.c b/tools/perf/util/top.c
index e4370ca..09fe579 100644
--- a/tools/perf/util/top.c
+++ b/tools/perf/util/top.c
@@ -69,11 +69,11 @@ size_t perf_top__header_snprintf(struct perf_top *top, char *bf, size_t size)
ret += SNPRINTF(bf + ret, size - ret, "], ");
- if (top->target_pid != -1)
- ret += SNPRINTF(bf + ret, size - ret, " (target_pid: %d",
+ if (top->target_pid)
+ ret += SNPRINTF(bf + ret, size - ret, " (target_pid: %s",
top->target_pid);
- else if (top->target_tid != -1)
- ret += SNPRINTF(bf + ret, size - ret, " (target_tid: %d",
+ else if (top->target_tid)
+ ret += SNPRINTF(bf + ret, size - ret, " (target_tid: %s",
top->target_tid);
else if (top->uid_str != NULL)
ret += SNPRINTF(bf + ret, size - ret, " (uid: %s",
@@ -85,7 +85,7 @@ size_t perf_top__header_snprintf(struct perf_top *top, char *bf, size_t size)
ret += SNPRINTF(bf + ret, size - ret, ", CPU%s: %s)",
top->evlist->cpus->nr > 1 ? "s" : "", top->cpu_list);
else {
- if (top->target_tid != -1)
+ if (top->target_tid)
ret += SNPRINTF(bf + ret, size - ret, ")");
else
ret += SNPRINTF(bf + ret, size - ret, ", %d CPU%s)",
diff --git a/tools/perf/util/top.h b/tools/perf/util/top.h
index def3e53..49eb848 100644
--- a/tools/perf/util/top.h
+++ b/tools/perf/util/top.h
@@ -23,7 +23,7 @@ struct perf_top {
u64 guest_us_samples, guest_kernel_samples;
int print_entries, count_filter, delay_secs;
int freq;
- pid_t target_pid, target_tid;
+ const char *target_pid, *target_tid;
uid_t uid;
bool hide_kernel_symbols, hide_user_symbols, zero;
bool system_wide;
diff --git a/tools/perf/util/usage.c b/tools/perf/util/usage.c
index d0c0139..1240bd6 100644
--- a/tools/perf/util/usage.c
+++ b/tools/perf/util/usage.c
@@ -83,7 +83,7 @@ void warning(const char *warn, ...)
va_end(params);
}
-uid_t parse_target_uid(const char *str, pid_t tid, pid_t pid)
+uid_t parse_target_uid(const char *str, const char *tid, const char *pid)
{
struct passwd pwd, *result;
char buf[1024];
@@ -91,8 +91,8 @@ uid_t parse_target_uid(const char *str, pid_t tid, pid_t pid)
if (str == NULL)
return UINT_MAX;
- /* CPU and PID are mutually exclusive */
- if (tid > 0 || pid > 0) {
+ /* UUID and PID are mutually exclusive */
+ if (tid || pid) {
ui__warning("PID/TID switch overriding UID\n");
sleep(1);
return UINT_MAX;
diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index 232d17e..7917b09 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -245,7 +245,7 @@ struct perf_event_attr;
void event_attr_init(struct perf_event_attr *attr);
-uid_t parse_target_uid(const char *str, pid_t tid, pid_t pid);
+uid_t parse_target_uid(const char *str, const char *tid, const char *pid);
#define _STR(x) #x
#define STR(x) _STR(x)
--
1.7.7.6
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH] perf tools: Allow multiple threads or processes in record, stat, top
2012-02-08 16:32 [PATCH] perf tools: Allow multiple threads or processes in record, stat, top David Ahern
@ 2012-02-09 1:19 ` Namhyung Kim
2012-02-09 2:52 ` David Ahern
2012-02-10 19:24 ` [PATCH] perf tools: Allow multiple threads or processes in record, stat, top Arnaldo Carvalho de Melo
2012-02-17 9:46 ` [tip:perf/core] " tip-bot for David Ahern
2 siblings, 1 reply; 22+ messages in thread
From: Namhyung Kim @ 2012-02-09 1:19 UTC (permalink / raw)
To: David Ahern; +Cc: acme, linux-kernel, mingo, peterz, fweisbec, paulus, tglx
Hello,
2012-02-09 1:32 AM, David Ahern wrote:
> Allow a user to collect events for multiple threads or processes
> using a comma separated list.
>
> e.g., collect data on a VM and its vhost thread:
> perf top -p 21483,21485
> perf stat -p 21483,21485 -ddd
> perf record -p 21483,21485
>
> or monitoring vcpu threads
> perf top -t 21488,21489
> perf stat -t 21488,21489 -ddd
> perf record -t 21488,21489
>
> Signed-off-by: David Ahern <dsahern@gmail.com>
Nice work, thanks. Some minor nits below..
> ---
> tools/perf/Documentation/perf-record.txt | 4 +-
> tools/perf/Documentation/perf-stat.txt | 4 +-
> tools/perf/Documentation/perf-top.txt | 4 +-
> tools/perf/builtin-record.c | 10 +-
> tools/perf/builtin-stat.c | 31 +++---
> tools/perf/builtin-test.c | 2 -
> tools/perf/builtin-top.c | 12 +--
> tools/perf/perf.h | 4 +-
> tools/perf/util/evlist.c | 10 +-
> tools/perf/util/evlist.h | 4 +-
> tools/perf/util/evsel.c | 2 +-
> tools/perf/util/thread_map.c | 152 ++++++++++++++++++++++++++++++
> tools/perf/util/thread_map.h | 4 +
> tools/perf/util/top.c | 10 +-
> tools/perf/util/top.h | 2 +-
> tools/perf/util/usage.c | 6 +-
> tools/perf/util/util.h | 2 +-
> 17 files changed, 207 insertions(+), 56 deletions(-)
>
[snip]
> diff --git a/tools/perf/util/thread_map.c b/tools/perf/util/thread_map.c
> index 3d4b6c5..e793c16 100644
> --- a/tools/perf/util/thread_map.c
> +++ b/tools/perf/util/thread_map.c
> @@ -6,7 +6,10 @@
> #include <sys/types.h>
> #include <sys/stat.h>
> #include <unistd.h>
> +#include <ctype.h>
I was trying to remove ctype.h, you might use util.h here.
> +#include <string.h>
> #include "thread_map.h"
> +#include "debug.h"
>
> /* Skip "." and ".." directories */
> static int filter(const struct dirent *dir)
> @@ -152,6 +155,155 @@ struct thread_map *thread_map__new(pid_t pid, pid_t tid, uid_t uid)
> return thread_map__new_by_tid(tid);
> }
>
> +
> +static pid_t get_next_task_id(char **str)
> +{
> + char *p, *pend;
> + pid_t ret = -1;
> + bool adv_pend = false;
> +
> + pend = p = *str;
> +
> + /* empty strings */
> + if (*pend == '\0')
> + return -1;
> +
> + /* only expecting digits separated by commas */
> + while (isdigit(*pend))
> + ++pend;
> +
> + if (*pend == ',') {
> + *pend = '\0';
> + adv_pend = true;
> + /* catch strings ending in a comma - like '1234,' */
> + if (*(pend+1) == '\0') {
> + ui__error("task id strings should not end in a comma\n");
> + goto out;
> + }
> + }
> +
> + /* only convert if all characters are valid */
> + if (*pend == '\0') {
> + ret = atoi(p);
> + if (adv_pend)
> + pend++;
> + *str = pend;
> + }
> +
> +out:
> + return ret;
> +}
> +
> +static struct thread_map *thread_map__new_by_pid_str(const char *pid_str)
> +{
> + struct thread_map *threads = NULL;
> + char name[256];
> + int items, total_tasks = 0;
> + struct dirent **namelist = NULL;
> + int i, j = 0;
> + char *ostr, *str;
> + pid_t pid;
> +
> + ostr = strdup(pid_str);
> + if (!ostr)
> + return NULL;
> + str = ostr;
> +
> + while (*str) {
> + pid = get_next_task_id(&str);
> + if (pid< 0) {
> + free(threads);
> + threads = NULL;
> + break;
> + }
> +
> + sprintf(name, "/proc/%d/task", pid);
> + items = scandir(name,&namelist, filter, NULL);
> + if (items<= 0)
> + break;
> +
> + total_tasks += items;
> + if (threads)
> + threads = realloc(threads,
> + sizeof(*threads) + sizeof(pid_t)*total_tasks);
> + else
> + threads = malloc(sizeof(*threads) + sizeof(pid_t)*items);
> +
> + if (threads) {
> + for (i = 0; i< items; i++)
> + threads->map[j++] = atoi(namelist[i]->d_name);
> + threads->nr = total_tasks;
> + }
> +
> + for (i = 0; i< items; i++)
> + free(namelist[i]);
> + free(namelist);
> +
> + if (!threads)
> + break;
> + }
> +
> + free(ostr);
> +
> + return threads;
> +}
> +
> +static struct thread_map *thread_map__new_by_tid_str(const char *tid_str)
> +{
> + struct thread_map *threads = NULL;
> + char *str;
> + int ntasks = 0;
> + pid_t tid;
> +
> + /* perf-stat expects threads to be generated even if tid not given */
> + if (!tid_str) {
> + threads = malloc(sizeof(*threads) + sizeof(pid_t));
> + threads->map[1] = -1;
> + threads->nr = 1;
> + return threads;
> + }
> +
> + str = strdup(tid_str);
> + if (!str)
> + return NULL;
> +
> + while (*str) {
> + tid = get_next_task_id(&str);
> + if (tid < 0) {
> + free(threads);
> + threads = NULL;
> + break;
> + }
> +
> + ntasks++;
> + if (threads)
> + threads = realloc(threads,
> + sizeof(*threads) + sizeof(pid_t) * ntasks);
> + else
> + threads = malloc(sizeof(*threads) + sizeof(pid_t));
> +
> + if (threads == NULL)
> + break;
> +
> + threads->map[ntasks-1] = tid;
> + threads->nr = ntasks;
> + }
> +
> + return threads;
> +}
> +
This will ends up being a code duplication. How about something like below?
while (*str) {
tid = get_next_task_id(&str);
if (tid < 0)
goto out_err;
tmp = thread_map__new_by_tid(tid); /* and for pid too */
if (tmp == NULL)
goto out_err;
threads = thread_map__merge(threads, tmp); /* should be implemented */
if (threads == NULL)
break;
}
> +struct thread_map *thread_map__new_str(const char *pid, const char *tid,
> + uid_t uid)
> +{
> + if (pid)
> + return thread_map__new_by_pid_str(pid);
> +
> + if (!tid&& uid != UINT_MAX)
> + return thread_map__new_by_uid(uid);
> +
> + return thread_map__new_by_tid_str(tid);
> +}
> +
> void thread_map__delete(struct thread_map *threads)
> {
> free(threads);
[snip]
> diff --git a/tools/perf/util/usage.c b/tools/perf/util/usage.c
> index d0c0139..1240bd6 100644
> --- a/tools/perf/util/usage.c
> +++ b/tools/perf/util/usage.c
> @@ -83,7 +83,7 @@ void warning(const char *warn, ...)
> va_end(params);
> }
>
> -uid_t parse_target_uid(const char *str, pid_t tid, pid_t pid)
> +uid_t parse_target_uid(const char *str, const char *tid, const char *pid)
> {
> struct passwd pwd, *result;
> char buf[1024];
> @@ -91,8 +91,8 @@ uid_t parse_target_uid(const char *str, pid_t tid, pid_t pid)
> if (str == NULL)
> return UINT_MAX;
>
> - /* CPU and PID are mutually exclusive */
> - if (tid > 0 || pid > 0) {
> + /* UUID and PID are mutually exclusive */
s/UUID/UID/ ?
Thanks.
Namhyung
> + if (tid || pid) {
> ui__warning("PID/TID switch overriding UID\n");
> sleep(1);
> return UINT_MAX;
> diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
> index 232d17e..7917b09 100644
> --- a/tools/perf/util/util.h
> +++ b/tools/perf/util/util.h
> @@ -245,7 +245,7 @@ struct perf_event_attr;
>
> void event_attr_init(struct perf_event_attr *attr);
>
> -uid_t parse_target_uid(const char *str, pid_t tid, pid_t pid);
> +uid_t parse_target_uid(const char *str, const char *tid, const char *pid);
>
> #define _STR(x) #x
> #define STR(x) _STR(x)
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] perf tools: Allow multiple threads or processes in record, stat, top
2012-02-09 1:19 ` Namhyung Kim
@ 2012-02-09 2:52 ` David Ahern
2012-02-09 5:36 ` Namhyung Kim
2012-02-09 7:37 ` Ingo Molnar
0 siblings, 2 replies; 22+ messages in thread
From: David Ahern @ 2012-02-09 2:52 UTC (permalink / raw)
To: Namhyung Kim; +Cc: acme, linux-kernel, mingo, peterz, fweisbec, paulus, tglx
On 02/08/2012 06:19 PM, Namhyung Kim wrote:
>> diff --git a/tools/perf/util/thread_map.c b/tools/perf/util/thread_map.c
>> index 3d4b6c5..e793c16 100644
>> --- a/tools/perf/util/thread_map.c
>> +++ b/tools/perf/util/thread_map.c
>> @@ -6,7 +6,10 @@
>> #include <sys/types.h>
>> #include <sys/stat.h>
>> #include <unistd.h>
>> +#include <ctype.h>
>
> I was trying to remove ctype.h, you might use util.h here.
Right, knew that. But, in this case I am adding a call to isdigit which
means a direct dependency on ctype.h. I would prefer a direct
relationship versus an indirect via util.h
>
>
>> +#include <string.h>
>> #include "thread_map.h"
>> +#include "debug.h"
>>
>> /* Skip "." and ".." directories */
>> static int filter(const struct dirent *dir)
>> @@ -152,6 +155,155 @@ struct thread_map *thread_map__new(pid_t pid, pid_t tid, uid_t uid)
>> return thread_map__new_by_tid(tid);
>> }
>>
>> +
>> +static pid_t get_next_task_id(char **str)
>> +{
>> + char *p, *pend;
>> + pid_t ret = -1;
>> + bool adv_pend = false;
>> +
>> + pend = p = *str;
>> +
>> + /* empty strings */
>> + if (*pend == '\0')
>> + return -1;
>> +
>> + /* only expecting digits separated by commas */
>> + while (isdigit(*pend))
>> + ++pend;
>> +
>> + if (*pend == ',') {
>> + *pend = '\0';
>> + adv_pend = true;
>> + /* catch strings ending in a comma - like '1234,' */
>> + if (*(pend+1) == '\0') {
>> + ui__error("task id strings should not end in a comma\n");
>> + goto out;
>> + }
>> + }
>> +
>> + /* only convert if all characters are valid */
>> + if (*pend == '\0') {
>> + ret = atoi(p);
>> + if (adv_pend)
>> + pend++;
>> + *str = pend;
>> + }
>> +
>> +out:
>> + return ret;
>> +}
>> +
>> +static struct thread_map *thread_map__new_by_pid_str(const char *pid_str)
>> +{
>> + struct thread_map *threads = NULL;
>> + char name[256];
>> + int items, total_tasks = 0;
>> + struct dirent **namelist = NULL;
>> + int i, j = 0;
>> + char *ostr, *str;
>> + pid_t pid;
>> +
>> + ostr = strdup(pid_str);
>> + if (!ostr)
>> + return NULL;
>> + str = ostr;
>> +
>> + while (*str) {
>> + pid = get_next_task_id(&str);
>> + if (pid< 0) {
>> + free(threads);
>> + threads = NULL;
>> + break;
>> + }
>> +
>> + sprintf(name, "/proc/%d/task", pid);
>> + items = scandir(name,&namelist, filter, NULL);
>> + if (items<= 0)
>> + break;
>> +
>> + total_tasks += items;
>> + if (threads)
>> + threads = realloc(threads,
>> + sizeof(*threads) + sizeof(pid_t)*total_tasks);
>> + else
>> + threads = malloc(sizeof(*threads) + sizeof(pid_t)*items);
>> +
>> + if (threads) {
>> + for (i = 0; i< items; i++)
>> + threads->map[j++] = atoi(namelist[i]->d_name);
>> + threads->nr = total_tasks;
>> + }
>> +
>> + for (i = 0; i< items; i++)
>> + free(namelist[i]);
>> + free(namelist);
>> +
>> + if (!threads)
>> + break;
>> + }
>> +
>> + free(ostr);
>> +
>> + return threads;
>> +}
>> +
>> +static struct thread_map *thread_map__new_by_tid_str(const char *tid_str)
>> +{
>> + struct thread_map *threads = NULL;
>> + char *str;
>> + int ntasks = 0;
>> + pid_t tid;
>> +
>> + /* perf-stat expects threads to be generated even if tid not given */
>> + if (!tid_str) {
>> + threads = malloc(sizeof(*threads) + sizeof(pid_t));
>> + threads->map[1] = -1;
>> + threads->nr = 1;
>> + return threads;
>> + }
>> +
>> + str = strdup(tid_str);
>> + if (!str)
>> + return NULL;
>> +
>> + while (*str) {
>> + tid = get_next_task_id(&str);
>> + if (tid < 0) {
>> + free(threads);
>> + threads = NULL;
>> + break;
>> + }
>> +
>> + ntasks++;
>> + if (threads)
>> + threads = realloc(threads,
>> + sizeof(*threads) + sizeof(pid_t) * ntasks);
>> + else
>> + threads = malloc(sizeof(*threads) + sizeof(pid_t));
>> +
>> + if (threads == NULL)
>> + break;
>> +
>> + threads->map[ntasks-1] = tid;
>> + threads->nr = ntasks;
>> + }
>> +
>> + return threads;
>> +}
>> +
>
> This will ends up being a code duplication. How about something like below?
>
> while (*str) {
> tid = get_next_task_id(&str);
> if (tid < 0)
> goto out_err;
>
> tmp = thread_map__new_by_tid(tid); /* and for pid too */
> if (tmp == NULL)
> goto out_err;
>
> threads = thread_map__merge(threads, tmp); /* should be implemented */
> if (threads == NULL)
> break;
> }
>
The pid and tid functions are implemented differently. The commonality
is parsing a CSV string in a while loop.
>
>> +struct thread_map *thread_map__new_str(const char *pid, const char *tid,
>> + uid_t uid)
>> +{
>> + if (pid)
>> + return thread_map__new_by_pid_str(pid);
>> +
>> + if (!tid&& uid != UINT_MAX)
>> + return thread_map__new_by_uid(uid);
>> +
>> + return thread_map__new_by_tid_str(tid);
>> +}
>> +
>> void thread_map__delete(struct thread_map *threads)
>> {
>> free(threads);
> [snip]
>> diff --git a/tools/perf/util/usage.c b/tools/perf/util/usage.c
>> index d0c0139..1240bd6 100644
>> --- a/tools/perf/util/usage.c
>> +++ b/tools/perf/util/usage.c
>> @@ -83,7 +83,7 @@ void warning(const char *warn, ...)
>> va_end(params);
>> }
>>
>> -uid_t parse_target_uid(const char *str, pid_t tid, pid_t pid)
>> +uid_t parse_target_uid(const char *str, const char *tid, const char *pid)
>> {
>> struct passwd pwd, *result;
>> char buf[1024];
>> @@ -91,8 +91,8 @@ uid_t parse_target_uid(const char *str, pid_t tid, pid_t pid)
>> if (str == NULL)
>> return UINT_MAX;
>>
>> - /* CPU and PID are mutually exclusive */
>> - if (tid > 0 || pid > 0) {
>> + /* UUID and PID are mutually exclusive */
>
> s/UUID/UID/ ?
Yes. I was trying to fix the CPU typo when the function was created.
Will remove the extra 'U'.
David
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] perf tools: Allow multiple threads or processes in record, stat, top
2012-02-09 2:52 ` David Ahern
@ 2012-02-09 5:36 ` Namhyung Kim
2012-02-09 6:04 ` David Ahern
2012-02-09 7:37 ` Ingo Molnar
1 sibling, 1 reply; 22+ messages in thread
From: Namhyung Kim @ 2012-02-09 5:36 UTC (permalink / raw)
To: David Ahern; +Cc: acme, linux-kernel, mingo, peterz, fweisbec, paulus, tglx
2012-02-09 11:52 AM, David Ahern wrote:
> On 02/08/2012 06:19 PM, Namhyung Kim wrote:
>>> diff --git a/tools/perf/util/thread_map.c b/tools/perf/util/thread_map.c
>>> index 3d4b6c5..e793c16 100644
>>> --- a/tools/perf/util/thread_map.c
>>> +++ b/tools/perf/util/thread_map.c
>>> @@ -6,7 +6,10 @@
>>> #include<sys/types.h>
>>> #include<sys/stat.h>
>>> #include<unistd.h>
>>> +#include<ctype.h>
>>
>> I was trying to remove ctype.h, you might use util.h here.
>
> Right, knew that. But, in this case I am adding a call to isdigit which
> means a direct dependency on ctype.h. I would prefer a direct
> relationship versus an indirect via util.h
>
OK, not a big deal.
>>
>>
>>> +#include<string.h>
>>> #include "thread_map.h"
>>> +#include "debug.h"
>>>
>>> /* Skip "." and ".." directories */
>>> static int filter(const struct dirent *dir)
>>> @@ -152,6 +155,155 @@ struct thread_map *thread_map__new(pid_t pid, pid_t tid, uid_t uid)
>>> return thread_map__new_by_tid(tid);
>>> }
>>>
>>> +
>>> +static pid_t get_next_task_id(char **str)
>>> +{
>>> + char *p, *pend;
>>> + pid_t ret = -1;
>>> + bool adv_pend = false;
>>> +
>>> + pend = p = *str;
>>> +
>>> + /* empty strings */
>>> + if (*pend == '\0')
>>> + return -1;
>>> +
>>> + /* only expecting digits separated by commas */
>>> + while (isdigit(*pend))
>>> + ++pend;
>>> +
>>> + if (*pend == ',') {
>>> + *pend = '\0';
>>> + adv_pend = true;
>>> + /* catch strings ending in a comma - like '1234,' */
>>> + if (*(pend+1) == '\0') {
>>> + ui__error("task id strings should not end in a comma\n");
>>> + goto out;
>>> + }
>>> + }
>>> +
>>> + /* only convert if all characters are valid */
>>> + if (*pend == '\0') {
>>> + ret = atoi(p);
>>> + if (adv_pend)
>>> + pend++;
>>> + *str = pend;
>>> + }
>>> +
>>> +out:
>>> + return ret;
>>> +}
>>> +
>>> +static struct thread_map *thread_map__new_by_pid_str(const char *pid_str)
>>> +{
>>> + struct thread_map *threads = NULL;
>>> + char name[256];
>>> + int items, total_tasks = 0;
>>> + struct dirent **namelist = NULL;
>>> + int i, j = 0;
>>> + char *ostr, *str;
>>> + pid_t pid;
>>> +
>>> + ostr = strdup(pid_str);
>>> + if (!ostr)
>>> + return NULL;
>>> + str = ostr;
>>> +
>>> + while (*str) {
>>> + pid = get_next_task_id(&str);
>>> + if (pid< 0) {
>>> + free(threads);
>>> + threads = NULL;
>>> + break;
>>> + }
>>> +
>>> + sprintf(name, "/proc/%d/task", pid);
>>> + items = scandir(name,&namelist, filter, NULL);
>>> + if (items<= 0)
>>> + break;
>>> +
>>> + total_tasks += items;
>>> + if (threads)
>>> + threads = realloc(threads,
>>> + sizeof(*threads) + sizeof(pid_t)*total_tasks);
>>> + else
>>> + threads = malloc(sizeof(*threads) + sizeof(pid_t)*items);
>>> +
>>> + if (threads) {
>>> + for (i = 0; i< items; i++)
>>> + threads->map[j++] = atoi(namelist[i]->d_name);
>>> + threads->nr = total_tasks;
>>> + }
>>> +
>>> + for (i = 0; i< items; i++)
>>> + free(namelist[i]);
>>> + free(namelist);
>>> +
>>> + if (!threads)
>>> + break;
>>> + }
>>> +
>>> + free(ostr);
>>> +
>>> + return threads;
>>> +}
>>> +
>>> +static struct thread_map *thread_map__new_by_tid_str(const char *tid_str)
>>> +{
>>> + struct thread_map *threads = NULL;
>>> + char *str;
>>> + int ntasks = 0;
>>> + pid_t tid;
>>> +
>>> + /* perf-stat expects threads to be generated even if tid not given */
>>> + if (!tid_str) {
>>> + threads = malloc(sizeof(*threads) + sizeof(pid_t));
>>> + threads->map[1] = -1;
>>> + threads->nr = 1;
>>> + return threads;
>>> + }
>>> +
>>> + str = strdup(tid_str);
>>> + if (!str)
>>> + return NULL;
>>> +
>>> + while (*str) {
>>> + tid = get_next_task_id(&str);
>>> + if (tid< 0) {
>>> + free(threads);
>>> + threads = NULL;
>>> + break;
>>> + }
>>> +
>>> + ntasks++;
>>> + if (threads)
>>> + threads = realloc(threads,
>>> + sizeof(*threads) + sizeof(pid_t) * ntasks);
>>> + else
>>> + threads = malloc(sizeof(*threads) + sizeof(pid_t));
>>> +
>>> + if (threads == NULL)
>>> + break;
>>> +
>>> + threads->map[ntasks-1] = tid;
>>> + threads->nr = ntasks;
>>> + }
>>> +
>>> + return threads;
>>> +}
>>> +
>>
>> This will ends up being a code duplication. How about something like below?
>>
>> while (*str) {
>> tid = get_next_task_id(&str);
>> if (tid< 0)
>> goto out_err;
>>
>> tmp = thread_map__new_by_tid(tid); /* and for pid too */
>> if (tmp == NULL)
>> goto out_err;
>>
>> threads = thread_map__merge(threads, tmp); /* should be implemented */
>> if (threads == NULL)
>> break;
>> }
>>
>
> The pid and tid functions are implemented differently. The commonality
> is parsing a CSV string in a while loop.
>
Oh, I meant this:
static struct thread_map *thread_map__new_by_pid_str(const char *pid_str)
{
...
while (*str) {
...
tmp = thread_map__new_by_pid(pid);
...
}
...
}
static struct thread_map *thread_map__new_by_tid_str(const char *tid_str)
{
...
while (*str) {
...
tmp = thread_map__new_by_tid(tid);
...
}
...
}
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] perf tools: Allow multiple threads or processes in record, stat, top
2012-02-09 5:36 ` Namhyung Kim
@ 2012-02-09 6:04 ` David Ahern
0 siblings, 0 replies; 22+ messages in thread
From: David Ahern @ 2012-02-09 6:04 UTC (permalink / raw)
To: Namhyung Kim; +Cc: acme, linux-kernel, mingo, peterz, fweisbec, paulus, tglx
On 02/08/2012 10:36 PM, Namhyung Kim wrote:
> Oh, I meant this:
>
> static struct thread_map *thread_map__new_by_pid_str(const char *pid_str)
> {
> ...
> while (*str) {
> ...
> tmp = thread_map__new_by_pid(pid);
> ...
> }
> ...
> }
>
> static struct thread_map *thread_map__new_by_tid_str(const char *tid_str)
> {
> ...
> while (*str) {
> ...
> tmp = thread_map__new_by_tid(tid);
> ...
> }
> ...
> }
I'm still not seeing the code simplification here. For example,
new_by_tid(tid) allocates threads, sets the first element (it only
supports one) and returns it. In new_by_tid_str we loop over a CSV,
allocating/re-allocating on additional and then setting the current i-th
tid as opposed to the 0th element.
Similarly, for pid.
Personally, I liked my first patch better which dropped the current
tid/pid functions; they are only used by perf-test and it can be updated
to convert getpid() to a string and use the new API.
David
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] perf tools: Allow multiple threads or processes in record, stat, top
2012-02-09 2:52 ` David Ahern
2012-02-09 5:36 ` Namhyung Kim
@ 2012-02-09 7:37 ` Ingo Molnar
2012-02-09 14:34 ` Arnaldo Carvalho de Melo
1 sibling, 1 reply; 22+ messages in thread
From: Ingo Molnar @ 2012-02-09 7:37 UTC (permalink / raw)
To: David Ahern
Cc: Namhyung Kim, acme, linux-kernel, peterz, fweisbec, paulus, tglx
* David Ahern <dsahern@gmail.com> wrote:
> > I was trying to remove ctype.h, you might use util.h here.
>
> Right, knew that. But, in this case I am adding a call to
> isdigit which means a direct dependency on ctype.h. I would
> prefer a direct relationship versus an indirect via util.h
Please just remove ctype.h *altogether* from perf, it's just an
insane header.
Have a look at how Git solves these types of problems, it
defines sane string functions in git-compat-util.h:
/* Sane ctype - no locale, and works with signed chars */
#undef isascii
#undef isspace
#undef isdigit
#undef isalpha
#undef isalnum
#undef tolower
#undef toupper
extern unsigned char sane_ctype[256];
#define GIT_SPACE 0x01
#define GIT_DIGIT 0x02
#define GIT_ALPHA 0x04
#define GIT_GLOB_SPECIAL 0x08
#define GIT_REGEX_SPECIAL 0x10
Thanks,
Ingo
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] perf tools: Allow multiple threads or processes in record, stat, top
2012-02-09 7:37 ` Ingo Molnar
@ 2012-02-09 14:34 ` Arnaldo Carvalho de Melo
2012-02-09 14:44 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 22+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-02-09 14:34 UTC (permalink / raw)
To: Ingo Molnar
Cc: David Ahern, Namhyung Kim, linux-kernel, peterz, fweisbec, paulus, tglx
Em Thu, Feb 09, 2012 at 08:37:27AM +0100, Ingo Molnar escreveu:
>
> * David Ahern <dsahern@gmail.com> wrote:
>
> > > I was trying to remove ctype.h, you might use util.h here.
> >
> > Right, knew that. But, in this case I am adding a call to
> > isdigit which means a direct dependency on ctype.h. I would
> > prefer a direct relationship versus an indirect via util.h
>
> Please just remove ctype.h *altogether* from perf, it's just an
> insane header.
>
> Have a look at how Git solves these types of problems, it
> defines sane string functions in git-compat-util.h:
Yeah, these are in util.h, that doesn't includes ctype.h
I'm fixing this up and also that s/UUID/UID/g Kim pointed out,
then testing if the python binding still is ok with these changes.
> /* Sane ctype - no locale, and works with signed chars */
> #undef isascii
> #undef isspace
> #undef isdigit
> #undef isalpha
> #undef isalnum
> #undef tolower
> #undef toupper
> extern unsigned char sane_ctype[256];
> #define GIT_SPACE 0x01
> #define GIT_DIGIT 0x02
> #define GIT_ALPHA 0x04
> #define GIT_GLOB_SPECIAL 0x08
> #define GIT_REGEX_SPECIAL 0x10
>
> Thanks,
>
> Ingo
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] perf tools: Allow multiple threads or processes in record, stat, top
2012-02-09 14:34 ` Arnaldo Carvalho de Melo
@ 2012-02-09 14:44 ` Arnaldo Carvalho de Melo
2012-02-10 5:42 ` Namhyung Kim
0 siblings, 1 reply; 22+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-02-09 14:44 UTC (permalink / raw)
To: David Ahern
Cc: Ingo Molnar, Namhyung Kim, linux-kernel, peterz, fweisbec, paulus, tglx
Em Thu, Feb 09, 2012 at 12:34:49PM -0200, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Feb 09, 2012 at 08:37:27AM +0100, Ingo Molnar escreveu:
> >
> > * David Ahern <dsahern@gmail.com> wrote:
> >
> > > > I was trying to remove ctype.h, you might use util.h here.
> > >
> > > Right, knew that. But, in this case I am adding a call to
> > > isdigit which means a direct dependency on ctype.h. I would
> > > prefer a direct relationship versus an indirect via util.h
> >
> > Please just remove ctype.h *altogether* from perf, it's just an
> > insane header.
> >
> > Have a look at how Git solves these types of problems, it
> > defines sane string functions in git-compat-util.h:
>
> Yeah, these are in util.h, that doesn't includes ctype.h
>
> I'm fixing this up and also that s/UUID/UID/g Kim pointed out,
> then testing if the python binding still is ok with these changes.
[root@aninha linux]# tools/perf/python/twatch.py
Traceback (most recent call last):
File "tools/perf/python/twatch.py", line 16, in <module>
import perf
ImportError: /home/acme/git/build/perf/python/perf.so: undefined symbol:
ui__error
[root@aninha linux]#
it breaks, I'll check an alternative way to report problems without
calling ui__ methods from thread_map.
- Arnaldo
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] perf tools: Allow multiple threads or processes in record, stat, top
2012-02-09 14:44 ` Arnaldo Carvalho de Melo
@ 2012-02-10 5:42 ` Namhyung Kim
2012-02-10 14:28 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 22+ messages in thread
From: Namhyung Kim @ 2012-02-10 5:42 UTC (permalink / raw)
To: linux-kernel
Cc: David Ahern, Ingo Molnar, linux-kernel, peterz, fweisbec, paulus, tglx
Hello,
2012-02-09 11:44 PM, Arnaldo Carvalho de Melo wrote:
> Em Thu, Feb 09, 2012 at 12:34:49PM -0200, Arnaldo Carvalho de Melo escreveu:
>> Em Thu, Feb 09, 2012 at 08:37:27AM +0100, Ingo Molnar escreveu:
>>>
>>> * David Ahern<dsahern@gmail.com> wrote:
>>>
>>>>> I was trying to remove ctype.h, you might use util.h here.
>>>>
>>>> Right, knew that. But, in this case I am adding a call to
>>>> isdigit which means a direct dependency on ctype.h. I would
>>>> prefer a direct relationship versus an indirect via util.h
>>>
>>> Please just remove ctype.h *altogether* from perf, it's just an
>>> insane header.
>>>
>>> Have a look at how Git solves these types of problems, it
>>> defines sane string functions in git-compat-util.h:
>>
>> Yeah, these are in util.h, that doesn't includes ctype.h
>>
>> I'm fixing this up and also that s/UUID/UID/g Kim pointed out,
>> then testing if the python binding still is ok with these changes.
>
> [root@aninha linux]# tools/perf/python/twatch.py
> Traceback (most recent call last):
> File "tools/perf/python/twatch.py", line 16, in<module>
> import perf
> ImportError: /home/acme/git/build/perf/python/perf.so: undefined symbol:
> ui__error
> [root@aninha linux]#
>
> it breaks, I'll check an alternative way to report problems without
> calling ui__ methods from thread_map.
>
> - Arnaldo
I have a different result:
$ git checkout tip/perf/core
...
HEAD is now at c98fdeaa9273... x86/sched/perf/AMD: Set sched_clock_stable
$
$ patch -p1 < perf-allow-multiple-threads.patch
patching file tools/perf/Documentation/perf-record.txt
patching file tools/perf/Documentation/perf-stat.txt
patching file tools/perf/Documentation/perf-top.txt
patching file tools/perf/builtin-record.c
patching file tools/perf/builtin-stat.c
patching file tools/perf/builtin-test.c
patching file tools/perf/builtin-top.c
patching file tools/perf/perf.h
patching file tools/perf/util/evlist.c
patching file tools/perf/util/evlist.h
patching file tools/perf/util/evsel.c
patching file tools/perf/util/thread_map.c
patching file tools/perf/util/thread_map.h
patching file tools/perf/util/top.c
patching file tools/perf/util/top.h
patching file tools/perf/util/usage.c
patching file tools/perf/util/util.h
patch unexpectedly ends in middle of line
$
$ cd tools/perf
$ make -j8
Makefile:417: No libdw.h found or old libdw.h found or elfutils is older than 0.138, disables dwarf support.
Please install new elfutils-devel/libdw-dev
Makefile:604: No bfd.h/libbfd found, install binutils-dev[el]/zlib-static to gain symbol demangling
GEN common-cmds.h
...
AR libperf.a
LINK perf
$
$ sudo python/twatch.py
Traceback (most recent call last):
File "python/twatch.py", line 41, in <module>
main()
File "python/twatch.py", line 25, in main
evsel.open(cpus = cpus, threads = threads);
OSError: [Errno 22] Invalid argument
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] perf tools: Allow multiple threads or processes in record, stat, top
2012-02-10 5:42 ` Namhyung Kim
@ 2012-02-10 14:28 ` Arnaldo Carvalho de Melo
2012-02-12 10:45 ` [PATCH] perf tools: Fix build dependency of perf python extension Namhyung Kim
0 siblings, 1 reply; 22+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-02-10 14:28 UTC (permalink / raw)
To: Namhyung Kim
Cc: David Ahern, Ingo Molnar, linux-kernel, peterz, fweisbec, paulus, tglx
Em Fri, Feb 10, 2012 at 02:42:01PM +0900, Namhyung Kim escreveu:
> 2012-02-09 11:44 PM, Arnaldo Carvalho de Melo wrote:
> > [root@aninha linux]# tools/perf/python/twatch.py
> > Traceback (most recent call last):
> > File "tools/perf/python/twatch.py", line 16, in<module>
> > import perf
> > ImportError: /home/acme/git/build/perf/python/perf.so: undefined symbol: ui__error
> > it breaks, I'll check an alternative way to report problems without
> > calling ui__ methods from thread_map.
>
> I have a different result:
> $ git checkout tip/perf/core
> HEAD is now at c98fdeaa9273... x86/sched/perf/AMD: Set sched_clock_stable
> $ patch -p1 < perf-allow-multiple-threads.patch
> patching file tools/perf/Documentation/perf-record.txt
<SNIP>
> $ cd tools/perf
> $ make -j8
<SNIP>
> $ sudo python/twatch.py
> Traceback (most recent call last):
> File "python/twatch.py", line 41, in <module>
> main()
> File "python/twatch.py", line 25, in main
> evsel.open(cpus = cpus, threads = threads);
> OSError: [Errno 22] Invalid argument
This is another problem and one I would appreciate if you could fix :-)
I mentioned it to David in a private message and there was at least one
instance where Linus was involved and I promised to fix this but haven't
find the time to do so.
The problem is that one needs to research how to make the python binding
to be rebuilt when one of the files that are linked into it changes.
Please take a look at tools/perf/util/setup.py and you'll see:
perf = Extension('perf',
sources = ['util/python.c', 'util/ctype.c', 'util/evlist.c',
'util/evsel.c', 'util/cpumap.c', 'util/thread_map.c',
'util/util.c', 'util/xyarray.c', 'util/cgroup.c',
'util/debugfs.c'],
include_dirs = ['util/include'],
extra_compile_args = cflags,
)
Probably is something simple in the Makefiles.
So please try again after doing:
cd tools/perf
rm -rf python/perf.so
make
And you should get the same problem. Also check
/proc/sys/kernel/perf_event_paranoid as this script does syswide sampling.
I'll try to fix this problem now that I took the time to think about it
8-)
- Arnaldo
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] perf tools: Allow multiple threads or processes in record, stat, top
2012-02-08 16:32 [PATCH] perf tools: Allow multiple threads or processes in record, stat, top David Ahern
2012-02-09 1:19 ` Namhyung Kim
@ 2012-02-10 19:24 ` Arnaldo Carvalho de Melo
2012-02-10 19:32 ` David Ahern
2012-02-17 9:46 ` [tip:perf/core] " tip-bot for David Ahern
2 siblings, 1 reply; 22+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-02-10 19:24 UTC (permalink / raw)
To: David Ahern; +Cc: linux-kernel, mingo, peterz, fweisbec, paulus, tglx
Em Wed, Feb 08, 2012 at 09:32:52AM -0700, David Ahern escreveu:
> Allow a user to collect events for multiple threads or processes
> using a comma separated list.
>
> e.g., collect data on a VM and its vhost thread:
> perf top -p 21483,21485
> perf stat -p 21483,21485 -ddd
> perf record -p 21483,21485
>
> or monitoring vcpu threads
> perf top -t 21488,21489
> perf stat -t 21488,21489 -ddd
> perf record -t 21488,21489
I found some problems below:
> diff --git a/tools/perf/util/thread_map.c b/tools/perf/util/thread_map.c
> index 3d4b6c5..e793c16 100644
> --- a/tools/perf/util/thread_map.c
> +++ b/tools/perf/util/thread_map.c
> +static struct thread_map *thread_map__new_by_pid_str(const char *pid_str)
> +{
> + struct thread_map *threads = NULL;
<SNIP>
> + if (threads)
> + threads = realloc(threads,
> + sizeof(*threads) + sizeof(pid_t)*total_tasks);
> + else
> + threads = malloc(sizeof(*threads) + sizeof(pid_t)*items);
If realloc fails and threads was allocated before... we leak memory.
We don't need to use this if clause, realloc handles threads == NULL
just fines and then works as malloc.
Also I didn't use ctype altogether + that CSV parsing routine, we have
strlist for that, it even will allow us to trow away duplicate pids/tids
and also supports passing a file with a list of threads to monitor, for
free.
Take a look at the modified patch below, if you're ok with it I can keep
your autorship and put a [committer note: use strlist] or take autorship
and state that it was based on a patch made by you, definetely your
call. I'd go for the committer note.
- Arnaldo
diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index ff9a66e..a5766b4 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -52,11 +52,11 @@ OPTIONS
-p::
--pid=::
- Record events on existing process ID.
+ Record events on existing process ID (comma separated list).
-t::
--tid=::
- Record events on existing thread ID.
+ Record events on existing thread ID (comma separated list).
-u::
--uid=::
diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
index 8966b9a..2fa173b 100644
--- a/tools/perf/Documentation/perf-stat.txt
+++ b/tools/perf/Documentation/perf-stat.txt
@@ -35,11 +35,11 @@ OPTIONS
child tasks do not inherit counters
-p::
--pid=<pid>::
- stat events on existing process id
+ stat events on existing process id (comma separated list)
-t::
--tid=<tid>::
- stat events on existing thread id
+ stat events on existing thread id (comma separated list)
-a::
diff --git a/tools/perf/Documentation/perf-top.txt b/tools/perf/Documentation/perf-top.txt
index ab1454e..4a5680c 100644
--- a/tools/perf/Documentation/perf-top.txt
+++ b/tools/perf/Documentation/perf-top.txt
@@ -72,11 +72,11 @@ Default is to monitor all CPUS.
-p <pid>::
--pid=<pid>::
- Profile events on existing Process ID.
+ Profile events on existing Process ID (comma separated list).
-t <tid>::
--tid=<tid>::
- Profile events on existing thread ID.
+ Profile events on existing thread ID (comma separated list).
-u::
--uid=::
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index d6d1c6c..08ed24b 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -645,8 +645,6 @@ static const char * const record_usage[] = {
*/
static struct perf_record record = {
.opts = {
- .target_pid = -1,
- .target_tid = -1,
.mmap_pages = UINT_MAX,
.user_freq = UINT_MAX,
.user_interval = ULLONG_MAX,
@@ -670,9 +668,9 @@ const struct option record_options[] = {
parse_events_option),
OPT_CALLBACK(0, "filter", &record.evlist, "filter",
"event filter", parse_filter),
- OPT_INTEGER('p', "pid", &record.opts.target_pid,
+ OPT_STRING('p', "pid", &record.opts.target_pid, "pid",
"record events on existing process id"),
- OPT_INTEGER('t', "tid", &record.opts.target_tid,
+ OPT_STRING('t', "tid", &record.opts.target_tid, "tid",
"record events on existing thread id"),
OPT_INTEGER('r', "realtime", &record.realtime_prio,
"collect data with this RT SCHED_FIFO priority"),
@@ -739,7 +737,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 == -1 && rec->opts.target_tid == -1 &&
+ if (!argc && !rec->opts.target_pid && !rec->opts.target_tid &&
!rec->opts.system_wide && !rec->opts.cpu_list && !rec->uid_str)
usage_with_options(record_usage, record_options);
@@ -785,7 +783,7 @@ int cmd_record(int argc, const char **argv, const char *prefix __used)
if (rec->uid_str != NULL && rec->opts.uid == UINT_MAX - 1)
goto out_free_fd;
- if (rec->opts.target_pid != -1)
+ if (rec->opts.target_pid)
rec->opts.target_tid = rec->opts.target_pid;
if (perf_evlist__create_maps(evsel_list, rec->opts.target_pid,
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index d14b37a..ea40e4e 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -182,8 +182,8 @@ static int run_count = 1;
static bool no_inherit = false;
static bool scale = true;
static bool no_aggr = false;
-static pid_t target_pid = -1;
-static pid_t target_tid = -1;
+static const char *target_pid;
+static const char *target_tid;
static pid_t child_pid = -1;
static bool null_run = false;
static int detailed_run = 0;
@@ -296,7 +296,7 @@ static int create_perf_stat_counter(struct perf_evsel *evsel,
if (system_wide)
return perf_evsel__open_per_cpu(evsel, evsel_list->cpus,
group, group_fd);
- if (target_pid == -1 && target_tid == -1) {
+ if (!target_pid && !target_tid) {
attr->disabled = 1;
attr->enable_on_exec = 1;
}
@@ -446,7 +446,7 @@ static int run_perf_stat(int argc __used, const char **argv)
exit(-1);
}
- if (target_tid == -1 && target_pid == -1 && !system_wide)
+ if (!target_tid && !target_pid && !system_wide)
evsel_list->threads->map[0] = child_pid;
/*
@@ -968,14 +968,14 @@ static void print_stat(int argc, const char **argv)
if (!csv_output) {
fprintf(output, "\n");
fprintf(output, " Performance counter stats for ");
- if(target_pid == -1 && target_tid == -1) {
+ if (!target_pid && !target_tid) {
fprintf(output, "\'%s", argv[0]);
for (i = 1; i < argc; i++)
fprintf(output, " %s", argv[i]);
- } else if (target_pid != -1)
- fprintf(output, "process id \'%d", target_pid);
+ } else if (target_pid)
+ fprintf(output, "process id \'%s", target_pid);
else
- fprintf(output, "thread id \'%d", target_tid);
+ fprintf(output, "thread id \'%s", target_tid);
fprintf(output, "\'");
if (run_count > 1)
@@ -1049,10 +1049,10 @@ static const struct option options[] = {
"event filter", parse_filter),
OPT_BOOLEAN('i', "no-inherit", &no_inherit,
"child tasks do not inherit counters"),
- OPT_INTEGER('p', "pid", &target_pid,
- "stat events on existing process id"),
- OPT_INTEGER('t', "tid", &target_tid,
- "stat events on existing thread id"),
+ OPT_STRING('p', "pid", &target_pid, "pid",
+ "stat events on existing process id"),
+ OPT_STRING('t', "tid", &target_tid, "tid",
+ "stat events on existing thread id"),
OPT_BOOLEAN('a', "all-cpus", &system_wide,
"system-wide collection from all CPUs"),
OPT_BOOLEAN('g', "group", &group,
@@ -1190,7 +1190,7 @@ 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 == -1 && target_tid == -1)
+ if (!argc && !target_pid && !target_tid)
usage_with_options(stat_usage, options);
if (run_count <= 0)
usage_with_options(stat_usage, options);
@@ -1206,10 +1206,11 @@ int cmd_stat(int argc, const char **argv, const char *prefix __used)
if (add_default_attributes())
goto out;
- if (target_pid != -1)
+ if (target_pid)
target_tid = target_pid;
- evsel_list->threads = thread_map__new(target_pid, target_tid, UINT_MAX);
+ evsel_list->threads = thread_map__new_str(target_pid,
+ target_tid, UINT_MAX);
if (evsel_list->threads == NULL) {
pr_err("Problems finding threads of monitor\n");
usage_with_options(stat_usage, options);
diff --git a/tools/perf/builtin-test.c b/tools/perf/builtin-test.c
index 70c4eb2..0f15195 100644
--- a/tools/perf/builtin-test.c
+++ b/tools/perf/builtin-test.c
@@ -1010,8 +1010,6 @@ realloc:
static int test__PERF_RECORD(void)
{
struct perf_record_opts opts = {
- .target_pid = -1,
- .target_tid = -1,
.no_delay = true,
.freq = 10,
.mmap_pages = 256,
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index d869b21..5ed62a8 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -965,7 +965,7 @@ static int __cmd_top(struct perf_top *top)
if (ret)
goto out_delete;
- if (top->target_tid != -1 || top->uid != UINT_MAX)
+ if (top->target_tid || top->uid != UINT_MAX)
perf_event__synthesize_thread_map(&top->tool, top->evlist->threads,
perf_event__process,
&top->session->host_machine);
@@ -1103,8 +1103,6 @@ int cmd_top(int argc, const char **argv, const char *prefix __used)
struct perf_top top = {
.count_filter = 5,
.delay_secs = 2,
- .target_pid = -1,
- .target_tid = -1,
.uid = UINT_MAX,
.freq = 1000, /* 1 KHz */
.sample_id_all_avail = true,
@@ -1118,9 +1116,9 @@ int cmd_top(int argc, const char **argv, const char *prefix __used)
parse_events_option),
OPT_INTEGER('c', "count", &top.default_interval,
"event period to sample"),
- OPT_INTEGER('p', "pid", &top.target_pid,
+ OPT_STRING('p', "pid", &top.target_pid, "pid",
"profile events on existing process id"),
- OPT_INTEGER('t', "tid", &top.target_tid,
+ OPT_STRING('t', "tid", &top.target_tid, "tid",
"profile events on existing thread id"),
OPT_BOOLEAN('a', "all-cpus", &top.system_wide,
"system-wide collection from all CPUs"),
@@ -1210,19 +1208,19 @@ int cmd_top(int argc, const char **argv, const char *prefix __used)
goto out_delete_evlist;
/* CPU and PID are mutually exclusive */
- if (top.target_tid > 0 && top.cpu_list) {
+ if (top.target_tid && top.cpu_list) {
printf("WARNING: PID switch overriding CPU\n");
sleep(1);
top.cpu_list = NULL;
}
- if (top.target_pid != -1)
+ if (top.target_pid)
top.target_tid = top.target_pid;
if (perf_evlist__create_maps(top.evlist, top.target_pid,
top.target_tid, top.uid, top.cpu_list) < 0)
usage_with_options(top_usage, options);
-
+
if (!top.evlist->nr_entries &&
perf_evlist__add_default(top.evlist) < 0) {
pr_err("Not enough memory for event selector list\n");
diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index 92af168..deb17db 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -186,8 +186,8 @@ extern const char perf_version_string[];
void pthread__unblock_sigwinch(void);
struct perf_record_opts {
- pid_t target_pid;
- pid_t target_tid;
+ const char *target_pid;
+ const char *target_tid;
uid_t uid;
bool call_graph;
bool group;
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index a57a8cf..5c61dc5 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -593,15 +593,15 @@ int perf_evlist__mmap(struct perf_evlist *evlist, unsigned int pages,
return perf_evlist__mmap_per_cpu(evlist, prot, mask);
}
-int perf_evlist__create_maps(struct perf_evlist *evlist, pid_t target_pid,
- pid_t target_tid, uid_t uid, const char *cpu_list)
+int perf_evlist__create_maps(struct perf_evlist *evlist, const char *target_pid,
+ const char *target_tid, uid_t uid, const char *cpu_list)
{
- evlist->threads = thread_map__new(target_pid, target_tid, uid);
+ evlist->threads = thread_map__new_str(target_pid, target_tid, uid);
if (evlist->threads == NULL)
return -1;
- if (uid != UINT_MAX || (cpu_list == NULL && target_tid != -1))
+ if (uid != UINT_MAX || (cpu_list == NULL && target_tid))
evlist->cpus = cpu_map__dummy_new();
else
evlist->cpus = cpu_map__new(cpu_list);
@@ -820,7 +820,7 @@ int perf_evlist__prepare_workload(struct perf_evlist *evlist,
exit(-1);
}
- if (!opts->system_wide && opts->target_tid == -1 && opts->target_pid == -1)
+ if (!opts->system_wide && !opts->target_tid && !opts->target_pid)
evlist->threads->map[0] = evlist->workload.pid;
close(child_ready_pipe[1]);
diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
index 1b4282b..21f1c9e 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -106,8 +106,8 @@ static inline void perf_evlist__set_maps(struct perf_evlist *evlist,
evlist->threads = threads;
}
-int perf_evlist__create_maps(struct perf_evlist *evlist, pid_t target_pid,
- pid_t tid, uid_t uid, const char *cpu_list);
+int perf_evlist__create_maps(struct perf_evlist *evlist, const char *target_pid,
+ const char *tid, uid_t uid, const char *cpu_list);
void perf_evlist__delete_maps(struct perf_evlist *evlist);
int perf_evlist__set_filters(struct perf_evlist *evlist);
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 9a11f9e..f910f50 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -130,7 +130,7 @@ void perf_evsel__config(struct perf_evsel *evsel, struct perf_record_opts *opts)
attr->mmap = track;
attr->comm = track;
- if (opts->target_pid == -1 && opts->target_tid == -1 && !opts->system_wide) {
+ if (!opts->target_pid && !opts->target_tid && !opts->system_wide) {
attr->disabled = 1;
attr->enable_on_exec = 1;
}
diff --git a/tools/perf/util/setup.py b/tools/perf/util/setup.py
index 36d4c56..4f85b8f 100644
--- a/tools/perf/util/setup.py
+++ b/tools/perf/util/setup.py
@@ -28,7 +28,8 @@ perf = Extension('perf',
sources = ['util/python.c', 'util/ctype.c', 'util/evlist.c',
'util/evsel.c', 'util/cpumap.c', 'util/thread_map.c',
'util/util.c', 'util/xyarray.c', 'util/cgroup.c',
- 'util/debugfs.c'],
+ 'util/debugfs.c', '../../lib/rbtree.c',
+ 'util/strlist.c'],
include_dirs = ['util/include'],
extra_compile_args = cflags,
)
diff --git a/tools/perf/util/thread_map.c b/tools/perf/util/thread_map.c
index 3d4b6c5..884f7e2 100644
--- a/tools/perf/util/thread_map.c
+++ b/tools/perf/util/thread_map.c
@@ -6,6 +6,8 @@
#include <sys/types.h>
#include <sys/stat.h>
#include <unistd.h>
+#include "strlist.h"
+#include <string.h>
#include "thread_map.h"
/* Skip "." and ".." directories */
@@ -152,6 +154,132 @@ struct thread_map *thread_map__new(pid_t pid, pid_t tid, uid_t uid)
return thread_map__new_by_tid(tid);
}
+static struct thread_map *thread_map__new_by_pid_str(const char *pid_str)
+{
+ struct thread_map *threads = NULL, *nt;
+ char name[256];
+ int items, total_tasks = 0;
+ struct dirent **namelist = NULL;
+ int i, j = 0;
+ pid_t pid, prev_pid = LONG_MAX;
+ char *end_ptr;
+ struct str_node *pos;
+ struct strlist *slist = strlist__new(false, pid_str);
+
+ if (!slist)
+ return NULL;
+
+ strlist__for_each(pos, slist) {
+ pid = strtol(pos->s, &end_ptr, 10);
+
+ if (pid == LONG_MIN || pid == LONG_MAX ||
+ (*end_ptr != '\0' && *end_ptr != ','))
+ goto out_free_threads;
+
+ if (pid == prev_pid)
+ continue;
+
+ sprintf(name, "/proc/%d/task", pid);
+ items = scandir(name, &namelist, filter, NULL);
+ if (items <= 0)
+ break;
+
+ total_tasks += items;
+ nt = realloc(threads, (sizeof(*threads) +
+ sizeof(pid_t) * total_tasks));
+ if (nt == NULL)
+ goto out_free_threads;
+
+ threads = nt;
+
+ if (threads) {
+ for (i = 0; i < items; i++)
+ threads->map[j++] = atoi(namelist[i]->d_name);
+ threads->nr = total_tasks;
+ }
+
+ for (i = 0; i < items; i++)
+ free(namelist[i]);
+ free(namelist);
+
+ if (!threads)
+ break;
+ }
+
+out:
+ strlist__delete(slist);
+ return threads;
+
+out_free_threads:
+ free(threads);
+ threads = NULL;
+ goto out;
+}
+
+static struct thread_map *thread_map__new_by_tid_str(const char *tid_str)
+{
+ struct thread_map *threads = NULL, *nt;
+ int ntasks = 0;
+ pid_t tid, prev_tid = LONG_MAX;
+ char *end_ptr;
+ struct str_node *pos;
+ struct strlist *slist;
+
+ /* perf-stat expects threads to be generated even if tid not given */
+ if (!tid_str) {
+ threads = malloc(sizeof(*threads) + sizeof(pid_t));
+ if (threads != NULL) {
+ threads->map[1] = -1;
+ threads->nr = 1;
+ }
+ return threads;
+ }
+
+ slist = strlist__new(false, tid_str);
+ if (!slist)
+ return NULL;
+
+ strlist__for_each(pos, slist) {
+ tid = strtol(pos->s, &end_ptr, 10);
+
+ if (tid == LONG_MIN || tid == LONG_MAX ||
+ (*end_ptr != '\0' && *end_ptr != ','))
+ goto out_free_threads;
+
+ if (tid == prev_tid)
+ continue;
+
+ ntasks++;
+ nt = realloc(threads, sizeof(*threads) + sizeof(pid_t) * ntasks);
+
+ if (nt == NULL)
+ goto out_free_threads;
+
+ threads = nt;
+ threads->map[ntasks - 1] = tid;
+ threads->nr = ntasks;
+ }
+out:
+ return threads;
+
+out_free_threads:
+ free(threads);
+ threads = NULL;
+ goto out;
+}
+
+struct thread_map *thread_map__new_str(const char *pid, const char *tid,
+ uid_t uid)
+{
+ if (pid)
+ return thread_map__new_by_pid_str(pid);
+
+ if (!tid && uid != UINT_MAX)
+ return thread_map__new_by_uid(uid);
+
+ return thread_map__new_by_tid_str(tid);
+}
+
void thread_map__delete(struct thread_map *threads)
{
free(threads);
diff --git a/tools/perf/util/thread_map.h b/tools/perf/util/thread_map.h
index c75ddba..7da80f1 100644
--- a/tools/perf/util/thread_map.h
+++ b/tools/perf/util/thread_map.h
@@ -13,6 +13,10 @@ struct thread_map *thread_map__new_by_pid(pid_t pid);
struct thread_map *thread_map__new_by_tid(pid_t tid);
struct thread_map *thread_map__new_by_uid(uid_t uid);
struct thread_map *thread_map__new(pid_t pid, pid_t tid, uid_t uid);
+
+struct thread_map *thread_map__new_str(const char *pid,
+ const char *tid, uid_t uid);
+
void thread_map__delete(struct thread_map *threads);
size_t thread_map__fprintf(struct thread_map *threads, FILE *fp);
diff --git a/tools/perf/util/top.c b/tools/perf/util/top.c
index e4370ca..09fe579 100644
--- a/tools/perf/util/top.c
+++ b/tools/perf/util/top.c
@@ -69,11 +69,11 @@ size_t perf_top__header_snprintf(struct perf_top *top, char *bf, size_t size)
ret += SNPRINTF(bf + ret, size - ret, "], ");
- if (top->target_pid != -1)
- ret += SNPRINTF(bf + ret, size - ret, " (target_pid: %d",
+ if (top->target_pid)
+ ret += SNPRINTF(bf + ret, size - ret, " (target_pid: %s",
top->target_pid);
- else if (top->target_tid != -1)
- ret += SNPRINTF(bf + ret, size - ret, " (target_tid: %d",
+ else if (top->target_tid)
+ ret += SNPRINTF(bf + ret, size - ret, " (target_tid: %s",
top->target_tid);
else if (top->uid_str != NULL)
ret += SNPRINTF(bf + ret, size - ret, " (uid: %s",
@@ -85,7 +85,7 @@ size_t perf_top__header_snprintf(struct perf_top *top, char *bf, size_t size)
ret += SNPRINTF(bf + ret, size - ret, ", CPU%s: %s)",
top->evlist->cpus->nr > 1 ? "s" : "", top->cpu_list);
else {
- if (top->target_tid != -1)
+ if (top->target_tid)
ret += SNPRINTF(bf + ret, size - ret, ")");
else
ret += SNPRINTF(bf + ret, size - ret, ", %d CPU%s)",
diff --git a/tools/perf/util/top.h b/tools/perf/util/top.h
index def3e53..49eb848 100644
--- a/tools/perf/util/top.h
+++ b/tools/perf/util/top.h
@@ -23,7 +23,7 @@ struct perf_top {
u64 guest_us_samples, guest_kernel_samples;
int print_entries, count_filter, delay_secs;
int freq;
- pid_t target_pid, target_tid;
+ const char *target_pid, *target_tid;
uid_t uid;
bool hide_kernel_symbols, hide_user_symbols, zero;
bool system_wide;
diff --git a/tools/perf/util/usage.c b/tools/perf/util/usage.c
index d0c0139..52bb07c 100644
--- a/tools/perf/util/usage.c
+++ b/tools/perf/util/usage.c
@@ -83,7 +83,7 @@ void warning(const char *warn, ...)
va_end(params);
}
-uid_t parse_target_uid(const char *str, pid_t tid, pid_t pid)
+uid_t parse_target_uid(const char *str, const char *tid, const char *pid)
{
struct passwd pwd, *result;
char buf[1024];
@@ -91,8 +91,8 @@ uid_t parse_target_uid(const char *str, pid_t tid, pid_t pid)
if (str == NULL)
return UINT_MAX;
- /* CPU and PID are mutually exclusive */
- if (tid > 0 || pid > 0) {
+ /* UID and PID are mutually exclusive */
+ if (tid || pid) {
ui__warning("PID/TID switch overriding UID\n");
sleep(1);
return UINT_MAX;
diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index 232d17e..7917b09 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -245,7 +245,7 @@ struct perf_event_attr;
void event_attr_init(struct perf_event_attr *attr);
-uid_t parse_target_uid(const char *str, pid_t tid, pid_t pid);
+uid_t parse_target_uid(const char *str, const char *tid, const char *pid);
#define _STR(x) #x
#define STR(x) _STR(x)
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH] perf tools: Allow multiple threads or processes in record, stat, top
2012-02-10 19:24 ` [PATCH] perf tools: Allow multiple threads or processes in record, stat, top Arnaldo Carvalho de Melo
@ 2012-02-10 19:32 ` David Ahern
2012-02-10 19:34 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 22+ messages in thread
From: David Ahern @ 2012-02-10 19:32 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: linux-kernel, mingo, peterz, fweisbec, paulus, tglx
On 02/10/2012 12:24 PM, Arnaldo Carvalho de Melo wrote:
> Em Wed, Feb 08, 2012 at 09:32:52AM -0700, David Ahern escreveu:
>> Allow a user to collect events for multiple threads or processes
>> using a comma separated list.
>>
>> e.g., collect data on a VM and its vhost thread:
>> perf top -p 21483,21485
>> perf stat -p 21483,21485 -ddd
>> perf record -p 21483,21485
>>
>> or monitoring vcpu threads
>> perf top -t 21488,21489
>> perf stat -t 21488,21489 -ddd
>> perf record -t 21488,21489
>
> I found some problems below:
>
>> diff --git a/tools/perf/util/thread_map.c b/tools/perf/util/thread_map.c
>> index 3d4b6c5..e793c16 100644
>> --- a/tools/perf/util/thread_map.c
>> +++ b/tools/perf/util/thread_map.c
>> +static struct thread_map *thread_map__new_by_pid_str(const char *pid_str)
>> +{
>> + struct thread_map *threads = NULL;
> <SNIP>
>> + if (threads)
>> + threads = realloc(threads,
>> + sizeof(*threads) + sizeof(pid_t)*total_tasks);
>> + else
>> + threads = malloc(sizeof(*threads) + sizeof(pid_t)*items);
>
> If realloc fails and threads was allocated before... we leak memory.
ok.
>
> We don't need to use this if clause, realloc handles threads == NULL
> just fines and then works as malloc.
ok.
>
> Also I didn't use ctype altogether + that CSV parsing routine, we have
> strlist for that, it even will allow us to trow away duplicate pids/tids
> and also supports passing a file with a list of threads to monitor, for
> free.
Interesting. I did look at strlist before going with the string parsing.
It was not obvious to me how to use it for this case.
>
> Take a look at the modified patch below, if you're ok with it I can keep
> your autorship and put a [committer note: use strlist] or take autorship
> and state that it was based on a patch made by you, definetely your
> call. I'd go for the committer note.
I'm fine with either one. Patch below looks fine. If needed:
Acked-by: David Ahern <dsahern@gmail.com>
David
>
> - Arnaldo
>
> diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
> index ff9a66e..a5766b4 100644
> --- a/tools/perf/Documentation/perf-record.txt
> +++ b/tools/perf/Documentation/perf-record.txt
> @@ -52,11 +52,11 @@ OPTIONS
>
> -p::
> --pid=::
> - Record events on existing process ID.
> + Record events on existing process ID (comma separated list).
>
> -t::
> --tid=::
> - Record events on existing thread ID.
> + Record events on existing thread ID (comma separated list).
>
> -u::
> --uid=::
> diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
> index 8966b9a..2fa173b 100644
> --- a/tools/perf/Documentation/perf-stat.txt
> +++ b/tools/perf/Documentation/perf-stat.txt
> @@ -35,11 +35,11 @@ OPTIONS
> child tasks do not inherit counters
> -p::
> --pid=<pid>::
> - stat events on existing process id
> + stat events on existing process id (comma separated list)
>
> -t::
> --tid=<tid>::
> - stat events on existing thread id
> + stat events on existing thread id (comma separated list)
>
>
> -a::
> diff --git a/tools/perf/Documentation/perf-top.txt b/tools/perf/Documentation/perf-top.txt
> index ab1454e..4a5680c 100644
> --- a/tools/perf/Documentation/perf-top.txt
> +++ b/tools/perf/Documentation/perf-top.txt
> @@ -72,11 +72,11 @@ Default is to monitor all CPUS.
>
> -p <pid>::
> --pid=<pid>::
> - Profile events on existing Process ID.
> + Profile events on existing Process ID (comma separated list).
>
> -t <tid>::
> --tid=<tid>::
> - Profile events on existing thread ID.
> + Profile events on existing thread ID (comma separated list).
>
> -u::
> --uid=::
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index d6d1c6c..08ed24b 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -645,8 +645,6 @@ static const char * const record_usage[] = {
> */
> static struct perf_record record = {
> .opts = {
> - .target_pid = -1,
> - .target_tid = -1,
> .mmap_pages = UINT_MAX,
> .user_freq = UINT_MAX,
> .user_interval = ULLONG_MAX,
> @@ -670,9 +668,9 @@ const struct option record_options[] = {
> parse_events_option),
> OPT_CALLBACK(0, "filter", &record.evlist, "filter",
> "event filter", parse_filter),
> - OPT_INTEGER('p', "pid", &record.opts.target_pid,
> + OPT_STRING('p', "pid", &record.opts.target_pid, "pid",
> "record events on existing process id"),
> - OPT_INTEGER('t', "tid", &record.opts.target_tid,
> + OPT_STRING('t', "tid", &record.opts.target_tid, "tid",
> "record events on existing thread id"),
> OPT_INTEGER('r', "realtime", &record.realtime_prio,
> "collect data with this RT SCHED_FIFO priority"),
> @@ -739,7 +737,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 == -1 && rec->opts.target_tid == -1 &&
> + if (!argc && !rec->opts.target_pid && !rec->opts.target_tid &&
> !rec->opts.system_wide && !rec->opts.cpu_list && !rec->uid_str)
> usage_with_options(record_usage, record_options);
>
> @@ -785,7 +783,7 @@ int cmd_record(int argc, const char **argv, const char *prefix __used)
> if (rec->uid_str != NULL && rec->opts.uid == UINT_MAX - 1)
> goto out_free_fd;
>
> - if (rec->opts.target_pid != -1)
> + if (rec->opts.target_pid)
> rec->opts.target_tid = rec->opts.target_pid;
>
> if (perf_evlist__create_maps(evsel_list, rec->opts.target_pid,
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index d14b37a..ea40e4e 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -182,8 +182,8 @@ static int run_count = 1;
> static bool no_inherit = false;
> static bool scale = true;
> static bool no_aggr = false;
> -static pid_t target_pid = -1;
> -static pid_t target_tid = -1;
> +static const char *target_pid;
> +static const char *target_tid;
> static pid_t child_pid = -1;
> static bool null_run = false;
> static int detailed_run = 0;
> @@ -296,7 +296,7 @@ static int create_perf_stat_counter(struct perf_evsel *evsel,
> if (system_wide)
> return perf_evsel__open_per_cpu(evsel, evsel_list->cpus,
> group, group_fd);
> - if (target_pid == -1 && target_tid == -1) {
> + if (!target_pid && !target_tid) {
> attr->disabled = 1;
> attr->enable_on_exec = 1;
> }
> @@ -446,7 +446,7 @@ static int run_perf_stat(int argc __used, const char **argv)
> exit(-1);
> }
>
> - if (target_tid == -1 && target_pid == -1 && !system_wide)
> + if (!target_tid && !target_pid && !system_wide)
> evsel_list->threads->map[0] = child_pid;
>
> /*
> @@ -968,14 +968,14 @@ static void print_stat(int argc, const char **argv)
> if (!csv_output) {
> fprintf(output, "\n");
> fprintf(output, " Performance counter stats for ");
> - if(target_pid == -1 && target_tid == -1) {
> + if (!target_pid && !target_tid) {
> fprintf(output, "\'%s", argv[0]);
> for (i = 1; i < argc; i++)
> fprintf(output, " %s", argv[i]);
> - } else if (target_pid != -1)
> - fprintf(output, "process id \'%d", target_pid);
> + } else if (target_pid)
> + fprintf(output, "process id \'%s", target_pid);
> else
> - fprintf(output, "thread id \'%d", target_tid);
> + fprintf(output, "thread id \'%s", target_tid);
>
> fprintf(output, "\'");
> if (run_count > 1)
> @@ -1049,10 +1049,10 @@ static const struct option options[] = {
> "event filter", parse_filter),
> OPT_BOOLEAN('i', "no-inherit", &no_inherit,
> "child tasks do not inherit counters"),
> - OPT_INTEGER('p', "pid", &target_pid,
> - "stat events on existing process id"),
> - OPT_INTEGER('t', "tid", &target_tid,
> - "stat events on existing thread id"),
> + OPT_STRING('p', "pid", &target_pid, "pid",
> + "stat events on existing process id"),
> + OPT_STRING('t', "tid", &target_tid, "tid",
> + "stat events on existing thread id"),
> OPT_BOOLEAN('a', "all-cpus", &system_wide,
> "system-wide collection from all CPUs"),
> OPT_BOOLEAN('g', "group", &group,
> @@ -1190,7 +1190,7 @@ 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 == -1 && target_tid == -1)
> + if (!argc && !target_pid && !target_tid)
> usage_with_options(stat_usage, options);
> if (run_count <= 0)
> usage_with_options(stat_usage, options);
> @@ -1206,10 +1206,11 @@ int cmd_stat(int argc, const char **argv, const char *prefix __used)
> if (add_default_attributes())
> goto out;
>
> - if (target_pid != -1)
> + if (target_pid)
> target_tid = target_pid;
>
> - evsel_list->threads = thread_map__new(target_pid, target_tid, UINT_MAX);
> + evsel_list->threads = thread_map__new_str(target_pid,
> + target_tid, UINT_MAX);
> if (evsel_list->threads == NULL) {
> pr_err("Problems finding threads of monitor\n");
> usage_with_options(stat_usage, options);
> diff --git a/tools/perf/builtin-test.c b/tools/perf/builtin-test.c
> index 70c4eb2..0f15195 100644
> --- a/tools/perf/builtin-test.c
> +++ b/tools/perf/builtin-test.c
> @@ -1010,8 +1010,6 @@ realloc:
> static int test__PERF_RECORD(void)
> {
> struct perf_record_opts opts = {
> - .target_pid = -1,
> - .target_tid = -1,
> .no_delay = true,
> .freq = 10,
> .mmap_pages = 256,
> diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> index d869b21..5ed62a8 100644
> --- a/tools/perf/builtin-top.c
> +++ b/tools/perf/builtin-top.c
> @@ -965,7 +965,7 @@ static int __cmd_top(struct perf_top *top)
> if (ret)
> goto out_delete;
>
> - if (top->target_tid != -1 || top->uid != UINT_MAX)
> + if (top->target_tid || top->uid != UINT_MAX)
> perf_event__synthesize_thread_map(&top->tool, top->evlist->threads,
> perf_event__process,
> &top->session->host_machine);
> @@ -1103,8 +1103,6 @@ int cmd_top(int argc, const char **argv, const char *prefix __used)
> struct perf_top top = {
> .count_filter = 5,
> .delay_secs = 2,
> - .target_pid = -1,
> - .target_tid = -1,
> .uid = UINT_MAX,
> .freq = 1000, /* 1 KHz */
> .sample_id_all_avail = true,
> @@ -1118,9 +1116,9 @@ int cmd_top(int argc, const char **argv, const char *prefix __used)
> parse_events_option),
> OPT_INTEGER('c', "count", &top.default_interval,
> "event period to sample"),
> - OPT_INTEGER('p', "pid", &top.target_pid,
> + OPT_STRING('p', "pid", &top.target_pid, "pid",
> "profile events on existing process id"),
> - OPT_INTEGER('t', "tid", &top.target_tid,
> + OPT_STRING('t', "tid", &top.target_tid, "tid",
> "profile events on existing thread id"),
> OPT_BOOLEAN('a', "all-cpus", &top.system_wide,
> "system-wide collection from all CPUs"),
> @@ -1210,19 +1208,19 @@ int cmd_top(int argc, const char **argv, const char *prefix __used)
> goto out_delete_evlist;
>
> /* CPU and PID are mutually exclusive */
> - if (top.target_tid > 0 && top.cpu_list) {
> + if (top.target_tid && top.cpu_list) {
> printf("WARNING: PID switch overriding CPU\n");
> sleep(1);
> top.cpu_list = NULL;
> }
>
> - if (top.target_pid != -1)
> + if (top.target_pid)
> top.target_tid = top.target_pid;
>
> if (perf_evlist__create_maps(top.evlist, top.target_pid,
> top.target_tid, top.uid, top.cpu_list) < 0)
> usage_with_options(top_usage, options);
> -
> +
> if (!top.evlist->nr_entries &&
> perf_evlist__add_default(top.evlist) < 0) {
> pr_err("Not enough memory for event selector list\n");
> diff --git a/tools/perf/perf.h b/tools/perf/perf.h
> index 92af168..deb17db 100644
> --- a/tools/perf/perf.h
> +++ b/tools/perf/perf.h
> @@ -186,8 +186,8 @@ extern const char perf_version_string[];
> void pthread__unblock_sigwinch(void);
>
> struct perf_record_opts {
> - pid_t target_pid;
> - pid_t target_tid;
> + const char *target_pid;
> + const char *target_tid;
> uid_t uid;
> bool call_graph;
> bool group;
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index a57a8cf..5c61dc5 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -593,15 +593,15 @@ int perf_evlist__mmap(struct perf_evlist *evlist, unsigned int pages,
> return perf_evlist__mmap_per_cpu(evlist, prot, mask);
> }
>
> -int perf_evlist__create_maps(struct perf_evlist *evlist, pid_t target_pid,
> - pid_t target_tid, uid_t uid, const char *cpu_list)
> +int perf_evlist__create_maps(struct perf_evlist *evlist, const char *target_pid,
> + const char *target_tid, uid_t uid, const char *cpu_list)
> {
> - evlist->threads = thread_map__new(target_pid, target_tid, uid);
> + evlist->threads = thread_map__new_str(target_pid, target_tid, uid);
>
> if (evlist->threads == NULL)
> return -1;
>
> - if (uid != UINT_MAX || (cpu_list == NULL && target_tid != -1))
> + if (uid != UINT_MAX || (cpu_list == NULL && target_tid))
> evlist->cpus = cpu_map__dummy_new();
> else
> evlist->cpus = cpu_map__new(cpu_list);
> @@ -820,7 +820,7 @@ int perf_evlist__prepare_workload(struct perf_evlist *evlist,
> exit(-1);
> }
>
> - if (!opts->system_wide && opts->target_tid == -1 && opts->target_pid == -1)
> + if (!opts->system_wide && !opts->target_tid && !opts->target_pid)
> evlist->threads->map[0] = evlist->workload.pid;
>
> close(child_ready_pipe[1]);
> diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
> index 1b4282b..21f1c9e 100644
> --- a/tools/perf/util/evlist.h
> +++ b/tools/perf/util/evlist.h
> @@ -106,8 +106,8 @@ static inline void perf_evlist__set_maps(struct perf_evlist *evlist,
> evlist->threads = threads;
> }
>
> -int perf_evlist__create_maps(struct perf_evlist *evlist, pid_t target_pid,
> - pid_t tid, uid_t uid, const char *cpu_list);
> +int perf_evlist__create_maps(struct perf_evlist *evlist, const char *target_pid,
> + const char *tid, uid_t uid, const char *cpu_list);
> void perf_evlist__delete_maps(struct perf_evlist *evlist);
> int perf_evlist__set_filters(struct perf_evlist *evlist);
>
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index 9a11f9e..f910f50 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -130,7 +130,7 @@ void perf_evsel__config(struct perf_evsel *evsel, struct perf_record_opts *opts)
> attr->mmap = track;
> attr->comm = track;
>
> - if (opts->target_pid == -1 && opts->target_tid == -1 && !opts->system_wide) {
> + if (!opts->target_pid && !opts->target_tid && !opts->system_wide) {
> attr->disabled = 1;
> attr->enable_on_exec = 1;
> }
> diff --git a/tools/perf/util/setup.py b/tools/perf/util/setup.py
> index 36d4c56..4f85b8f 100644
> --- a/tools/perf/util/setup.py
> +++ b/tools/perf/util/setup.py
> @@ -28,7 +28,8 @@ perf = Extension('perf',
> sources = ['util/python.c', 'util/ctype.c', 'util/evlist.c',
> 'util/evsel.c', 'util/cpumap.c', 'util/thread_map.c',
> 'util/util.c', 'util/xyarray.c', 'util/cgroup.c',
> - 'util/debugfs.c'],
> + 'util/debugfs.c', '../../lib/rbtree.c',
> + 'util/strlist.c'],
> include_dirs = ['util/include'],
> extra_compile_args = cflags,
> )
> diff --git a/tools/perf/util/thread_map.c b/tools/perf/util/thread_map.c
> index 3d4b6c5..884f7e2 100644
> --- a/tools/perf/util/thread_map.c
> +++ b/tools/perf/util/thread_map.c
> @@ -6,6 +6,8 @@
> #include <sys/types.h>
> #include <sys/stat.h>
> #include <unistd.h>
> +#include "strlist.h"
> +#include <string.h>
> #include "thread_map.h"
>
> /* Skip "." and ".." directories */
> @@ -152,6 +154,132 @@ struct thread_map *thread_map__new(pid_t pid, pid_t tid, uid_t uid)
> return thread_map__new_by_tid(tid);
> }
>
> +static struct thread_map *thread_map__new_by_pid_str(const char *pid_str)
> +{
> + struct thread_map *threads = NULL, *nt;
> + char name[256];
> + int items, total_tasks = 0;
> + struct dirent **namelist = NULL;
> + int i, j = 0;
> + pid_t pid, prev_pid = LONG_MAX;
> + char *end_ptr;
> + struct str_node *pos;
> + struct strlist *slist = strlist__new(false, pid_str);
> +
> + if (!slist)
> + return NULL;
> +
> + strlist__for_each(pos, slist) {
> + pid = strtol(pos->s, &end_ptr, 10);
> +
> + if (pid == LONG_MIN || pid == LONG_MAX ||
> + (*end_ptr != '\0' && *end_ptr != ','))
> + goto out_free_threads;
> +
> + if (pid == prev_pid)
> + continue;
> +
> + sprintf(name, "/proc/%d/task", pid);
> + items = scandir(name, &namelist, filter, NULL);
> + if (items <= 0)
> + break;
> +
> + total_tasks += items;
> + nt = realloc(threads, (sizeof(*threads) +
> + sizeof(pid_t) * total_tasks));
> + if (nt == NULL)
> + goto out_free_threads;
> +
> + threads = nt;
> +
> + if (threads) {
> + for (i = 0; i < items; i++)
> + threads->map[j++] = atoi(namelist[i]->d_name);
> + threads->nr = total_tasks;
> + }
> +
> + for (i = 0; i < items; i++)
> + free(namelist[i]);
> + free(namelist);
> +
> + if (!threads)
> + break;
> + }
> +
> +out:
> + strlist__delete(slist);
> + return threads;
> +
> +out_free_threads:
> + free(threads);
> + threads = NULL;
> + goto out;
> +}
> +
> +static struct thread_map *thread_map__new_by_tid_str(const char *tid_str)
> +{
> + struct thread_map *threads = NULL, *nt;
> + int ntasks = 0;
> + pid_t tid, prev_tid = LONG_MAX;
> + char *end_ptr;
> + struct str_node *pos;
> + struct strlist *slist;
> +
> + /* perf-stat expects threads to be generated even if tid not given */
> + if (!tid_str) {
> + threads = malloc(sizeof(*threads) + sizeof(pid_t));
> + if (threads != NULL) {
> + threads->map[1] = -1;
> + threads->nr = 1;
> + }
> + return threads;
> + }
> +
> + slist = strlist__new(false, tid_str);
> + if (!slist)
> + return NULL;
> +
> + strlist__for_each(pos, slist) {
> + tid = strtol(pos->s, &end_ptr, 10);
> +
> + if (tid == LONG_MIN || tid == LONG_MAX ||
> + (*end_ptr != '\0' && *end_ptr != ','))
> + goto out_free_threads;
> +
> + if (tid == prev_tid)
> + continue;
> +
> + ntasks++;
> + nt = realloc(threads, sizeof(*threads) + sizeof(pid_t) * ntasks);
> +
> + if (nt == NULL)
> + goto out_free_threads;
> +
> + threads = nt;
> + threads->map[ntasks - 1] = tid;
> + threads->nr = ntasks;
> + }
> +out:
> + return threads;
> +
> +out_free_threads:
> + free(threads);
> + threads = NULL;
> + goto out;
> +}
> +
> +struct thread_map *thread_map__new_str(const char *pid, const char *tid,
> + uid_t uid)
> +{
> + if (pid)
> + return thread_map__new_by_pid_str(pid);
> +
> + if (!tid && uid != UINT_MAX)
> + return thread_map__new_by_uid(uid);
> +
> + return thread_map__new_by_tid_str(tid);
> +}
> +
> void thread_map__delete(struct thread_map *threads)
> {
> free(threads);
> diff --git a/tools/perf/util/thread_map.h b/tools/perf/util/thread_map.h
> index c75ddba..7da80f1 100644
> --- a/tools/perf/util/thread_map.h
> +++ b/tools/perf/util/thread_map.h
> @@ -13,6 +13,10 @@ struct thread_map *thread_map__new_by_pid(pid_t pid);
> struct thread_map *thread_map__new_by_tid(pid_t tid);
> struct thread_map *thread_map__new_by_uid(uid_t uid);
> struct thread_map *thread_map__new(pid_t pid, pid_t tid, uid_t uid);
> +
> +struct thread_map *thread_map__new_str(const char *pid,
> + const char *tid, uid_t uid);
> +
> void thread_map__delete(struct thread_map *threads);
>
> size_t thread_map__fprintf(struct thread_map *threads, FILE *fp);
> diff --git a/tools/perf/util/top.c b/tools/perf/util/top.c
> index e4370ca..09fe579 100644
> --- a/tools/perf/util/top.c
> +++ b/tools/perf/util/top.c
> @@ -69,11 +69,11 @@ size_t perf_top__header_snprintf(struct perf_top *top, char *bf, size_t size)
>
> ret += SNPRINTF(bf + ret, size - ret, "], ");
>
> - if (top->target_pid != -1)
> - ret += SNPRINTF(bf + ret, size - ret, " (target_pid: %d",
> + if (top->target_pid)
> + ret += SNPRINTF(bf + ret, size - ret, " (target_pid: %s",
> top->target_pid);
> - else if (top->target_tid != -1)
> - ret += SNPRINTF(bf + ret, size - ret, " (target_tid: %d",
> + else if (top->target_tid)
> + ret += SNPRINTF(bf + ret, size - ret, " (target_tid: %s",
> top->target_tid);
> else if (top->uid_str != NULL)
> ret += SNPRINTF(bf + ret, size - ret, " (uid: %s",
> @@ -85,7 +85,7 @@ size_t perf_top__header_snprintf(struct perf_top *top, char *bf, size_t size)
> ret += SNPRINTF(bf + ret, size - ret, ", CPU%s: %s)",
> top->evlist->cpus->nr > 1 ? "s" : "", top->cpu_list);
> else {
> - if (top->target_tid != -1)
> + if (top->target_tid)
> ret += SNPRINTF(bf + ret, size - ret, ")");
> else
> ret += SNPRINTF(bf + ret, size - ret, ", %d CPU%s)",
> diff --git a/tools/perf/util/top.h b/tools/perf/util/top.h
> index def3e53..49eb848 100644
> --- a/tools/perf/util/top.h
> +++ b/tools/perf/util/top.h
> @@ -23,7 +23,7 @@ struct perf_top {
> u64 guest_us_samples, guest_kernel_samples;
> int print_entries, count_filter, delay_secs;
> int freq;
> - pid_t target_pid, target_tid;
> + const char *target_pid, *target_tid;
> uid_t uid;
> bool hide_kernel_symbols, hide_user_symbols, zero;
> bool system_wide;
> diff --git a/tools/perf/util/usage.c b/tools/perf/util/usage.c
> index d0c0139..52bb07c 100644
> --- a/tools/perf/util/usage.c
> +++ b/tools/perf/util/usage.c
> @@ -83,7 +83,7 @@ void warning(const char *warn, ...)
> va_end(params);
> }
>
> -uid_t parse_target_uid(const char *str, pid_t tid, pid_t pid)
> +uid_t parse_target_uid(const char *str, const char *tid, const char *pid)
> {
> struct passwd pwd, *result;
> char buf[1024];
> @@ -91,8 +91,8 @@ uid_t parse_target_uid(const char *str, pid_t tid, pid_t pid)
> if (str == NULL)
> return UINT_MAX;
>
> - /* CPU and PID are mutually exclusive */
> - if (tid > 0 || pid > 0) {
> + /* UID and PID are mutually exclusive */
> + if (tid || pid) {
> ui__warning("PID/TID switch overriding UID\n");
> sleep(1);
> return UINT_MAX;
> diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
> index 232d17e..7917b09 100644
> --- a/tools/perf/util/util.h
> +++ b/tools/perf/util/util.h
> @@ -245,7 +245,7 @@ struct perf_event_attr;
>
> void event_attr_init(struct perf_event_attr *attr);
>
> -uid_t parse_target_uid(const char *str, pid_t tid, pid_t pid);
> +uid_t parse_target_uid(const char *str, const char *tid, const char *pid);
>
> #define _STR(x) #x
> #define STR(x) _STR(x)
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] perf tools: Allow multiple threads or processes in record, stat, top
2012-02-10 19:32 ` David Ahern
@ 2012-02-10 19:34 ` Arnaldo Carvalho de Melo
2012-02-10 19:46 ` David Ahern
0 siblings, 1 reply; 22+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-02-10 19:34 UTC (permalink / raw)
To: David Ahern; +Cc: linux-kernel, mingo, peterz, fweisbec, paulus, tglx
Em Fri, Feb 10, 2012 at 12:32:09PM -0700, David Ahern escreveu:
>
>
> On 02/10/2012 12:24 PM, Arnaldo Carvalho de Melo wrote:
> > Em Wed, Feb 08, 2012 at 09:32:52AM -0700, David Ahern escreveu:
> >> Allow a user to collect events for multiple threads or processes
> >> using a comma separated list.
> >>
> >> e.g., collect data on a VM and its vhost thread:
> >> perf top -p 21483,21485
> >> perf stat -p 21483,21485 -ddd
> >> perf record -p 21483,21485
> >>
> >> or monitoring vcpu threads
> >> perf top -t 21488,21489
> >> perf stat -t 21488,21489 -ddd
> >> perf record -t 21488,21489
> >
> > I found some problems below:
> >
> >> diff --git a/tools/perf/util/thread_map.c b/tools/perf/util/thread_map.c
> >> index 3d4b6c5..e793c16 100644
> >> --- a/tools/perf/util/thread_map.c
> >> +++ b/tools/perf/util/thread_map.c
> >> +static struct thread_map *thread_map__new_by_pid_str(const char *pid_str)
> >> +{
> >> + struct thread_map *threads = NULL;
> > <SNIP>
> >> + if (threads)
> >> + threads = realloc(threads,
> >> + sizeof(*threads) + sizeof(pid_t)*total_tasks);
> >> + else
> >> + threads = malloc(sizeof(*threads) + sizeof(pid_t)*items);
> >
> > If realloc fails and threads was allocated before... we leak memory.
>
> ok.
>
> >
> > We don't need to use this if clause, realloc handles threads == NULL
> > just fines and then works as malloc.
>
> ok.
>
> >
> > Also I didn't use ctype altogether + that CSV parsing routine, we have
> > strlist for that, it even will allow us to trow away duplicate pids/tids
> > and also supports passing a file with a list of threads to monitor, for
> > free.
>
> Interesting. I did look at strlist before going with the string parsing.
> It was not obvious to me how to use it for this case.
>
> >
> > Take a look at the modified patch below, if you're ok with it I can keep
> > your autorship and put a [committer note: use strlist] or take autorship
> > and state that it was based on a patch made by you, definetely your
> > call. I'd go for the committer note.
>
> I'm fine with either one. Patch below looks fine. If needed:
>
> Acked-by: David Ahern <dsahern@gmail.com>
I assume you tested it in a few scenarios (I know I did, but hey, more
testing is always good) and that I can add another stamp, a Tested-by:
ya, right?
- Arnaldo
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] perf tools: Allow multiple threads or processes in record, stat, top
2012-02-10 19:34 ` Arnaldo Carvalho de Melo
@ 2012-02-10 19:46 ` David Ahern
2012-02-10 19:58 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 22+ messages in thread
From: David Ahern @ 2012-02-10 19:46 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: linux-kernel, mingo, peterz, fweisbec, paulus, tglx
On 02/10/2012 12:34 PM, Arnaldo Carvalho de Melo wrote:
> Em Fri, Feb 10, 2012 at 12:32:09PM -0700, David Ahern escreveu:
>>
>>
>> On 02/10/2012 12:24 PM, Arnaldo Carvalho de Melo wrote:
>>> Em Wed, Feb 08, 2012 at 09:32:52AM -0700, David Ahern escreveu:
>>>> Allow a user to collect events for multiple threads or processes
>>>> using a comma separated list.
>>>>
>>>> e.g., collect data on a VM and its vhost thread:
>>>> perf top -p 21483,21485
>>>> perf stat -p 21483,21485 -ddd
>>>> perf record -p 21483,21485
>>>>
>>>> or monitoring vcpu threads
>>>> perf top -t 21488,21489
>>>> perf stat -t 21488,21489 -ddd
>>>> perf record -t 21488,21489
>>>
>>> I found some problems below:
>>>
>>>> diff --git a/tools/perf/util/thread_map.c b/tools/perf/util/thread_map.c
>>>> index 3d4b6c5..e793c16 100644
>>>> --- a/tools/perf/util/thread_map.c
>>>> +++ b/tools/perf/util/thread_map.c
>>>> +static struct thread_map *thread_map__new_by_pid_str(const char *pid_str)
>>>> +{
>>>> + struct thread_map *threads = NULL;
>>> <SNIP>
>>>> + if (threads)
>>>> + threads = realloc(threads,
>>>> + sizeof(*threads) + sizeof(pid_t)*total_tasks);
>>>> + else
>>>> + threads = malloc(sizeof(*threads) + sizeof(pid_t)*items);
>>>
>>> If realloc fails and threads was allocated before... we leak memory.
>>
>> ok.
>>
>>>
>>> We don't need to use this if clause, realloc handles threads == NULL
>>> just fines and then works as malloc.
>>
>> ok.
>>
>>>
>>> Also I didn't use ctype altogether + that CSV parsing routine, we have
>>> strlist for that, it even will allow us to trow away duplicate pids/tids
>>> and also supports passing a file with a list of threads to monitor, for
>>> free.
>>
>> Interesting. I did look at strlist before going with the string parsing.
>> It was not obvious to me how to use it for this case.
>>
>>>
>>> Take a look at the modified patch below, if you're ok with it I can keep
>>> your autorship and put a [committer note: use strlist] or take autorship
>>> and state that it was based on a patch made by you, definetely your
>>> call. I'd go for the committer note.
>>
>> I'm fine with either one. Patch below looks fine. If needed:
>>
>> Acked-by: David Ahern <dsahern@gmail.com>
>
> I assume you tested it in a few scenarios (I know I did, but hey, more
> testing is always good) and that I can add another stamp, a Tested-by:
> ya, right?
>
> - Arnaldo
Hmmm... that was a radical idea. Build breaks on Fedora 16, x86_64:
util/thread_map.c: In function ‘thread_map__new_by_pid_str’:
util/thread_map.c:164:2: error: overflow in implicit constant conversion
[-Werror=overflow]
util/thread_map.c:175:3: error: comparison is always false due to
limited range of data type [-Werror=type-limits]
util/thread_map.c:175:3: error: comparison is always false due to
limited range of data type [-Werror=type-limits]
util/thread_map.c: In function ‘thread_map__new_by_tid_str’:
util/thread_map.c:223:2: error: overflow in implicit constant conversion
[-Werror=overflow]
util/thread_map.c:245:3: error: comparison is always false due to
limited range of data type [-Werror=type-limits]
util/thread_map.c:245:3: error: comparison is always false due to
limited range of data type [-Werror=type-limits]
cc1: all warnings being treated as errors
I think you want INT_MAX not LONG_MAX.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] perf tools: Allow multiple threads or processes in record, stat, top
2012-02-10 19:46 ` David Ahern
@ 2012-02-10 19:58 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 22+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-02-10 19:58 UTC (permalink / raw)
To: David Ahern; +Cc: linux-kernel, mingo, peterz, fweisbec, paulus, tglx
Em Fri, Feb 10, 2012 at 12:46:40PM -0700, David Ahern escreveu:
> On 02/10/2012 12:34 PM, Arnaldo Carvalho de Melo wrote:
> > I assume you tested it in a few scenarios (I know I did, but hey, more
> > testing is always good) and that I can add another stamp, a Tested-by:
> > ya, right?
> >
> > - Arnaldo
>
> Hmmm... that was a radical idea. Build breaks on Fedora 16, x86_64:
>
> util/thread_map.c: In function ‘thread_map__new_by_pid_str’:
> util/thread_map.c:164:2: error: overflow in implicit constant conversion
> [-Werror=overflow]
> util/thread_map.c:175:3: error: comparison is always false due to
> limited range of data type [-Werror=type-limits]
> util/thread_map.c:175:3: error: comparison is always false due to
> limited range of data type [-Werror=type-limits]
> util/thread_map.c: In function ‘thread_map__new_by_tid_str’:
> util/thread_map.c:223:2: error: overflow in implicit constant conversion
> [-Werror=overflow]
> util/thread_map.c:245:3: error: comparison is always false due to
> limited range of data type [-Werror=type-limits]
> util/thread_map.c:245:3: error: comparison is always false due to
> limited range of data type [-Werror=type-limits]
> cc1: all warnings being treated as errors
>
>
> I think you want INT_MAX not LONG_MAX.
yeah, I tested it only on 32-bit :-\
I'll re-read the strtol man page and rework this patch, thanks for
testing!
- Arnaldo
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] perf tools: Fix build dependency of perf python extension
2012-02-10 14:28 ` Arnaldo Carvalho de Melo
@ 2012-02-12 10:45 ` Namhyung Kim
2012-02-17 9:44 ` [tip:perf/core] " tip-bot for Namhyung Kim
0 siblings, 1 reply; 22+ messages in thread
From: Namhyung Kim @ 2012-02-12 10:45 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: David Ahern, Ingo Molnar, Peter Zijlstra, Paul Mackerras,
fweisbec, tglx, linux-kernel
The perf python extention (perf.so) file lacks its dependencies in the
Makefile so that it cannot be refreshed if one of source files it depends
is changed. Fix it by putting them in a separate file and processing it in
both of Makefile and setup.py.
Signed-off-by: Namhyung Kim <namhyung@gmail.com>
---
Hello, Arnaldo.
This is my attempt to fix the problem you said. I used a separate file to
track the dependency and it was used by both Makefile and setup.py. It
seemed work well for me. Please let me know if I missed something.
Thanks.
tools/perf/Makefile | 5 ++++-
tools/perf/util/python-ext-sources | 17 +++++++++++++++++
tools/perf/util/setup.py | 8 ++++----
3 files changed, 25 insertions(+), 5 deletions(-)
create mode 100644 tools/perf/util/python-ext-sources
diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 64df5de12ca8..8359fa140d2d 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -183,7 +183,10 @@ SCRIPT_SH += perf-archive.sh
grep-libs = $(filter -l%,$(1))
strip-libs = $(filter-out -l%,$(1))
-$(OUTPUT)python/perf.so: $(PYRF_OBJS)
+PYTHON_EXT_SRCS := $(shell grep -v ^\# util/python-ext-sources)
+PYTHON_EXT_DEPS := util/python-ext-sources util/setup.py
+
+$(OUTPUT)python/perf.so: $(PYRF_OBJS) $(PYTHON_EXT_SRCS) $(PYTHON_EXT_DEPS)
$(QUIET_GEN)CFLAGS='$(BASIC_CFLAGS)' $(PYTHON_WORD) util/setup.py \
--quiet build_ext; \
mkdir -p $(OUTPUT)python && \
diff --git a/tools/perf/util/python-ext-sources b/tools/perf/util/python-ext-sources
new file mode 100644
index 000000000000..ff606f482a7c
--- /dev/null
+++ b/tools/perf/util/python-ext-sources
@@ -0,0 +1,17 @@
+#
+# List of files needed by perf python extention
+#
+# Each source file must be placed on its own line so that it can be
+# processed by Makefile and util/setup.py accordingly.
+#
+
+util/python.c
+util/ctype.c
+util/evlist.c
+util/evsel.c
+util/cpumap.c
+util/thread_map.c
+util/util.c
+util/xyarray.c
+util/cgroup.c
+util/debugfs.c
diff --git a/tools/perf/util/setup.py b/tools/perf/util/setup.py
index 36d4c5619575..d0f9f29cf181 100644
--- a/tools/perf/util/setup.py
+++ b/tools/perf/util/setup.py
@@ -24,11 +24,11 @@ cflags += getenv('CFLAGS', '').split()
build_lib = getenv('PYTHON_EXTBUILD_LIB')
build_tmp = getenv('PYTHON_EXTBUILD_TMP')
+ext_sources = [f.strip() for f in file('util/python-ext-sources')
+ if len(f.strip()) > 0 and f[0] != '#']
+
perf = Extension('perf',
- sources = ['util/python.c', 'util/ctype.c', 'util/evlist.c',
- 'util/evsel.c', 'util/cpumap.c', 'util/thread_map.c',
- 'util/util.c', 'util/xyarray.c', 'util/cgroup.c',
- 'util/debugfs.c'],
+ sources = ext_sources,
include_dirs = ['util/include'],
extra_compile_args = cflags,
)
--
1.7.8.2
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [tip:perf/core] perf tools: Fix build dependency of perf python extension
2012-02-12 10:45 ` [PATCH] perf tools: Fix build dependency of perf python extension Namhyung Kim
@ 2012-02-17 9:44 ` tip-bot for Namhyung Kim
0 siblings, 0 replies; 22+ messages in thread
From: tip-bot for Namhyung Kim @ 2012-02-17 9:44 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, paulus, acme, hpa, mingo, a.p.zijlstra, namhyung,
fweisbec, dsahern, tglx, mingo
Commit-ID: 6a5c13aff49ac9b3fea38d5f84b436718cb2780d
Gitweb: http://git.kernel.org/tip/6a5c13aff49ac9b3fea38d5f84b436718cb2780d
Author: Namhyung Kim <namhyung@gmail.com>
AuthorDate: Sun, 12 Feb 2012 19:45:24 +0900
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 13 Feb 2012 18:01:25 -0200
perf tools: Fix build dependency of perf python extension
The perf python extention (perf.so) file lacks its dependencies in the
Makefile so that it cannot be refreshed if one of source files it depends
is changed. Fix it by putting them in a separate file and processing it in
both of Makefile and setup.py.
Reported-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/1329043524-12470-1-git-send-email-namhyung@gmail.com
Signed-off-by: Namhyung Kim <namhyung@gmail.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/Makefile | 5 ++++-
tools/perf/util/python-ext-sources | 17 +++++++++++++++++
tools/perf/util/setup.py | 8 ++++----
3 files changed, 25 insertions(+), 5 deletions(-)
diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 64df5de..8359fa1 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -183,7 +183,10 @@ SCRIPT_SH += perf-archive.sh
grep-libs = $(filter -l%,$(1))
strip-libs = $(filter-out -l%,$(1))
-$(OUTPUT)python/perf.so: $(PYRF_OBJS)
+PYTHON_EXT_SRCS := $(shell grep -v ^\# util/python-ext-sources)
+PYTHON_EXT_DEPS := util/python-ext-sources util/setup.py
+
+$(OUTPUT)python/perf.so: $(PYRF_OBJS) $(PYTHON_EXT_SRCS) $(PYTHON_EXT_DEPS)
$(QUIET_GEN)CFLAGS='$(BASIC_CFLAGS)' $(PYTHON_WORD) util/setup.py \
--quiet build_ext; \
mkdir -p $(OUTPUT)python && \
diff --git a/tools/perf/util/python-ext-sources b/tools/perf/util/python-ext-sources
new file mode 100644
index 0000000..ff606f4
--- /dev/null
+++ b/tools/perf/util/python-ext-sources
@@ -0,0 +1,17 @@
+#
+# List of files needed by perf python extention
+#
+# Each source file must be placed on its own line so that it can be
+# processed by Makefile and util/setup.py accordingly.
+#
+
+util/python.c
+util/ctype.c
+util/evlist.c
+util/evsel.c
+util/cpumap.c
+util/thread_map.c
+util/util.c
+util/xyarray.c
+util/cgroup.c
+util/debugfs.c
diff --git a/tools/perf/util/setup.py b/tools/perf/util/setup.py
index 36d4c56..d0f9f29 100644
--- a/tools/perf/util/setup.py
+++ b/tools/perf/util/setup.py
@@ -24,11 +24,11 @@ cflags += getenv('CFLAGS', '').split()
build_lib = getenv('PYTHON_EXTBUILD_LIB')
build_tmp = getenv('PYTHON_EXTBUILD_TMP')
+ext_sources = [f.strip() for f in file('util/python-ext-sources')
+ if len(f.strip()) > 0 and f[0] != '#']
+
perf = Extension('perf',
- sources = ['util/python.c', 'util/ctype.c', 'util/evlist.c',
- 'util/evsel.c', 'util/cpumap.c', 'util/thread_map.c',
- 'util/util.c', 'util/xyarray.c', 'util/cgroup.c',
- 'util/debugfs.c'],
+ sources = ext_sources,
include_dirs = ['util/include'],
extra_compile_args = cflags,
)
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [tip:perf/core] perf tools: Allow multiple threads or processes in record, stat, top
2012-02-08 16:32 [PATCH] perf tools: Allow multiple threads or processes in record, stat, top David Ahern
2012-02-09 1:19 ` Namhyung Kim
2012-02-10 19:24 ` [PATCH] perf tools: Allow multiple threads or processes in record, stat, top Arnaldo Carvalho de Melo
@ 2012-02-17 9:46 ` tip-bot for David Ahern
2012-03-02 10:56 ` Ingo Molnar
2 siblings, 1 reply; 22+ messages in thread
From: tip-bot for David Ahern @ 2012-02-17 9:46 UTC (permalink / raw)
To: linux-tip-commits
Cc: acme, linux-kernel, paulus, hpa, mingo, peterz, fweisbec,
dsahern, tglx, mingo
Commit-ID: b52956c961be3a04182ae7b776623531601e0fb7
Gitweb: http://git.kernel.org/tip/b52956c961be3a04182ae7b776623531601e0fb7
Author: David Ahern <dsahern@gmail.com>
AuthorDate: Wed, 8 Feb 2012 09:32:52 -0700
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 13 Feb 2012 22:54:11 -0200
perf tools: Allow multiple threads or processes in record, stat, top
Allow a user to collect events for multiple threads or processes
using a comma separated list.
e.g., collect data on a VM and its vhost thread:
perf top -p 21483,21485
perf stat -p 21483,21485 -ddd
perf record -p 21483,21485
or monitoring vcpu threads
perf top -t 21488,21489
perf stat -t 21488,21489 -ddd
perf record -t 21488,21489
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/1328718772-16688-1-git-send-email-dsahern@gmail.com
Signed-off-by: David Ahern <dsahern@gmail.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/Documentation/perf-record.txt | 4 +-
tools/perf/Documentation/perf-stat.txt | 4 +-
tools/perf/Documentation/perf-top.txt | 4 +-
tools/perf/builtin-record.c | 10 +--
tools/perf/builtin-stat.c | 31 ++++----
tools/perf/builtin-test.c | 2 -
tools/perf/builtin-top.c | 12 +--
tools/perf/perf.h | 4 +-
tools/perf/util/evlist.c | 10 +-
tools/perf/util/evlist.h | 4 +-
tools/perf/util/evsel.c | 2 +-
tools/perf/util/python-ext-sources | 2 +
tools/perf/util/thread_map.c | 128 ++++++++++++++++++++++++++++++
tools/perf/util/thread_map.h | 4 +
tools/perf/util/top.c | 10 +-
tools/perf/util/top.h | 2 +-
tools/perf/util/usage.c | 6 +-
tools/perf/util/util.h | 2 +-
18 files changed, 185 insertions(+), 56 deletions(-)
diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index ff9a66e..a5766b4 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -52,11 +52,11 @@ OPTIONS
-p::
--pid=::
- Record events on existing process ID.
+ Record events on existing process ID (comma separated list).
-t::
--tid=::
- Record events on existing thread ID.
+ Record events on existing thread ID (comma separated list).
-u::
--uid=::
diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
index 8966b9a..2fa173b 100644
--- a/tools/perf/Documentation/perf-stat.txt
+++ b/tools/perf/Documentation/perf-stat.txt
@@ -35,11 +35,11 @@ OPTIONS
child tasks do not inherit counters
-p::
--pid=<pid>::
- stat events on existing process id
+ stat events on existing process id (comma separated list)
-t::
--tid=<tid>::
- stat events on existing thread id
+ stat events on existing thread id (comma separated list)
-a::
diff --git a/tools/perf/Documentation/perf-top.txt b/tools/perf/Documentation/perf-top.txt
index ab1454e..4a5680c 100644
--- a/tools/perf/Documentation/perf-top.txt
+++ b/tools/perf/Documentation/perf-top.txt
@@ -72,11 +72,11 @@ Default is to monitor all CPUS.
-p <pid>::
--pid=<pid>::
- Profile events on existing Process ID.
+ Profile events on existing Process ID (comma separated list).
-t <tid>::
--tid=<tid>::
- Profile events on existing thread ID.
+ Profile events on existing thread ID (comma separated list).
-u::
--uid=::
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index d6d1c6c..08ed24b 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -645,8 +645,6 @@ static const char * const record_usage[] = {
*/
static struct perf_record record = {
.opts = {
- .target_pid = -1,
- .target_tid = -1,
.mmap_pages = UINT_MAX,
.user_freq = UINT_MAX,
.user_interval = ULLONG_MAX,
@@ -670,9 +668,9 @@ const struct option record_options[] = {
parse_events_option),
OPT_CALLBACK(0, "filter", &record.evlist, "filter",
"event filter", parse_filter),
- OPT_INTEGER('p', "pid", &record.opts.target_pid,
+ OPT_STRING('p', "pid", &record.opts.target_pid, "pid",
"record events on existing process id"),
- OPT_INTEGER('t', "tid", &record.opts.target_tid,
+ OPT_STRING('t', "tid", &record.opts.target_tid, "tid",
"record events on existing thread id"),
OPT_INTEGER('r', "realtime", &record.realtime_prio,
"collect data with this RT SCHED_FIFO priority"),
@@ -739,7 +737,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 == -1 && rec->opts.target_tid == -1 &&
+ if (!argc && !rec->opts.target_pid && !rec->opts.target_tid &&
!rec->opts.system_wide && !rec->opts.cpu_list && !rec->uid_str)
usage_with_options(record_usage, record_options);
@@ -785,7 +783,7 @@ int cmd_record(int argc, const char **argv, const char *prefix __used)
if (rec->uid_str != NULL && rec->opts.uid == UINT_MAX - 1)
goto out_free_fd;
- if (rec->opts.target_pid != -1)
+ if (rec->opts.target_pid)
rec->opts.target_tid = rec->opts.target_pid;
if (perf_evlist__create_maps(evsel_list, rec->opts.target_pid,
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index d14b37a..ea40e4e 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -182,8 +182,8 @@ static int run_count = 1;
static bool no_inherit = false;
static bool scale = true;
static bool no_aggr = false;
-static pid_t target_pid = -1;
-static pid_t target_tid = -1;
+static const char *target_pid;
+static const char *target_tid;
static pid_t child_pid = -1;
static bool null_run = false;
static int detailed_run = 0;
@@ -296,7 +296,7 @@ static int create_perf_stat_counter(struct perf_evsel *evsel,
if (system_wide)
return perf_evsel__open_per_cpu(evsel, evsel_list->cpus,
group, group_fd);
- if (target_pid == -1 && target_tid == -1) {
+ if (!target_pid && !target_tid) {
attr->disabled = 1;
attr->enable_on_exec = 1;
}
@@ -446,7 +446,7 @@ static int run_perf_stat(int argc __used, const char **argv)
exit(-1);
}
- if (target_tid == -1 && target_pid == -1 && !system_wide)
+ if (!target_tid && !target_pid && !system_wide)
evsel_list->threads->map[0] = child_pid;
/*
@@ -968,14 +968,14 @@ static void print_stat(int argc, const char **argv)
if (!csv_output) {
fprintf(output, "\n");
fprintf(output, " Performance counter stats for ");
- if(target_pid == -1 && target_tid == -1) {
+ if (!target_pid && !target_tid) {
fprintf(output, "\'%s", argv[0]);
for (i = 1; i < argc; i++)
fprintf(output, " %s", argv[i]);
- } else if (target_pid != -1)
- fprintf(output, "process id \'%d", target_pid);
+ } else if (target_pid)
+ fprintf(output, "process id \'%s", target_pid);
else
- fprintf(output, "thread id \'%d", target_tid);
+ fprintf(output, "thread id \'%s", target_tid);
fprintf(output, "\'");
if (run_count > 1)
@@ -1049,10 +1049,10 @@ static const struct option options[] = {
"event filter", parse_filter),
OPT_BOOLEAN('i', "no-inherit", &no_inherit,
"child tasks do not inherit counters"),
- OPT_INTEGER('p', "pid", &target_pid,
- "stat events on existing process id"),
- OPT_INTEGER('t', "tid", &target_tid,
- "stat events on existing thread id"),
+ OPT_STRING('p', "pid", &target_pid, "pid",
+ "stat events on existing process id"),
+ OPT_STRING('t', "tid", &target_tid, "tid",
+ "stat events on existing thread id"),
OPT_BOOLEAN('a', "all-cpus", &system_wide,
"system-wide collection from all CPUs"),
OPT_BOOLEAN('g', "group", &group,
@@ -1190,7 +1190,7 @@ 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 == -1 && target_tid == -1)
+ if (!argc && !target_pid && !target_tid)
usage_with_options(stat_usage, options);
if (run_count <= 0)
usage_with_options(stat_usage, options);
@@ -1206,10 +1206,11 @@ int cmd_stat(int argc, const char **argv, const char *prefix __used)
if (add_default_attributes())
goto out;
- if (target_pid != -1)
+ if (target_pid)
target_tid = target_pid;
- evsel_list->threads = thread_map__new(target_pid, target_tid, UINT_MAX);
+ evsel_list->threads = thread_map__new_str(target_pid,
+ target_tid, UINT_MAX);
if (evsel_list->threads == NULL) {
pr_err("Problems finding threads of monitor\n");
usage_with_options(stat_usage, options);
diff --git a/tools/perf/builtin-test.c b/tools/perf/builtin-test.c
index 70c4eb2..0f15195 100644
--- a/tools/perf/builtin-test.c
+++ b/tools/perf/builtin-test.c
@@ -1010,8 +1010,6 @@ realloc:
static int test__PERF_RECORD(void)
{
struct perf_record_opts opts = {
- .target_pid = -1,
- .target_tid = -1,
.no_delay = true,
.freq = 10,
.mmap_pages = 256,
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index d869b21..94d55cb 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -965,7 +965,7 @@ static int __cmd_top(struct perf_top *top)
if (ret)
goto out_delete;
- if (top->target_tid != -1 || top->uid != UINT_MAX)
+ if (top->target_tid || top->uid != UINT_MAX)
perf_event__synthesize_thread_map(&top->tool, top->evlist->threads,
perf_event__process,
&top->session->host_machine);
@@ -1103,8 +1103,6 @@ int cmd_top(int argc, const char **argv, const char *prefix __used)
struct perf_top top = {
.count_filter = 5,
.delay_secs = 2,
- .target_pid = -1,
- .target_tid = -1,
.uid = UINT_MAX,
.freq = 1000, /* 1 KHz */
.sample_id_all_avail = true,
@@ -1118,9 +1116,9 @@ int cmd_top(int argc, const char **argv, const char *prefix __used)
parse_events_option),
OPT_INTEGER('c', "count", &top.default_interval,
"event period to sample"),
- OPT_INTEGER('p', "pid", &top.target_pid,
+ OPT_STRING('p', "pid", &top.target_pid, "pid",
"profile events on existing process id"),
- OPT_INTEGER('t', "tid", &top.target_tid,
+ OPT_STRING('t', "tid", &top.target_tid, "tid",
"profile events on existing thread id"),
OPT_BOOLEAN('a', "all-cpus", &top.system_wide,
"system-wide collection from all CPUs"),
@@ -1210,13 +1208,13 @@ int cmd_top(int argc, const char **argv, const char *prefix __used)
goto out_delete_evlist;
/* CPU and PID are mutually exclusive */
- if (top.target_tid > 0 && top.cpu_list) {
+ if (top.target_tid && top.cpu_list) {
printf("WARNING: PID switch overriding CPU\n");
sleep(1);
top.cpu_list = NULL;
}
- if (top.target_pid != -1)
+ if (top.target_pid)
top.target_tid = top.target_pid;
if (perf_evlist__create_maps(top.evlist, top.target_pid,
diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index 92af168..deb17db 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -186,8 +186,8 @@ extern const char perf_version_string[];
void pthread__unblock_sigwinch(void);
struct perf_record_opts {
- pid_t target_pid;
- pid_t target_tid;
+ const char *target_pid;
+ const char *target_tid;
uid_t uid;
bool call_graph;
bool group;
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index a57a8cf..5c61dc5 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -593,15 +593,15 @@ int perf_evlist__mmap(struct perf_evlist *evlist, unsigned int pages,
return perf_evlist__mmap_per_cpu(evlist, prot, mask);
}
-int perf_evlist__create_maps(struct perf_evlist *evlist, pid_t target_pid,
- pid_t target_tid, uid_t uid, const char *cpu_list)
+int perf_evlist__create_maps(struct perf_evlist *evlist, const char *target_pid,
+ const char *target_tid, uid_t uid, const char *cpu_list)
{
- evlist->threads = thread_map__new(target_pid, target_tid, uid);
+ evlist->threads = thread_map__new_str(target_pid, target_tid, uid);
if (evlist->threads == NULL)
return -1;
- if (uid != UINT_MAX || (cpu_list == NULL && target_tid != -1))
+ if (uid != UINT_MAX || (cpu_list == NULL && target_tid))
evlist->cpus = cpu_map__dummy_new();
else
evlist->cpus = cpu_map__new(cpu_list);
@@ -820,7 +820,7 @@ int perf_evlist__prepare_workload(struct perf_evlist *evlist,
exit(-1);
}
- if (!opts->system_wide && opts->target_tid == -1 && opts->target_pid == -1)
+ if (!opts->system_wide && !opts->target_tid && !opts->target_pid)
evlist->threads->map[0] = evlist->workload.pid;
close(child_ready_pipe[1]);
diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
index 1b4282b..21f1c9e 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -106,8 +106,8 @@ static inline void perf_evlist__set_maps(struct perf_evlist *evlist,
evlist->threads = threads;
}
-int perf_evlist__create_maps(struct perf_evlist *evlist, pid_t target_pid,
- pid_t tid, uid_t uid, const char *cpu_list);
+int perf_evlist__create_maps(struct perf_evlist *evlist, const char *target_pid,
+ const char *tid, uid_t uid, const char *cpu_list);
void perf_evlist__delete_maps(struct perf_evlist *evlist);
int perf_evlist__set_filters(struct perf_evlist *evlist);
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 9a11f9e..f910f50 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -130,7 +130,7 @@ void perf_evsel__config(struct perf_evsel *evsel, struct perf_record_opts *opts)
attr->mmap = track;
attr->comm = track;
- if (opts->target_pid == -1 && opts->target_tid == -1 && !opts->system_wide) {
+ if (!opts->target_pid && !opts->target_tid && !opts->system_wide) {
attr->disabled = 1;
attr->enable_on_exec = 1;
}
diff --git a/tools/perf/util/python-ext-sources b/tools/perf/util/python-ext-sources
index ff606f4..2884e67 100644
--- a/tools/perf/util/python-ext-sources
+++ b/tools/perf/util/python-ext-sources
@@ -15,3 +15,5 @@ util/util.c
util/xyarray.c
util/cgroup.c
util/debugfs.c
+util/strlist.c
+../../lib/rbtree.c
diff --git a/tools/perf/util/thread_map.c b/tools/perf/util/thread_map.c
index 3d4b6c5..e15983c 100644
--- a/tools/perf/util/thread_map.c
+++ b/tools/perf/util/thread_map.c
@@ -6,6 +6,8 @@
#include <sys/types.h>
#include <sys/stat.h>
#include <unistd.h>
+#include "strlist.h"
+#include <string.h>
#include "thread_map.h"
/* Skip "." and ".." directories */
@@ -152,6 +154,132 @@ struct thread_map *thread_map__new(pid_t pid, pid_t tid, uid_t uid)
return thread_map__new_by_tid(tid);
}
+static struct thread_map *thread_map__new_by_pid_str(const char *pid_str)
+{
+ struct thread_map *threads = NULL, *nt;
+ char name[256];
+ int items, total_tasks = 0;
+ struct dirent **namelist = NULL;
+ int i, j = 0;
+ pid_t pid, prev_pid = INT_MAX;
+ char *end_ptr;
+ struct str_node *pos;
+ struct strlist *slist = strlist__new(false, pid_str);
+
+ if (!slist)
+ return NULL;
+
+ strlist__for_each(pos, slist) {
+ pid = strtol(pos->s, &end_ptr, 10);
+
+ if (pid == INT_MIN || pid == INT_MAX ||
+ (*end_ptr != '\0' && *end_ptr != ','))
+ goto out_free_threads;
+
+ if (pid == prev_pid)
+ continue;
+
+ sprintf(name, "/proc/%d/task", pid);
+ items = scandir(name, &namelist, filter, NULL);
+ if (items <= 0)
+ goto out_free_threads;
+
+ total_tasks += items;
+ nt = realloc(threads, (sizeof(*threads) +
+ sizeof(pid_t) * total_tasks));
+ if (nt == NULL)
+ goto out_free_threads;
+
+ threads = nt;
+
+ if (threads) {
+ for (i = 0; i < items; i++)
+ threads->map[j++] = atoi(namelist[i]->d_name);
+ threads->nr = total_tasks;
+ }
+
+ for (i = 0; i < items; i++)
+ free(namelist[i]);
+ free(namelist);
+
+ if (!threads)
+ break;
+ }
+
+out:
+ strlist__delete(slist);
+ return threads;
+
+out_free_threads:
+ free(threads);
+ threads = NULL;
+ goto out;
+}
+
+static struct thread_map *thread_map__new_by_tid_str(const char *tid_str)
+{
+ struct thread_map *threads = NULL, *nt;
+ int ntasks = 0;
+ pid_t tid, prev_tid = INT_MAX;
+ char *end_ptr;
+ struct str_node *pos;
+ struct strlist *slist;
+
+ /* perf-stat expects threads to be generated even if tid not given */
+ if (!tid_str) {
+ threads = malloc(sizeof(*threads) + sizeof(pid_t));
+ if (threads != NULL) {
+ threads->map[1] = -1;
+ threads->nr = 1;
+ }
+ return threads;
+ }
+
+ slist = strlist__new(false, tid_str);
+ if (!slist)
+ return NULL;
+
+ strlist__for_each(pos, slist) {
+ tid = strtol(pos->s, &end_ptr, 10);
+
+ if (tid == INT_MIN || tid == INT_MAX ||
+ (*end_ptr != '\0' && *end_ptr != ','))
+ goto out_free_threads;
+
+ if (tid == prev_tid)
+ continue;
+
+ ntasks++;
+ nt = realloc(threads, sizeof(*threads) + sizeof(pid_t) * ntasks);
+
+ if (nt == NULL)
+ goto out_free_threads;
+
+ threads = nt;
+ threads->map[ntasks - 1] = tid;
+ threads->nr = ntasks;
+ }
+out:
+ return threads;
+
+out_free_threads:
+ free(threads);
+ threads = NULL;
+ goto out;
+}
+
+struct thread_map *thread_map__new_str(const char *pid, const char *tid,
+ uid_t uid)
+{
+ if (pid)
+ return thread_map__new_by_pid_str(pid);
+
+ if (!tid && uid != UINT_MAX)
+ return thread_map__new_by_uid(uid);
+
+ return thread_map__new_by_tid_str(tid);
+}
+
void thread_map__delete(struct thread_map *threads)
{
free(threads);
diff --git a/tools/perf/util/thread_map.h b/tools/perf/util/thread_map.h
index c75ddba..7da80f1 100644
--- a/tools/perf/util/thread_map.h
+++ b/tools/perf/util/thread_map.h
@@ -13,6 +13,10 @@ struct thread_map *thread_map__new_by_pid(pid_t pid);
struct thread_map *thread_map__new_by_tid(pid_t tid);
struct thread_map *thread_map__new_by_uid(uid_t uid);
struct thread_map *thread_map__new(pid_t pid, pid_t tid, uid_t uid);
+
+struct thread_map *thread_map__new_str(const char *pid,
+ const char *tid, uid_t uid);
+
void thread_map__delete(struct thread_map *threads);
size_t thread_map__fprintf(struct thread_map *threads, FILE *fp);
diff --git a/tools/perf/util/top.c b/tools/perf/util/top.c
index e4370ca..09fe579 100644
--- a/tools/perf/util/top.c
+++ b/tools/perf/util/top.c
@@ -69,11 +69,11 @@ size_t perf_top__header_snprintf(struct perf_top *top, char *bf, size_t size)
ret += SNPRINTF(bf + ret, size - ret, "], ");
- if (top->target_pid != -1)
- ret += SNPRINTF(bf + ret, size - ret, " (target_pid: %d",
+ if (top->target_pid)
+ ret += SNPRINTF(bf + ret, size - ret, " (target_pid: %s",
top->target_pid);
- else if (top->target_tid != -1)
- ret += SNPRINTF(bf + ret, size - ret, " (target_tid: %d",
+ else if (top->target_tid)
+ ret += SNPRINTF(bf + ret, size - ret, " (target_tid: %s",
top->target_tid);
else if (top->uid_str != NULL)
ret += SNPRINTF(bf + ret, size - ret, " (uid: %s",
@@ -85,7 +85,7 @@ size_t perf_top__header_snprintf(struct perf_top *top, char *bf, size_t size)
ret += SNPRINTF(bf + ret, size - ret, ", CPU%s: %s)",
top->evlist->cpus->nr > 1 ? "s" : "", top->cpu_list);
else {
- if (top->target_tid != -1)
+ if (top->target_tid)
ret += SNPRINTF(bf + ret, size - ret, ")");
else
ret += SNPRINTF(bf + ret, size - ret, ", %d CPU%s)",
diff --git a/tools/perf/util/top.h b/tools/perf/util/top.h
index def3e53..49eb848 100644
--- a/tools/perf/util/top.h
+++ b/tools/perf/util/top.h
@@ -23,7 +23,7 @@ struct perf_top {
u64 guest_us_samples, guest_kernel_samples;
int print_entries, count_filter, delay_secs;
int freq;
- pid_t target_pid, target_tid;
+ const char *target_pid, *target_tid;
uid_t uid;
bool hide_kernel_symbols, hide_user_symbols, zero;
bool system_wide;
diff --git a/tools/perf/util/usage.c b/tools/perf/util/usage.c
index d0c0139..52bb07c 100644
--- a/tools/perf/util/usage.c
+++ b/tools/perf/util/usage.c
@@ -83,7 +83,7 @@ void warning(const char *warn, ...)
va_end(params);
}
-uid_t parse_target_uid(const char *str, pid_t tid, pid_t pid)
+uid_t parse_target_uid(const char *str, const char *tid, const char *pid)
{
struct passwd pwd, *result;
char buf[1024];
@@ -91,8 +91,8 @@ uid_t parse_target_uid(const char *str, pid_t tid, pid_t pid)
if (str == NULL)
return UINT_MAX;
- /* CPU and PID are mutually exclusive */
- if (tid > 0 || pid > 0) {
+ /* UID and PID are mutually exclusive */
+ if (tid || pid) {
ui__warning("PID/TID switch overriding UID\n");
sleep(1);
return UINT_MAX;
diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index 232d17e..7917b09 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -245,7 +245,7 @@ struct perf_event_attr;
void event_attr_init(struct perf_event_attr *attr);
-uid_t parse_target_uid(const char *str, pid_t tid, pid_t pid);
+uid_t parse_target_uid(const char *str, const char *tid, const char *pid);
#define _STR(x) #x
#define STR(x) _STR(x)
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [tip:perf/core] perf tools: Allow multiple threads or processes in record, stat, top
2012-02-17 9:46 ` [tip:perf/core] " tip-bot for David Ahern
@ 2012-03-02 10:56 ` Ingo Molnar
2012-03-02 14:21 ` David Ahern
2012-03-02 14:52 ` David Ahern
0 siblings, 2 replies; 22+ messages in thread
From: Ingo Molnar @ 2012-03-02 10:56 UTC (permalink / raw)
To: mingo, hpa, paulus, linux-kernel, acme, fweisbec, peterz, dsahern, tglx
Cc: linux-tip-commits, Stephane Eranian
David,
* tip-bot for David Ahern <dsahern@gmail.com> wrote:
> Commit-ID: b52956c961be3a04182ae7b776623531601e0fb7
> Gitweb: http://git.kernel.org/tip/b52956c961be3a04182ae7b776623531601e0fb7
> Author: David Ahern <dsahern@gmail.com>
> AuthorDate: Wed, 8 Feb 2012 09:32:52 -0700
> Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
> CommitDate: Mon, 13 Feb 2012 22:54:11 -0200
>
> perf tools: Allow multiple threads or processes in record, stat, top
>
> Allow a user to collect events for multiple threads or processes
> using a comma separated list.
>
> e.g., collect data on a VM and its vhost thread:
> perf top -p 21483,21485
> perf stat -p 21483,21485 -ddd
> perf record -p 21483,21485
>
> or monitoring vcpu threads
> perf top -t 21488,21489
> perf stat -t 21488,21489 -ddd
> perf record -t 21488,21489
>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Link: http://lkml.kernel.org/r/1328718772-16688-1-git-send-email-dsahern@gmail.com
> Signed-off-by: David Ahern <dsahern@gmail.com>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> ---
> tools/perf/Documentation/perf-record.txt | 4 +-
> tools/perf/Documentation/perf-stat.txt | 4 +-
> tools/perf/Documentation/perf-top.txt | 4 +-
> tools/perf/builtin-record.c | 10 +--
> tools/perf/builtin-stat.c | 31 ++++----
> tools/perf/builtin-test.c | 2 -
> tools/perf/builtin-top.c | 12 +--
> tools/perf/perf.h | 4 +-
> tools/perf/util/evlist.c | 10 +-
> tools/perf/util/evlist.h | 4 +-
> tools/perf/util/evsel.c | 2 +-
> tools/perf/util/python-ext-sources | 2 +
> tools/perf/util/thread_map.c | 128 ++++++++++++++++++++++++++++++
> tools/perf/util/thread_map.h | 4 +
> tools/perf/util/top.c | 10 +-
> tools/perf/util/top.h | 2 +-
> tools/perf/util/usage.c | 6 +-
> tools/perf/util/util.h | 2 +-
> 18 files changed, 185 insertions(+), 56 deletions(-)
I have a bigger NUMA testbox where perf sampling (perf top, perf
record, etc.) stopped working (perf stat is fine), and I've
bisected it back to the above commit.
b52956c961be3a04182ae7b776623531601e0fb7 is the first bad commit
Checking out eca1c3e3f937 works fine, b52956c961b is broken,
repeatedly - so the bisection is reliable.
One symptom is no sampling records in the perf.data:
phoenix:~> perf record -a sleep 1
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.023 MB perf.data (~988 samples) ]
phoenix:~> perf report --stdio
Warning:
The perf.data file has no samples!
# ========
# captured on: Fri Mar 2 12:01:41 2012
# hostname : phoenix
# os release : 3.3.0-rc5+
# perf version : 3.3.rc5.1698.g68a63a.dirty
# arch : x86_64
# nrcpus online : 16
# nrcpus avail : 16
# cpudesc : Quad-Core AMD Opteron(tm) Processor 8356
# cpuid : AuthenticAMD,16,2,3
# total memory : 33012240 kB
# cmdline : /home/mingo/bin/perf record -a sleep 1
# event : name = cycles, type = 0, config = 0x0, config1 = 0x0, config2 = 0x0, excl_usr = 0, excl_k
# HEADER_CPU_TOPOLOGY info available, use -I to display
# HEADER_NUMA_TOPOLOGY info available, use -I to display
# ========
phoenix:~> perf --version
perf version 3.3.rc5.1698.g68a63a.dirty
phoenix:~>
The later fixes to b52956c9, such as:
6b1bee9035d4: perf tools: fix broken perf record -a mode
did not fix this system - it's still broken as of today's -tip.
Another system running the same kernel does not exhibit this
problem, so it's somehow specific to this system.
I have not looked deep into this, let me know if you need more
data.
Thanks,
Ingo
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [tip:perf/core] perf tools: Allow multiple threads or processes in record, stat, top
2012-03-02 10:56 ` Ingo Molnar
@ 2012-03-02 14:21 ` David Ahern
2012-03-02 14:52 ` David Ahern
1 sibling, 0 replies; 22+ messages in thread
From: David Ahern @ 2012-03-02 14:21 UTC (permalink / raw)
To: Ingo Molnar
Cc: mingo, hpa, paulus, linux-kernel, acme, fweisbec, peterz, tglx,
linux-tip-commits, Stephane Eranian
On 3/2/12 3:56 AM, Ingo Molnar wrote:
> I have a bigger NUMA testbox where perf sampling (perf top, perf
> record, etc.) stopped working (perf stat is fine), and I've
> bisected it back to the above commit.
>
> b52956c961be3a04182ae7b776623531601e0fb7 is the first bad commit
>
> Checking out eca1c3e3f937 works fine, b52956c961b is broken,
> repeatedly - so the bisection is reliable.
>
> One symptom is no sampling records in the perf.data:
>
> phoenix:~> perf record -a sleep 1
> [ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 0.023 MB perf.data (~988 samples) ]
> phoenix:~> perf report --stdio
> Warning:
> The perf.data file has no samples!
> # ========
> # captured on: Fri Mar 2 12:01:41 2012
> # hostname : phoenix
> # os release : 3.3.0-rc5+
> # perf version : 3.3.rc5.1698.g68a63a.dirty
> # arch : x86_64
> # nrcpus online : 16
> # nrcpus avail : 16
> # cpudesc : Quad-Core AMD Opteron(tm) Processor 8356
> # cpuid : AuthenticAMD,16,2,3
> # total memory : 33012240 kB
> # cmdline : /home/mingo/bin/perf record -a sleep 1
> # event : name = cycles, type = 0, config = 0x0, config1 = 0x0, config2 = 0x0, excl_usr = 0, excl_k
> # HEADER_CPU_TOPOLOGY info available, use -I to display
> # HEADER_NUMA_TOPOLOGY info available, use -I to display
> # ========
> phoenix:~> perf --version
> perf version 3.3.rc5.1698.g68a63a.dirty
> phoenix:~>
>
> The later fixes to b52956c9, such as:
>
> 6b1bee9035d4: perf tools: fix broken perf record -a mode
>
> did not fix this system - it's still broken as of today's -tip.
>
> Another system running the same kernel does not exhibit this
> problem, so it's somehow specific to this system.
>
> I have not looked deep into this, let me know if you need more
> data.
>
> Thanks,
>
> Ingo
Ok. I have a similar Intel based box - 2 socket, E5540 with 24G; this is
the box I typically use for testing. I'll take a look at this later
today and see what I can find.
David
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [tip:perf/core] perf tools: Allow multiple threads or processes in record, stat, top
2012-03-02 10:56 ` Ingo Molnar
2012-03-02 14:21 ` David Ahern
@ 2012-03-02 14:52 ` David Ahern
2012-03-03 7:42 ` Ingo Molnar
1 sibling, 1 reply; 22+ messages in thread
From: David Ahern @ 2012-03-02 14:52 UTC (permalink / raw)
To: Ingo Molnar
Cc: mingo, hpa, paulus, linux-kernel, acme, fweisbec, peterz, tglx,
linux-tip-commits, Stephane Eranian
That first cup of coffee is so important.
On 3/2/12 3:56 AM, Ingo Molnar wrote:
> phoenix:~> perf record -a sleep 1
> [ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 0.023 MB perf.data (~988 samples) ]
> phoenix:~> perf report --stdio
> Warning:
> The perf.data file has no samples!
> # ========
> # captured on: Fri Mar 2 12:01:41 2012
> # hostname : phoenix
> # os release : 3.3.0-rc5+
> # perf version : 3.3.rc5.1698.g68a63a.dirty
> # arch : x86_64
> # nrcpus online : 16
> # nrcpus avail : 16
> # cpudesc : Quad-Core AMD Opteron(tm) Processor 8356
> # cpuid : AuthenticAMD,16,2,3
Would you mind adding this patch:
https://lkml.org/lkml/2012/2/29/217
and see if it fixes your problem?
Quick test with an updated tree:
$ /tmp/pbuild/perf record -a sleep 1
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.046 MB perf.data (~1990 samples) ]
$ /tmp/pbuild/perf script
<samples dumped>
$ /tmp/pbuild/perf --version
perf version perf.core.for.mingo.36.g8eedce
This is your tip.git repository, tip/perf/core branch:
[remote "tip"]
url = git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git
fetch = +refs/heads/*:refs/remotes/tip/*
[branch "tip-perf-core"]
remote = tip
merge = refs/heads/perf/core
I did noticed that this branch only shows 3.3.0-rc2 as the latest kernel
version.
> # total memory : 33012240 kB
> # cmdline : /home/mingo/bin/perf record -a sleep 1
> # event : name = cycles, type = 0, config = 0x0, config1 = 0x0, config2 = 0x0, excl_usr = 0, excl_k
> # HEADER_CPU_TOPOLOGY info available, use -I to display
> # HEADER_NUMA_TOPOLOGY info available, use -I to display
> # ========
> phoenix:~> perf --version
> perf version 3.3.rc5.1698.g68a63a.dirty
> phoenix:~>
>
> The later fixes to b52956c9, such as:
>
> 6b1bee9035d4: perf tools: fix broken perf record -a mode
>
> did not fix this system - it's still broken as of today's -tip.
>
> Another system running the same kernel does not exhibit this
> problem, so it's somehow specific to this system.
Is the other system Intel based?
David
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [tip:perf/core] perf tools: Allow multiple threads or processes in record, stat, top
2012-03-02 14:52 ` David Ahern
@ 2012-03-03 7:42 ` Ingo Molnar
0 siblings, 0 replies; 22+ messages in thread
From: Ingo Molnar @ 2012-03-03 7:42 UTC (permalink / raw)
To: David Ahern
Cc: mingo, hpa, paulus, linux-kernel, acme, fweisbec, peterz, tglx,
linux-tip-commits, Stephane Eranian
* David Ahern <dsahern@gmail.com> wrote:
> That first cup of coffee is so important.
>
> On 3/2/12 3:56 AM, Ingo Molnar wrote:
> >phoenix:~> perf record -a sleep 1
> >[ perf record: Woken up 1 times to write data ]
> >[ perf record: Captured and wrote 0.023 MB perf.data (~988 samples) ]
> >phoenix:~> perf report --stdio
> >Warning:
> >The perf.data file has no samples!
> ># ========
> ># captured on: Fri Mar 2 12:01:41 2012
> ># hostname : phoenix
> ># os release : 3.3.0-rc5+
> ># perf version : 3.3.rc5.1698.g68a63a.dirty
> ># arch : x86_64
> ># nrcpus online : 16
> ># nrcpus avail : 16
> ># cpudesc : Quad-Core AMD Opteron(tm) Processor 8356
> ># cpuid : AuthenticAMD,16,2,3
>
> Would you mind adding this patch:
> https://lkml.org/lkml/2012/2/29/217
> and see if it fixes your problem?
That one appears to have done the trick - I committed it
yesterday, shortly after I did the testing - unknowing that it
would fix this bug as well :-)
Thanks,
Ingo
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2012-03-03 7:42 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-02-08 16:32 [PATCH] perf tools: Allow multiple threads or processes in record, stat, top David Ahern
2012-02-09 1:19 ` Namhyung Kim
2012-02-09 2:52 ` David Ahern
2012-02-09 5:36 ` Namhyung Kim
2012-02-09 6:04 ` David Ahern
2012-02-09 7:37 ` Ingo Molnar
2012-02-09 14:34 ` Arnaldo Carvalho de Melo
2012-02-09 14:44 ` Arnaldo Carvalho de Melo
2012-02-10 5:42 ` Namhyung Kim
2012-02-10 14:28 ` Arnaldo Carvalho de Melo
2012-02-12 10:45 ` [PATCH] perf tools: Fix build dependency of perf python extension Namhyung Kim
2012-02-17 9:44 ` [tip:perf/core] " tip-bot for Namhyung Kim
2012-02-10 19:24 ` [PATCH] perf tools: Allow multiple threads or processes in record, stat, top Arnaldo Carvalho de Melo
2012-02-10 19:32 ` David Ahern
2012-02-10 19:34 ` Arnaldo Carvalho de Melo
2012-02-10 19:46 ` David Ahern
2012-02-10 19:58 ` Arnaldo Carvalho de Melo
2012-02-17 9:46 ` [tip:perf/core] " tip-bot for David Ahern
2012-03-02 10:56 ` Ingo Molnar
2012-03-02 14:21 ` David Ahern
2012-03-02 14:52 ` David Ahern
2012-03-03 7:42 ` Ingo Molnar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).