* [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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).