Messages in this thread | | | From | Rob Herring <> | Date | Fri, 12 Mar 2021 07:34:39 -0700 | Subject | Re: [PATCH v6 04/10] libperf: Add evsel mmap support |
| |
On Fri, Mar 12, 2021 at 6:59 AM Jiri Olsa <jolsa@redhat.com> wrote: > > On Wed, Mar 10, 2021 at 05:08:31PM -0700, Rob Herring wrote: > > SNIP > > > + > > static int > > sys_perf_event_open(struct perf_event_attr *attr, > > pid_t pid, int cpu, int group_fd, > > @@ -137,6 +147,8 @@ void perf_evsel__free_fd(struct perf_evsel *evsel) > > { > > xyarray__delete(evsel->fd); > > evsel->fd = NULL; > > + xyarray__delete(evsel->mmap); > > + evsel->mmap = NULL; > > } > > > > void perf_evsel__close(struct perf_evsel *evsel) > > @@ -156,6 +168,45 @@ void perf_evsel__close_cpu(struct perf_evsel *evsel, int cpu) > > perf_evsel__close_fd_cpu(evsel, cpu); > > } > > > > +int perf_evsel__mmap(struct perf_evsel *evsel, int pages) > > +{ > > + int ret, cpu, thread; > > + struct perf_mmap_param mp = { > > + .prot = PROT_READ | PROT_WRITE, > > + .mask = (pages * page_size) - 1, > > + }; > > I don't mind using evsel->fd for dimensions below, > but we need to check in here that it's defined, > that perf_evsel__open was called
Right, so I'll add this here:
if (evsel->fd == NULL) return -EINVAL;
Note that struct evsel has dimensions in it, but they are only set in the evlist code. I couldn't tell if that was by design or mistake.
BTW, I just noticed perf_evsel__open is leaking memory on most of its error paths.
> > jirka > > > + > > + if (evsel->mmap == NULL && > > + perf_evsel__alloc_mmap(evsel, xyarray__max_x(evsel->fd), xyarray__max_y(evsel->fd)) < 0) > > + return -ENOMEM; > > + > > + for (cpu = 0; cpu < xyarray__max_x(evsel->fd); cpu++) { > > + for (thread = 0; thread < xyarray__max_y(evsel->fd); thread++) { > > + int fd = FD(evsel, cpu, thread); > > + struct perf_mmap *map = MMAP(evsel, cpu, thread); > > + > > + if (fd < 0) > > + continue; > > + > > + perf_mmap__init(map, NULL, false, NULL); > > + > > + ret = perf_mmap__mmap(map, &mp, fd, cpu); > > + if (ret) > > + return -1; > > + } > > + } > > + > > + return 0; > > +} > > + > > SNIP >
| |