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

[David Ahern <dsahern@gmail.com>]

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@google.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?

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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/