On 05.03.2019 15:25, Jiri Olsa wrote:
> On Fri, Mar 01, 2019 at 06:41:44PM +0300, Alexey Budankov wrote:
>
> SNIP
>
>> static int process_synthesized_event(struct perf_tool *tool,
>> union perf_event *event,
>> struct perf_sample *sample __maybe_unused,
>> @@ -543,7 +566,8 @@ static int record__mmap_evlist(struct record *rec,
>> if (perf_evlist__mmap_ex(evlist, opts->mmap_pages,
>> opts->auxtrace_mmap_pages,
>> opts->auxtrace_snapshot_mode,
>> - opts->nr_cblocks, opts->affinity) < 0) {
>> + opts->nr_cblocks, opts->affinity,
>> + opts->mmap_flush) < 0) {
>> if (errno == EPERM) {
>> pr_err("Permission error mapping pages.\n"
>> "Consider increasing "
>> @@ -733,7 +757,7 @@ static void record__adjust_affinity(struct record *rec,
>> struct perf_mmap *map)
>> }
>>
>> static int record__mmap_read_evlist(struct record *rec, struct perf_evlist
>> *evlist,
>> - bool overwrite)
>> + bool overwrite, bool sync)
>> {
>> u64 bytes_written = rec->bytes_written;
>> int i;
>> @@ -756,12 +780,19 @@ static int record__mmap_read_evlist(struct record
>> *rec, struct perf_evlist *evli
>> off = record__aio_get_pos(trace_fd);
>>
>> for (i = 0; i < evlist->nr_mmaps; i++) {
>> + u64 flush = MMAP_FLUSH_DEFAULT;
>
> hum, why does this need the initial value?
Compiler reported some warning or even error so staying on the safe side.
>
>> struct perf_mmap *map = &maps[i];
>>
>> if (map->base) {
>> record__adjust_affinity(rec, map);
>> + if (sync) {
>> + flush = map->flush;
>> + map->flush = MMAP_FLUSH_DEFAULT;
>> + }
>> if (!record__aio_enabled(rec)) {
>> if (perf_mmap__push(map, rec, record__pushfn)
>> != 0) {
>> + if (sync)
>> + map->flush = flush;
>> rc = -1;
>> goto out;
>> }
>> @@ -774,10 +805,14 @@ static int record__mmap_read_evlist(struct record
>> *rec, struct perf_evlist *evli
>> idx = record__aio_sync(map, false);
>> if (perf_mmap__aio_push(map, rec, idx,
>> record__aio_pushfn, &off) != 0) {
>> record__aio_set_pos(trace_fd, off);
>> + if (sync)
>> + map->flush = flush;
>> rc = -1;
>> goto out;
>> }
>> }
>> + if (sync)
>> + map->flush = flush;
>
> is there a point to set this back in case of sync == true?
> we are going out at this point, right?
Right, but function should work independently of any external assumption.
~Alexey
>
> jirka
>
