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

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

[Stephane Eranian]

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;
+
+	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,

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

[David Ahern]

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/

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

[Stephane Eranian]

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

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

[David Ahern]

On 08/26/2011 12:02 PM, Stephane Eranian wrote:

>>> +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.

I meant in that function you added above -- sample_read2u64. You are
moving the array pointer forward and essentially throwing away data that
was pushed to userspace. Why not add a struct and save that data for use
elsewhere in the builtin code? And the order of the data is a function
of whether PERF_FORMAT_GROUP is set (see perf_event.h file).

David