Re: [PATCH 1/4 -v2] perf: rework the whole read vs group stuff

From: stephane eranian
Date: Fri Aug 21 2009 - 10:47:52 EST

Next message: Frans Pop: "Re: [PATCH] acpi: thermal: display forced passive trip points in proc"
Previous message: Ingo Molnar: "Re: [PATCH v2 1/2] tracing: Move tracepoint callbacks into DEFINE"
Next in thread: Peter Zijlstra: "Re: [PATCH 1/4 -v2] perf: rework the whole read vs group stuff"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hi,

Well, I tried that and it brings more complexity than needed
especially with regards to
extracting the ID for events when you're doing grouping.

To extract the ID, one has to read out a struct as follows:
* { u64 nr;
* { u64 time_enabled; } && PERF_FORMAT_ENABLED
* { u64 time_running; } && PERF_FORMAT_RUNNING
* { u64 value;
* { u64 id; } && PERF_FORMAT_ID
* } cntr[nr];
* } && PERF_FORMAT_GROUP

Supposedly, you should have to do this only once per group. Reading
this stuff using the group leader should yield the values of all the other
events. This is not what I have observed. All events report the same
ID as the leader. Not clear why.

As I suggested in a previous message, I don't think all of this is necessary.
If a tool was able to pass the ID to associate with an event, then there
would be no need for a read(). Furthermore, it would make it easier to pick
an ID which suites the tool's data structure. For instance, if you create
4 events in a group, the ID could be 0,1,2,3 and would most likely map to
an index in the array used by the tool to manage the perf_counter structures.
That would also make it easier in the critical path in the signal handler. No
need to have a lookup table to map "random" ID to ID more relevant
for the tool. The ID does not need to be very wide. IDs are relevant only if
one uses group sampling. Therefore the ID needs to identify an event within
a group. Could use a reserved field in perf_counter_attr or add an ioctl() to
assign an ID.

On Thu, Aug 13, 2009 at 11:47 AM, Peter Zijlstra<a.p.zijlstra@xxxxxxxxx> wrote:
> Replace PERF_SAMPLE_GROUP with PERF_SAMPLE_READ and introduce
> PERF_FORMAT_GROUP to deal with group reads in a more generic way.
>
> This allows you to get group reads out of read() as well.
>
> Signed-off-by: Peter Zijlstra <a.p.zijlstra@xxxxxxxxx>
> ---
> Âinclude/linux/perf_counter.h | Â 47 +++++--
> Âkernel/perf_counter.c Â Â Â Â| Â274 +++++++++++++++++++++++++++++++------------
> Â2 files changed, 238 insertions(+), 83 deletions(-)
>
> Index: linux-2.6/include/linux/perf_counter.h
> ===================================================================
> --- linux-2.6.orig/include/linux/perf_counter.h
> +++ linux-2.6/include/linux/perf_counter.h
> @@ -115,7 +115,7 @@ enum perf_counter_sample_format {
> Â Â Â ÂPERF_SAMPLE_TID Â Â Â Â Â Â Â Â Â Â Â Â = 1U << 1,
> Â Â Â ÂPERF_SAMPLE_TIME Â Â Â Â Â Â Â Â Â Â Â Â= 1U << 2,
> Â Â Â ÂPERF_SAMPLE_ADDR Â Â Â Â Â Â Â Â Â Â Â Â= 1U << 3,
> - Â Â Â PERF_SAMPLE_GROUP Â Â Â Â Â Â Â Â Â Â Â = 1U << 4,
> + Â Â Â PERF_SAMPLE_READ Â Â Â Â Â Â Â Â Â Â Â Â= 1U << 4,
> Â Â Â ÂPERF_SAMPLE_CALLCHAIN Â Â Â Â Â Â Â Â Â = 1U << 5,
> Â Â Â ÂPERF_SAMPLE_ID Â Â Â Â Â Â Â Â Â Â Â Â Â= 1U << 6,
> Â Â Â ÂPERF_SAMPLE_CPU Â Â Â Â Â Â Â Â Â Â Â Â = 1U << 7,
> @@ -127,16 +127,32 @@ enum perf_counter_sample_format {
> Â};
>
> Â/*
> - * Bits that can be set in attr.read_format to request that
> - * reads on the counter should return the indicated quantities,
> - * in increasing order of bit value, after the counter value.
> + * The format of the data returned by read() on a perf counter fd,
> + * as specified by attr.read_format:
> + *
> + * struct read_format {
> + * Â Â { u64 Â Â Â Â Â value;
> + * Â Â Â { u64 Â Â Â Â time_enabled; } && PERF_FORMAT_ENABLED
> + * Â Â Â { u64 Â Â Â Â time_running; } && PERF_FORMAT_RUNNING
> + * Â Â Â { u64 Â Â Â Â id; Â Â Â Â Â } && PERF_FORMAT_ID
> + * Â Â } && !PERF_FORMAT_GROUP
> + *
> + * Â Â { u64 Â Â Â Â Â nr;
> + * Â Â Â { u64 Â Â Â Â time_enabled; } && PERF_FORMAT_ENABLED
> + * Â Â Â { u64 Â Â Â Â time_running; } && PERF_FORMAT_RUNNING
> + * Â Â Â { u64 Â Â Â Â value;
> + * Â Â Â Â { u64 Â Â Â id; Â Â Â Â Â } && PERF_FORMAT_ID
> + * Â Â Â } Â Â Â Â Â Â cntr[nr];
> + * Â Â } && PERF_FORMAT_GROUP
> + * };
> Â*/
> Âenum perf_counter_read_format {
> Â Â Â ÂPERF_FORMAT_TOTAL_TIME_ENABLED Â Â Â Â Â= 1U << 0,
> Â Â Â ÂPERF_FORMAT_TOTAL_TIME_RUNNING Â Â Â Â Â= 1U << 1,
> Â Â Â ÂPERF_FORMAT_ID Â Â Â Â Â Â Â Â Â Â Â Â Â= 1U << 2,
> + Â Â Â PERF_FORMAT_GROUP Â Â Â Â Â Â Â Â Â Â Â = 1U << 3,
>
> - Â Â Â PERF_FORMAT_MAX = 1U << 3, Â Â Â Â Â Â Â/* non-ABI */
> + Â Â Â PERF_FORMAT_MAX = 1U << 4, Â Â Â Â Â Â Â/* non-ABI */
> Â};
>
> Â#define PERF_ATTR_SIZE_VER0 Â Â64 Â Â Â/* sizeof first published struct */
> @@ -343,10 +359,8 @@ enum perf_event_type {
> Â Â Â Â * struct {
> Â Â Â Â * Â Â Âstruct perf_event_header Â Â Â Âheader;
> Â Â Â Â * Â Â Âu32 Â Â Â Â Â Â Â Â Â Â Â Â Â Â pid, tid;
> - Â Â Â Â* Â Â Âu64 Â Â Â Â Â Â Â Â Â Â Â Â Â Â value;
> - Â Â Â Â* Â Â Â{ u64 Â Â Â Â Â time_enabled; Â } && PERF_FORMAT_ENABLED
> - Â Â Â Â* Â Â Â{ u64 Â Â Â Â Â time_running; Â } && PERF_FORMAT_RUNNING
> - Â Â Â Â* Â Â Â{ u64 Â Â Â Â Â parent_id; Â Â Â} && PERF_FORMAT_ID
> + Â Â Â Â*
> + Â Â Â Â* Â Â Âstruct read_format Â Â Â Â Â Â Âvalues;
> Â Â Â Â * };
> Â Â Â Â */
> Â Â Â ÂPERF_EVENT_READ Â Â Â Â Â Â Â Â = 8,
> @@ -364,11 +378,22 @@ enum perf_event_type {
> Â Â Â Â * Â Â Â{ u32 Â Â Â Â Â Â Â Â Â cpu, res; } && PERF_SAMPLE_CPU
> Â Â Â Â * Â Â Â{ u64 Â Â Â Â Â Â Â Â Â period; Â } && PERF_SAMPLE_PERIOD
> Â Â Â Â *
> - Â Â Â Â* Â Â Â{ u64 Â Â Â Â Â Â Â Â Â nr;
> - Â Â Â Â* Â Â Â Â{ u64 id, val; } Â Â Âcnt[nr]; Â} && PERF_SAMPLE_GROUP
> + Â Â Â Â* Â Â Â{ struct read_format Â Âvalues; Â } && PERF_SAMPLE_READ
> Â Â Â Â *
> Â Â Â Â * Â Â Â{ u64 Â Â Â Â Â Â Â Â Â nr,
> Â Â Â Â * Â Â Â Âu64 Â Â Â Â Â Â Â Â Â ips[nr]; Â} && PERF_SAMPLE_CALLCHAIN
> + Â Â Â Â*
> + Â Â Â Â* Â Â Â#
> + Â Â Â Â* Â Â Â# The RAW record below is opaque data wrt the ABI
> + Â Â Â Â* Â Â Â#
> + Â Â Â Â* Â Â Â# That is, the ABI doesn't make any promises wrt to
> + Â Â Â Â* Â Â Â# the stability of its content, it may vary depending
> + Â Â Â Â* Â Â Â# on event, hardware, kernel version and phase of
> + Â Â Â Â* Â Â Â# the moon.
> + Â Â Â Â* Â Â Â#
> + Â Â Â Â* Â Â Â# In other words, PERF_SAMPLE_RAW contents are not an ABI.
> + Â Â Â Â* Â Â Â#
> + Â Â Â Â*
> Â Â Â Â * Â Â Â{ u32 Â Â Â Â Â Â Â Â Â size;
> Â Â Â Â * Â Â Â Âchar Â Â Â Â Â Â Â Â Âdata[size];}&& PERF_SAMPLE_RAW
> Â Â Â Â * };
> Index: linux-2.6/kernel/perf_counter.c
> ===================================================================
> --- linux-2.6.orig/kernel/perf_counter.c
> +++ linux-2.6/kernel/perf_counter.c
> @@ -1692,7 +1692,32 @@ static int perf_release(struct inode *in
> Â Â Â Âreturn 0;
> Â}
>
> -static u64 perf_counter_read_tree(struct perf_counter *counter)
> +static int perf_counter_read_size(struct perf_counter *counter)
> +{
> + Â Â Â int entry = sizeof(u64); /* value */
> + Â Â Â int size = 0;
> + Â Â Â int nr = 1;
> +
> + Â Â Â if (counter->attr.read_format & PERF_FORMAT_TOTAL_TIME_ENABLED)
> + Â Â Â Â Â Â Â size += sizeof(u64);
> +
> + Â Â Â if (counter->attr.read_format & PERF_FORMAT_TOTAL_TIME_RUNNING)
> + Â Â Â Â Â Â Â size += sizeof(u64);
> +
> + Â Â Â if (counter->attr.read_format & PERF_FORMAT_ID)
> + Â Â Â Â Â Â Â entry += sizeof(u64);
> +
> + Â Â Â if (counter->attr.read_format & PERF_FORMAT_GROUP) {
> + Â Â Â Â Â Â Â nr += counter->group_leader->nr_siblings;
> + Â Â Â Â Â Â Â size += sizeof(u64);
> + Â Â Â }
> +
> + Â Â Â size += entry * nr;
> +
> + Â Â Â return size;
> +}
> +
> +static u64 perf_counter_read_value(struct perf_counter *counter)
> Â{
> Â Â Â Âstruct perf_counter *child;
> Â Â Â Âu64 total = 0;
> @@ -1704,14 +1729,96 @@ static u64 perf_counter_read_tree(struct
> Â Â Â Âreturn total;
> Â}
>
> +static int perf_counter_read_entry(struct perf_counter *counter,
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âu64 read_format, char __user *buf)
> +{
> + Â Â Â int n = 0, count = 0;
> + Â Â Â u64 values[2];
> +
> + Â Â Â values[n++] = perf_counter_read_value(counter);
> + Â Â Â if (read_format & PERF_FORMAT_ID)
> + Â Â Â Â Â Â Â values[n++] = primary_counter_id(counter);
> +
> + Â Â Â count = n * sizeof(u64);
> +
> + Â Â Â if (copy_to_user(buf, values, count))
> + Â Â Â Â Â Â Â return -EFAULT;
> +
> + Â Â Â return count;
> +}
> +
> +static int perf_counter_read_group(struct perf_counter *counter,
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âu64 read_format, char __user *buf)
> +{
> + Â Â Â struct perf_counter *leader = counter->group_leader, *sub;
> + Â Â Â int n = 0, size = 0, err = -EFAULT;
> + Â Â Â u64 values[3];
> +
> + Â Â Â values[n++] = 1 + leader->nr_siblings;
> + Â Â Â if (read_format & PERF_FORMAT_TOTAL_TIME_ENABLED) {
> + Â Â Â Â Â Â Â values[n++] = leader->total_time_enabled +
> + Â Â Â Â Â Â Â Â Â Â Â atomic64_read(&leader->child_total_time_enabled);
> + Â Â Â }
> + Â Â Â if (read_format & PERF_FORMAT_TOTAL_TIME_RUNNING) {
> + Â Â Â Â Â Â Â values[n++] = leader->total_time_running +
> + Â Â Â Â Â Â Â Â Â Â Â atomic64_read(&leader->child_total_time_running);
> + Â Â Â }
> +
> + Â Â Â size = n * sizeof(u64);
> +
> + Â Â Â if (copy_to_user(buf, values, size))
> + Â Â Â Â Â Â Â return -EFAULT;
> +
> + Â Â Â err = perf_counter_read_entry(leader, read_format, buf + size);
> + Â Â Â if (err < 0)
> + Â Â Â Â Â Â Â return err;
> +
> + Â Â Â size += err;
> +
> + Â Â Â list_for_each_entry(sub, &leader->sibling_list, list_entry) {
> + Â Â Â Â Â Â Â err = perf_counter_read_entry(counter, read_format,
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â buf + size);
> + Â Â Â Â Â Â Â if (err < 0)
> + Â Â Â Â Â Â Â Â Â Â Â return err;
> +
> + Â Â Â Â Â Â Â size += err;
> + Â Â Â }
> +
> + Â Â Â return size;
> +}
> +
> +static int perf_counter_read_one(struct perf_counter *counter,
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âu64 read_format, char __user *buf)
> +{
> + Â Â Â u64 values[4];
> + Â Â Â int n = 0;
> +
> + Â Â Â values[n++] = perf_counter_read_value(counter);
> + Â Â Â if (read_format & PERF_FORMAT_TOTAL_TIME_ENABLED) {
> + Â Â Â Â Â Â Â values[n++] = counter->total_time_enabled +
> + Â Â Â Â Â Â Â Â Â Â Â atomic64_read(&counter->child_total_time_enabled);
> + Â Â Â }
> + Â Â Â if (read_format & PERF_FORMAT_TOTAL_TIME_RUNNING) {
> + Â Â Â Â Â Â Â values[n++] = counter->total_time_running +
> + Â Â Â Â Â Â Â Â Â Â Â atomic64_read(&counter->child_total_time_running);
> + Â Â Â }
> + Â Â Â if (read_format & PERF_FORMAT_ID)
> + Â Â Â Â Â Â Â values[n++] = primary_counter_id(counter);
> +
> + Â Â Â if (copy_to_user(buf, values, n * sizeof(u64)))
> + Â Â Â Â Â Â Â return -EFAULT;
> +
> + Â Â Â return n * sizeof(u64);
> +}
> +
> Â/*
> Â* Read the performance counter - simple non blocking version for now
> Â*/
> Âstatic ssize_t
> Âperf_read_hw(struct perf_counter *counter, char __user *buf, size_t count)
> Â{
> - Â Â Â u64 values[4];
> - Â Â Â int n;
> + Â Â Â u64 read_format = counter->attr.read_format;
> + Â Â Â int ret;
>
> Â Â Â Â/*
> Â Â Â Â * Return end-of-file for a read on a counter that is in
> @@ -1721,28 +1828,18 @@ perf_read_hw(struct perf_counter *counte
> Â Â Â Âif (counter->state == PERF_COUNTER_STATE_ERROR)
> Â Â Â Â Â Â Â Âreturn 0;
>
> + Â Â Â if (count < perf_counter_read_size(counter))
> + Â Â Â Â Â Â Â return -ENOSPC;
> +
> Â Â Â ÂWARN_ON_ONCE(counter->ctx->parent_ctx);
> Â Â Â Âmutex_lock(&counter->child_mutex);
> - Â Â Â values[0] = perf_counter_read_tree(counter);
> - Â Â Â n = 1;
> - Â Â Â if (counter->attr.read_format & PERF_FORMAT_TOTAL_TIME_ENABLED)
> - Â Â Â Â Â Â Â values[n++] = counter->total_time_enabled +
> - Â Â Â Â Â Â Â Â Â Â Â atomic64_read(&counter->child_total_time_enabled);
> - Â Â Â if (counter->attr.read_format & PERF_FORMAT_TOTAL_TIME_RUNNING)
> - Â Â Â Â Â Â Â values[n++] = counter->total_time_running +
> - Â Â Â Â Â Â Â Â Â Â Â atomic64_read(&counter->child_total_time_running);
> - Â Â Â if (counter->attr.read_format & PERF_FORMAT_ID)
> - Â Â Â Â Â Â Â values[n++] = primary_counter_id(counter);
> + Â Â Â if (read_format & PERF_FORMAT_GROUP)
> + Â Â Â Â Â Â Â ret = perf_counter_read_group(counter, read_format, buf);
> + Â Â Â else
> + Â Â Â Â Â Â Â ret = perf_counter_read_one(counter, read_format, buf);
> Â Â Â Âmutex_unlock(&counter->child_mutex);
>
> - Â Â Â if (count < n * sizeof(u64))
> - Â Â Â Â Â Â Â return -EINVAL;
> - Â Â Â count = n * sizeof(u64);
> -
> - Â Â Â if (copy_to_user(buf, values, count))
> - Â Â Â Â Â Â Â return -EFAULT;
> -
> - Â Â Â return count;
> + Â Â Â return ret;
> Â}
>
> Âstatic ssize_t
> @@ -2631,6 +2728,79 @@ static u32 perf_counter_tid(struct perf_
> Â Â Â Âreturn task_pid_nr_ns(p, counter->ns);
> Â}
>
> +static void perf_output_read_one(struct perf_output_handle *handle,
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âstruct perf_counter *counter)
> +{
> + Â Â Â u64 read_format = counter->attr.read_format;
> + Â Â Â u64 values[4];
> + Â Â Â int n = 0;
> +
> + Â Â Â values[n++] = atomic64_read(&counter->count);
> + Â Â Â if (read_format & PERF_FORMAT_TOTAL_TIME_ENABLED) {
> + Â Â Â Â Â Â Â values[n++] = counter->total_time_enabled +
> + Â Â Â Â Â Â Â Â Â Â Â atomic64_read(&counter->child_total_time_enabled);
> + Â Â Â }
> + Â Â Â if (read_format & PERF_FORMAT_TOTAL_TIME_RUNNING) {
> + Â Â Â Â Â Â Â values[n++] = counter->total_time_running +
> + Â Â Â Â Â Â Â Â Â Â Â atomic64_read(&counter->child_total_time_running);
> + Â Â Â }
> + Â Â Â if (read_format & PERF_FORMAT_ID)
> + Â Â Â Â Â Â Â values[n++] = primary_counter_id(counter);
> +
> + Â Â Â perf_output_copy(handle, values, n * sizeof(u64));
> +}
> +
> +/*
> + * XXX PERF_FORMAT_GROUP vs inherited counters seems difficult.
> + */
> +static void perf_output_read_group(struct perf_output_handle *handle,
> + Â Â Â Â Â Â Â Â Â Â Â Â Â struct perf_counter *counter)
> +{
> + Â Â Â struct perf_counter *leader = counter->group_leader, *sub;
> + Â Â Â u64 read_format = counter->attr.read_format;
> + Â Â Â u64 values[5];
> + Â Â Â int n = 0;
> +
> + Â Â Â values[n++] = 1 + leader->nr_siblings;
> +
> + Â Â Â if (read_format & PERF_FORMAT_TOTAL_TIME_ENABLED)
> + Â Â Â Â Â Â Â values[n++] = leader->total_time_enabled;
> +
> + Â Â Â if (read_format & PERF_FORMAT_TOTAL_TIME_RUNNING)
> + Â Â Â Â Â Â Â values[n++] = leader->total_time_running;
> +
> + Â Â Â if (leader != counter)
> + Â Â Â Â Â Â Â leader->pmu->read(leader);
> +
> + Â Â Â values[n++] = atomic64_read(&leader->count);
> + Â Â Â if (read_format & PERF_FORMAT_ID)
> + Â Â Â Â Â Â Â values[n++] = primary_counter_id(leader);
> +
> + Â Â Â perf_output_copy(handle, values, n * sizeof(u64));
> +
> + Â Â Â list_for_each_entry(sub, &leader->sibling_list, list_entry) {
> + Â Â Â Â Â Â Â n = 0;
> +
> + Â Â Â Â Â Â Â if (sub != counter)
> + Â Â Â Â Â Â Â Â Â Â Â sub->pmu->read(sub);
> +
> + Â Â Â Â Â Â Â values[n++] = atomic64_read(&sub->count);
> + Â Â Â Â Â Â Â if (read_format & PERF_FORMAT_ID)
> + Â Â Â Â Â Â Â Â Â Â Â values[n++] = primary_counter_id(sub);
> +
> + Â Â Â Â Â Â Â perf_output_copy(handle, values, n * sizeof(u64));
> + Â Â Â }
> +}
> +
> +static void perf_output_read(struct perf_output_handle *handle,
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Âstruct perf_counter *counter)
> +{
> + Â Â Â if (counter->attr.read_format & PERF_FORMAT_GROUP)
> + Â Â Â Â Â Â Â perf_output_read_group(handle, counter);
> + Â Â Â else
> + Â Â Â Â Â Â Â perf_output_read_one(handle, counter);
> +}
> +
> Âvoid perf_counter_output(struct perf_counter *counter, int nmi,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âstruct perf_sample_data *data)
> Â{
> @@ -2642,10 +2812,6 @@ void perf_counter_output(struct perf_cou
> Â Â Â Âstruct {
> Â Â Â Â Â Â Â Âu32 pid, tid;
> Â Â Â Â} tid_entry;
> - Â Â Â struct {
> - Â Â Â Â Â Â Â u64 id;
> - Â Â Â Â Â Â Â u64 counter;
> - Â Â Â } group_entry;
> Â Â Â Âstruct perf_callchain_entry *callchain = NULL;
> Â Â Â Âint callchain_size = 0;
> Â Â Â Âu64 time;
> @@ -2700,10 +2866,8 @@ void perf_counter_output(struct perf_cou
> Â Â Â Âif (sample_type & PERF_SAMPLE_PERIOD)
> Â Â Â Â Â Â Â Âheader.size += sizeof(u64);
>
> - Â Â Â if (sample_type & PERF_SAMPLE_GROUP) {
> - Â Â Â Â Â Â Â header.size += sizeof(u64) +
> - Â Â Â Â Â Â Â Â Â Â Â counter->nr_siblings * sizeof(group_entry);
> - Â Â Â }
> + Â Â Â if (sample_type & PERF_SAMPLE_READ)
> + Â Â Â Â Â Â Â header.size += perf_counter_read_size(counter);
>
> Â Â Â Âif (sample_type & PERF_SAMPLE_CALLCHAIN) {
> Â Â Â Â Â Â Â Âcallchain = perf_callchain(data->regs);
> @@ -2760,26 +2924,8 @@ void perf_counter_output(struct perf_cou
> Â Â Â Âif (sample_type & PERF_SAMPLE_PERIOD)
> Â Â Â Â Â Â Â Âperf_output_put(&handle, data->period);
>
> - Â Â Â /*
> - Â Â Â Â* XXX PERF_SAMPLE_GROUP vs inherited counters seems difficult.
> - Â Â Â Â*/
> - Â Â Â if (sample_type & PERF_SAMPLE_GROUP) {
> - Â Â Â Â Â Â Â struct perf_counter *leader, *sub;
> - Â Â Â Â Â Â Â u64 nr = counter->nr_siblings;
> -
> - Â Â Â Â Â Â Â perf_output_put(&handle, nr);
> -
> - Â Â Â Â Â Â Â leader = counter->group_leader;
> - Â Â Â Â Â Â Â list_for_each_entry(sub, &leader->sibling_list, list_entry) {
> - Â Â Â Â Â Â Â Â Â Â Â if (sub != counter)
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â sub->pmu->read(sub);
> -
> - Â Â Â Â Â Â Â Â Â Â Â group_entry.id = primary_counter_id(sub);
> - Â Â Â Â Â Â Â Â Â Â Â group_entry.counter = atomic64_read(&sub->count);
> -
> - Â Â Â Â Â Â Â Â Â Â Â perf_output_put(&handle, group_entry);
> - Â Â Â Â Â Â Â }
> - Â Â Â }
> + Â Â Â if (sample_type & PERF_SAMPLE_READ)
> + Â Â Â Â Â Â Â perf_output_read(&handle, counter);
>
> Â Â Â Âif (sample_type & PERF_SAMPLE_CALLCHAIN) {
> Â Â Â Â Â Â Â Âif (callchain)
> @@ -2818,8 +2964,6 @@ struct perf_read_event {
>
> Â Â Â Âu32 Â Â Â Â Â Â Â Â Â Â Â Â Â Â pid;
> Â Â Â Âu32 Â Â Â Â Â Â Â Â Â Â Â Â Â Â tid;
> - Â Â Â u64 Â Â Â Â Â Â Â Â Â Â Â Â Â Â value;
> - Â Â Â u64 Â Â Â Â Â Â Â Â Â Â Â Â Â Â format[3];
> Â};
>
> Âstatic void
> @@ -2831,34 +2975,20 @@ perf_counter_read_event(struct perf_coun
> Â Â Â Â Â Â Â Â.header = {
> Â Â Â Â Â Â Â Â Â Â Â Â.type = PERF_EVENT_READ,
> Â Â Â Â Â Â Â Â Â Â Â Â.misc = 0,
> - Â Â Â Â Â Â Â Â Â Â Â .size = sizeof(event) - sizeof(event.format),
> + Â Â Â Â Â Â Â Â Â Â Â .size = sizeof(event) + perf_counter_read_size(counter),
> Â Â Â Â Â Â Â Â},
> Â Â Â Â Â Â Â Â.pid = perf_counter_pid(counter, task),
> Â Â Â Â Â Â Â Â.tid = perf_counter_tid(counter, task),
> - Â Â Â Â Â Â Â .value = atomic64_read(&counter->count),
> Â Â Â Â};
> - Â Â Â int ret, i = 0;
> -
> - Â Â Â if (counter->attr.read_format & PERF_FORMAT_TOTAL_TIME_ENABLED) {
> - Â Â Â Â Â Â Â event.header.size += sizeof(u64);
> - Â Â Â Â Â Â Â event.format[i++] = counter->total_time_enabled;
> - Â Â Â }
> -
> - Â Â Â if (counter->attr.read_format & PERF_FORMAT_TOTAL_TIME_RUNNING) {
> - Â Â Â Â Â Â Â event.header.size += sizeof(u64);
> - Â Â Â Â Â Â Â event.format[i++] = counter->total_time_running;
> - Â Â Â }
> -
> - Â Â Â if (counter->attr.read_format & PERF_FORMAT_ID) {
> - Â Â Â Â Â Â Â event.header.size += sizeof(u64);
> - Â Â Â Â Â Â Â event.format[i++] = primary_counter_id(counter);
> - Â Â Â }
> + Â Â Â int ret;
>
> Â Â Â Âret = perf_output_begin(&handle, counter, event.header.size, 0, 0);
> Â Â Â Âif (ret)
> Â Â Â Â Â Â Â Âreturn;
>
> - Â Â Â perf_output_copy(&handle, &event, event.header.size);
> + Â Â Â perf_output_put(&handle, event);
> + Â Â Â perf_output_read(&handle, counter);
> +
> Â Â Â Âperf_output_end(&handle);
> Â}
>
> @@ -3929,9 +4059,9 @@ perf_counter_alloc(struct perf_counter_a
> Â Â Â Âatomic64_set(&hwc->period_left, hwc->sample_period);
>
> Â Â Â Â/*
> - Â Â Â Â* we currently do not support PERF_SAMPLE_GROUP on inherited counters
> + Â Â Â Â* we currently do not support PERF_FORMAT_GROUP on inherited counters
> Â Â Â Â */
> - Â Â Â if (attr->inherit && (attr->sample_type & PERF_SAMPLE_GROUP))
> + Â Â Â if (attr->inherit && (attr->read_format & PERF_FORMAT_GROUP))
> Â Â Â Â Â Â Â Âgoto done;
>
> Â Â Â Âswitch (attr->type) {
>
> --
>
>
èº{.nÇ+‰·Ÿ®‰†+%ŠËlzwm…ébëæìr¸›zX§»®w¥Š{ayºÊÚë,j¢f£¢·hš‹àz¹®w¥¢¸¢·¦j:+v‰¨ŠwèjØm¶Ÿÿ¾«‘êçzZ+ƒùšŽŠÝj"ú!¶iO•æ¬z·švØ^¶m§ÿðÃnÆàþY&—

Next message: Frans Pop: "Re: [PATCH] acpi: thermal: display forced passive trip points in proc"
Previous message: Ingo Molnar: "Re: [PATCH v2 1/2] tracing: Move tracepoint callbacks into DEFINE"
Next in thread: Peter Zijlstra: "Re: [PATCH 1/4 -v2] perf: rework the whole read vs group stuff"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]