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

[Liang, Kan]

> 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