Messages in this thread | | | Date | Thu, 23 Aug 2018 11:30:05 -0300 | From | Arnaldo Carvalho de Melo <> | Subject | Re: [PATCH v1 1/2]: perf util: map data buffer for preserving collected data |
| |
Em Thu, Aug 23, 2018 at 01:30:47PM +0300, Alexey Budankov escreveu: > +++ b/tools/perf/util/evlist.c > @@ -718,6 +718,8 @@ static void perf_evlist__munmap_nofree(struct perf_evlist *evlist) > void perf_evlist__munmap(struct perf_evlist *evlist) > { > perf_evlist__munmap_nofree(evlist); > + if (&evlist->mmap_aio)
Is the above test for evlist->mmap_aio being NULL? I think that '&' is not needed here, with it this test will always be true, right?
> + zfree(&evlist->mmap_aio);
> zfree(&evlist->mmap); > zfree(&evlist->overwrite_mmap); > } > @@ -749,6 +751,13 @@ static struct perf_mmap *perf_evlist__alloc_mmap(struct perf_evlist *evlist, > */ > refcount_set(&map[i].refcnt, 0); > } > + > + evlist->mmap_aio = zalloc(evlist->nr_mmaps * sizeof(struct aiocb *));
Right, and here you could have used calloc(evlist->nr_mmaps, sizeof(struct aiocb *))
> + if (!evlist->mmap_aio) { > + zfree(&map);
If you use zfree(&map); then map becomes NULL and you do not return NULL in the next line, if you insist in using the following 'return NULL;', then you could as well use just 'free(map);', as 'map' is a local variable and thus we need not set it to NULL :-)
> + return NULL; > + } > + > return map; > } > > diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h > index dc66436add98..f98b949561fd 100644 > --- a/tools/perf/util/evlist.h > +++ b/tools/perf/util/evlist.h > @@ -15,6 +15,7 @@ > #include "util.h" > #include <signal.h> > #include <unistd.h> > +#include <aio.h> > > struct pollfd; > struct thread_map; > @@ -43,6 +44,7 @@ struct perf_evlist { > } workload; > struct fdarray pollfd; > struct perf_mmap *mmap; > + struct aiocb **mmap_aio; > struct perf_mmap *overwrite_mmap; > struct thread_map *threads; > struct cpu_map *cpus; > diff --git a/tools/perf/util/mmap.c b/tools/perf/util/mmap.c > index fc832676a798..e71d46cb01cc 100644 > --- a/tools/perf/util/mmap.c > +++ b/tools/perf/util/mmap.c > @@ -155,6 +155,10 @@ void __weak auxtrace_mmap_params__set_idx(struct auxtrace_mmap_params *mp __mayb > > void perf_mmap__munmap(struct perf_mmap *map) > { > + if (map->data != NULL) { > + munmap(map->data, perf_mmap__mmap_len(map)); > + map->data = NULL; > + } > if (map->base != NULL) { > munmap(map->base, perf_mmap__mmap_len(map)); > map->base = NULL; > @@ -190,6 +194,14 @@ int perf_mmap__mmap(struct perf_mmap *map, struct mmap_params *mp, int fd) > map->base = NULL; > return -1; > } > + map->data = mmap(NULL, perf_mmap__mmap_len(map), PROT_READ | PROT_WRITE, > + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); > + if (map->data == MAP_FAILED) { > + pr_debug2("failed to mmap perf event data buffer, error %d\n", > + errno); > + map->data = NULL; > + return -1; > + } > map->fd = fd; > > if (auxtrace_mmap__mmap(&map->auxtrace_mmap, > diff --git a/tools/perf/util/mmap.h b/tools/perf/util/mmap.h > index d82294db1295..1974e621e36b 100644 > --- a/tools/perf/util/mmap.h > +++ b/tools/perf/util/mmap.h > @@ -6,6 +6,7 @@ > #include <linux/types.h> > #include <asm/barrier.h> > #include <stdbool.h> > +#include <aio.h> > #include "auxtrace.h" > #include "event.h" > > @@ -25,6 +26,8 @@ struct perf_mmap { > bool overwrite; > struct auxtrace_mmap auxtrace_mmap; > char event_copy[PERF_SAMPLE_MAX_SIZE] __aligned(8); > + void *data; > + struct aiocb cblock; > }; > > /* >
| |