[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <YS6Dd6lZoftrKGXl@kernel.org>
Date: Tue, 31 Aug 2021 16:31:03 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Riccardo Mancini <rickyman7@...il.com>
Cc: Ian Rogers <irogers@...gle.com>,
Namhyung Kim <namhyung@...nel.org>,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>,
Mark Rutland <mark.rutland@....com>,
Jiri Olsa <jolsa@...hat.com>, linux-kernel@...r.kernel.org,
linux-perf-users@...r.kernel.org
Subject: Re: [RFC PATCH v1 16/37] perf evsel: save open flags in evsel
Em Sat, Aug 21, 2021 at 11:19:22AM +0200, Riccardo Mancini escreveu:
> This patch caches the flags used in perf_event_open inside evsel, so
> that they can be set in __evsel__prepare_open (this will be useful
> in following patches, when the fallback mechanisms will be handled
> outside the open itself).
> This also optimizes the code, by not having to recompute them everytime.
Nice, thanks, applied, I'll make available what I have in tmp.perf/core
so that you can take a look before I push it all to Linus, this week.
- Arnaldo
> Since flags are now saved in evsel, the flags argument in
> perf_event_open is removed.
>
> Signed-off-by: Riccardo Mancini <rickyman7@...il.com>
> ---
> tools/perf/util/evsel.c | 24 ++++++++++++------------
> tools/perf/util/evsel.h | 1 +
> 2 files changed, 13 insertions(+), 12 deletions(-)
>
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index ddf324e2e17a0951..509a2970a94b3142 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -1710,17 +1710,16 @@ static void display_attr(struct perf_event_attr *attr)
> }
>
> static int perf_event_open(struct evsel *evsel,
> - pid_t pid, int cpu, int group_fd,
> - unsigned long flags)
> + pid_t pid, int cpu, int group_fd)
> {
> int precise_ip = evsel->core.attr.precise_ip;
> int fd;
>
> while (1) {
> pr_debug2_peo("sys_perf_event_open: pid %d cpu %d group_fd %d flags %#lx",
> - pid, cpu, group_fd, flags);
> + pid, cpu, group_fd, evsel->open_flags);
>
> - fd = sys_perf_event_open(&evsel->core.attr, pid, cpu, group_fd, flags);
> + fd = sys_perf_event_open(&evsel->core.attr, pid, cpu, group_fd, evsel->open_flags);
> if (fd >= 0)
> break;
>
> @@ -1788,6 +1787,10 @@ static int __evsel__prepare_open(struct evsel *evsel, struct perf_cpu_map *cpus,
> perf_evsel__alloc_fd(&evsel->core, cpus->nr, nthreads) < 0)
> return -ENOMEM;
>
> + evsel->open_flags = PERF_FLAG_FD_CLOEXEC;
> + if (evsel->cgrp)
> + evsel->open_flags |= PERF_FLAG_PID_CGROUP;
> +
> return 0;
> }
>
> @@ -1796,7 +1799,6 @@ static int evsel__open_cpu(struct evsel *evsel, struct perf_cpu_map *cpus,
> int start_cpu, int end_cpu)
> {
> int cpu, thread, nthreads;
> - unsigned long flags = PERF_FLAG_FD_CLOEXEC;
> int pid = -1, err, old_errno;
> enum { NO_CHANGE, SET_TO_MAX, INCREASED_MAX } set_rlimit = NO_CHANGE;
>
> @@ -1815,10 +1817,8 @@ static int evsel__open_cpu(struct evsel *evsel, struct perf_cpu_map *cpus,
> else
> nthreads = threads->nr;
>
> - if (evsel->cgrp) {
> - flags |= PERF_FLAG_PID_CGROUP;
> + if (evsel->cgrp)
> pid = evsel->cgrp->fd;
> - }
>
> fallback_missing_features:
> if (perf_missing_features.weight_struct) {
> @@ -1832,7 +1832,7 @@ static int evsel__open_cpu(struct evsel *evsel, struct perf_cpu_map *cpus,
> evsel->core.attr.clockid = 0;
> }
> if (perf_missing_features.cloexec)
> - flags &= ~(unsigned long)PERF_FLAG_FD_CLOEXEC;
> + evsel->open_flags &= ~(unsigned long)PERF_FLAG_FD_CLOEXEC;
> if (perf_missing_features.mmap2)
> evsel->core.attr.mmap2 = 0;
> if (perf_missing_features.exclude_guest)
> @@ -1866,7 +1866,7 @@ static int evsel__open_cpu(struct evsel *evsel, struct perf_cpu_map *cpus,
> test_attr__ready();
>
> fd = perf_event_open(evsel, pid, cpus->map[cpu],
> - group_fd, flags);
> + group_fd);
>
> FD(evsel, cpu, thread) = fd;
>
> @@ -1874,7 +1874,7 @@ static int evsel__open_cpu(struct evsel *evsel, struct perf_cpu_map *cpus,
>
> if (unlikely(test_attr__enabled)) {
> test_attr__open(&evsel->core.attr, pid, cpus->map[cpu],
> - fd, group_fd, flags);
> + fd, group_fd, evsel->open_flags);
> }
>
> if (fd < 0) {
> @@ -2012,7 +2012,7 @@ static int evsel__open_cpu(struct evsel *evsel, struct perf_cpu_map *cpus,
> perf_missing_features.clockid = true;
> pr_debug2_peo("switching off use_clockid\n");
> goto fallback_missing_features;
> - } else if (!perf_missing_features.cloexec && (flags & PERF_FLAG_FD_CLOEXEC)) {
> + } else if (!perf_missing_features.cloexec && (evsel->open_flags & PERF_FLAG_FD_CLOEXEC)) {
> perf_missing_features.cloexec = true;
> pr_debug2_peo("switching off cloexec flag\n");
> goto fallback_missing_features;
> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> index eabccce406886320..1c0057e80d080f2f 100644
> --- a/tools/perf/util/evsel.h
> +++ b/tools/perf/util/evsel.h
> @@ -149,6 +149,7 @@ struct evsel {
> struct bperf_leader_bpf *leader_skel;
> struct bperf_follower_bpf *follower_skel;
> };
> + unsigned long open_flags;
> };
>
> struct perf_missing_features {
> --
> 2.31.1
--
- Arnaldo
Powered by blists - more mailing lists