Re: [RFC PATCH 2/2] tools/perf: Make group_fd static and move itsplace in __perf_evsel__open()

From: Deng-Cheng Zhu
Date: Wed Oct 26 2011 - 02:26:28 EST

On 10/25/2011 11:23 PM, Arnaldo Carvalho de Melo wrote:
Em Mon, Oct 24, 2011 at 12:03:28PM -0200, Arnaldo Carvalho de Melo escreveu:
Em Mon, Oct 24, 2011 at 06:57:00PM +0800, Deng-Cheng Zhu escreveu:
__perf_evsel__open() is called per event, it does not work for all the
grouped events at one time. So, currently group_fd will alway be -1 for
the events in a group. This patch fixes it.

Signed-off-by: Deng-Cheng Zhu<dczhu@xxxxxxxx>
Cc: Peter Zijlstra<a.p.zijlstra@xxxxxxxxx>
Cc: Paul Mackerras<paulus@xxxxxxxxx>
Cc: Ingo Molnar<mingo@xxxxxxx>
Cc: Arnaldo Carvalho de Melo<acme@xxxxxxxxxxxxxxxxxx>
tools/perf/util/evsel.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index e389815..7bd0d9d 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -219,9 +219,8 @@ static int __perf_evsel__open(struct perf_evsel *evsel, struct cpu_map *cpus,

for (cpu = 0; cpu< cpus->nr; cpu++) {
- int group_fd = -1;
for (thread = 0; thread< threads->nr; thread++) {
+ static int group_fd = -1;

if (!evsel->cgrp)
pid = threads->map[thread];

Lets not do it that way, using statics for this is humm, ugly, IMHO.

Just pass an integer pointer that is a member of perf_evlist, I'll work
on a patch now.

Thanks for reporting the bug tho!

Can you try this patch?

I tested it with:

[root@emilia ~]# perf top -e cycles -e instructions --group


[root@emilia ~]# perf record -e cycles -e instructions --group -a sleep 2

Your patch does fix the group fd issue. But to get event grouping
workable, the event state fix is still needed. Please see the discussion

With _only_ your patch applied, I tested with the following commands on
MIPS 74K (4 counters available in total):

perf stat -g -e L1-dcache-load-misses,cycles,LLC-load-misses,iTLB-loads,instructions find / >/dev/null

I tried to group up to 5 events in the hope of seeing NOSPC error. But
the command didn't fail and output:

Performance counter stats for 'find /':

9300823 L1-dcache-load-misses
<not counted> cycles
<not counted> LLC-load-misses
<not counted> iTLB-loads
<not counted> instructions

8.463207591 seconds time elapsed

This is due to the event state check in validate_group() filtering out
the grouped events in OFF state. They are in OFF state because we are
running the command with the perf tool as opposed to attaching to an
existing task:


if (target_pid == -1 && target_tid == -1) {
attr->disabled = 1;
attr->enable_on_exec = 1;

I suppose X86 has this issue too -- collect_events() in validate_group()
won't do real work in the bottom half of the function.

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at