All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/7] perf tools: Clarify overwrite and backward, bugfix
@ 2017-11-13  1:38 Wang Nan
  2017-11-13  1:38 ` [PATCH 1/7] perf mmap: Fix perf backward recording Wang Nan
                   ` (7 more replies)
  0 siblings, 8 replies; 21+ messages in thread
From: Wang Nan @ 2017-11-13  1:38 UTC (permalink / raw)
  To: linux-kernel, kan.liang, acme, jolsa, namhyung; +Cc: Wang Nan

Based on previous discussion, perf needs to support only two types
of ringbuffer: read-write + forward, readonly + backward. This patchset
completly removes the concept of 'overwrite' from code level, controls
mapping permission using write_backward instead.

Wang Nan (7):
  perf mmap: Fix perf backward recording
  perf tests: Set evlist of test__backward_ring_buffer() to !overwrite
  perf tests: Set evlist of test__sw_clock_freq() to !overwrite
  perf tests: Set evlist of test__basic_mmap() to !overwrite
  perf tests: Set evlist of test__task_exit() to !overwrite
  perf tools: Remove 'overwrite' concept from code level
  perf tools: Remove prot field in mmap param

 tools/perf/arch/x86/tests/perf-time-to-tsc.c |  2 +-
 tools/perf/builtin-kvm.c                     |  2 +-
 tools/perf/builtin-record.c                  |  4 ++--
 tools/perf/builtin-top.c                     |  2 +-
 tools/perf/builtin-trace.c                   |  2 +-
 tools/perf/tests/backward-ring-buffer.c      |  2 +-
 tools/perf/tests/bpf.c                       |  2 +-
 tools/perf/tests/code-reading.c              |  2 +-
 tools/perf/tests/keep-tracking.c             |  2 +-
 tools/perf/tests/mmap-basic.c                |  2 +-
 tools/perf/tests/openat-syscall-tp-fields.c  |  2 +-
 tools/perf/tests/perf-record.c               |  2 +-
 tools/perf/tests/sw-clock.c                  |  2 +-
 tools/perf/tests/switch-tracking.c           |  2 +-
 tools/perf/tests/task-exit.c                 |  2 +-
 tools/perf/util/evlist.c                     | 21 ++++++++++-----------
 tools/perf/util/evlist.h                     |  6 ++----
 tools/perf/util/mmap.c                       | 10 +++++-----
 tools/perf/util/mmap.h                       |  6 +++---
 tools/perf/util/python.c                     | 10 +++++-----
 20 files changed, 41 insertions(+), 44 deletions(-)

-- 
2.10.1

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 1/7] perf mmap: Fix perf backward recording
  2017-11-13  1:38 [PATCH 0/7] perf tools: Clarify overwrite and backward, bugfix Wang Nan
@ 2017-11-13  1:38 ` Wang Nan
  2017-11-13  1:38 ` [PATCH 2/7] perf tests: Set evlist of test__backward_ring_buffer() to !overwrite Wang Nan
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 21+ messages in thread
From: Wang Nan @ 2017-11-13  1:38 UTC (permalink / raw)
  To: linux-kernel, kan.liang, acme, jolsa, namhyung; +Cc: Wang Nan

perf record backward recording doesn't work as we expected: it never
overwrite when ring buffer full.

Test:

(Run a busy printing task background like this:

 while True:
     print 123

send SIGUSR2 to perf to capture snapshot.)

 # ./perf record --overwrite -e raw_syscalls:sys_enter -e raw_syscalls:sys_exit --exclude-perf -a --switch-output
 [ perf record: dump data: Woken up 1 times ]
 [ perf record: Dump perf.data.2017110101520743 ]
 [ perf record: dump data: Woken up 1 times ]
 [ perf record: Dump perf.data.2017110101521251 ]
 [ perf record: dump data: Woken up 1 times ]
 [ perf record: Dump perf.data.2017110101521692 ]
 ^C[ perf record: Woken up 1 times to write data ]
 [ perf record: Dump perf.data.2017110101521936 ]
 [ perf record: Captured and wrote 0.826 MB perf.data.<timestamp> ]

 # ./perf script -i ./perf.data.2017110101520743 | head -n3
             perf  2717 [000] 12449.310785: raw_syscalls:sys_enter: NR 16 (5, 2400, 0, 59, 100, 0)
             perf  2717 [000] 12449.310790: raw_syscalls:sys_enter: NR 7 (4112340, 2, ffffffff, 3df, 100, 0)
           python  2545 [000] 12449.310800:  raw_syscalls:sys_exit: NR 1 = 4
 # ./perf script -i ./perf.data.2017110101521251 | head -n3
             perf  2717 [000] 12449.310785: raw_syscalls:sys_enter: NR 16 (5, 2400, 0, 59, 100, 0)
             perf  2717 [000] 12449.310790: raw_syscalls:sys_enter: NR 7 (4112340, 2, ffffffff, 3df, 100, 0)
           python  2545 [000] 12449.310800:  raw_syscalls:sys_exit: NR 1 = 4
 # ./perf script -i ./perf.data.2017110101521692 | head -n3
             perf  2717 [000] 12449.310785: raw_syscalls:sys_enter: NR 16 (5, 2400, 0, 59, 100, 0)
             perf  2717 [000] 12449.310790: raw_syscalls:sys_enter: NR 7 (4112340, 2, ffffffff, 3df, 100, 0)
           python  2545 [000] 12449.310800:  raw_syscalls:sys_exit: NR 1 = 4

Timestamps are never change, but my background task is a dead loop, can
easily overwhelme the ring buffer.

This patch fix it by force unsetting PROT_WRITE for backward ring
buffer, so all backward ring buffer become overwrite ring buffer.

Test result:

 # ./perf record --overwrite -e raw_syscalls:sys_enter -e raw_syscalls:sys_exit --exclude-perf -a --switch-output
 [ perf record: dump data: Woken up 1 times ]
 [ perf record: Dump perf.data.2017110101285323 ]
 [ perf record: dump data: Woken up 1 times ]
 [ perf record: Dump perf.data.2017110101290053 ]
 [ perf record: dump data: Woken up 1 times ]
 [ perf record: Dump perf.data.2017110101290446 ]
 ^C[ perf record: Woken up 1 times to write data ]
 [ perf record: Dump perf.data.2017110101290837 ]
 [ perf record: Captured and wrote 0.826 MB perf.data.<timestamp> ]
 # ./perf script -i ./perf.data.2017110101285323 | head -n3
           python  2545 [000] 11064.268083:  raw_syscalls:sys_exit: NR 1 = 4
           python  2545 [000] 11064.268084: raw_syscalls:sys_enter: NR 1 (1, 12cc330, 4, 7fc237280370, 7fc2373d0700, 2c7b0)
           python  2545 [000] 11064.268086:  raw_syscalls:sys_exit: NR 1 = 4
 # ./perf script -i ./perf.data.2017110101290 | head -n3
 failed to open ./perf.data.2017110101290: No such file or directory
 # ./perf script -i ./perf.data.2017110101290053 | head -n3
           python  2545 [000] 11071.564062: raw_syscalls:sys_enter: NR 1 (1, 12cc330, 4, 7fc237280370, 7fc2373d0700, 2c7b0)
           python  2545 [000] 11071.564064:  raw_syscalls:sys_exit: NR 1 = 4
           python  2545 [000] 11071.564066: raw_syscalls:sys_enter: NR 1 (1, 12cc330, 4, 7fc237280370, 7fc2373d0700, 2c7b0)
 # ./perf script -i ./perf.data.2017110101290 | head -n3
 perf.data.2017110101290053  perf.data.2017110101290446  perf.data.2017110101290837
 # ./perf script -i ./perf.data.2017110101290446 | head -n3
             sshd  1321 [000] 11075.499473:  raw_syscalls:sys_exit: NR 14 = 0
             sshd  1321 [000] 11075.499474: raw_syscalls:sys_enter: NR 14 (2, 7ffe98899490, 0, 8, 0, 3000)
             sshd  1321 [000] 11075.499474:  raw_syscalls:sys_exit: NR 14 = 0
 # ./perf script -i ./perf.data.2017110101290837 | head -n3
           python  2545 [000] 11079.280844:  raw_syscalls:sys_exit: NR 1 = 4
           python  2545 [000] 11079.280847: raw_syscalls:sys_enter: NR 1 (1, 12cc330, 4, 7fc237280370, 7fc2373d0700, 2c7b0)
           python  2545 [000] 11079.280850:  raw_syscalls:sys_exit: NR 1 = 4

Signed-off-by: Wang Nan <wangnan0@huawei.com>
---
 tools/perf/util/evlist.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index c6c891e..4c5daba 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -799,22 +799,28 @@ 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;
 
 	evlist__for_each_entry(evlist, evsel) {
 		struct perf_mmap *maps = evlist->mmap;
+		struct mmap_params rdonly_mp;
 		int *output = _output;
 		int fd;
 		int cpu;
 
+		mp = _mp;
 		if (evsel->attr.write_backward) {
 			output = _output_backward;
 			maps = evlist->backward_mmap;
+			rdonly_mp = *_mp;
+			rdonly_mp.prot &= ~PROT_WRITE;
+			mp = &rdonly_mp;
 
 			if (!maps) {
 				maps = perf_evlist__alloc_mmap(evlist);
-- 
2.10.1

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 2/7] perf tests: Set evlist of test__backward_ring_buffer() to !overwrite
  2017-11-13  1:38 [PATCH 0/7] perf tools: Clarify overwrite and backward, bugfix Wang Nan
  2017-11-13  1:38 ` [PATCH 1/7] perf mmap: Fix perf backward recording Wang Nan
@ 2017-11-13  1:38 ` Wang Nan
  2017-11-13 18:30   ` Arnaldo Carvalho de Melo
  2017-11-18  8:30   ` [tip:perf/core] " tip-bot for Wang Nan
  2017-11-13  1:38 ` [PATCH 3/7] perf tests: Set evlist of test__sw_clock_freq() " Wang Nan
                   ` (5 subsequent siblings)
  7 siblings, 2 replies; 21+ messages in thread
From: Wang Nan @ 2017-11-13  1:38 UTC (permalink / raw)
  To: linux-kernel, kan.liang, acme, jolsa, namhyung; +Cc: Wang Nan

Setting overwrite in perf_evlist__mmap() is meaningless because the
event in this evlist is already have 'overwrite' postfix and goes to
backward ring buffer automatically. Pass 'false' to perf_evlist__mmap()
to make it similar to others.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
---
 tools/perf/tests/backward-ring-buffer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/tests/backward-ring-buffer.c b/tools/perf/tests/backward-ring-buffer.c
index d233ad3..992c917 100644
--- a/tools/perf/tests/backward-ring-buffer.c
+++ b/tools/perf/tests/backward-ring-buffer.c
@@ -58,7 +58,7 @@ static int do_test(struct perf_evlist *evlist, int mmap_pages,
 	int err;
 	char sbuf[STRERR_BUFSIZE];
 
-	err = perf_evlist__mmap(evlist, mmap_pages, true);
+	err = perf_evlist__mmap(evlist, mmap_pages, false);
 	if (err < 0) {
 		pr_debug("perf_evlist__mmap: %s\n",
 			 str_error_r(errno, sbuf, sizeof(sbuf)));
-- 
2.10.1

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 3/7] perf tests: Set evlist of test__sw_clock_freq() to !overwrite
  2017-11-13  1:38 [PATCH 0/7] perf tools: Clarify overwrite and backward, bugfix Wang Nan
  2017-11-13  1:38 ` [PATCH 1/7] perf mmap: Fix perf backward recording Wang Nan
  2017-11-13  1:38 ` [PATCH 2/7] perf tests: Set evlist of test__backward_ring_buffer() to !overwrite Wang Nan
@ 2017-11-13  1:38 ` Wang Nan
  2017-11-13 18:31   ` Arnaldo Carvalho de Melo
  2017-11-18  8:30   ` [tip:perf/core] " tip-bot for Wang Nan
  2017-11-13  1:38 ` [PATCH 4/7] perf tests: Set evlist of test__basic_mmap() " Wang Nan
                   ` (4 subsequent siblings)
  7 siblings, 2 replies; 21+ messages in thread
From: Wang Nan @ 2017-11-13  1:38 UTC (permalink / raw)
  To: linux-kernel, kan.liang, acme, jolsa, namhyung; +Cc: Wang Nan

Unsetting overwrite when calling perf_evlist__mmap is harmless. This commit
passes false to it, makes following commits eliminate the overwrite argument
easier.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
---
 tools/perf/tests/sw-clock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/tests/sw-clock.c b/tools/perf/tests/sw-clock.c
index d88511f..c468e6c 100644
--- a/tools/perf/tests/sw-clock.c
+++ b/tools/perf/tests/sw-clock.c
@@ -77,7 +77,7 @@ static int __test__sw_clock_freq(enum perf_sw_ids clock_id)
 		goto out_delete_evlist;
 	}
 
-	err = perf_evlist__mmap(evlist, 128, true);
+	err = perf_evlist__mmap(evlist, 128, false);
 	if (err < 0) {
 		pr_debug("failed to mmap event: %d (%s)\n", errno,
 			 str_error_r(errno, sbuf, sizeof(sbuf)));
-- 
2.10.1

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 4/7] perf tests: Set evlist of test__basic_mmap() to !overwrite
  2017-11-13  1:38 [PATCH 0/7] perf tools: Clarify overwrite and backward, bugfix Wang Nan
                   ` (2 preceding siblings ...)
  2017-11-13  1:38 ` [PATCH 3/7] perf tests: Set evlist of test__sw_clock_freq() " Wang Nan
@ 2017-11-13  1:38 ` Wang Nan
  2017-11-13 18:31   ` Arnaldo Carvalho de Melo
  2017-11-18  8:31   ` [tip:perf/core] " tip-bot for Wang Nan
  2017-11-13  1:38 ` [PATCH 5/7] perf tests: Set evlist of test__task_exit() " Wang Nan
                   ` (3 subsequent siblings)
  7 siblings, 2 replies; 21+ messages in thread
From: Wang Nan @ 2017-11-13  1:38 UTC (permalink / raw)
  To: linux-kernel, kan.liang, acme, jolsa, namhyung; +Cc: Wang Nan

In this test, a large ring buffer is required so all events can feed into,
so overwrite or not is meaningless.

Change to !overwrite so following commits can remove this argument.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
---
 tools/perf/tests/mmap-basic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/tests/mmap-basic.c b/tools/perf/tests/mmap-basic.c
index bc8a70e..667d696 100644
--- a/tools/perf/tests/mmap-basic.c
+++ b/tools/perf/tests/mmap-basic.c
@@ -93,7 +93,7 @@ int test__basic_mmap(struct test *test __maybe_unused, int subtest __maybe_unuse
 		expected_nr_events[i] = 1 + rand() % 127;
 	}
 
-	if (perf_evlist__mmap(evlist, 128, true) < 0) {
+	if (perf_evlist__mmap(evlist, 128, false) < 0) {
 		pr_debug("failed to mmap events: %d (%s)\n", errno,
 			 str_error_r(errno, sbuf, sizeof(sbuf)));
 		goto out_delete_evlist;
-- 
2.10.1

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 5/7] perf tests: Set evlist of test__task_exit() to !overwrite
  2017-11-13  1:38 [PATCH 0/7] perf tools: Clarify overwrite and backward, bugfix Wang Nan
                   ` (3 preceding siblings ...)
  2017-11-13  1:38 ` [PATCH 4/7] perf tests: Set evlist of test__basic_mmap() " Wang Nan
@ 2017-11-13  1:38 ` Wang Nan
  2017-11-13 18:32   ` Arnaldo Carvalho de Melo
  2017-11-18  8:31   ` [tip:perf/core] " tip-bot for Wang Nan
  2017-11-13  1:38 ` [PATCH 6/7] perf tools: Remove 'overwrite' concept from code level Wang Nan
                   ` (2 subsequent siblings)
  7 siblings, 2 replies; 21+ messages in thread
From: Wang Nan @ 2017-11-13  1:38 UTC (permalink / raw)
  To: linux-kernel, kan.liang, acme, jolsa, namhyung; +Cc: Wang Nan

Changing ringbuffer to !overwrite in this task is harmless because
this test uses a very low frequency (1) and using a very simple
program (true). There should have only 3 events in the whole test.
Overwriting is impossible to happen.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
---
 tools/perf/tests/task-exit.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/tests/task-exit.c b/tools/perf/tests/task-exit.c
index f0881d0..4fb6609 100644
--- a/tools/perf/tests/task-exit.c
+++ b/tools/perf/tests/task-exit.c
@@ -96,7 +96,7 @@ int test__task_exit(struct test *test __maybe_unused, int subtest __maybe_unused
 		goto out_delete_evlist;
 	}
 
-	if (perf_evlist__mmap(evlist, 128, true) < 0) {
+	if (perf_evlist__mmap(evlist, 128, false) < 0) {
 		pr_debug("failed to mmap events: %d (%s)\n", errno,
 			 str_error_r(errno, sbuf, sizeof(sbuf)));
 		goto out_delete_evlist;
-- 
2.10.1

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 6/7] perf tools: Remove 'overwrite' concept from code level
  2017-11-13  1:38 [PATCH 0/7] perf tools: Clarify overwrite and backward, bugfix Wang Nan
                   ` (4 preceding siblings ...)
  2017-11-13  1:38 ` [PATCH 5/7] perf tests: Set evlist of test__task_exit() " Wang Nan
@ 2017-11-13  1:38 ` Wang Nan
  2017-11-13 11:52   ` Jiri Olsa
  2017-11-13 15:07   ` Liang, Kan
  2017-11-13  1:38 ` [PATCH 7/7] perf tools: Remove prot field in mmap param Wang Nan
  2017-11-13 14:49 ` [PATCH 0/7] perf tools: Clarify overwrite and backward, bugfix Liang, Kan
  7 siblings, 2 replies; 21+ messages in thread
From: Wang Nan @ 2017-11-13  1:38 UTC (permalink / raw)
  To: linux-kernel, kan.liang, acme, jolsa, namhyung; +Cc: Wang Nan

Since all 'overwrite' usage are cleaned and no one really use a readonly main
ringbuffer, remove 'overwrite' from function arguments and evlist. The concept
of 'overwrite' and 'write_backward' are cleanner than before:

  1. In code level, there's no 'overwrite' concept. Each evlist has two
     ringbuffer groups. One is read-write/forward, another is readonly/backward.
     Don't support read-write/backward and readonly/forward.

  2. In user interface, we keep '--overwrite' and translate it into write_backward
     in each event.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
---
 tools/perf/arch/x86/tests/perf-time-to-tsc.c |  2 +-
 tools/perf/builtin-kvm.c                     |  2 +-
 tools/perf/builtin-record.c                  |  4 ++--
 tools/perf/builtin-top.c                     |  2 +-
 tools/perf/builtin-trace.c                   |  2 +-
 tools/perf/tests/backward-ring-buffer.c      |  2 +-
 tools/perf/tests/bpf.c                       |  2 +-
 tools/perf/tests/code-reading.c              |  2 +-
 tools/perf/tests/keep-tracking.c             |  2 +-
 tools/perf/tests/mmap-basic.c                |  2 +-
 tools/perf/tests/openat-syscall-tp-fields.c  |  2 +-
 tools/perf/tests/perf-record.c               |  2 +-
 tools/perf/tests/sw-clock.c                  |  2 +-
 tools/perf/tests/switch-tracking.c           |  2 +-
 tools/perf/tests/task-exit.c                 |  2 +-
 tools/perf/util/evlist.c                     | 14 ++++++--------
 tools/perf/util/evlist.h                     |  6 ++----
 tools/perf/util/mmap.c                       |  6 +++---
 tools/perf/util/mmap.h                       |  2 +-
 tools/perf/util/python.c                     | 10 +++++-----
 20 files changed, 33 insertions(+), 37 deletions(-)

diff --git a/tools/perf/arch/x86/tests/perf-time-to-tsc.c b/tools/perf/arch/x86/tests/perf-time-to-tsc.c
index 5dd7efb..c7ea843 100644
--- a/tools/perf/arch/x86/tests/perf-time-to-tsc.c
+++ b/tools/perf/arch/x86/tests/perf-time-to-tsc.c
@@ -83,7 +83,7 @@ int test__perf_time_to_tsc(struct test *test __maybe_unused, int subtest __maybe
 
 	CHECK__(perf_evlist__open(evlist));
 
-	CHECK__(perf_evlist__mmap(evlist, UINT_MAX, false));
+	CHECK__(perf_evlist__mmap(evlist, UINT_MAX));
 
 	pc = evlist->mmap[0].base;
 	ret = perf_read_tsc_conversion(pc, &tc);
diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
index 0af4c09..e3e2a80 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -1043,7 +1043,7 @@ static int kvm_live_open_events(struct perf_kvm_stat *kvm)
 		goto out;
 	}
 
-	if (perf_evlist__mmap(evlist, kvm->opts.mmap_pages, false) < 0) {
+	if (perf_evlist__mmap(evlist, kvm->opts.mmap_pages) < 0) {
 		ui__error("Failed to mmap the events: %s\n",
 			  str_error_r(errno, sbuf, sizeof(sbuf)));
 		perf_evlist__close(evlist);
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index f4d9fc5..b3ef33f 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -300,7 +300,7 @@ static int record__mmap_evlist(struct record *rec,
 	struct record_opts *opts = &rec->opts;
 	char msg[512];
 
-	if (perf_evlist__mmap_ex(evlist, opts->mmap_pages, false,
+	if (perf_evlist__mmap_ex(evlist, opts->mmap_pages,
 				 opts->auxtrace_mmap_pages,
 				 opts->auxtrace_snapshot_mode) < 0) {
 		if (errno == EPERM) {
@@ -481,7 +481,7 @@ static int record__mmap_read_evlist(struct record *rec, struct perf_evlist *evli
 		struct auxtrace_mmap *mm = &maps[i].auxtrace_mmap;
 
 		if (maps[i].base) {
-			if (perf_mmap__push(&maps[i], evlist->overwrite, backward, rec, record__pushfn) != 0) {
+			if (perf_mmap__push(&maps[i], backward, rec, record__pushfn) != 0) {
 				rc = -1;
 				goto out;
 			}
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 477a869..83d2ae2 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -902,7 +902,7 @@ static int perf_top__start_counters(struct perf_top *top)
 		}
 	}
 
-	if (perf_evlist__mmap(evlist, opts->mmap_pages, false) < 0) {
+	if (perf_evlist__mmap(evlist, opts->mmap_pages) < 0) {
 		ui__error("Failed to mmap with %d (%s)\n",
 			    errno, str_error_r(errno, msg, sizeof(msg)));
 		goto out_err;
diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index c373f9a..c3f2f98 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -2404,7 +2404,7 @@ static int trace__run(struct trace *trace, int argc, const char **argv)
 	if (err < 0)
 		goto out_error_apply_filters;
 
-	err = perf_evlist__mmap(evlist, trace->opts.mmap_pages, false);
+	err = perf_evlist__mmap(evlist, trace->opts.mmap_pages);
 	if (err < 0)
 		goto out_error_mmap;
 
diff --git a/tools/perf/tests/backward-ring-buffer.c b/tools/perf/tests/backward-ring-buffer.c
index 992c917..bdebcf9 100644
--- a/tools/perf/tests/backward-ring-buffer.c
+++ b/tools/perf/tests/backward-ring-buffer.c
@@ -58,7 +58,7 @@ static int do_test(struct perf_evlist *evlist, int mmap_pages,
 	int err;
 	char sbuf[STRERR_BUFSIZE];
 
-	err = perf_evlist__mmap(evlist, mmap_pages, false);
+	err = perf_evlist__mmap(evlist, mmap_pages);
 	if (err < 0) {
 		pr_debug("perf_evlist__mmap: %s\n",
 			 str_error_r(errno, sbuf, sizeof(sbuf)));
diff --git a/tools/perf/tests/bpf.c b/tools/perf/tests/bpf.c
index 34c22cd..c433dd3 100644
--- a/tools/perf/tests/bpf.c
+++ b/tools/perf/tests/bpf.c
@@ -167,7 +167,7 @@ static int do_test(struct bpf_object *obj, int (*func)(void),
 		goto out_delete_evlist;
 	}
 
-	err = perf_evlist__mmap(evlist, opts.mmap_pages, false);
+	err = perf_evlist__mmap(evlist, opts.mmap_pages);
 	if (err < 0) {
 		pr_debug("perf_evlist__mmap: %s\n",
 			 str_error_r(errno, sbuf, sizeof(sbuf)));
diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c
index 466a462..b6c813e 100644
--- a/tools/perf/tests/code-reading.c
+++ b/tools/perf/tests/code-reading.c
@@ -638,7 +638,7 @@ static int do_test_code_reading(bool try_kcore)
 		break;
 	}
 
-	ret = perf_evlist__mmap(evlist, UINT_MAX, false);
+	ret = perf_evlist__mmap(evlist, UINT_MAX);
 	if (ret < 0) {
 		pr_debug("perf_evlist__mmap failed\n");
 		goto out_put;
diff --git a/tools/perf/tests/keep-tracking.c b/tools/perf/tests/keep-tracking.c
index 7394286..b282916 100644
--- a/tools/perf/tests/keep-tracking.c
+++ b/tools/perf/tests/keep-tracking.c
@@ -94,7 +94,7 @@ int test__keep_tracking(struct test *test __maybe_unused, int subtest __maybe_un
 		goto out_err;
 	}
 
-	CHECK__(perf_evlist__mmap(evlist, UINT_MAX, false));
+	CHECK__(perf_evlist__mmap(evlist, UINT_MAX));
 
 	/*
 	 * First, test that a 'comm' event can be found when the event is
diff --git a/tools/perf/tests/mmap-basic.c b/tools/perf/tests/mmap-basic.c
index 667d696..f43d862 100644
--- a/tools/perf/tests/mmap-basic.c
+++ b/tools/perf/tests/mmap-basic.c
@@ -93,7 +93,7 @@ int test__basic_mmap(struct test *test __maybe_unused, int subtest __maybe_unuse
 		expected_nr_events[i] = 1 + rand() % 127;
 	}
 
-	if (perf_evlist__mmap(evlist, 128, false) < 0) {
+	if (perf_evlist__mmap(evlist, 128) < 0) {
 		pr_debug("failed to mmap events: %d (%s)\n", errno,
 			 str_error_r(errno, sbuf, sizeof(sbuf)));
 		goto out_delete_evlist;
diff --git a/tools/perf/tests/openat-syscall-tp-fields.c b/tools/perf/tests/openat-syscall-tp-fields.c
index b6ee1c4..f68d818 100644
--- a/tools/perf/tests/openat-syscall-tp-fields.c
+++ b/tools/perf/tests/openat-syscall-tp-fields.c
@@ -63,7 +63,7 @@ int test__syscall_openat_tp_fields(struct test *test __maybe_unused, int subtest
 		goto out_delete_evlist;
 	}
 
-	err = perf_evlist__mmap(evlist, UINT_MAX, false);
+	err = perf_evlist__mmap(evlist, UINT_MAX);
 	if (err < 0) {
 		pr_debug("perf_evlist__mmap: %s\n",
 			 str_error_r(errno, sbuf, sizeof(sbuf)));
diff --git a/tools/perf/tests/perf-record.c b/tools/perf/tests/perf-record.c
index 19b6500..f9fbd8b 100644
--- a/tools/perf/tests/perf-record.c
+++ b/tools/perf/tests/perf-record.c
@@ -140,7 +140,7 @@ int test__PERF_RECORD(struct test *test __maybe_unused, int subtest __maybe_unus
 	 * fds in the same CPU to be injected in the same mmap ring buffer
 	 * (using ioctl(PERF_EVENT_IOC_SET_OUTPUT)).
 	 */
-	err = perf_evlist__mmap(evlist, opts.mmap_pages, false);
+	err = perf_evlist__mmap(evlist, opts.mmap_pages);
 	if (err < 0) {
 		pr_debug("perf_evlist__mmap: %s\n",
 			 str_error_r(errno, sbuf, sizeof(sbuf)));
diff --git a/tools/perf/tests/sw-clock.c b/tools/perf/tests/sw-clock.c
index c468e6c..fd01b24 100644
--- a/tools/perf/tests/sw-clock.c
+++ b/tools/perf/tests/sw-clock.c
@@ -77,7 +77,7 @@ static int __test__sw_clock_freq(enum perf_sw_ids clock_id)
 		goto out_delete_evlist;
 	}
 
-	err = perf_evlist__mmap(evlist, 128, false);
+	err = perf_evlist__mmap(evlist, 128);
 	if (err < 0) {
 		pr_debug("failed to mmap event: %d (%s)\n", errno,
 			 str_error_r(errno, sbuf, sizeof(sbuf)));
diff --git a/tools/perf/tests/switch-tracking.c b/tools/perf/tests/switch-tracking.c
index 2acd785..9fee2a0 100644
--- a/tools/perf/tests/switch-tracking.c
+++ b/tools/perf/tests/switch-tracking.c
@@ -448,7 +448,7 @@ int test__switch_tracking(struct test *test __maybe_unused, int subtest __maybe_
 		goto out;
 	}
 
-	err = perf_evlist__mmap(evlist, UINT_MAX, false);
+	err = perf_evlist__mmap(evlist, UINT_MAX);
 	if (err) {
 		pr_debug("perf_evlist__mmap failed!\n");
 		goto out_err;
diff --git a/tools/perf/tests/task-exit.c b/tools/perf/tests/task-exit.c
index 4fb6609..4ba5a27 100644
--- a/tools/perf/tests/task-exit.c
+++ b/tools/perf/tests/task-exit.c
@@ -96,7 +96,7 @@ int test__task_exit(struct test *test __maybe_unused, int subtest __maybe_unused
 		goto out_delete_evlist;
 	}
 
-	if (perf_evlist__mmap(evlist, 128, false) < 0) {
+	if (perf_evlist__mmap(evlist, 128) < 0) {
 		pr_debug("failed to mmap events: %d (%s)\n", errno,
 			 str_error_r(errno, sbuf, sizeof(sbuf)));
 		goto out_delete_evlist;
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 4c5daba..4948d3d 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -711,7 +711,7 @@ union perf_event *perf_evlist__mmap_read_forward(struct perf_evlist *evlist, int
 	 * No need for read-write ring buffer: kernel stop outputting when
 	 * it hit md->prev (perf_mmap__consume()).
 	 */
-	return perf_mmap__read_forward(md, evlist->overwrite);
+	return perf_mmap__read_forward(md, false);
 }
 
 union perf_event *perf_evlist__mmap_read_backward(struct perf_evlist *evlist, int idx)
@@ -738,7 +738,7 @@ void perf_evlist__mmap_read_catchup(struct perf_evlist *evlist, int idx)
 
 void perf_evlist__mmap_consume(struct perf_evlist *evlist, int idx)
 {
-	perf_mmap__consume(&evlist->mmap[idx], evlist->overwrite);
+	perf_mmap__consume(&evlist->mmap[idx], false);
 }
 
 static void perf_evlist__munmap_nofree(struct perf_evlist *evlist)
@@ -1058,14 +1058,14 @@ int perf_evlist__parse_mmap_pages(const struct option *opt, const char *str,
  * Return: %0 on success, negative error code otherwise.
  */
 int perf_evlist__mmap_ex(struct perf_evlist *evlist, unsigned int pages,
-			 bool overwrite, unsigned int auxtrace_pages,
+			 unsigned int auxtrace_pages,
 			 bool auxtrace_overwrite)
 {
 	struct perf_evsel *evsel;
 	const struct cpu_map *cpus = evlist->cpus;
 	const struct thread_map *threads = evlist->threads;
 	struct mmap_params mp = {
-		.prot = PROT_READ | (overwrite ? 0 : PROT_WRITE),
+		.prot = PROT_READ | PROT_WRITE,
 	};
 
 	if (!evlist->mmap)
@@ -1076,7 +1076,6 @@ int perf_evlist__mmap_ex(struct perf_evlist *evlist, unsigned int pages,
 	if (evlist->pollfd.entries == NULL && perf_evlist__alloc_pollfd(evlist) < 0)
 		return -ENOMEM;
 
-	evlist->overwrite = overwrite;
 	evlist->mmap_len = perf_evlist__mmap_size(pages);
 	pr_debug("mmap size %zuB\n", evlist->mmap_len);
 	mp.mask = evlist->mmap_len - page_size - 1;
@@ -1097,10 +1096,9 @@ int perf_evlist__mmap_ex(struct perf_evlist *evlist, unsigned int pages,
 	return perf_evlist__mmap_per_cpu(evlist, &mp);
 }
 
-int perf_evlist__mmap(struct perf_evlist *evlist, unsigned int pages,
-		      bool overwrite)
+int perf_evlist__mmap(struct perf_evlist *evlist, unsigned int pages)
 {
-	return perf_evlist__mmap_ex(evlist, pages, overwrite, 0, false);
+	return perf_evlist__mmap_ex(evlist, pages, 0, false);
 }
 
 int perf_evlist__create_maps(struct perf_evlist *evlist, struct target *target)
diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
index 8c433e9..1dd3291 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -30,7 +30,6 @@ struct perf_evlist {
 	int		 nr_entries;
 	int		 nr_groups;
 	int		 nr_mmaps;
-	bool		 overwrite;
 	bool		 enabled;
 	bool		 has_user_cpus;
 	size_t		 mmap_len;
@@ -168,10 +167,9 @@ int perf_evlist__parse_mmap_pages(const struct option *opt,
 unsigned long perf_event_mlock_kb_in_pages(void);
 
 int perf_evlist__mmap_ex(struct perf_evlist *evlist, unsigned int pages,
-			 bool overwrite, unsigned int auxtrace_pages,
+			 unsigned int auxtrace_pages,
 			 bool auxtrace_overwrite);
-int perf_evlist__mmap(struct perf_evlist *evlist, unsigned int pages,
-		      bool overwrite);
+int perf_evlist__mmap(struct perf_evlist *evlist, unsigned int pages);
 void perf_evlist__munmap(struct perf_evlist *evlist);
 
 size_t perf_evlist__mmap_size(unsigned long pages);
diff --git a/tools/perf/util/mmap.c b/tools/perf/util/mmap.c
index 9fe5f9c..703ed41 100644
--- a/tools/perf/util/mmap.c
+++ b/tools/perf/util/mmap.c
@@ -299,7 +299,7 @@ static int rb_find_range(void *data, int mask, u64 head, u64 old,
 	return backward_rb_find_range(data, mask, head, start, end);
 }
 
-int perf_mmap__push(struct perf_mmap *md, bool overwrite, bool backward,
+int perf_mmap__push(struct perf_mmap *md, bool backward,
 		    void *to, int push(void *to, void *buf, size_t size))
 {
 	u64 head = perf_mmap__read_head(md);
@@ -321,7 +321,7 @@ int perf_mmap__push(struct perf_mmap *md, bool overwrite, bool backward,
 		WARN_ONCE(1, "failed to keep up with mmap data. (warn only once)\n");
 
 		md->prev = head;
-		perf_mmap__consume(md, overwrite || backward);
+		perf_mmap__consume(md, backward);
 		return 0;
 	}
 
@@ -346,7 +346,7 @@ int perf_mmap__push(struct perf_mmap *md, bool overwrite, bool backward,
 	}
 
 	md->prev = head;
-	perf_mmap__consume(md, overwrite || backward);
+	perf_mmap__consume(md, backward);
 out:
 	return rc;
 }
diff --git a/tools/perf/util/mmap.h b/tools/perf/util/mmap.h
index efd78b8..2c3d291 100644
--- a/tools/perf/util/mmap.h
+++ b/tools/perf/util/mmap.h
@@ -89,7 +89,7 @@ static inline void perf_mmap__write_tail(struct perf_mmap *md, u64 tail)
 union perf_event *perf_mmap__read_forward(struct perf_mmap *map, bool check_messup);
 union perf_event *perf_mmap__read_backward(struct perf_mmap *map);
 
-int perf_mmap__push(struct perf_mmap *md, bool overwrite, bool backward,
+int perf_mmap__push(struct perf_mmap *md, bool backward,
 		    void *to, int push(void *to, void *buf, size_t size));
 
 size_t perf_mmap__mmap_len(struct perf_mmap *map);
diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
index c129e99..ece33b4 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -856,14 +856,14 @@ static PyObject *pyrf_evlist__mmap(struct pyrf_evlist *pevlist,
 				   PyObject *args, PyObject *kwargs)
 {
 	struct perf_evlist *evlist = &pevlist->evlist;
-	static char *kwlist[] = { "pages", "overwrite", NULL };
-	int pages = 128, overwrite = false;
+	static char *kwlist[] = { "pages", NULL };
+	int pages = 128;
 
-	if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|ii", kwlist,
-					 &pages, &overwrite))
+	if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|i", kwlist,
+					 &pages))
 		return NULL;
 
-	if (perf_evlist__mmap(evlist, pages, overwrite) < 0) {
+	if (perf_evlist__mmap(evlist, pages) < 0) {
 		PyErr_SetFromErrno(PyExc_OSError);
 		return NULL;
 	}
-- 
2.10.1

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 7/7] perf tools: Remove prot field in mmap param
  2017-11-13  1:38 [PATCH 0/7] perf tools: Clarify overwrite and backward, bugfix Wang Nan
                   ` (5 preceding siblings ...)
  2017-11-13  1:38 ` [PATCH 6/7] perf tools: Remove 'overwrite' concept from code level Wang Nan
@ 2017-11-13  1:38 ` Wang Nan
  2017-11-13 11:52   ` Jiri Olsa
  2017-11-13 11:52   ` Jiri Olsa
  2017-11-13 14:49 ` [PATCH 0/7] perf tools: Clarify overwrite and backward, bugfix Liang, Kan
  7 siblings, 2 replies; 21+ messages in thread
From: Wang Nan @ 2017-11-13  1:38 UTC (permalink / raw)
  To: linux-kernel, kan.liang, acme, jolsa, namhyung; +Cc: Wang Nan

After removing the concept of 'overwrite' in code level, now the
prot is determinated by write_backward. There's no need to pass
prot from perf_evlist__mmap_ex().

Signed-off-by: Wang Nan <wangnan0@huawei.com>
---
 tools/perf/util/evlist.c | 17 ++++++-----------
 tools/perf/util/mmap.c   |  4 ++--
 tools/perf/util/mmap.h   |  4 ++--
 3 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 4948d3d..0d713e0 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -799,28 +799,23 @@ 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;
 
 	evlist__for_each_entry(evlist, evsel) {
 		struct perf_mmap *maps = evlist->mmap;
-		struct mmap_params rdonly_mp;
 		int *output = _output;
 		int fd;
 		int cpu;
+		int prot = PROT_READ;
 
-		mp = _mp;
 		if (evsel->attr.write_backward) {
 			output = _output_backward;
 			maps = evlist->backward_mmap;
-			rdonly_mp = *_mp;
-			rdonly_mp.prot &= ~PROT_WRITE;
-			mp = &rdonly_mp;
 
 			if (!maps) {
 				maps = perf_evlist__alloc_mmap(evlist);
@@ -830,6 +825,8 @@ static int perf_evlist__mmap_per_evsel(struct perf_evlist *evlist, int idx,
 				if (evlist->bkw_mmap_state == BKW_MMAP_NOTREADY)
 					perf_evlist__toggle_bkw_mmap(evlist, BKW_MMAP_RUNNING);
 			}
+		} else {
+			prot |= PROT_WRITE;
 		}
 
 		if (evsel->system_wide && thread)
@@ -844,7 +841,7 @@ static int perf_evlist__mmap_per_evsel(struct perf_evlist *evlist, int idx,
 		if (*output == -1) {
 			*output = fd;
 
-			if (perf_mmap__mmap(&maps[idx], mp, *output)  < 0)
+			if (perf_mmap__mmap(&maps[idx], mp, prot, *output)  < 0)
 				return -1;
 		} else {
 			if (ioctl(fd, PERF_EVENT_IOC_SET_OUTPUT, *output) != 0)
@@ -1064,9 +1061,7 @@ int perf_evlist__mmap_ex(struct perf_evlist *evlist, unsigned int pages,
 	struct perf_evsel *evsel;
 	const struct cpu_map *cpus = evlist->cpus;
 	const struct thread_map *threads = evlist->threads;
-	struct mmap_params mp = {
-		.prot = PROT_READ | PROT_WRITE,
-	};
+	struct mmap_params mp;
 
 	if (!evlist->mmap)
 		evlist->mmap = perf_evlist__alloc_mmap(evlist);
diff --git a/tools/perf/util/mmap.c b/tools/perf/util/mmap.c
index 703ed41..40e91a0 100644
--- a/tools/perf/util/mmap.c
+++ b/tools/perf/util/mmap.c
@@ -219,7 +219,7 @@ void perf_mmap__munmap(struct perf_mmap *map)
 	auxtrace_mmap__munmap(&map->auxtrace_mmap);
 }
 
-int perf_mmap__mmap(struct perf_mmap *map, struct mmap_params *mp, int fd)
+int perf_mmap__mmap(struct perf_mmap *map, struct mmap_params *mp, int prot, int fd)
 {
 	/*
 	 * The last one will be done at perf_evlist__mmap_consume(), so that we
@@ -237,7 +237,7 @@ int perf_mmap__mmap(struct perf_mmap *map, struct mmap_params *mp, int fd)
 	refcount_set(&map->refcnt, 2);
 	map->prev = 0;
 	map->mask = mp->mask;
-	map->base = mmap(NULL, perf_mmap__mmap_len(map), mp->prot,
+	map->base = mmap(NULL, perf_mmap__mmap_len(map), prot,
 			 MAP_SHARED, fd, 0);
 	if (map->base == MAP_FAILED) {
 		pr_debug2("failed to mmap perf event ring buffer, error %d\n",
diff --git a/tools/perf/util/mmap.h b/tools/perf/util/mmap.h
index 2c3d291..1f6fcc6 100644
--- a/tools/perf/util/mmap.h
+++ b/tools/perf/util/mmap.h
@@ -53,11 +53,11 @@ enum bkw_mmap_state {
 };
 
 struct mmap_params {
-	int			    prot, mask;
+	int			    mask;
 	struct auxtrace_mmap_params auxtrace_mp;
 };
 
-int perf_mmap__mmap(struct perf_mmap *map, struct mmap_params *mp, int fd);
+int perf_mmap__mmap(struct perf_mmap *map, struct mmap_params *mp, int prot, int fd);
 void perf_mmap__munmap(struct perf_mmap *map);
 
 void perf_mmap__get(struct perf_mmap *map);
-- 
2.10.1

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH 7/7] perf tools: Remove prot field in mmap param
  2017-11-13  1:38 ` [PATCH 7/7] perf tools: Remove prot field in mmap param Wang Nan
@ 2017-11-13 11:52   ` Jiri Olsa
  2017-11-13 11:52   ` Jiri Olsa
  1 sibling, 0 replies; 21+ messages in thread
From: Jiri Olsa @ 2017-11-13 11:52 UTC (permalink / raw)
  To: Wang Nan; +Cc: linux-kernel, kan.liang, acme, namhyung

On Mon, Nov 13, 2017 at 01:38:09AM +0000, Wang Nan wrote:
> After removing the concept of 'overwrite' in code level, now the
> prot is determinated by write_backward. There's no need to pass
> prot from perf_evlist__mmap_ex().
> 
> Signed-off-by: Wang Nan <wangnan0@huawei.com>
> ---
>  tools/perf/util/evlist.c | 17 ++++++-----------
>  tools/perf/util/mmap.c   |  4 ++--
>  tools/perf/util/mmap.h   |  4 ++--
>  3 files changed, 10 insertions(+), 15 deletions(-)
> 
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index 4948d3d..0d713e0 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -799,28 +799,23 @@ 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;
>  
>  	evlist__for_each_entry(evlist, evsel) {
>  		struct perf_mmap *maps = evlist->mmap;
> -		struct mmap_params rdonly_mp;
>  		int *output = _output;
>  		int fd;
>  		int cpu;
> +		int prot = PROT_READ;

can't you set the PROT_READ in struct mmap_params *mp as its default value?

		mp->prot = PROT_READ;

>  
> -		mp = _mp;
>  		if (evsel->attr.write_backward) {
>  			output = _output_backward;
>  			maps = evlist->backward_mmap;
> -			rdonly_mp = *_mp;
> -			rdonly_mp.prot &= ~PROT_WRITE;
> -			mp = &rdonly_mp;
>  
>  			if (!maps) {
>  				maps = perf_evlist__alloc_mmap(evlist);
> @@ -830,6 +825,8 @@ static int perf_evlist__mmap_per_evsel(struct perf_evlist *evlist, int idx,
>  				if (evlist->bkw_mmap_state == BKW_MMAP_NOTREADY)
>  					perf_evlist__toggle_bkw_mmap(evlist, BKW_MMAP_RUNNING);
>  			}
> +		} else {
> +			prot |= PROT_WRITE;
>  		}
>  
>  		if (evsel->system_wide && thread)
> @@ -844,7 +841,7 @@ static int perf_evlist__mmap_per_evsel(struct perf_evlist *evlist, int idx,
>  		if (*output == -1) {
>  			*output = fd;
>  
> -			if (perf_mmap__mmap(&maps[idx], mp, *output)  < 0)
> +			if (perf_mmap__mmap(&maps[idx], mp, prot, *output)  < 0)

so there's no need for the extra 'prot' param in here

jirka

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 6/7] perf tools: Remove 'overwrite' concept from code level
  2017-11-13  1:38 ` [PATCH 6/7] perf tools: Remove 'overwrite' concept from code level Wang Nan
@ 2017-11-13 11:52   ` Jiri Olsa
  2017-11-13 15:07   ` Liang, Kan
  1 sibling, 0 replies; 21+ messages in thread
From: Jiri Olsa @ 2017-11-13 11:52 UTC (permalink / raw)
  To: Wang Nan; +Cc: linux-kernel, kan.liang, acme, namhyung

On Mon, Nov 13, 2017 at 01:38:08AM +0000, Wang Nan wrote:

SNIP

>  size_t perf_mmap__mmap_len(struct perf_mmap *map);
> diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> index c129e99..ece33b4 100644
> --- a/tools/perf/util/python.c
> +++ b/tools/perf/util/python.c
> @@ -856,14 +856,14 @@ static PyObject *pyrf_evlist__mmap(struct pyrf_evlist *pevlist,
>  				   PyObject *args, PyObject *kwargs)
>  {
>  	struct perf_evlist *evlist = &pevlist->evlist;
> -	static char *kwlist[] = { "pages", "overwrite", NULL };

unlikely, but there might be already some users of this..

I think the best would be to keep the "overwrite" here and
don't use it.. maybe warn or update docs, if there's any ;-)

jirka

> -	int pages = 128, overwrite = false;
> +	static char *kwlist[] = { "pages", NULL };
> +	int pages = 128;
>  
> -	if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|ii", kwlist,
> -					 &pages, &overwrite))
> +	if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|i", kwlist,
> +					 &pages))
>  		return NULL;
>  
> -	if (perf_evlist__mmap(evlist, pages, overwrite) < 0) {
> +	if (perf_evlist__mmap(evlist, pages) < 0) {
>  		PyErr_SetFromErrno(PyExc_OSError);
>  		return NULL;
>  	}
> -- 
> 2.10.1
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 7/7] perf tools: Remove prot field in mmap param
  2017-11-13  1:38 ` [PATCH 7/7] perf tools: Remove prot field in mmap param Wang Nan
  2017-11-13 11:52   ` Jiri Olsa
@ 2017-11-13 11:52   ` Jiri Olsa
  1 sibling, 0 replies; 21+ messages in thread
From: Jiri Olsa @ 2017-11-13 11:52 UTC (permalink / raw)
  To: Wang Nan; +Cc: linux-kernel, kan.liang, acme, namhyung

On Mon, Nov 13, 2017 at 01:38:09AM +0000, Wang Nan wrote:
> After removing the concept of 'overwrite' in code level, now the
> prot is determinated by write_backward. There's no need to pass
> prot from perf_evlist__mmap_ex().
> 
> Signed-off-by: Wang Nan <wangnan0@huawei.com>
> ---
>  tools/perf/util/evlist.c | 17 ++++++-----------
>  tools/perf/util/mmap.c   |  4 ++--
>  tools/perf/util/mmap.h   |  4 ++--
>  3 files changed, 10 insertions(+), 15 deletions(-)
> 
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index 4948d3d..0d713e0 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -799,28 +799,23 @@ 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;
>  
>  	evlist__for_each_entry(evlist, evsel) {
>  		struct perf_mmap *maps = evlist->mmap;
> -		struct mmap_params rdonly_mp;
>  		int *output = _output;
>  		int fd;
>  		int cpu;
> +		int prot = PROT_READ;
>  
> -		mp = _mp;
>  		if (evsel->attr.write_backward) {
>  			output = _output_backward;
>  			maps = evlist->backward_mmap;
> -			rdonly_mp = *_mp;
> -			rdonly_mp.prot &= ~PROT_WRITE;
> -			mp = &rdonly_mp;

I dont think we need the first patch, you're reverting it in here

jirka

^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: [PATCH 0/7] perf tools: Clarify overwrite and backward, bugfix
  2017-11-13  1:38 [PATCH 0/7] perf tools: Clarify overwrite and backward, bugfix Wang Nan
                   ` (6 preceding siblings ...)
  2017-11-13  1:38 ` [PATCH 7/7] perf tools: Remove prot field in mmap param Wang Nan
@ 2017-11-13 14:49 ` Liang, Kan
  7 siblings, 0 replies; 21+ messages in thread
From: Liang, Kan @ 2017-11-13 14:49 UTC (permalink / raw)
  To: Wang Nan, linux-kernel, acme, jolsa, namhyung

> Based on previous discussion, perf needs to support only two types
> of ringbuffer: read-write + forward, readonly + backward. This patchset
> completly removes the concept of 'overwrite' from code level, controls
> mapping permission using write_backward instead.

I think I suggested to remove the concept of backward/forward, keep
the overwrite. That's from user's perspective.
Here you use read-write + forward and readonly + backward, which
should be from kernel 's perspective. It's OK for me either.
However, we should make the concepts connected by clearly documented,
not just in changelog.

I suggest you may create a new file, ringbuffer.txt, in Documentation to
explain, the types of ringbuffer, how do they work, what's connections
between overwrite and readonly + backward, and so on. 


> 
> Wang Nan (7):
>   perf mmap: Fix perf backward recording
>   perf tests: Set evlist of test__backward_ring_buffer() to !overwrite
>   perf tests: Set evlist of test__sw_clock_freq() to !overwrite
>   perf tests: Set evlist of test__basic_mmap() to !overwrite
>   perf tests: Set evlist of test__task_exit() to !overwrite
>   perf tools: Remove 'overwrite' concept from code level
>   perf tools: Remove prot field in mmap param
> 

You had another related fix "perf tool: Don't discard prev in backward
mode" https://lkml.org/lkml/2017/10/12/408
It has not been merged yet. It needs to backport.
I think you may want to add it in this series as well.


Thanks,
Kan

>  tools/perf/arch/x86/tests/perf-time-to-tsc.c |  2 +-
>  tools/perf/builtin-kvm.c                     |  2 +-
>  tools/perf/builtin-record.c                  |  4 ++--
>  tools/perf/builtin-top.c                     |  2 +-
>  tools/perf/builtin-trace.c                   |  2 +-
>  tools/perf/tests/backward-ring-buffer.c      |  2 +-
>  tools/perf/tests/bpf.c                       |  2 +-
>  tools/perf/tests/code-reading.c              |  2 +-
>  tools/perf/tests/keep-tracking.c             |  2 +-
>  tools/perf/tests/mmap-basic.c                |  2 +-
>  tools/perf/tests/openat-syscall-tp-fields.c  |  2 +-
>  tools/perf/tests/perf-record.c               |  2 +-
>  tools/perf/tests/sw-clock.c                  |  2 +-
>  tools/perf/tests/switch-tracking.c           |  2 +-
>  tools/perf/tests/task-exit.c                 |  2 +-
>  tools/perf/util/evlist.c                     | 21 ++++++++++-----------
>  tools/perf/util/evlist.h                     |  6 ++----
>  tools/perf/util/mmap.c                       | 10 +++++-----
>  tools/perf/util/mmap.h                       |  6 +++---
>  tools/perf/util/python.c                     | 10 +++++-----
>  20 files changed, 41 insertions(+), 44 deletions(-)
> 
> --
> 2.10.1

^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: [PATCH 6/7] perf tools: Remove 'overwrite' concept from code level
  2017-11-13  1:38 ` [PATCH 6/7] perf tools: Remove 'overwrite' concept from code level Wang Nan
  2017-11-13 11:52   ` Jiri Olsa
@ 2017-11-13 15:07   ` Liang, Kan
  1 sibling, 0 replies; 21+ messages in thread
From: Liang, Kan @ 2017-11-13 15:07 UTC (permalink / raw)
  To: Wang Nan, linux-kernel, acme, jolsa, namhyung

> Since all 'overwrite' usage are cleaned and no one really use a readonly main
> ringbuffer, remove 'overwrite' from function arguments and evlist. The
> concept
> of 'overwrite' and 'write_backward' are cleanner than before:
> 
>   1. In code level, there's no 'overwrite' concept. Each evlist has two
>      ringbuffer groups. One is read-write/forward, another is
> readonly/backward.
>      Don't support read-write/backward and readonly/forward.
> 
>   2. In user interface, we keep '--overwrite' and translate it into
> write_backward
>      in each event.
> 
> Signed-off-by: Wang Nan <wangnan0@huawei.com>
> ---
>  tools/perf/arch/x86/tests/perf-time-to-tsc.c |  2 +-
>  tools/perf/builtin-kvm.c                     |  2 +-
>  tools/perf/builtin-record.c                  |  4 ++--
>  tools/perf/builtin-top.c                     |  2 +-
>  tools/perf/builtin-trace.c                   |  2 +-
>  tools/perf/tests/backward-ring-buffer.c      |  2 +-
>  tools/perf/tests/bpf.c                       |  2 +-
>  tools/perf/tests/code-reading.c              |  2 +-
>  tools/perf/tests/keep-tracking.c             |  2 +-
>  tools/perf/tests/mmap-basic.c                |  2 +-
>  tools/perf/tests/openat-syscall-tp-fields.c  |  2 +-
>  tools/perf/tests/perf-record.c               |  2 +-
>  tools/perf/tests/sw-clock.c                  |  2 +-
>  tools/perf/tests/switch-tracking.c           |  2 +-
>  tools/perf/tests/task-exit.c                 |  2 +-
>  tools/perf/util/evlist.c                     | 14 ++++++--------
>  tools/perf/util/evlist.h                     |  6 ++----
>  tools/perf/util/mmap.c                       |  6 +++---
>  tools/perf/util/mmap.h                       |  2 +-
>  tools/perf/util/python.c                     | 10 +++++-----
>  20 files changed, 33 insertions(+), 37 deletions(-)
> 
> diff --git a/tools/perf/arch/x86/tests/perf-time-to-tsc.c
> b/tools/perf/arch/x86/tests/perf-time-to-tsc.c
> index 5dd7efb..c7ea843 100644
> --- a/tools/perf/arch/x86/tests/perf-time-to-tsc.c
> +++ b/tools/perf/arch/x86/tests/perf-time-to-tsc.c
> @@ -83,7 +83,7 @@ int test__perf_time_to_tsc(struct test *test
> __maybe_unused, int subtest __maybe
> 
>  	CHECK__(perf_evlist__open(evlist));
> 
> -	CHECK__(perf_evlist__mmap(evlist, UINT_MAX, false));
> +	CHECK__(perf_evlist__mmap(evlist, UINT_MAX));
> 
>  	pc = evlist->mmap[0].base;
>  	ret = perf_read_tsc_conversion(pc, &tc);
> diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
> index 0af4c09..e3e2a80 100644
> --- a/tools/perf/builtin-kvm.c
> +++ b/tools/perf/builtin-kvm.c
> @@ -1043,7 +1043,7 @@ static int kvm_live_open_events(struct
> perf_kvm_stat *kvm)
>  		goto out;
>  	}
> 
> -	if (perf_evlist__mmap(evlist, kvm->opts.mmap_pages, false) < 0) {
> +	if (perf_evlist__mmap(evlist, kvm->opts.mmap_pages) < 0) {
>  		ui__error("Failed to mmap the events: %s\n",
>  			  str_error_r(errno, sbuf, sizeof(sbuf)));
>  		perf_evlist__close(evlist);
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index f4d9fc5..b3ef33f 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -300,7 +300,7 @@ static int record__mmap_evlist(struct record *rec,
>  	struct record_opts *opts = &rec->opts;
>  	char msg[512];
> 
> -	if (perf_evlist__mmap_ex(evlist, opts->mmap_pages, false,
> +	if (perf_evlist__mmap_ex(evlist, opts->mmap_pages,
>  				 opts->auxtrace_mmap_pages,
>  				 opts->auxtrace_snapshot_mode) < 0) {
>  		if (errno == EPERM) {
> @@ -481,7 +481,7 @@ static int record__mmap_read_evlist(struct record
> *rec, struct perf_evlist *evli
>  		struct auxtrace_mmap *mm = &maps[i].auxtrace_mmap;
> 
>  		if (maps[i].base) {
> -			if (perf_mmap__push(&maps[i], evlist->overwrite,
> backward, rec, record__pushfn) != 0) {
> +			if (perf_mmap__push(&maps[i], backward, rec,
> record__pushfn) != 0) {
>  				rc = -1;
>  				goto out;
>  			}
> diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> index 477a869..83d2ae2 100644
> --- a/tools/perf/builtin-top.c
> +++ b/tools/perf/builtin-top.c
> @@ -902,7 +902,7 @@ static int perf_top__start_counters(struct perf_top
> *top)
>  		}
>  	}
> 
> -	if (perf_evlist__mmap(evlist, opts->mmap_pages, false) < 0) {
> +	if (perf_evlist__mmap(evlist, opts->mmap_pages) < 0) {
>  		ui__error("Failed to mmap with %d (%s)\n",
>  			    errno, str_error_r(errno, msg, sizeof(msg)));
>  		goto out_err;
> diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> index c373f9a..c3f2f98 100644
> --- a/tools/perf/builtin-trace.c
> +++ b/tools/perf/builtin-trace.c
> @@ -2404,7 +2404,7 @@ static int trace__run(struct trace *trace, int argc,
> const char **argv)
>  	if (err < 0)
>  		goto out_error_apply_filters;
> 
> -	err = perf_evlist__mmap(evlist, trace->opts.mmap_pages, false);
> +	err = perf_evlist__mmap(evlist, trace->opts.mmap_pages);
>  	if (err < 0)
>  		goto out_error_mmap;
> 
> diff --git a/tools/perf/tests/backward-ring-buffer.c
> b/tools/perf/tests/backward-ring-buffer.c
> index 992c917..bdebcf9 100644
> --- a/tools/perf/tests/backward-ring-buffer.c
> +++ b/tools/perf/tests/backward-ring-buffer.c
> @@ -58,7 +58,7 @@ static int do_test(struct perf_evlist *evlist, int
> mmap_pages,
>  	int err;
>  	char sbuf[STRERR_BUFSIZE];
> 
> -	err = perf_evlist__mmap(evlist, mmap_pages, false);
> +	err = perf_evlist__mmap(evlist, mmap_pages);
>  	if (err < 0) {
>  		pr_debug("perf_evlist__mmap: %s\n",
>  			 str_error_r(errno, sbuf, sizeof(sbuf)));
> diff --git a/tools/perf/tests/bpf.c b/tools/perf/tests/bpf.c
> index 34c22cd..c433dd3 100644
> --- a/tools/perf/tests/bpf.c
> +++ b/tools/perf/tests/bpf.c
> @@ -167,7 +167,7 @@ static int do_test(struct bpf_object *obj, int
> (*func)(void),
>  		goto out_delete_evlist;
>  	}
> 
> -	err = perf_evlist__mmap(evlist, opts.mmap_pages, false);
> +	err = perf_evlist__mmap(evlist, opts.mmap_pages);
>  	if (err < 0) {
>  		pr_debug("perf_evlist__mmap: %s\n",
>  			 str_error_r(errno, sbuf, sizeof(sbuf)));
> diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c
> index 466a462..b6c813e 100644
> --- a/tools/perf/tests/code-reading.c
> +++ b/tools/perf/tests/code-reading.c
> @@ -638,7 +638,7 @@ static int do_test_code_reading(bool try_kcore)
>  		break;
>  	}
> 
> -	ret = perf_evlist__mmap(evlist, UINT_MAX, false);
> +	ret = perf_evlist__mmap(evlist, UINT_MAX);
>  	if (ret < 0) {
>  		pr_debug("perf_evlist__mmap failed\n");
>  		goto out_put;
> diff --git a/tools/perf/tests/keep-tracking.c b/tools/perf/tests/keep-tracking.c
> index 7394286..b282916 100644
> --- a/tools/perf/tests/keep-tracking.c
> +++ b/tools/perf/tests/keep-tracking.c
> @@ -94,7 +94,7 @@ int test__keep_tracking(struct test *test
> __maybe_unused, int subtest __maybe_un
>  		goto out_err;
>  	}
> 
> -	CHECK__(perf_evlist__mmap(evlist, UINT_MAX, false));
> +	CHECK__(perf_evlist__mmap(evlist, UINT_MAX));
> 
>  	/*
>  	 * First, test that a 'comm' event can be found when the event is
> diff --git a/tools/perf/tests/mmap-basic.c b/tools/perf/tests/mmap-basic.c
> index 667d696..f43d862 100644
> --- a/tools/perf/tests/mmap-basic.c
> +++ b/tools/perf/tests/mmap-basic.c
> @@ -93,7 +93,7 @@ int test__basic_mmap(struct test *test
> __maybe_unused, int subtest __maybe_unuse
>  		expected_nr_events[i] = 1 + rand() % 127;
>  	}
> 
> -	if (perf_evlist__mmap(evlist, 128, false) < 0) {
> +	if (perf_evlist__mmap(evlist, 128) < 0) {
>  		pr_debug("failed to mmap events: %d (%s)\n", errno,
>  			 str_error_r(errno, sbuf, sizeof(sbuf)));
>  		goto out_delete_evlist;
> diff --git a/tools/perf/tests/openat-syscall-tp-fields.c
> b/tools/perf/tests/openat-syscall-tp-fields.c
> index b6ee1c4..f68d818 100644
> --- a/tools/perf/tests/openat-syscall-tp-fields.c
> +++ b/tools/perf/tests/openat-syscall-tp-fields.c
> @@ -63,7 +63,7 @@ int test__syscall_openat_tp_fields(struct test *test
> __maybe_unused, int subtest
>  		goto out_delete_evlist;
>  	}
> 
> -	err = perf_evlist__mmap(evlist, UINT_MAX, false);
> +	err = perf_evlist__mmap(evlist, UINT_MAX);
>  	if (err < 0) {
>  		pr_debug("perf_evlist__mmap: %s\n",
>  			 str_error_r(errno, sbuf, sizeof(sbuf)));
> diff --git a/tools/perf/tests/perf-record.c b/tools/perf/tests/perf-record.c
> index 19b6500..f9fbd8b 100644
> --- a/tools/perf/tests/perf-record.c
> +++ b/tools/perf/tests/perf-record.c
> @@ -140,7 +140,7 @@ int test__PERF_RECORD(struct test *test
> __maybe_unused, int subtest __maybe_unus
>  	 * fds in the same CPU to be injected in the same mmap ring buffer
>  	 * (using ioctl(PERF_EVENT_IOC_SET_OUTPUT)).
>  	 */
> -	err = perf_evlist__mmap(evlist, opts.mmap_pages, false);
> +	err = perf_evlist__mmap(evlist, opts.mmap_pages);
>  	if (err < 0) {
>  		pr_debug("perf_evlist__mmap: %s\n",
>  			 str_error_r(errno, sbuf, sizeof(sbuf)));
> diff --git a/tools/perf/tests/sw-clock.c b/tools/perf/tests/sw-clock.c
> index c468e6c..fd01b24 100644
> --- a/tools/perf/tests/sw-clock.c
> +++ b/tools/perf/tests/sw-clock.c
> @@ -77,7 +77,7 @@ static int __test__sw_clock_freq(enum perf_sw_ids
> clock_id)
>  		goto out_delete_evlist;
>  	}
> 
> -	err = perf_evlist__mmap(evlist, 128, false);
> +	err = perf_evlist__mmap(evlist, 128);
>  	if (err < 0) {
>  		pr_debug("failed to mmap event: %d (%s)\n", errno,
>  			 str_error_r(errno, sbuf, sizeof(sbuf)));
> diff --git a/tools/perf/tests/switch-tracking.c b/tools/perf/tests/switch-
> tracking.c
> index 2acd785..9fee2a0 100644
> --- a/tools/perf/tests/switch-tracking.c
> +++ b/tools/perf/tests/switch-tracking.c
> @@ -448,7 +448,7 @@ int test__switch_tracking(struct test *test
> __maybe_unused, int subtest __maybe_
>  		goto out;
>  	}
> 
> -	err = perf_evlist__mmap(evlist, UINT_MAX, false);
> +	err = perf_evlist__mmap(evlist, UINT_MAX);
>  	if (err) {
>  		pr_debug("perf_evlist__mmap failed!\n");
>  		goto out_err;
> diff --git a/tools/perf/tests/task-exit.c b/tools/perf/tests/task-exit.c
> index 4fb6609..4ba5a27 100644
> --- a/tools/perf/tests/task-exit.c
> +++ b/tools/perf/tests/task-exit.c
> @@ -96,7 +96,7 @@ int test__task_exit(struct test *test __maybe_unused,
> int subtest __maybe_unused
>  		goto out_delete_evlist;
>  	}
> 
> -	if (perf_evlist__mmap(evlist, 128, false) < 0) {
> +	if (perf_evlist__mmap(evlist, 128) < 0) {
>  		pr_debug("failed to mmap events: %d (%s)\n", errno,
>  			 str_error_r(errno, sbuf, sizeof(sbuf)));
>  		goto out_delete_evlist;
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index 4c5daba..4948d3d 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -711,7 +711,7 @@ union perf_event
> *perf_evlist__mmap_read_forward(struct perf_evlist *evlist, int
>  	 * No need for read-write ring buffer: kernel stop outputting when
>  	 * it hit md->prev (perf_mmap__consume()).
>  	 */
> -	return perf_mmap__read_forward(md, evlist->overwrite);
> +	return perf_mmap__read_forward(md, false);

If we remove the 'overwrite' in forward, I think 'check_messup' is useless. 
It's better to remove it as well.

>  }
> 
>  union perf_event *perf_evlist__mmap_read_backward(struct perf_evlist
> *evlist, int idx)
> @@ -738,7 +738,7 @@ void perf_evlist__mmap_read_catchup(struct
> perf_evlist *evlist, int idx)
> 
>  void perf_evlist__mmap_consume(struct perf_evlist *evlist, int idx)
>  {
> -	perf_mmap__consume(&evlist->mmap[idx], evlist->overwrite);
> +	perf_mmap__consume(&evlist->mmap[idx], false);

I think we still need to remove 'overwrite' in the implementation of
perf_mmap__consume.


There are too many generic functions are changed in this patch.
I think it's better to split into several patches.
It's better to have one generic function changed in one patch.

Thanks,
Kan

>  }
> 
>  static void perf_evlist__munmap_nofree(struct perf_evlist *evlist)
> @@ -1058,14 +1058,14 @@ int perf_evlist__parse_mmap_pages(const
> struct option *opt, const char *str,
>   * Return: %0 on success, negative error code otherwise.
>   */
>  int perf_evlist__mmap_ex(struct perf_evlist *evlist, unsigned int pages,
> -			 bool overwrite, unsigned int auxtrace_pages,
> +			 unsigned int auxtrace_pages,
>  			 bool auxtrace_overwrite)
>  {
>  	struct perf_evsel *evsel;
>  	const struct cpu_map *cpus = evlist->cpus;
>  	const struct thread_map *threads = evlist->threads;
>  	struct mmap_params mp = {
> -		.prot = PROT_READ | (overwrite ? 0 : PROT_WRITE),
> +		.prot = PROT_READ | PROT_WRITE,
>  	};
> 
>  	if (!evlist->mmap)
> @@ -1076,7 +1076,6 @@ int perf_evlist__mmap_ex(struct perf_evlist *evlist,
> unsigned int pages,
>  	if (evlist->pollfd.entries == NULL && perf_evlist__alloc_pollfd(evlist) <
> 0)
>  		return -ENOMEM;
> 
> -	evlist->overwrite = overwrite;
>  	evlist->mmap_len = perf_evlist__mmap_size(pages);
>  	pr_debug("mmap size %zuB\n", evlist->mmap_len);
>  	mp.mask = evlist->mmap_len - page_size - 1;
> @@ -1097,10 +1096,9 @@ int perf_evlist__mmap_ex(struct perf_evlist
> *evlist, unsigned int pages,
>  	return perf_evlist__mmap_per_cpu(evlist, &mp);
>  }
> 
> -int perf_evlist__mmap(struct perf_evlist *evlist, unsigned int pages,
> -		      bool overwrite)
> +int perf_evlist__mmap(struct perf_evlist *evlist, unsigned int pages)
>  {
> -	return perf_evlist__mmap_ex(evlist, pages, overwrite, 0, false);
> +	return perf_evlist__mmap_ex(evlist, pages, 0, false);
>  }
> 
>  int perf_evlist__create_maps(struct perf_evlist *evlist, struct target *target)
> diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
> index 8c433e9..1dd3291 100644
> --- a/tools/perf/util/evlist.h
> +++ b/tools/perf/util/evlist.h
> @@ -30,7 +30,6 @@ struct perf_evlist {
>  	int		 nr_entries;
>  	int		 nr_groups;
>  	int		 nr_mmaps;
> -	bool		 overwrite;
>  	bool		 enabled;
>  	bool		 has_user_cpus;
>  	size_t		 mmap_len;
> @@ -168,10 +167,9 @@ int perf_evlist__parse_mmap_pages(const struct
> option *opt,
>  unsigned long perf_event_mlock_kb_in_pages(void);
> 
>  int perf_evlist__mmap_ex(struct perf_evlist *evlist, unsigned int pages,
> -			 bool overwrite, unsigned int auxtrace_pages,
> +			 unsigned int auxtrace_pages,
>  			 bool auxtrace_overwrite);
> -int perf_evlist__mmap(struct perf_evlist *evlist, unsigned int pages,
> -		      bool overwrite);
> +int perf_evlist__mmap(struct perf_evlist *evlist, unsigned int pages);
>  void perf_evlist__munmap(struct perf_evlist *evlist);
> 
>  size_t perf_evlist__mmap_size(unsigned long pages);
> diff --git a/tools/perf/util/mmap.c b/tools/perf/util/mmap.c
> index 9fe5f9c..703ed41 100644
> --- a/tools/perf/util/mmap.c
> +++ b/tools/perf/util/mmap.c
> @@ -299,7 +299,7 @@ static int rb_find_range(void *data, int mask, u64
> head, u64 old,
>  	return backward_rb_find_range(data, mask, head, start, end);
>  }
> 
> -int perf_mmap__push(struct perf_mmap *md, bool overwrite, bool
> backward,
> +int perf_mmap__push(struct perf_mmap *md, bool backward,
>  		    void *to, int push(void *to, void *buf, size_t size))
>  {
>  	u64 head = perf_mmap__read_head(md);
> @@ -321,7 +321,7 @@ int perf_mmap__push(struct perf_mmap *md, bool
> overwrite, bool backward,
>  		WARN_ONCE(1, "failed to keep up with mmap data. (warn
> only once)\n");
> 
>  		md->prev = head;
> -		perf_mmap__consume(md, overwrite || backward);
> +		perf_mmap__consume(md, backward);
>  		return 0;
>  	}
> 
> @@ -346,7 +346,7 @@ int perf_mmap__push(struct perf_mmap *md, bool
> overwrite, bool backward,
>  	}
> 
>  	md->prev = head;
> -	perf_mmap__consume(md, overwrite || backward);
> +	perf_mmap__consume(md, backward);
>  out:
>  	return rc;
>  }
> diff --git a/tools/perf/util/mmap.h b/tools/perf/util/mmap.h
> index efd78b8..2c3d291 100644
> --- a/tools/perf/util/mmap.h
> +++ b/tools/perf/util/mmap.h
> @@ -89,7 +89,7 @@ static inline void perf_mmap__write_tail(struct
> perf_mmap *md, u64 tail)
>  union perf_event *perf_mmap__read_forward(struct perf_mmap *map,
> bool check_messup);
>  union perf_event *perf_mmap__read_backward(struct perf_mmap *map);
> 
> -int perf_mmap__push(struct perf_mmap *md, bool overwrite, bool
> backward,
> +int perf_mmap__push(struct perf_mmap *md, bool backward,
>  		    void *to, int push(void *to, void *buf, size_t size));
> 
>  size_t perf_mmap__mmap_len(struct perf_mmap *map);
> diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> index c129e99..ece33b4 100644
> --- a/tools/perf/util/python.c
> +++ b/tools/perf/util/python.c
> @@ -856,14 +856,14 @@ static PyObject *pyrf_evlist__mmap(struct
> pyrf_evlist *pevlist,
>  				   PyObject *args, PyObject *kwargs)
>  {
>  	struct perf_evlist *evlist = &pevlist->evlist;
> -	static char *kwlist[] = { "pages", "overwrite", NULL };
> -	int pages = 128, overwrite = false;
> +	static char *kwlist[] = { "pages", NULL };
> +	int pages = 128;
> 
> -	if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|ii", kwlist,
> -					 &pages, &overwrite))
> +	if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|i", kwlist,
> +					 &pages))
>  		return NULL;
> 
> -	if (perf_evlist__mmap(evlist, pages, overwrite) < 0) {
> +	if (perf_evlist__mmap(evlist, pages) < 0) {
>  		PyErr_SetFromErrno(PyExc_OSError);
>  		return NULL;
>  	}
> --
> 2.10.1

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 2/7] perf tests: Set evlist of test__backward_ring_buffer() to !overwrite
  2017-11-13  1:38 ` [PATCH 2/7] perf tests: Set evlist of test__backward_ring_buffer() to !overwrite Wang Nan
@ 2017-11-13 18:30   ` Arnaldo Carvalho de Melo
  2017-11-18  8:30   ` [tip:perf/core] " tip-bot for Wang Nan
  1 sibling, 0 replies; 21+ messages in thread
From: Arnaldo Carvalho de Melo @ 2017-11-13 18:30 UTC (permalink / raw)
  To: Wang Nan; +Cc: linux-kernel, kan.liang, jolsa, namhyung

Em Mon, Nov 13, 2017 at 01:38:04AM +0000, Wang Nan escreveu:
> Setting overwrite in perf_evlist__mmap() is meaningless because the
> event in this evlist is already have 'overwrite' postfix and goes to
> backward ring buffer automatically. Pass 'false' to perf_evlist__mmap()
> to make it similar to others.

applied
 
> Signed-off-by: Wang Nan <wangnan0@huawei.com>
> ---
>  tools/perf/tests/backward-ring-buffer.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/perf/tests/backward-ring-buffer.c b/tools/perf/tests/backward-ring-buffer.c
> index d233ad3..992c917 100644
> --- a/tools/perf/tests/backward-ring-buffer.c
> +++ b/tools/perf/tests/backward-ring-buffer.c
> @@ -58,7 +58,7 @@ static int do_test(struct perf_evlist *evlist, int mmap_pages,
>  	int err;
>  	char sbuf[STRERR_BUFSIZE];
>  
> -	err = perf_evlist__mmap(evlist, mmap_pages, true);
> +	err = perf_evlist__mmap(evlist, mmap_pages, false);
>  	if (err < 0) {
>  		pr_debug("perf_evlist__mmap: %s\n",
>  			 str_error_r(errno, sbuf, sizeof(sbuf)));
> -- 
> 2.10.1

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 3/7] perf tests: Set evlist of test__sw_clock_freq() to !overwrite
  2017-11-13  1:38 ` [PATCH 3/7] perf tests: Set evlist of test__sw_clock_freq() " Wang Nan
@ 2017-11-13 18:31   ` Arnaldo Carvalho de Melo
  2017-11-18  8:30   ` [tip:perf/core] " tip-bot for Wang Nan
  1 sibling, 0 replies; 21+ messages in thread
From: Arnaldo Carvalho de Melo @ 2017-11-13 18:31 UTC (permalink / raw)
  To: Wang Nan; +Cc: linux-kernel, kan.liang, jolsa, namhyung

Em Mon, Nov 13, 2017 at 01:38:05AM +0000, Wang Nan escreveu:
> Unsetting overwrite when calling perf_evlist__mmap is harmless. This commit
> passes false to it, makes following commits eliminate the overwrite argument
> easier.

applied
 
> Signed-off-by: Wang Nan <wangnan0@huawei.com>
> ---
>  tools/perf/tests/sw-clock.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/perf/tests/sw-clock.c b/tools/perf/tests/sw-clock.c
> index d88511f..c468e6c 100644
> --- a/tools/perf/tests/sw-clock.c
> +++ b/tools/perf/tests/sw-clock.c
> @@ -77,7 +77,7 @@ static int __test__sw_clock_freq(enum perf_sw_ids clock_id)
>  		goto out_delete_evlist;
>  	}
>  
> -	err = perf_evlist__mmap(evlist, 128, true);
> +	err = perf_evlist__mmap(evlist, 128, false);
>  	if (err < 0) {
>  		pr_debug("failed to mmap event: %d (%s)\n", errno,
>  			 str_error_r(errno, sbuf, sizeof(sbuf)));
> -- 
> 2.10.1

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 4/7] perf tests: Set evlist of test__basic_mmap() to !overwrite
  2017-11-13  1:38 ` [PATCH 4/7] perf tests: Set evlist of test__basic_mmap() " Wang Nan
@ 2017-11-13 18:31   ` Arnaldo Carvalho de Melo
  2017-11-18  8:31   ` [tip:perf/core] " tip-bot for Wang Nan
  1 sibling, 0 replies; 21+ messages in thread
From: Arnaldo Carvalho de Melo @ 2017-11-13 18:31 UTC (permalink / raw)
  To: Wang Nan; +Cc: linux-kernel, kan.liang, jolsa, namhyung

Em Mon, Nov 13, 2017 at 01:38:06AM +0000, Wang Nan escreveu:
> In this test, a large ring buffer is required so all events can feed into,
> so overwrite or not is meaningless.
> 
> Change to !overwrite so following commits can remove this argument.

applied
 
> Signed-off-by: Wang Nan <wangnan0@huawei.com>
> ---
>  tools/perf/tests/mmap-basic.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/perf/tests/mmap-basic.c b/tools/perf/tests/mmap-basic.c
> index bc8a70e..667d696 100644
> --- a/tools/perf/tests/mmap-basic.c
> +++ b/tools/perf/tests/mmap-basic.c
> @@ -93,7 +93,7 @@ int test__basic_mmap(struct test *test __maybe_unused, int subtest __maybe_unuse
>  		expected_nr_events[i] = 1 + rand() % 127;
>  	}
>  
> -	if (perf_evlist__mmap(evlist, 128, true) < 0) {
> +	if (perf_evlist__mmap(evlist, 128, false) < 0) {
>  		pr_debug("failed to mmap events: %d (%s)\n", errno,
>  			 str_error_r(errno, sbuf, sizeof(sbuf)));
>  		goto out_delete_evlist;
> -- 
> 2.10.1

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 5/7] perf tests: Set evlist of test__task_exit() to !overwrite
  2017-11-13  1:38 ` [PATCH 5/7] perf tests: Set evlist of test__task_exit() " Wang Nan
@ 2017-11-13 18:32   ` Arnaldo Carvalho de Melo
  2017-11-18  8:31   ` [tip:perf/core] " tip-bot for Wang Nan
  1 sibling, 0 replies; 21+ messages in thread
From: Arnaldo Carvalho de Melo @ 2017-11-13 18:32 UTC (permalink / raw)
  To: Wang Nan; +Cc: linux-kernel, kan.liang, jolsa, namhyung

Em Mon, Nov 13, 2017 at 01:38:07AM +0000, Wang Nan escreveu:
> Changing ringbuffer to !overwrite in this task is harmless because
> this test uses a very low frequency (1) and using a very simple
> program (true). There should have only 3 events in the whole test.
> Overwriting is impossible to happen.

applied
 
> Signed-off-by: Wang Nan <wangnan0@huawei.com>
> ---
>  tools/perf/tests/task-exit.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/perf/tests/task-exit.c b/tools/perf/tests/task-exit.c
> index f0881d0..4fb6609 100644
> --- a/tools/perf/tests/task-exit.c
> +++ b/tools/perf/tests/task-exit.c
> @@ -96,7 +96,7 @@ int test__task_exit(struct test *test __maybe_unused, int subtest __maybe_unused
>  		goto out_delete_evlist;
>  	}
>  
> -	if (perf_evlist__mmap(evlist, 128, true) < 0) {
> +	if (perf_evlist__mmap(evlist, 128, false) < 0) {
>  		pr_debug("failed to mmap events: %d (%s)\n", errno,
>  			 str_error_r(errno, sbuf, sizeof(sbuf)));
>  		goto out_delete_evlist;
> -- 
> 2.10.1

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [tip:perf/core] perf tests: Set evlist of test__backward_ring_buffer() to !overwrite
  2017-11-13  1:38 ` [PATCH 2/7] perf tests: Set evlist of test__backward_ring_buffer() to !overwrite Wang Nan
  2017-11-13 18:30   ` Arnaldo Carvalho de Melo
@ 2017-11-18  8:30   ` tip-bot for Wang Nan
  1 sibling, 0 replies; 21+ messages in thread
From: tip-bot for Wang Nan @ 2017-11-18  8:30 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, jolsa, tglx, mingo, hpa, linux-kernel, kan.liang, wangnan0,
	namhyung

Commit-ID:  d492326f160e44e08fcf132a63163b36dd8e8839
Gitweb:     https://git.kernel.org/tip/d492326f160e44e08fcf132a63163b36dd8e8839
Author:     Wang Nan <wangnan0@huawei.com>
AuthorDate: Mon, 13 Nov 2017 01:38:04 +0000
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 16 Nov 2017 14:49:57 -0300

perf tests: Set evlist of test__backward_ring_buffer() to !overwrite

Setting overwrite in perf_evlist__mmap() is meaningless because the
event in this evlist is already have 'overwrite' postfix and goes to
backward ring buffer automatically. Pass 'false' to perf_evlist__mmap()
to make it similar to others.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Link: http://lkml.kernel.org/r/20171113013809.212417-3-wangnan0@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/tests/backward-ring-buffer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/tests/backward-ring-buffer.c b/tools/perf/tests/backward-ring-buffer.c
index 71b9a0b..43a8c6a 100644
--- a/tools/perf/tests/backward-ring-buffer.c
+++ b/tools/perf/tests/backward-ring-buffer.c
@@ -59,7 +59,7 @@ static int do_test(struct perf_evlist *evlist, int mmap_pages,
 	int err;
 	char sbuf[STRERR_BUFSIZE];
 
-	err = perf_evlist__mmap(evlist, mmap_pages, true);
+	err = perf_evlist__mmap(evlist, mmap_pages, false);
 	if (err < 0) {
 		pr_debug("perf_evlist__mmap: %s\n",
 			 str_error_r(errno, sbuf, sizeof(sbuf)));

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [tip:perf/core] perf tests: Set evlist of test__sw_clock_freq() to !overwrite
  2017-11-13  1:38 ` [PATCH 3/7] perf tests: Set evlist of test__sw_clock_freq() " Wang Nan
  2017-11-13 18:31   ` Arnaldo Carvalho de Melo
@ 2017-11-18  8:30   ` tip-bot for Wang Nan
  1 sibling, 0 replies; 21+ messages in thread
From: tip-bot for Wang Nan @ 2017-11-18  8:30 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: mingo, kan.liang, wangnan0, hpa, tglx, acme, namhyung, jolsa,
	linux-kernel

Commit-ID:  677b0601768881934f658bebb1713c3c843893fa
Gitweb:     https://git.kernel.org/tip/677b0601768881934f658bebb1713c3c843893fa
Author:     Wang Nan <wangnan0@huawei.com>
AuthorDate: Mon, 13 Nov 2017 01:38:05 +0000
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 16 Nov 2017 14:49:57 -0300

perf tests: Set evlist of test__sw_clock_freq() to !overwrite

Unsetting overwrite when calling perf_evlist__mmap is harmless. This
commit passes false to it, makes following commits eliminate the
overwrite argument easier.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Link: http://lkml.kernel.org/r/20171113013809.212417-4-wangnan0@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/tests/sw-clock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/tests/sw-clock.c b/tools/perf/tests/sw-clock.c
index 725a196..c6937ed 100644
--- a/tools/perf/tests/sw-clock.c
+++ b/tools/perf/tests/sw-clock.c
@@ -78,7 +78,7 @@ static int __test__sw_clock_freq(enum perf_sw_ids clock_id)
 		goto out_delete_evlist;
 	}
 
-	err = perf_evlist__mmap(evlist, 128, true);
+	err = perf_evlist__mmap(evlist, 128, false);
 	if (err < 0) {
 		pr_debug("failed to mmap event: %d (%s)\n", errno,
 			 str_error_r(errno, sbuf, sizeof(sbuf)));

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [tip:perf/core] perf tests: Set evlist of test__basic_mmap() to !overwrite
  2017-11-13  1:38 ` [PATCH 4/7] perf tests: Set evlist of test__basic_mmap() " Wang Nan
  2017-11-13 18:31   ` Arnaldo Carvalho de Melo
@ 2017-11-18  8:31   ` tip-bot for Wang Nan
  1 sibling, 0 replies; 21+ messages in thread
From: tip-bot for Wang Nan @ 2017-11-18  8:31 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: jolsa, acme, hpa, kan.liang, wangnan0, linux-kernel, mingo,
	namhyung, tglx

Commit-ID:  301d724aa19add1c0cf3ec8cad0d10151d30393f
Gitweb:     https://git.kernel.org/tip/301d724aa19add1c0cf3ec8cad0d10151d30393f
Author:     Wang Nan <wangnan0@huawei.com>
AuthorDate: Mon, 13 Nov 2017 01:38:06 +0000
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 16 Nov 2017 14:49:58 -0300

perf tests: Set evlist of test__basic_mmap() to !overwrite

In this test, a large ring buffer is required so all events can feed
into, so overwrite or not is meaningless.

Change to !overwrite so following commits can remove this argument.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Link: http://lkml.kernel.org/r/20171113013809.212417-5-wangnan0@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/tests/mmap-basic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/tests/mmap-basic.c b/tools/perf/tests/mmap-basic.c
index 5a8bf31..91f10d6 100644
--- a/tools/perf/tests/mmap-basic.c
+++ b/tools/perf/tests/mmap-basic.c
@@ -94,7 +94,7 @@ int test__basic_mmap(struct test *test __maybe_unused, int subtest __maybe_unuse
 		expected_nr_events[i] = 1 + rand() % 127;
 	}
 
-	if (perf_evlist__mmap(evlist, 128, true) < 0) {
+	if (perf_evlist__mmap(evlist, 128, false) < 0) {
 		pr_debug("failed to mmap events: %d (%s)\n", errno,
 			 str_error_r(errno, sbuf, sizeof(sbuf)));
 		goto out_delete_evlist;

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [tip:perf/core] perf tests: Set evlist of test__task_exit() to !overwrite
  2017-11-13  1:38 ` [PATCH 5/7] perf tests: Set evlist of test__task_exit() " Wang Nan
  2017-11-13 18:32   ` Arnaldo Carvalho de Melo
@ 2017-11-18  8:31   ` tip-bot for Wang Nan
  1 sibling, 0 replies; 21+ messages in thread
From: tip-bot for Wang Nan @ 2017-11-18  8:31 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, mingo, linux-kernel, wangnan0, kan.liang, namhyung, hpa,
	jolsa, tglx

Commit-ID:  a0e3dd79cdd8ad838cbcefeff530a15193f8336e
Gitweb:     https://git.kernel.org/tip/a0e3dd79cdd8ad838cbcefeff530a15193f8336e
Author:     Wang Nan <wangnan0@huawei.com>
AuthorDate: Mon, 13 Nov 2017 01:38:07 +0000
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 16 Nov 2017 14:49:58 -0300

perf tests: Set evlist of test__task_exit() to !overwrite

Changing ringbuffer to !overwrite in this task is harmless because
this test uses a very low frequency (1) and using a very simple program
(true). There should have only 3 events in the whole test.  Overwriting
is impossible to happen.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Link: http://lkml.kernel.org/r/20171113013809.212417-6-wangnan0@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/tests/task-exit.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/tests/task-exit.c b/tools/perf/tests/task-exit.c
index bc4a734..98c0984 100644
--- a/tools/perf/tests/task-exit.c
+++ b/tools/perf/tests/task-exit.c
@@ -97,7 +97,7 @@ int test__task_exit(struct test *test __maybe_unused, int subtest __maybe_unused
 		goto out_delete_evlist;
 	}
 
-	if (perf_evlist__mmap(evlist, 128, true) < 0) {
+	if (perf_evlist__mmap(evlist, 128, false) < 0) {
 		pr_debug("failed to mmap events: %d (%s)\n", errno,
 			 str_error_r(errno, sbuf, sizeof(sbuf)));
 		goto out_delete_evlist;

^ permalink raw reply related	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2017-11-18  8:32 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-13  1:38 [PATCH 0/7] perf tools: Clarify overwrite and backward, bugfix Wang Nan
2017-11-13  1:38 ` [PATCH 1/7] perf mmap: Fix perf backward recording Wang Nan
2017-11-13  1:38 ` [PATCH 2/7] perf tests: Set evlist of test__backward_ring_buffer() to !overwrite Wang Nan
2017-11-13 18:30   ` Arnaldo Carvalho de Melo
2017-11-18  8:30   ` [tip:perf/core] " tip-bot for Wang Nan
2017-11-13  1:38 ` [PATCH 3/7] perf tests: Set evlist of test__sw_clock_freq() " Wang Nan
2017-11-13 18:31   ` Arnaldo Carvalho de Melo
2017-11-18  8:30   ` [tip:perf/core] " tip-bot for Wang Nan
2017-11-13  1:38 ` [PATCH 4/7] perf tests: Set evlist of test__basic_mmap() " Wang Nan
2017-11-13 18:31   ` Arnaldo Carvalho de Melo
2017-11-18  8:31   ` [tip:perf/core] " tip-bot for Wang Nan
2017-11-13  1:38 ` [PATCH 5/7] perf tests: Set evlist of test__task_exit() " Wang Nan
2017-11-13 18:32   ` Arnaldo Carvalho de Melo
2017-11-18  8:31   ` [tip:perf/core] " tip-bot for Wang Nan
2017-11-13  1:38 ` [PATCH 6/7] perf tools: Remove 'overwrite' concept from code level Wang Nan
2017-11-13 11:52   ` Jiri Olsa
2017-11-13 15:07   ` Liang, Kan
2017-11-13  1:38 ` [PATCH 7/7] perf tools: Remove prot field in mmap param Wang Nan
2017-11-13 11:52   ` Jiri Olsa
2017-11-13 11:52   ` Jiri Olsa
2017-11-13 14:49 ` [PATCH 0/7] perf tools: Clarify overwrite and backward, bugfix Liang, Kan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.