>From 6bb91a9b3ca76ad0311d72989655a2b31ef2ef4b Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Mon, 11 Apr 2011 09:33:59 -0300 Subject: [PATCH 1/1] perf evsel: Fix use of inherit in non mmap case perf stat doesn't mmap and its perfectly fine for it to use task-bound counters with inheritance. So don't do any change in the perf_evsel__open parameters, leaving the syscall itself to validate this. When the mmap fails perf_evlist__mmap will just emit a warning if this is the failure reason. Reported-by: Peter Zijlstra Cc: Frederic Weisbecker Cc: Ingo Molnar Cc: Mike Galbraith Cc: Paul Mackerras Cc: Peter Zijlstra Cc: Stephane Eranian Cc: Tom Zanussi Link: http://lkml.kernel.org/n/tip-zkjs8vstfnq9yq9r2jkh8kz0@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-record.c | 3 +++ tools/perf/util/evlist.c | 14 ++++++++++---- tools/perf/util/evsel.c | 15 ++------------- 3 files changed, 15 insertions(+), 17 deletions(-) diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c index 17d1dcb..426f4f4 100644 --- a/tools/perf/builtin-record.c +++ b/tools/perf/builtin-record.c @@ -251,6 +251,9 @@ static void open_counters(struct perf_evlist *evlist) { struct perf_evsel *pos; + if (evlist->cpus->map[0] < 0) + no_inherit = true; + list_for_each_entry(pos, &evlist->entries, node) { struct perf_event_attr *attr = &pos->attr; /* diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c index d852cef..45da8d1 100644 --- a/tools/perf/util/evlist.c +++ b/tools/perf/util/evlist.c @@ -12,6 +12,7 @@ #include "evlist.h" #include "evsel.h" #include "util.h" +#include "debug.h" #include @@ -250,15 +251,19 @@ int perf_evlist__alloc_mmap(struct perf_evlist *evlist) return evlist->mmap != NULL ? 0 : -ENOMEM; } -static int __perf_evlist__mmap(struct perf_evlist *evlist, int cpu, int prot, - int mask, int fd) +static int __perf_evlist__mmap(struct perf_evlist *evlist, struct perf_evsel *evsel, + int cpu, int prot, int mask, int fd) { evlist->mmap[cpu].prev = 0; evlist->mmap[cpu].mask = mask; evlist->mmap[cpu].base = mmap(NULL, evlist->mmap_len, prot, MAP_SHARED, fd, 0); - if (evlist->mmap[cpu].base == MAP_FAILED) + if (evlist->mmap[cpu].base == MAP_FAILED) { + if (evlist->cpus->map[cpu] == -1 && evsel->attr.inherit) + ui__warning("Inherit is not allowed on per-task " + "events using mmap.\n"); return -1; + } perf_evlist__add_pollfd(evlist, fd); return 0; @@ -312,7 +317,8 @@ int perf_evlist__mmap(struct perf_evlist *evlist, int pages, bool overwrite) if (ioctl(fd, PERF_EVENT_IOC_SET_OUTPUT, FD(first_evsel, cpu, 0)) != 0) goto out_unmap; - } else if (__perf_evlist__mmap(evlist, cpu, prot, mask, fd) < 0) + } else if (__perf_evlist__mmap(evlist, evsel, cpu, + prot, mask, fd) < 0) goto out_unmap; if ((evsel->attr.read_format & PERF_FORMAT_ID) && diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c index 662596a..0feffc9 100644 --- a/tools/perf/util/evsel.c +++ b/tools/perf/util/evsel.c @@ -190,21 +190,10 @@ static int __perf_evsel__open(struct perf_evsel *evsel, struct cpu_map *cpus, pid = evsel->cgrp->fd; } + evsel->attr.inherit = inherit; + for (cpu = 0; cpu < cpus->nr; cpu++) { int group_fd = -1; - /* - * Don't allow mmap() of inherited per-task counters. This - * would create a performance issue due to all children writing - * to the same buffer. - * - * FIXME: - * Proper fix is not to pass 'inherit' to perf_evsel__open*, - * but a 'flags' parameter, with 'group' folded there as well, - * then introduce a PERF_O_{MMAP,GROUP,INHERIT} enum, and if - * O_MMAP is set, emit a warning if cpu < 0 and O_INHERIT is - * set. Lets go for the minimal fix first tho. - */ - evsel->attr.inherit = (cpus->map[cpu] >= 0) && inherit; for (thread = 0; thread < threads->nr; thread++) { -- 1.6.2.5