[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CABPqkBTDG_DcJTPpB-126HyuT8yR+w_JOssN=ix0rAP3tjzpuw@mail.gmail.com>
Date: Fri, 26 Aug 2011 20:02:57 +0200
From: Stephane Eranian <eranian@...gle.com>
To: David Ahern <dsahern@...il.com>
Cc: linux-kernel@...r.kernel.org, peterz@...radead.org, mingo@...e.hu,
acme@...hat.com, ming.m.lin@...el.com
Subject: Re: [PATCH] perf: collect multiplexing timing information in perf record
On Fri, Aug 26, 2011 at 6:32 PM, David Ahern <dsahern@...il.com> wrote:
>
>
> On 08/26/2011 09:22 AM, Stephane Eranian wrote:
>>
>> This patch modifies perf record to collect multiplexing timing
>> information (time_enabled, time_running) when the -R (raw) option
>> is used.
>>
>> This allows you to compare events captured on a single run and
>> compute multi-event metrics. Further it allows you to compare
>> measurements between different runs where the number of collected
>> events has changed and across different architectures where the
>> event count will surely change.
>>
>> To normalize an event (assuming a fixed period), you do:
>> total_events = n_samples * period * time_enabled / time_running
>>
>> You need the timing information to compensate for multiplexing
>> which may have happened.
>>
>> To add timing information in each sample, it is necessary to
>> set PERF_SAMPLE_READ as per the kernel API. That has the side
>> effect of also storing the value of the sampling event in each
>> sample. This patch modifies perf report to correctly parse
>> such samples.
>>
>> Signed-off-by: Stephane Eranian <eranian@...gle.com>
>>
>> ---
>> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
>> index 6b0519f..c54e580 100644
>> --- a/tools/perf/builtin-record.c
>> +++ b/tools/perf/builtin-record.c
>> @@ -166,7 +166,7 @@ static void config_attr(struct perf_evsel *evsel, struct perf_evlist *evlist)
>> PERF_FORMAT_TOTAL_TIME_RUNNING |
>> PERF_FORMAT_ID;
>>
>> - attr->sample_type |= PERF_SAMPLE_IP | PERF_SAMPLE_TID;
>> + attr->sample_type |= PERF_SAMPLE_IP | PERF_SAMPLE_TID;
>>
>> if (evlist->nr_entries > 1)
>> attr->sample_type |= PERF_SAMPLE_ID;
>> @@ -211,6 +211,7 @@ static void config_attr(struct perf_evsel *evsel, struct perf_evlist *evlist)
>> attr->sample_type |= PERF_SAMPLE_TIME;
>> attr->sample_type |= PERF_SAMPLE_RAW;
>> attr->sample_type |= PERF_SAMPLE_CPU;
>> + attr->sample_type |= PERF_SAMPLE_ID | PERF_SAMPLE_READ;
>> }
>>
>> if (nodelay) {
>> diff --git a/tools/perf/builtin-test.c b/tools/perf/builtin-test.c
>> index 55f4c76..997b7d7 100644
>> --- a/tools/perf/builtin-test.c
>> +++ b/tools/perf/builtin-test.c
>> @@ -561,7 +561,7 @@ static int test__basic_mmap(void)
>> }
>>
>> err = perf_event__parse_sample(event, attr.sample_type, sample_size,
>> - false, &sample);
>> + false, 0, &sample);
>> if (err) {
>> pr_err("Can't parse sample, err = %d\n", err);
>> goto out_munmap;
>> diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
>> index 1d7f664..ed193eb 100644
>> --- a/tools/perf/util/event.h
>> +++ b/tools/perf/util/event.h
>> @@ -186,6 +186,6 @@ const char *perf_event__name(unsigned int id);
>>
>> int perf_event__parse_sample(const union perf_event *event, u64 type,
>> int sample_size, bool sample_id_all,
>> - struct perf_sample *sample);
>> + u64 read_format, struct perf_sample *sample);
>>
>> #endif /* __PERF_RECORD_H */
>> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
>> index c12bd47..be0e30c 100644
>> --- a/tools/perf/util/evlist.c
>> +++ b/tools/perf/util/evlist.c
>> @@ -477,6 +477,14 @@ int perf_evlist__set_filters(struct perf_evlist *evlist)
>> return 0;
>> }
>>
>> +u64 perf_evlist__read_format(const struct perf_evlist *evlist)
>> +{
>> + struct perf_evsel *first;
>> +
>> + first = list_entry(evlist->entries.next, struct perf_evsel, node);
>> + return first->attr.read_format;
>> +}
>> +
>> bool perf_evlist__valid_sample_type(const struct perf_evlist *evlist)
>> {
>> struct perf_evsel *pos, *first;
>> diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
>> index ce85ae9..0c0d0ff 100644
>> --- a/tools/perf/util/evlist.h
>> +++ b/tools/perf/util/evlist.h
>> @@ -68,6 +68,7 @@ int perf_evlist__create_maps(struct perf_evlist *evlist, pid_t target_pid,
>> void perf_evlist__delete_maps(struct perf_evlist *evlist);
>> int perf_evlist__set_filters(struct perf_evlist *evlist);
>>
>> +u64 perf_evlist__read_format(const struct perf_evlist *evlist);
>> u64 perf_evlist__sample_type(const struct perf_evlist *evlist);
>> bool perf_evlist__sample_id_all(const const struct perf_evlist *evlist);
>>
>> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
>> index a03a36b..630a7da 100644
>> --- a/tools/perf/util/evsel.c
>> +++ b/tools/perf/util/evsel.c
>> @@ -340,9 +340,32 @@ static bool sample_overlap(const union perf_event *event,
>> return false;
>> }
>>
>> +static int
>> +sample_read2u64(const u64 *array, u64 fmt)
>> +{
>> + u64 nr = 1;
>> + int ret = 1; /* nr or value */
>> +
>> + if (fmt & PERF_FORMAT_TOTAL_TIME_ENABLED)
>> + ret++;
>> +
>> + if (fmt & PERF_FORMAT_TOTAL_TIME_RUNNING)
>> + ret++;
>> +
>> + if (fmt & PERF_FORMAT_GROUP) {
>> + nr = *(u64 *)array;
>> + ret += nr;
>> + }
>> +
>> + if (fmt & PERF_FORMAT_ID)
>> + ret += nr;
>
> Why not add
> struct read_format {
> u64 value;
> u64 time_enabled;
> u64 time_running;
> u64 id;
> };
>
> to perf_sample and save the data there?
>
I am not following you here.
Are you talking about the kernel API or perf tool internals?
The only way to have timing saved in each sample is via
PERF_SAMPLE_READ and a read_format which includes
time_enabled + time_running.
> David
>
>> +
>> + return ret;
>> +}
>> +
>> int perf_event__parse_sample(const union perf_event *event, u64 type,
>> int sample_size, bool sample_id_all,
>> - struct perf_sample *data)
>> + u64 read_format,struct perf_sample *data)
>> {
>> const u64 *array;
>>
>> @@ -405,10 +428,8 @@ int perf_event__parse_sample(const union perf_event *event, u64 type,
>> array++;
>> }
>>
>> - if (type & PERF_SAMPLE_READ) {
>> - fprintf(stderr, "PERF_SAMPLE_READ is unsuported for now\n");
>> - return -1;
>> - }
>> + if (type & PERF_SAMPLE_READ)
>> + array += sample_read2u64(array, read_format);
>>
>> if (type & PERF_SAMPLE_CALLCHAIN) {
>> if (sample_overlap(event, array, sizeof(data->callchain->nr)))
>> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
>> index 72458d9..25a4cf9 100644
>> --- a/tools/perf/util/session.c
>> +++ b/tools/perf/util/session.c
>> @@ -108,6 +108,7 @@ out:
>> void perf_session__update_sample_type(struct perf_session *self)
>> {
>> self->sample_type = perf_evlist__sample_type(self->evlist);
>> + self->read_format = perf_evlist__read_format(self->evlist);
>> self->sample_size = __perf_evsel__sample_size(self->sample_type);
>> self->sample_id_all = perf_evlist__sample_id_all(self->evlist);
>> perf_session__id_header_size(self);
>> diff --git a/tools/perf/util/session.h b/tools/perf/util/session.h
>> index 170601e..903fc1a 100644
>> --- a/tools/perf/util/session.h
>> +++ b/tools/perf/util/session.h
>> @@ -42,7 +42,8 @@ struct perf_session {
>> * perf.data file.
>> */
>> struct hists hists;
>> - u64 sample_type;
>> + u64 sample_type; /* identical for all events */
>> + u64 read_format; /* identical for all events */
>> int sample_size;
>> int fd;
>> bool fd_pipe;
>> @@ -162,7 +163,9 @@ static inline int perf_session__parse_sample(struct perf_session *session,
>> {
>> return perf_event__parse_sample(event, session->sample_type,
>> session->sample_size,
>> - session->sample_id_all, sample);
>> + session->sample_id_all,
>> + session->read_format,
>> + sample);
>> }
>>
>> struct perf_evsel *perf_session__find_first_evtype(struct perf_session *session,
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@...r.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at http://www.tux.org/lkml/
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists