[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20140115185026.GA14248@ghostprotocols.net>
Date: Wed, 15 Jan 2014 15:50:26 -0300
From: Arnaldo Carvalho de Melo <acme@...stprotocols.net>
To: Yann Droneaud <ydroneaud@...eya.com>
Cc: Peter Zijlstra <a.p.zijlstra@...llo.nl>,
Paul Mackerras <paulus@...ba.org>,
Ingo Molnar <mingo@...hat.com>, Jiri Olsa <jolsa@...hat.com>,
Namhyung Kim <namhyung@...nel.org>,
Andi Kleen <ak@...ux.intel.com>,
David Ahern <dsahern@...il.com>,
Frederic Weisbecker <fweisbec@...il.com>,
Mike Galbraith <efault@....de>,
Stephane Eranian <eranian@...gle.com>,
Adrian Hunter <adrian.hunter@...el.com>,
Benjamin Herrenschmidt <benh@...nel.crashing.org>,
Michael Ellerman <michael@...erman.id.au>,
linux-kernel@...r.kernel.org, Peter Zijlstra <peterz@...radead.org>
Subject: Re: [PATCHv2] perf tools: enable close-on-exec flag on perf file
descriptor
Em Mon, Jan 13, 2014 at 11:09:30AM +0100, Yann Droneaud escreveu:
> +++ b/tools/perf/util/cloexec.c
> @@ -0,0 +1,54 @@
> +#include "util.h"
> +#include "../perf.h"
> +#include "cloexec.h"
> +
> +static unsigned long flag = PERF_FLAG_FD_CLOEXEC;
> +
> +static int perf_flag_probe(void)
> +{
> + struct perf_event_attr attr;
> + int fd;
> + int err;
> +
> + /* check cloexec flag */
> + memset(&attr, 0, sizeof(attr));
If you do this you'll use attr.type = PERF_TYPE_HARDWARE and
attr.config then means PERF_COUNT_HW_CPU_CYCLES, which will fail
where we don't have a PMU, see perf_evsel__fallback(), perhaps it is
better for you to go with the fallback that is most likely supported in
all arches:
evsel->attr.type = PERF_TYPE_SOFTWARE; // 1
evsel->attr.config = PERF_COUNT_SW_CPU_CLOCK; // 0
I.e. just do a:
struct perf_event_attr attr = { .type = 1, }; and ditch the
memset (or do a attr.type = PERF_COUNT_SW_CPU_CLOCK;, up to you).
Thanks for doing the __perf_evsel__open fallbacking!
- Arnaldo
> + fd = sys_perf_event_open(&attr, 0, -1, -1,
> + PERF_FLAG_FD_CLOEXEC);
> + if (fd >= 0) {
> + close(fd);
> + return 1;
> + }
> +
> + if (errno != EINVAL) {
> + err = errno;
> + pr_warning("sys_perf_event_open() syscall returned "
> + "%d (%d: %s)\n", fd, err, strerror(err));
> + }
> +
> + /* not supported, confirm error related to PERF_FLAG_FD_CLOEXEC */
> + memset(&attr, 0, sizeof(attr));
> + fd = sys_perf_event_open(&attr, 0, -1, -1, 0);
> + if (fd >= 0) {
> + close(fd);
> + return 0;
> + }
> +
> + err = errno;
> + die("sys_perf_event_open() syscall returned "
> + "%d (%d: %s)\n", fd, err, strerror(err));
> +
> + return -1;
> +}
> +
> +unsigned long perf_event_open_cloexec_flag(void)
> +{
> + static bool probed;
> +
> + if (!probed) {
> + if (perf_flag_probe() <= 0)
> + flag = 0;
> + probed = true;
> + }
> +
> + return flag;
> +}
> diff --git a/tools/perf/util/cloexec.h b/tools/perf/util/cloexec.h
> new file mode 100644
> index 000000000000..94a5a7d829d5
> --- /dev/null
> +++ b/tools/perf/util/cloexec.h
> @@ -0,0 +1,6 @@
> +#ifndef __PERF_CLOEXEC_H
> +#define __PERF_CLOEXEC_H
> +
> +unsigned long perf_event_open_cloexec_flag(void);
> +
> +#endif /* __PERF_CLOEXEC_H */
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index ade8d9c1c431..ff845c7e7a4f 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -29,6 +29,7 @@ static struct {
> bool sample_id_all;
> bool exclude_guest;
> bool mmap2;
> + bool cloexec;
> } perf_missing_features;
>
> #define FD(e, x, y) (*(int *)xyarray__entry(e->fd, x, y))
> @@ -968,7 +969,7 @@ static int __perf_evsel__open(struct perf_evsel *evsel, struct cpu_map *cpus,
> struct thread_map *threads)
> {
> int cpu, thread;
> - unsigned long flags = 0;
> + unsigned long flags = PERF_FLAG_FD_CLOEXEC;
> int pid = -1, err;
> enum { NO_CHANGE, SET_TO_MAX, INCREASED_MAX } set_rlimit = NO_CHANGE;
>
> @@ -977,11 +978,13 @@ static int __perf_evsel__open(struct perf_evsel *evsel, struct cpu_map *cpus,
> return -ENOMEM;
>
> if (evsel->cgrp) {
> - flags = PERF_FLAG_PID_CGROUP;
> + flags |= PERF_FLAG_PID_CGROUP;
> pid = evsel->cgrp->fd;
> }
>
> fallback_missing_features:
> + if (perf_missing_features.cloexec)
> + flags &= ~(unsigned long)PERF_FLAG_FD_CLOEXEC;
> if (perf_missing_features.mmap2)
> evsel->attr.mmap2 = 0;
> if (perf_missing_features.exclude_guest)
> @@ -1050,7 +1053,10 @@ try_fallback:
> if (err != -EINVAL || cpu > 0 || thread > 0)
> goto out_close;
>
> - if (!perf_missing_features.mmap2 && evsel->attr.mmap2) {
> + if (!perf_missing_features.cloexec && (flags & PERF_FLAG_FD_CLOEXEC)) {
> + perf_missing_features.cloexec = true;
> + goto fallback_missing_features;
> + } else if (!perf_missing_features.mmap2 && evsel->attr.mmap2) {
> perf_missing_features.mmap2 = true;
> goto fallback_missing_features;
> } else if (!perf_missing_features.exclude_guest &&
> diff --git a/tools/perf/util/record.c b/tools/perf/util/record.c
> index 104a47563d39..6483ef5df31b 100644
> --- a/tools/perf/util/record.c
> +++ b/tools/perf/util/record.c
> @@ -4,6 +4,7 @@
> #include "parse-events.h"
> #include "fs.h"
> #include "util.h"
> +#include "cloexec.h"
>
> typedef void (*setup_probe_fn_t)(struct perf_evsel *evsel);
>
> @@ -11,6 +12,7 @@ static int perf_do_probe_api(setup_probe_fn_t fn, int cpu, const char *str)
> {
> struct perf_evlist *evlist;
> struct perf_evsel *evsel;
> + unsigned long flags = perf_event_open_cloexec_flag();
> int err = -EAGAIN, fd;
>
> evlist = perf_evlist__new();
> @@ -22,14 +24,14 @@ static int perf_do_probe_api(setup_probe_fn_t fn, int cpu, const char *str)
>
> evsel = perf_evlist__first(evlist);
>
> - fd = sys_perf_event_open(&evsel->attr, -1, cpu, -1, 0);
> + fd = sys_perf_event_open(&evsel->attr, -1, cpu, -1, flags);
> if (fd < 0)
> goto out_delete;
> close(fd);
>
> fn(evsel);
>
> - fd = sys_perf_event_open(&evsel->attr, -1, cpu, -1, 0);
> + fd = sys_perf_event_open(&evsel->attr, -1, cpu, -1, flags);
> if (fd < 0) {
> if (errno == EINVAL)
> err = -EINVAL;
> @@ -203,7 +205,8 @@ bool perf_evlist__can_select_event(struct perf_evlist *evlist, const char *str)
> cpu = evlist->cpus->map[0];
> }
>
> - fd = sys_perf_event_open(&evsel->attr, -1, cpu, -1, 0);
> + fd = sys_perf_event_open(&evsel->attr, -1, cpu, -1,
> + perf_event_open_cloexec_flag());
> if (fd >= 0) {
> close(fd);
> ret = true;
> --
> 1.8.4.2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists