RE: [PATCH] perf tools: fix: Force backward ring buffer mapped readonly

From: Liang, Kan
Date: Wed Oct 11 2017 - 09:16:20 EST



> perf record's --overwrite option doesn't work as we expect.
> For example:
>
> $ ~/linux/tools/perf$ sudo rm ./perf.data*
> rm: cannot remove './perf.data*': No such file or directory
> : ~/linux/tools/perf$ sudo ./perf record -m 4 -e raw_syscalls:* -g --
> overwrite \
> --switch-output=1s --tail-synthesize
> dd if=/dev/zero of=/dev/null
> [ perf record: dump data: Woken up 1 times ]
> [ perf record: Dump perf.data.2017101221460002 ]
> [ perf record: dump data: Woken up 1 times ]
> [ perf record: Dump perf.data.2017101221460102 ]
> [ perf record: dump data: Woken up 1 times ]
> [ perf record: Dump perf.data.2017101221460202 ]
> [ perf record: dump data: Woken up 1 times ]
> [ perf record: Dump perf.data.2017101221460302 ]
> ^C[ perf record: Woken up 1 times to write data ]
> 2066346+0 records in
> 2066346+0 records out
> 1057969152 bytes (1.1 GB, 1009 MiB) copied, 4.25983 s, 248 MB/s
> [ perf record: Dump perf.data.2017101221460332 ]
> [ perf record: Captured and wrote 0.034 MB perf.data.<timestamp> ]
>
> $ ~/linux/tools/perf$ sudo ./perf script -i ./perf.data.2017101221460002 |
> head -n 1
> dd 3918 [006] 182589.668954: raw_syscalls:sys_exit: NR 59 = 0
> $ ~/linux/tools/perf$ sudo ./perf script -i ./perf.data.2017101221460102 |
> head -n 1
> dd 3918 [006] 182589.668954: raw_syscalls:sys_exit: NR 59 = 0
> $ ~/linux/tools/perf$ sudo ./perf script -i ./perf.data.2017101221460202 |
> head -n 1
> dd 3918 [006] 182589.668954: raw_syscalls:sys_exit: NR 59 = 0
> $ ~/linux/tools/perf$ sudo ./perf script -i ./perf.data.2017101221460302 |
> head -n 1
> dd 3918 [006] 182589.668954: raw_syscalls:sys_exit: NR 59 = 0
>
> In the above example we get same records from the backward ring buffer
> all the time. Overwriting is not triggered.
>
> This commit maps backward ring buffers readonly, make it overwritable.
> It is safe because we assume backward ring buffer always overwritable
> in other part of code.
>
> After applying this patch:
>
> $ ~/linux/tools/perf$ sudo ./perf record -m 4 -e raw_syscalls:* -g --
> overwrite \
> --switch-output=1s --tail-synthesize dd
> if=/dev/zero of=/dev/null
> [ perf record: dump data: Woken up 1 times ]
> [ perf record: Dump perf.data.2017101221540350 ]
> [ perf record: dump data: Woken up 1 times ]
> [ perf record: Dump perf.data.2017101221540451 ]
> [ perf record: dump data: Woken up 1 times ]
> [ perf record: Dump perf.data.2017101221540551 ]
> [ perf record: dump data: Woken up 1 times ]
> [ perf record: Dump perf.data.2017101221540651 ]
> ^C[ perf record: Woken up 1 times to write data ]
> 1604606+0 records in
> 1604605+0 records out
> 821557760 bytes (822 MB, 783 MiB) copied, 4.42736 s, 186 MB/s
> [ perf record: Dump perf.data.2017101221540700 ]
> [ perf record: Captured and wrote 0.034 MB perf.data.<timestamp> ]
>
> $ ~/linux/tools/perf$ sudo ./perf script -i ./perf.data.2017101221540350 |
> head -n 1
> dd 5126 [003] 183074.104581: raw_syscalls:sys_exit: NR 0 = 512
> $ ~/linux/tools/perf$ sudo ./perf script -i ./perf.data.2017101221540451 |
> head -n 1
> dd 5126 [003] 183075.106496: raw_syscalls:sys_exit: NR 1 = 512
> $ ~/linux/tools/perf$ sudo ./perf script -i ./perf.data.2017101221540551 |
> head -n 1
> dd 5126 [003] 183076.108093: raw_syscalls:sys_enter: NR 1 (1, af8000, 200,
> 871, 0, af8060)
> $ ~/linux/tools/perf$ sudo ./perf script -i ./perf.data.2017101221540651 |
> head -n 1
> dd 5126 [003] 183077.109676: raw_syscalls:sys_exit: NR 1 = 512
>
>
> Signed-off-by: Wang Nan <wangnan0@xxxxxxxxxx>
> Cc: Liang Kan <kan.liang@xxxxxxxxx>
> Cc: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
> Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> Cc: Jiri Olsa <jolsa@xxxxxxxxxx>
> Cc: Namhyung Kim <namhyung@xxxxxxxxxx>
> Cc: Alexander Shishkin <alexander.shishkin@xxxxxxxxxxxxxxx>
> Cc: Adrian Hunter <adrian.hunter@xxxxxxxxx>
> Cc: Andi Kleen <ak@xxxxxxxxxxxxxxx>
> Cc: Li Zefan <lizefan@xxxxxxxxxx>
> ---
> tools/perf/util/evlist.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index c6c891e..a86b0d2 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -799,12 +799,14 @@ perf_evlist__should_poll(struct perf_evlist *evlist
> __maybe_unused,
> }
>
> static int perf_evlist__mmap_per_evsel(struct perf_evlist *evlist, int idx,
> - struct mmap_params *mp, int cpu_idx,
> + struct mmap_params *_mp, int cpu_idx,
> int thread, int *_output, int
> *_output_backward)
> {
> struct perf_evsel *evsel;
> int revent;
> int evlist_cpu = cpu_map__cpu(evlist->cpus, cpu_idx);
> + struct mmap_params *mp = _mp;
> + struct mmap_params backward_mp;
>
> evlist__for_each_entry(evlist, evsel) {
> struct perf_mmap *maps = evlist->mmap;
> @@ -815,6 +817,9 @@ static int perf_evlist__mmap_per_evsel(struct
> perf_evlist *evlist, int idx,
> if (evsel->attr.write_backward) {
> output = _output_backward;
> maps = evlist->backward_mmap;
> + backward_mp = *mp;
> + backward_mp.prot &= ~PROT_WRITE;
> + mp = &backward_mp;
>
> if (!maps) {
> maps = perf_evlist__alloc_mmap(evlist);

So it's trying to support per-event overwrite.
How about the global --overwrite option?
I think we should use opts->overwrite to replace the hard code 'false' for
perf_evlist__mmap_ex as well.

Thanks,
Kan