Re: [PATCH] perf: collect multiplexing timing information in perf record

[Stephane Eranian]

On Fri, Aug 26, 2011 at 6:32 PM, David Ahern <dsahern@xxxxxxxxx> 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@xxxxxxxxxx>
>>
>> ---
>> 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@xxxxxxxxxxxxxxx
>> 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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/