Hi Arnaldo,
On 23.08.2018 17:30, Arnaldo Carvalho de Melo wrote:
> 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?
Right, looks like a typo.
>
>> + 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 *))
yep, make sense.
>
>> + 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 :-)
yeah, I see. Let me resend. Thanks!
>
>> + 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;
>> };
>>
>> /*
>>
>
