LinuxLists.cc - [PATCH] perf tools: Allow multiple threads or processes in record, stat, top

[permalink] [raw]

Subject: Re: [PATCH] perf tools: Allow multiple threads or processes in record, stat, top

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 <[email protected]>

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)

2012-02-09 02:52:45

[permalink] [raw]

Subject: Re: [PATCH] perf tools: Allow multiple threads or processes in record, stat, top

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

2012-02-09 05:36:52

[permalink] [raw]

Subject: Re: [PATCH] perf tools: Allow multiple threads or processes in record, stat, top

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

2012-02-09 06:04:14

[permalink] [raw]

Subject: Re: [PATCH] perf tools: Allow multiple threads or processes in record, stat, top

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

2012-02-09 07:37:48

by Ingo Molnar

[permalink] [raw]

Subject: Re: [PATCH] perf tools: Allow multiple threads or processes in record, stat, top

* David Ahern <[email protected]> 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

2012-02-09 14:34:58

[permalink] [raw]

Subject: Re: [PATCH] perf tools: Allow multiple threads or processes in record, stat, top

Em Thu, Feb 09, 2012 at 08:37:27AM +0100, Ingo Molnar escreveu:
>
> * David Ahern <[email protected]> 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

2012-02-09 14:45:00

[permalink] [raw]

Subject: Re: [PATCH] perf tools: Allow multiple threads or processes in record, stat, top

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 <[email protected]> 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

2012-02-10 05:42:25

[permalink] [raw]

Subject: Re: [PATCH] perf tools: Allow multiple threads or processes in record, stat, top

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<[email protected]> 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

2012-02-10 14:28:30

[permalink] [raw]

Subject: Re: [PATCH] perf tools: Allow multiple threads or processes in record, stat, top

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

2012-02-10 19:24:13

[permalink] [raw]

Subject: Re: [PATCH] perf tools: Allow multiple threads or processes in record, stat, top

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)

2012-02-10 19:32:17

[permalink] [raw]

Subject: Re: [PATCH] perf tools: Allow multiple threads or processes in record, stat, top

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 <[email protected]>

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)

2012-02-10 19:34:37

[permalink] [raw]

Subject: Re: [PATCH] perf tools: Allow multiple threads or processes in record, stat, top

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 <[email protected]>

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

2012-02-10 19:46:46

[permalink] [raw]

Subject: Re: [PATCH] perf tools: Allow multiple threads or processes in record, stat, top

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 <[email protected]>
>
> 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.

2012-02-10 19:59:07

[permalink] [raw]

Subject: Re: [PATCH] perf tools: Allow multiple threads or processes in record, stat, top

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

2012-02-12 10:45:33

[permalink] [raw]

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

Signed-off-by: Namhyung Kim <[email protected]>
---
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

2012-02-17 09:45:19

[permalink] [raw]

Subject: [tip:perf/core] perf tools: Fix build dependency of perf python extension

Commit-ID: 6a5c13aff49ac9b3fea38d5f84b436718cb2780d
Gitweb: http://git.kernel.org/tip/6a5c13aff49ac9b3fea38d5f84b436718cb2780d
Author: Namhyung Kim <[email protected]>
AuthorDate: Sun, 12 Feb 2012 19:45:24 +0900
Committer: Arnaldo Carvalho de Melo <[email protected]>
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 <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Namhyung Kim <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
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,
)

2012-02-17 09:47:00