All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/8] perf tools: perf tools: Clarify overwrite and backward, bugfix
@ 2017-12-03  2:00 Wang Nan
  2017-12-03  2:00 ` [PATCH v2 1/8] perf tools: Remove 'overwrite' parameter from perf_evlist__mmap Wang Nan
                   ` (8 more replies)
  0 siblings, 9 replies; 17+ messages in thread
From: Wang Nan @ 2017-12-03  2:00 UTC (permalink / raw)
  To: linux-kernel, kan.liang, acme, jolsa, namhyung
  Cc: Wang Nan, Arnaldo Carvalho de Melo, Jiri Olsa, Zhang Mengting

THe final result of this patchset is removing the concept of
'forward/backward', merge them into the concept of 'overwrite'.

Patch 1 to 5 clear arguments lists of many functions, remove the
'overwrite'. Because all callers of these functions doesn't need
the overwrite be set, we can simply remove them from arguments
lists and adjust code as if a 'false' is given.

Patch 6 fix a bug that forget to setting readonly for overwrite
ring buffers.

Patch 7 is suggested by Liang Kan, prevent dumpping duplicated
data if there's no so many events between two dumpping commands.

Patch 8 is 's/backward/overwrite'. After patch 8, the concept of
'backward' is removed from most of the code, make it uniform with
user interface ('--overwrite').

Cc: Kan Liang <kan.liang@intel.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Zhang Mengting <zhangmengting@huawei.com>

Wang Nan (8):
  perf tools: Remove 'overwrite' parameter from perf_evlist__mmap
  perf tools: Remove 'overwrite' parameter from perf_evlist__mmap_ex
  perf tools: Remove evlist->overwrite
  perf tools: Remove overwrite from arguments list of perf_mmap__push
  perf tools: Remove overwrite and check_messup from mmap read
  perf mmap: Fix perf backward recording
  perf tool: Don't discard prev in backward mode
  perf tools: Replace 'backward' to 'overwrite' in evlist, mmap and
    record

 tools/perf/arch/x86/tests/perf-time-to-tsc.c |  2 +-
 tools/perf/builtin-kvm.c                     |  2 +-
 tools/perf/builtin-record.c                  | 16 +++---
 tools/perf/builtin-top.c                     |  2 +-
 tools/perf/builtin-trace.c                   |  2 +-
 tools/perf/tests/backward-ring-buffer.c      |  6 +--
 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                     | 53 ++++++++++----------
 tools/perf/util/evlist.h                     |  8 ++-
 tools/perf/util/mmap.c                       | 73 ++++++++++------------------
 tools/perf/util/mmap.h                       |  4 +-
 tools/perf/util/python.c                     |  2 +-
 20 files changed, 81 insertions(+), 107 deletions(-)

-- 
2.10.1

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

* [PATCH v2 1/8] perf tools: Remove 'overwrite' parameter from perf_evlist__mmap
  2017-12-03  2:00 [PATCH v2 0/8] perf tools: perf tools: Clarify overwrite and backward, bugfix Wang Nan
@ 2017-12-03  2:00 ` Wang Nan
  2017-12-06 16:41   ` [tip:perf/core] perf evlist: " tip-bot for Wang Nan
  2017-12-03  2:00 ` [PATCH v2 2/8] perf tools: Remove 'overwrite' parameter from perf_evlist__mmap_ex Wang Nan
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 17+ messages in thread
From: Wang Nan @ 2017-12-03  2:00 UTC (permalink / raw)
  To: linux-kernel, kan.liang, acme, jolsa, namhyung; +Cc: Wang Nan

Now all perf_evlist__mmap's users doesn't set 'overwrite'. Remove it from
arguments list.

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-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                     | 5 ++---
 tools/perf/util/evlist.h                     | 3 +--
 tools/perf/util/python.c                     | 2 +-
 17 files changed, 18 insertions(+), 20 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 b59678e..06abe81 100644
--- a/tools/perf/arch/x86/tests/perf-time-to-tsc.c
+++ b/tools/perf/arch/x86/tests/perf-time-to-tsc.c
@@ -84,7 +84,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 597c7de..9885316 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -1044,7 +1044,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-top.c b/tools/perf/builtin-top.c
index 0077724..540461f 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -907,7 +907,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 84debdb..7c57898 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -2437,7 +2437,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 43a8c6a..cf37e43 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, 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 fcc8984..3bf7b14 100644
--- a/tools/perf/tests/code-reading.c
+++ b/tools/perf/tests/code-reading.c
@@ -639,7 +639,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 842d336..c465309 100644
--- a/tools/perf/tests/keep-tracking.c
+++ b/tools/perf/tests/keep-tracking.c
@@ -95,7 +95,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 91f10d6..c0e971d 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, 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 d9619d2..97c9407 100644
--- a/tools/perf/tests/openat-syscall-tp-fields.c
+++ b/tools/perf/tests/openat-syscall-tp-fields.c
@@ -64,7 +64,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 c34904d..0afafab 100644
--- a/tools/perf/tests/perf-record.c
+++ b/tools/perf/tests/perf-record.c
@@ -141,7 +141,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 c6937ed..f6c72f9 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, 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 7d3f4bf..33e0029 100644
--- a/tools/perf/tests/switch-tracking.c
+++ b/tools/perf/tests/switch-tracking.c
@@ -449,7 +449,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 5d06ac8..01b62b8 100644
--- a/tools/perf/tests/task-exit.c
+++ b/tools/perf/tests/task-exit.c
@@ -101,7 +101,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 199bb82..3c1778b 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -1091,10 +1091,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, false, 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 4e8131d..f0f2c8b 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -171,8 +171,7 @@ 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,
 			 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/python.c b/tools/perf/util/python.c
index 8e49d9c..b1e999b 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -864,7 +864,7 @@ static PyObject *pyrf_evlist__mmap(struct pyrf_evlist *pevlist,
 					 &pages, &overwrite))
 		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] 17+ messages in thread

* [PATCH v2 2/8] perf tools: Remove 'overwrite' parameter from perf_evlist__mmap_ex
  2017-12-03  2:00 [PATCH v2 0/8] perf tools: perf tools: Clarify overwrite and backward, bugfix Wang Nan
  2017-12-03  2:00 ` [PATCH v2 1/8] perf tools: Remove 'overwrite' parameter from perf_evlist__mmap Wang Nan
@ 2017-12-03  2:00 ` Wang Nan
  2017-12-06 16:41   ` [tip:perf/core] perf evlist: " tip-bot for Wang Nan
  2017-12-03  2:00 ` [PATCH v2 3/8] perf tools: Remove evlist->overwrite Wang Nan
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 17+ messages in thread
From: Wang Nan @ 2017-12-03  2:00 UTC (permalink / raw)
  To: linux-kernel, kan.liang, acme, jolsa, namhyung; +Cc: Wang Nan

All users of perf_evlist__mmap_ex set !overwrite. Remove it from its
arguments list.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
---
 tools/perf/builtin-record.c | 2 +-
 tools/perf/util/evlist.c    | 8 ++++----
 tools/perf/util/evlist.h    | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index e304bc4..08070f8 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -301,7 +301,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) {
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 3c1778b..93272d9 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -1052,14 +1052,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)
@@ -1070,7 +1070,7 @@ 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->overwrite = false;
 	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;
@@ -1093,7 +1093,7 @@ int perf_evlist__mmap_ex(struct perf_evlist *evlist, unsigned int pages,
 
 int perf_evlist__mmap(struct perf_evlist *evlist, unsigned int pages)
 {
-	return perf_evlist__mmap_ex(evlist, pages, false, 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 f0f2c8b..424a3d6 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -169,7 +169,7 @@ 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);
 void perf_evlist__munmap(struct perf_evlist *evlist);
-- 
2.10.1

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

* [PATCH v2 3/8] perf tools: Remove evlist->overwrite
  2017-12-03  2:00 [PATCH v2 0/8] perf tools: perf tools: Clarify overwrite and backward, bugfix Wang Nan
  2017-12-03  2:00 ` [PATCH v2 1/8] perf tools: Remove 'overwrite' parameter from perf_evlist__mmap Wang Nan
  2017-12-03  2:00 ` [PATCH v2 2/8] perf tools: Remove 'overwrite' parameter from perf_evlist__mmap_ex Wang Nan
@ 2017-12-03  2:00 ` Wang Nan
  2017-12-06 16:42   ` [tip:perf/core] perf evlist: " tip-bot for Wang Nan
  2017-12-03  2:00 ` [PATCH v2 4/8] perf tools: Remove overwrite from arguments list of perf_mmap__push Wang Nan
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 17+ messages in thread
From: Wang Nan @ 2017-12-03  2:00 UTC (permalink / raw)
  To: linux-kernel, kan.liang, acme, jolsa, namhyung; +Cc: Wang Nan

evlist->overwrite is set to false in all users. It can be removed.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
---
 tools/perf/builtin-record.c | 2 +-
 tools/perf/util/evlist.c    | 5 ++---
 tools/perf/util/evlist.h    | 1 -
 3 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 08070f8..3bc6cee 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -500,7 +500,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], false, backward, rec, record__pushfn) != 0) {
 				rc = -1;
 				goto out;
 			}
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 93272d9..a59134f 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)
@@ -1070,7 +1070,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 = false;
 	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;
diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
index 424a3d6..eec3377 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -31,7 +31,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;
-- 
2.10.1

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

* [PATCH v2 4/8] perf tools: Remove overwrite from arguments list of perf_mmap__push
  2017-12-03  2:00 [PATCH v2 0/8] perf tools: perf tools: Clarify overwrite and backward, bugfix Wang Nan
                   ` (2 preceding siblings ...)
  2017-12-03  2:00 ` [PATCH v2 3/8] perf tools: Remove evlist->overwrite Wang Nan
@ 2017-12-03  2:00 ` Wang Nan
  2017-12-06 16:42   ` [tip:perf/core] perf mmap: " tip-bot for Wang Nan
  2017-12-03  2:00 ` [PATCH v2 5/8] perf tools: Remove overwrite and check_messup from mmap read Wang Nan
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 17+ messages in thread
From: Wang Nan @ 2017-12-03  2:00 UTC (permalink / raw)
  To: linux-kernel, kan.liang, acme, jolsa, namhyung; +Cc: Wang Nan

'overwrite' argument is always 'false'. Revmove it from arguments
list of perf_mmap__push.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
---
 tools/perf/builtin-record.c | 2 +-
 tools/perf/util/mmap.c      | 6 +++---
 tools/perf/util/mmap.h      | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 3bc6cee..26b8571 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -500,7 +500,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], false, 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/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);
-- 
2.10.1

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

* [PATCH v2 5/8] perf tools: Remove overwrite and check_messup from mmap read
  2017-12-03  2:00 [PATCH v2 0/8] perf tools: perf tools: Clarify overwrite and backward, bugfix Wang Nan
                   ` (3 preceding siblings ...)
  2017-12-03  2:00 ` [PATCH v2 4/8] perf tools: Remove overwrite from arguments list of perf_mmap__push Wang Nan
@ 2017-12-03  2:00 ` Wang Nan
  2017-12-06 16:42   ` [tip:perf/core] perf mmap: " tip-bot for Wang Nan
  2017-12-03  2:00 ` [PATCH v2 6/8] perf mmap: Fix perf backward recording Wang Nan
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 17+ messages in thread
From: Wang Nan @ 2017-12-03  2:00 UTC (permalink / raw)
  To: linux-kernel, kan.liang, acme, jolsa, namhyung; +Cc: Wang Nan

All perf_mmap__read_forward() read from read-write ring buffer,
so no need check_messup. Reading from backward ring buffer doesn't
require check_messup because it never mess up. Cleanup arguments
lists.

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

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index a59134f..68c1f95 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, false);
+	return perf_mmap__read_forward(md);
 }
 
 union perf_event *perf_evlist__mmap_read_backward(struct perf_evlist *evlist, int idx)
diff --git a/tools/perf/util/mmap.c b/tools/perf/util/mmap.c
index 703ed41..3f262e7 100644
--- a/tools/perf/util/mmap.c
+++ b/tools/perf/util/mmap.c
@@ -21,33 +21,13 @@ size_t perf_mmap__mmap_len(struct perf_mmap *map)
 }
 
 /* When check_messup is true, 'end' must points to a good entry */
-static union perf_event *perf_mmap__read(struct perf_mmap *map, bool check_messup,
+static union perf_event *perf_mmap__read(struct perf_mmap *map,
 					 u64 start, u64 end, u64 *prev)
 {
 	unsigned char *data = map->base + page_size;
 	union perf_event *event = NULL;
 	int diff = end - start;
 
-	if (check_messup) {
-		/*
-		 * If we're further behind than half the buffer, there's a chance
-		 * the writer will bite our tail and mess up the samples under us.
-		 *
-		 * If we somehow ended up ahead of the 'end', we got messed up.
-		 *
-		 * In either case, truncate and restart at 'end'.
-		 */
-		if (diff > map->mask / 2 || diff < 0) {
-			fprintf(stderr, "WARNING: failed to keep up with mmap data.\n");
-
-			/*
-			 * 'end' points to a known good entry, start there.
-			 */
-			start = end;
-			diff = 0;
-		}
-	}
-
 	if (diff >= (int)sizeof(event->header)) {
 		size_t size;
 
@@ -89,7 +69,7 @@ static union perf_event *perf_mmap__read(struct perf_mmap *map, bool check_messu
 	return event;
 }
 
-union perf_event *perf_mmap__read_forward(struct perf_mmap *map, bool check_messup)
+union perf_event *perf_mmap__read_forward(struct perf_mmap *map)
 {
 	u64 head;
 	u64 old = map->prev;
@@ -102,7 +82,7 @@ union perf_event *perf_mmap__read_forward(struct perf_mmap *map, bool check_mess
 
 	head = perf_mmap__read_head(map);
 
-	return perf_mmap__read(map, check_messup, old, head, &map->prev);
+	return perf_mmap__read(map, old, head, &map->prev);
 }
 
 union perf_event *perf_mmap__read_backward(struct perf_mmap *map)
@@ -138,7 +118,7 @@ union perf_event *perf_mmap__read_backward(struct perf_mmap *map)
 	else
 		end = head + map->mask + 1;
 
-	return perf_mmap__read(map, false, start, end, &map->prev);
+	return perf_mmap__read(map, start, end, &map->prev);
 }
 
 void perf_mmap__read_catchup(struct perf_mmap *map)
diff --git a/tools/perf/util/mmap.h b/tools/perf/util/mmap.h
index 2c3d291..d640273 100644
--- a/tools/perf/util/mmap.h
+++ b/tools/perf/util/mmap.h
@@ -86,7 +86,7 @@ static inline void perf_mmap__write_tail(struct perf_mmap *md, u64 tail)
 	pc->data_tail = tail;
 }
 
-union perf_event *perf_mmap__read_forward(struct perf_mmap *map, bool check_messup);
+union perf_event *perf_mmap__read_forward(struct perf_mmap *map);
 union perf_event *perf_mmap__read_backward(struct perf_mmap *map);
 
 int perf_mmap__push(struct perf_mmap *md, bool backward,
-- 
2.10.1

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

* [PATCH v2 6/8] perf mmap: Fix perf backward recording
  2017-12-03  2:00 [PATCH v2 0/8] perf tools: perf tools: Clarify overwrite and backward, bugfix Wang Nan
                   ` (4 preceding siblings ...)
  2017-12-03  2:00 ` [PATCH v2 5/8] perf tools: Remove overwrite and check_messup from mmap read Wang Nan
@ 2017-12-03  2:00 ` Wang Nan
  2017-12-04  5:37   ` Namhyung Kim
  2017-12-03  2:00 ` [PATCH v2 7/8] perf tools: Don't discard prev in backward mode Wang Nan
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 17+ messages in thread
From: Wang Nan @ 2017-12-03  2:00 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 | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 68c1f95..bb70aef 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -799,7 +799,7 @@ 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;
@@ -824,6 +824,7 @@ 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);
 			}
+			mp.prot &= ~PROT_WRITE;
 		}
 
 		if (evsel->system_wide && thread)
@@ -838,7 +839,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, *output)  < 0)
 				return -1;
 		} else {
 			if (ioctl(fd, PERF_EVENT_IOC_SET_OUTPUT, *output) != 0)
@@ -890,7 +891,7 @@ static int perf_evlist__mmap_per_cpu(struct perf_evlist *evlist,
 					      true);
 
 		for (thread = 0; thread < nr_threads; thread++) {
-			if (perf_evlist__mmap_per_evsel(evlist, cpu, mp, cpu,
+			if (perf_evlist__mmap_per_evsel(evlist, cpu, *mp, cpu,
 							thread, &output, &output_backward))
 				goto out_unmap;
 		}
@@ -917,7 +918,7 @@ static int perf_evlist__mmap_per_thread(struct perf_evlist *evlist,
 		auxtrace_mmap_params__set_idx(&mp->auxtrace_mp, evlist, thread,
 					      false);
 
-		if (perf_evlist__mmap_per_evsel(evlist, thread, mp, 0, thread,
+		if (perf_evlist__mmap_per_evsel(evlist, thread, *mp, 0, thread,
 						&output, &output_backward))
 			goto out_unmap;
 	}
-- 
2.10.1

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

* [PATCH v2 7/8] perf tools: Don't discard prev in backward mode
  2017-12-03  2:00 [PATCH v2 0/8] perf tools: perf tools: Clarify overwrite and backward, bugfix Wang Nan
                   ` (5 preceding siblings ...)
  2017-12-03  2:00 ` [PATCH v2 6/8] perf mmap: Fix perf backward recording Wang Nan
@ 2017-12-03  2:00 ` Wang Nan
  2017-12-03  2:00 ` [PATCH v2 8/8] perf tools: Replace 'backward' to 'overwrite' in evlist, mmap and record Wang Nan
  2017-12-04  5:39 ` [PATCH v2 0/8] perf tools: perf tools: Clarify overwrite and backward, bugfix Namhyung Kim
  8 siblings, 0 replies; 17+ messages in thread
From: Wang Nan @ 2017-12-03  2:00 UTC (permalink / raw)
  To: linux-kernel, kan.liang, acme, jolsa, namhyung; +Cc: Wang Nan

Perf record can switch output. The new output should only store the
data after switching. However, in overwrite backward mode, the new
output still have the data from old output. That also brings extra
overhead.

At the end of mmap_read, the position of processed ring buffer is
saved in md->prev. Next mmap_read should be end in md->prev if it is
not overwriten. That avoids to process duplicate data.
However, the md->prev is discarded. So next mmap_read has to process
whole valid ring buffer, which probably include the old processed
data.

Avoid calling backward_rb_find_range() when md->prev is still
available.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Tested-by: Kan Liang <kan.liang@intel.com>
---
 tools/perf/util/mmap.c | 33 +++++++++++++++------------------
 1 file changed, 15 insertions(+), 18 deletions(-)

diff --git a/tools/perf/util/mmap.c b/tools/perf/util/mmap.c
index 3f262e7..5f8cb15 100644
--- a/tools/perf/util/mmap.c
+++ b/tools/perf/util/mmap.c
@@ -267,18 +267,6 @@ static int backward_rb_find_range(void *buf, int mask, u64 head, u64 *start, u64
 	return -1;
 }
 
-static int rb_find_range(void *data, int mask, u64 head, u64 old,
-			 u64 *start, u64 *end, bool backward)
-{
-	if (!backward) {
-		*start = old;
-		*end = head;
-		return 0;
-	}
-
-	return backward_rb_find_range(data, mask, head, start, end);
-}
-
 int perf_mmap__push(struct perf_mmap *md, bool backward,
 		    void *to, int push(void *to, void *buf, size_t size))
 {
@@ -290,19 +278,28 @@ int perf_mmap__push(struct perf_mmap *md, bool backward,
 	void *buf;
 	int rc = 0;
 
-	if (rb_find_range(data, md->mask, head, old, &start, &end, backward))
-		return -1;
+	start = backward ? head : old;
+	end = backward ? old : head;
 
 	if (start == end)
 		return 0;
 
 	size = end - start;
 	if (size > (unsigned long)(md->mask) + 1) {
-		WARN_ONCE(1, "failed to keep up with mmap data. (warn only once)\n");
+		if (!backward) {
+			WARN_ONCE(1, "failed to keep up with mmap data. (warn only once)\n");
 
-		md->prev = head;
-		perf_mmap__consume(md, backward);
-		return 0;
+			md->prev = head;
+			perf_mmap__consume(md, backward);
+			return 0;
+		}
+
+		/*
+		 * Backward ring buffer is full. We still have a chance to read
+		 * most of data from it.
+		 */
+		if (backward_rb_find_range(data, md->mask, head, &start, &end))
+			return -1;
 	}
 
 	if ((start & md->mask) + size != (end & md->mask)) {
-- 
2.10.1

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

* [PATCH v2 8/8] perf tools: Replace 'backward' to 'overwrite' in evlist, mmap and record
  2017-12-03  2:00 [PATCH v2 0/8] perf tools: perf tools: Clarify overwrite and backward, bugfix Wang Nan
                   ` (6 preceding siblings ...)
  2017-12-03  2:00 ` [PATCH v2 7/8] perf tools: Don't discard prev in backward mode Wang Nan
@ 2017-12-03  2:00 ` Wang Nan
  2017-12-04  5:39 ` [PATCH v2 0/8] perf tools: perf tools: Clarify overwrite and backward, bugfix Namhyung Kim
  8 siblings, 0 replies; 17+ messages in thread
From: Wang Nan @ 2017-12-03  2:00 UTC (permalink / raw)
  To: linux-kernel, kan.liang, acme, jolsa, namhyung; +Cc: Wang Nan

Remove the backward/forward concept to make it uniform with user
interface (the '--overwrite' option).

Signed-off-by: Wang Nan <wangnan0@huawei.com>
---
 tools/perf/builtin-record.c             | 14 +++++++-------
 tools/perf/tests/backward-ring-buffer.c |  4 ++--
 tools/perf/util/evlist.c                | 30 +++++++++++++++---------------
 tools/perf/util/evlist.h                |  2 +-
 tools/perf/util/mmap.c                  | 22 +++++++++++-----------
 5 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 26b8571..0a5749e 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -479,7 +479,7 @@ static struct perf_event_header finished_round_event = {
 };
 
 static int record__mmap_read_evlist(struct record *rec, struct perf_evlist *evlist,
-				    bool backward)
+				    bool overwrite)
 {
 	u64 bytes_written = rec->bytes_written;
 	int i;
@@ -489,18 +489,18 @@ static int record__mmap_read_evlist(struct record *rec, struct perf_evlist *evli
 	if (!evlist)
 		return 0;
 
-	maps = backward ? evlist->backward_mmap : evlist->mmap;
+	maps = overwrite ? evlist->overwrite_mmap : evlist->mmap;
 	if (!maps)
 		return 0;
 
-	if (backward && evlist->bkw_mmap_state != BKW_MMAP_DATA_PENDING)
+	if (overwrite && evlist->bkw_mmap_state != BKW_MMAP_DATA_PENDING)
 		return 0;
 
 	for (i = 0; i < evlist->nr_mmaps; i++) {
 		struct auxtrace_mmap *mm = &maps[i].auxtrace_mmap;
 
 		if (maps[i].base) {
-			if (perf_mmap__push(&maps[i], backward, rec, record__pushfn) != 0) {
+			if (perf_mmap__push(&maps[i], overwrite, rec, record__pushfn) != 0) {
 				rc = -1;
 				goto out;
 			}
@@ -520,7 +520,7 @@ static int record__mmap_read_evlist(struct record *rec, struct perf_evlist *evli
 	if (bytes_written != rec->bytes_written)
 		rc = record__write(rec, &finished_round_event, sizeof(finished_round_event));
 
-	if (backward)
+	if (overwrite)
 		perf_evlist__toggle_bkw_mmap(evlist, BKW_MMAP_EMPTY);
 out:
 	return rc;
@@ -692,8 +692,8 @@ perf_evlist__pick_pc(struct perf_evlist *evlist)
 	if (evlist) {
 		if (evlist->mmap && evlist->mmap[0].base)
 			return evlist->mmap[0].base;
-		if (evlist->backward_mmap && evlist->backward_mmap[0].base)
-			return evlist->backward_mmap[0].base;
+		if (evlist->overwrite_mmap && evlist->overwrite_mmap[0].base)
+			return evlist->overwrite_mmap[0].base;
 	}
 	return NULL;
 }
diff --git a/tools/perf/tests/backward-ring-buffer.c b/tools/perf/tests/backward-ring-buffer.c
index cf37e43..4035d43 100644
--- a/tools/perf/tests/backward-ring-buffer.c
+++ b/tools/perf/tests/backward-ring-buffer.c
@@ -33,8 +33,8 @@ static int count_samples(struct perf_evlist *evlist, int *sample_count,
 	for (i = 0; i < evlist->nr_mmaps; i++) {
 		union perf_event *event;
 
-		perf_mmap__read_catchup(&evlist->backward_mmap[i]);
-		while ((event = perf_mmap__read_backward(&evlist->backward_mmap[i])) != NULL) {
+		perf_mmap__read_catchup(&evlist->overwrite_mmap[i]);
+		while ((event = perf_mmap__read_backward(&evlist->overwrite_mmap[i])) != NULL) {
 			const u32 type = event->header.type;
 
 			switch (type) {
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index bb70aef..2774528a 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -125,7 +125,7 @@ static void perf_evlist__purge(struct perf_evlist *evlist)
 void perf_evlist__exit(struct perf_evlist *evlist)
 {
 	zfree(&evlist->mmap);
-	zfree(&evlist->backward_mmap);
+	zfree(&evlist->overwrite_mmap);
 	fdarray__exit(&evlist->pollfd);
 }
 
@@ -675,11 +675,11 @@ static int perf_evlist__set_paused(struct perf_evlist *evlist, bool value)
 {
 	int i;
 
-	if (!evlist->backward_mmap)
+	if (!evlist->overwrite_mmap)
 		return 0;
 
 	for (i = 0; i < evlist->nr_mmaps; i++) {
-		int fd = evlist->backward_mmap[i].fd;
+		int fd = evlist->overwrite_mmap[i].fd;
 		int err;
 
 		if (fd < 0)
@@ -749,16 +749,16 @@ static void perf_evlist__munmap_nofree(struct perf_evlist *evlist)
 		for (i = 0; i < evlist->nr_mmaps; i++)
 			perf_mmap__munmap(&evlist->mmap[i]);
 
-	if (evlist->backward_mmap)
+	if (evlist->overwrite_mmap)
 		for (i = 0; i < evlist->nr_mmaps; i++)
-			perf_mmap__munmap(&evlist->backward_mmap[i]);
+			perf_mmap__munmap(&evlist->overwrite_mmap[i]);
 }
 
 void perf_evlist__munmap(struct perf_evlist *evlist)
 {
 	perf_evlist__munmap_nofree(evlist);
 	zfree(&evlist->mmap);
-	zfree(&evlist->backward_mmap);
+	zfree(&evlist->overwrite_mmap);
 }
 
 static struct perf_mmap *perf_evlist__alloc_mmap(struct perf_evlist *evlist)
@@ -800,7 +800,7 @@ 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,
-				       int thread, int *_output, int *_output_backward)
+				       int thread, int *_output, int *_output_overwrite)
 {
 	struct perf_evsel *evsel;
 	int revent;
@@ -813,14 +813,14 @@ static int perf_evlist__mmap_per_evsel(struct perf_evlist *evlist, int idx,
 		int cpu;
 
 		if (evsel->attr.write_backward) {
-			output = _output_backward;
-			maps = evlist->backward_mmap;
+			output = _output_overwrite;
+			maps = evlist->overwrite_mmap;
 
 			if (!maps) {
 				maps = perf_evlist__alloc_mmap(evlist);
 				if (!maps)
 					return -1;
-				evlist->backward_mmap = maps;
+				evlist->overwrite_mmap = maps;
 				if (evlist->bkw_mmap_state == BKW_MMAP_NOTREADY)
 					perf_evlist__toggle_bkw_mmap(evlist, BKW_MMAP_RUNNING);
 			}
@@ -885,14 +885,14 @@ static int perf_evlist__mmap_per_cpu(struct perf_evlist *evlist,
 	pr_debug2("perf event ring buffer mmapped per cpu\n");
 	for (cpu = 0; cpu < nr_cpus; cpu++) {
 		int output = -1;
-		int output_backward = -1;
+		int output_overwrite = -1;
 
 		auxtrace_mmap_params__set_idx(&mp->auxtrace_mp, evlist, cpu,
 					      true);
 
 		for (thread = 0; thread < nr_threads; thread++) {
 			if (perf_evlist__mmap_per_evsel(evlist, cpu, *mp, cpu,
-							thread, &output, &output_backward))
+							thread, &output, &output_overwrite))
 				goto out_unmap;
 		}
 	}
@@ -913,13 +913,13 @@ static int perf_evlist__mmap_per_thread(struct perf_evlist *evlist,
 	pr_debug2("perf event ring buffer mmapped per thread\n");
 	for (thread = 0; thread < nr_threads; thread++) {
 		int output = -1;
-		int output_backward = -1;
+		int output_overwrite = -1;
 
 		auxtrace_mmap_params__set_idx(&mp->auxtrace_mp, evlist, thread,
 					      false);
 
 		if (perf_evlist__mmap_per_evsel(evlist, thread, *mp, 0, thread,
-						&output, &output_backward))
+						&output, &output_overwrite))
 			goto out_unmap;
 	}
 
@@ -1749,7 +1749,7 @@ void perf_evlist__toggle_bkw_mmap(struct perf_evlist *evlist,
 		RESUME,
 	} action = NONE;
 
-	if (!evlist->backward_mmap)
+	if (!evlist->overwrite_mmap)
 		return;
 
 	switch (old_state) {
diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
index eec3377..7516066 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -44,7 +44,7 @@ struct perf_evlist {
 	} workload;
 	struct fdarray	 pollfd;
 	struct perf_mmap *mmap;
-	struct perf_mmap *backward_mmap;
+	struct perf_mmap *overwrite_mmap;
 	struct thread_map *threads;
 	struct cpu_map	  *cpus;
 	struct perf_evsel *selected;
diff --git a/tools/perf/util/mmap.c b/tools/perf/util/mmap.c
index 5f8cb15..05076e6 100644
--- a/tools/perf/util/mmap.c
+++ b/tools/perf/util/mmap.c
@@ -234,18 +234,18 @@ int perf_mmap__mmap(struct perf_mmap *map, struct mmap_params *mp, int fd)
 	return 0;
 }
 
-static int backward_rb_find_range(void *buf, int mask, u64 head, u64 *start, u64 *end)
+static int overwrite_rb_find_range(void *buf, int mask, u64 head, u64 *start, u64 *end)
 {
 	struct perf_event_header *pheader;
 	u64 evt_head = head;
 	int size = mask + 1;
 
-	pr_debug2("backward_rb_find_range: buf=%p, head=%"PRIx64"\n", buf, head);
+	pr_debug2("overwrite_rb_find_range: buf=%p, head=%"PRIx64"\n", buf, head);
 	pheader = (struct perf_event_header *)(buf + (head & mask));
 	*start = head;
 	while (true) {
 		if (evt_head - head >= (unsigned int)size) {
-			pr_debug("Finished reading backward ring buffer: rewind\n");
+			pr_debug("Finished reading overwrite ring buffer: rewind\n");
 			if (evt_head - head > (unsigned int)size)
 				evt_head -= pheader->size;
 			*end = evt_head;
@@ -255,7 +255,7 @@ static int backward_rb_find_range(void *buf, int mask, u64 head, u64 *start, u64
 		pheader = (struct perf_event_header *)(buf + (evt_head & mask));
 
 		if (pheader->size == 0) {
-			pr_debug("Finished reading backward ring buffer: get start\n");
+			pr_debug("Finished reading overwrite ring buffer: get start\n");
 			*end = evt_head;
 			return 0;
 		}
@@ -267,7 +267,7 @@ static int backward_rb_find_range(void *buf, int mask, u64 head, u64 *start, u64
 	return -1;
 }
 
-int perf_mmap__push(struct perf_mmap *md, bool backward,
+int perf_mmap__push(struct perf_mmap *md, bool overwrite,
 		    void *to, int push(void *to, void *buf, size_t size))
 {
 	u64 head = perf_mmap__read_head(md);
@@ -278,19 +278,19 @@ int perf_mmap__push(struct perf_mmap *md, bool backward,
 	void *buf;
 	int rc = 0;
 
-	start = backward ? head : old;
-	end = backward ? old : head;
+	start = overwrite ? head : old;
+	end = overwrite ? old : head;
 
 	if (start == end)
 		return 0;
 
 	size = end - start;
 	if (size > (unsigned long)(md->mask) + 1) {
-		if (!backward) {
+		if (!overwrite) {
 			WARN_ONCE(1, "failed to keep up with mmap data. (warn only once)\n");
 
 			md->prev = head;
-			perf_mmap__consume(md, backward);
+			perf_mmap__consume(md, overwrite);
 			return 0;
 		}
 
@@ -298,7 +298,7 @@ int perf_mmap__push(struct perf_mmap *md, bool backward,
 		 * Backward ring buffer is full. We still have a chance to read
 		 * most of data from it.
 		 */
-		if (backward_rb_find_range(data, md->mask, head, &start, &end))
+		if (overwrite_rb_find_range(data, md->mask, head, &start, &end))
 			return -1;
 	}
 
@@ -323,7 +323,7 @@ int perf_mmap__push(struct perf_mmap *md, bool backward,
 	}
 
 	md->prev = head;
-	perf_mmap__consume(md, backward);
+	perf_mmap__consume(md, overwrite);
 out:
 	return rc;
 }
-- 
2.10.1

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

* Re: [PATCH v2 6/8] perf mmap: Fix perf backward recording
  2017-12-03  2:00 ` [PATCH v2 6/8] perf mmap: Fix perf backward recording Wang Nan
@ 2017-12-04  5:37   ` Namhyung Kim
  2017-12-04 15:44     ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 17+ messages in thread
From: Namhyung Kim @ 2017-12-04  5:37 UTC (permalink / raw)
  To: Wang Nan; +Cc: linux-kernel, kan.liang, acme, jolsa, kernel-team

Hi,

On Sun, Dec 03, 2017 at 02:00:42AM +0000, Wang Nan wrote:
> 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 | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index 68c1f95..bb70aef 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -799,7 +799,7 @@ 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;
> @@ -824,6 +824,7 @@ 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);
>  			}
> +			mp.prot &= ~PROT_WRITE;
>  		}
>  
>  		if (evsel->system_wide && thread)
> @@ -838,7 +839,7 @@ static int perf_evlist__mmap_per_evsel(struct perf_evlist *evlist, int idx,
>  		if (*output == -1) {
>  			*output = fd;
>

I still prefer updating mp->prot based on evsel->attr.write_backward
here (instead of passing mp as value).  It'd create a minimal change.

Thanks,
Namhyung


> -			if (perf_mmap__mmap(&maps[idx], mp, *output)  < 0)
> +			if (perf_mmap__mmap(&maps[idx], &mp, *output)  < 0)
>  				return -1;
>  		} else {
>  			if (ioctl(fd, PERF_EVENT_IOC_SET_OUTPUT, *output) != 0)
> @@ -890,7 +891,7 @@ static int perf_evlist__mmap_per_cpu(struct perf_evlist *evlist,
>  					      true);
>  
>  		for (thread = 0; thread < nr_threads; thread++) {
> -			if (perf_evlist__mmap_per_evsel(evlist, cpu, mp, cpu,
> +			if (perf_evlist__mmap_per_evsel(evlist, cpu, *mp, cpu,
>  							thread, &output, &output_backward))
>  				goto out_unmap;
>  		}
> @@ -917,7 +918,7 @@ static int perf_evlist__mmap_per_thread(struct perf_evlist *evlist,
>  		auxtrace_mmap_params__set_idx(&mp->auxtrace_mp, evlist, thread,
>  					      false);
>  
> -		if (perf_evlist__mmap_per_evsel(evlist, thread, mp, 0, thread,
> +		if (perf_evlist__mmap_per_evsel(evlist, thread, *mp, 0, thread,
>  						&output, &output_backward))
>  			goto out_unmap;
>  	}
> -- 
> 2.10.1
> 

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

* Re: [PATCH v2 0/8] perf tools: perf tools: Clarify overwrite and backward, bugfix
  2017-12-03  2:00 [PATCH v2 0/8] perf tools: perf tools: Clarify overwrite and backward, bugfix Wang Nan
                   ` (7 preceding siblings ...)
  2017-12-03  2:00 ` [PATCH v2 8/8] perf tools: Replace 'backward' to 'overwrite' in evlist, mmap and record Wang Nan
@ 2017-12-04  5:39 ` Namhyung Kim
  8 siblings, 0 replies; 17+ messages in thread
From: Namhyung Kim @ 2017-12-04  5:39 UTC (permalink / raw)
  To: Wang Nan
  Cc: linux-kernel, kan.liang, acme, jolsa, Arnaldo Carvalho de Melo,
	Jiri Olsa, Zhang Mengting, kernel-team

On Sun, Dec 03, 2017 at 02:00:36AM +0000, Wang Nan wrote:
> THe final result of this patchset is removing the concept of
> 'forward/backward', merge them into the concept of 'overwrite'.
> 
> Patch 1 to 5 clear arguments lists of many functions, remove the
> 'overwrite'. Because all callers of these functions doesn't need
> the overwrite be set, we can simply remove them from arguments
> lists and adjust code as if a 'false' is given.
> 
> Patch 6 fix a bug that forget to setting readonly for overwrite
> ring buffers.
> 
> Patch 7 is suggested by Liang Kan, prevent dumpping duplicated
> data if there's no so many events between two dumpping commands.
> 
> Patch 8 is 's/backward/overwrite'. After patch 8, the concept of
> 'backward' is removed from most of the code, make it uniform with
> user interface ('--overwrite').
> 
> Cc: Kan Liang <kan.liang@intel.com>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Zhang Mengting <zhangmengting@huawei.com>

I only have a nitpick on the patch 6, but otherwise

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung


> 
> Wang Nan (8):
>   perf tools: Remove 'overwrite' parameter from perf_evlist__mmap
>   perf tools: Remove 'overwrite' parameter from perf_evlist__mmap_ex
>   perf tools: Remove evlist->overwrite
>   perf tools: Remove overwrite from arguments list of perf_mmap__push
>   perf tools: Remove overwrite and check_messup from mmap read
>   perf mmap: Fix perf backward recording
>   perf tool: Don't discard prev in backward mode
>   perf tools: Replace 'backward' to 'overwrite' in evlist, mmap and
>     record
> 
>  tools/perf/arch/x86/tests/perf-time-to-tsc.c |  2 +-
>  tools/perf/builtin-kvm.c                     |  2 +-
>  tools/perf/builtin-record.c                  | 16 +++---
>  tools/perf/builtin-top.c                     |  2 +-
>  tools/perf/builtin-trace.c                   |  2 +-
>  tools/perf/tests/backward-ring-buffer.c      |  6 +--
>  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                     | 53 ++++++++++----------
>  tools/perf/util/evlist.h                     |  8 ++-
>  tools/perf/util/mmap.c                       | 73 ++++++++++------------------
>  tools/perf/util/mmap.h                       |  4 +-
>  tools/perf/util/python.c                     |  2 +-
>  20 files changed, 81 insertions(+), 107 deletions(-)
> 
> -- 
> 2.10.1
> 

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

* Re: [PATCH v2 6/8] perf mmap: Fix perf backward recording
  2017-12-04  5:37   ` Namhyung Kim
@ 2017-12-04 15:44     ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 17+ messages in thread
From: Arnaldo Carvalho de Melo @ 2017-12-04 15:44 UTC (permalink / raw)
  To: Namhyung Kim; +Cc: Wang Nan, linux-kernel, kan.liang, jolsa, kernel-team

Em Mon, Dec 04, 2017 at 02:37:44PM +0900, Namhyung Kim escreveu:
> Hi,
> 
> On Sun, Dec 03, 2017 at 02:00:42AM +0000, Wang Nan wrote:
> > 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 | 9 +++++----
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> > index 68c1f95..bb70aef 100644
> > --- a/tools/perf/util/evlist.c
> > +++ b/tools/perf/util/evlist.c
> > @@ -799,7 +799,7 @@ 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;
> > @@ -824,6 +824,7 @@ 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);
> >  			}
> > +			mp.prot &= ~PROT_WRITE;
> >  		}
> >  
> >  		if (evsel->system_wide && thread)
> > @@ -838,7 +839,7 @@ static int perf_evlist__mmap_per_evsel(struct perf_evlist *evlist, int idx,
> >  		if (*output == -1) {
> >  			*output = fd;
> >
> 
> I still prefer updating mp->prot based on evsel->attr.write_backward
> here (instead of passing mp as value).  It'd create a minimal change.

Wang? I applied 1-5 in this series, waiting now for this to settle,

- Arnaldo

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

* [tip:perf/core] perf evlist: Remove 'overwrite' parameter from perf_evlist__mmap
  2017-12-03  2:00 ` [PATCH v2 1/8] perf tools: Remove 'overwrite' parameter from perf_evlist__mmap Wang Nan
@ 2017-12-06 16:41   ` tip-bot for Wang Nan
  0 siblings, 0 replies; 17+ messages in thread
From: tip-bot for Wang Nan @ 2017-12-06 16:41 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: kan.liang, linux-kernel, tglx, acme, namhyung, hpa, mingo,
	wangnan0, jolsa

Commit-ID:  f74b9d3a1ac2b9c3ae1475f474ca0e6644746fbf
Gitweb:     https://git.kernel.org/tip/f74b9d3a1ac2b9c3ae1475f474ca0e6644746fbf
Author:     Wang Nan <wangnan0@huawei.com>
AuthorDate: Sun, 3 Dec 2017 02:00:37 +0000
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 5 Dec 2017 15:43:53 -0300

perf evlist: Remove 'overwrite' parameter from perf_evlist__mmap

Now all perf_evlist__mmap's users doesn't set 'overwrite'. Remove it
from arguments list.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Kan Liang <kan.liang@intel.com>
Link: http://lkml.kernel.org/r/20171203020044.81680-2-wangnan0@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/arch/x86/tests/perf-time-to-tsc.c | 2 +-
 tools/perf/builtin-kvm.c                     | 2 +-
 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                     | 5 ++---
 tools/perf/util/evlist.h                     | 3 +--
 tools/perf/util/python.c                     | 2 +-
 17 files changed, 18 insertions(+), 20 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 b59678e..06abe81 100644
--- a/tools/perf/arch/x86/tests/perf-time-to-tsc.c
+++ b/tools/perf/arch/x86/tests/perf-time-to-tsc.c
@@ -84,7 +84,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 597c7de..9885316 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -1044,7 +1044,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-top.c b/tools/perf/builtin-top.c
index 0077724..540461f 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -907,7 +907,7 @@ try_again:
 		}
 	}
 
-	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 84debdb..7c57898 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -2437,7 +2437,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 43a8c6a..cf37e43 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, 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 fcc8984..3bf7b14 100644
--- a/tools/perf/tests/code-reading.c
+++ b/tools/perf/tests/code-reading.c
@@ -639,7 +639,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 842d336..c465309 100644
--- a/tools/perf/tests/keep-tracking.c
+++ b/tools/perf/tests/keep-tracking.c
@@ -95,7 +95,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 91f10d6..c0e971d 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, 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 d9619d2..97c9407 100644
--- a/tools/perf/tests/openat-syscall-tp-fields.c
+++ b/tools/perf/tests/openat-syscall-tp-fields.c
@@ -64,7 +64,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 c34904d..0afafab 100644
--- a/tools/perf/tests/perf-record.c
+++ b/tools/perf/tests/perf-record.c
@@ -141,7 +141,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 c6937ed..f6c72f9 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, 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 7d3f4bf..33e0029 100644
--- a/tools/perf/tests/switch-tracking.c
+++ b/tools/perf/tests/switch-tracking.c
@@ -449,7 +449,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 5d06ac8..01b62b8 100644
--- a/tools/perf/tests/task-exit.c
+++ b/tools/perf/tests/task-exit.c
@@ -101,7 +101,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 199bb82..3c1778b 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -1091,10 +1091,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, false, 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 4e8131d..f0f2c8b 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -171,8 +171,7 @@ 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,
 			 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/python.c b/tools/perf/util/python.c
index 8e49d9c..b1e999b 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -864,7 +864,7 @@ static PyObject *pyrf_evlist__mmap(struct pyrf_evlist *pevlist,
 					 &pages, &overwrite))
 		return NULL;
 
-	if (perf_evlist__mmap(evlist, pages, overwrite) < 0) {
+	if (perf_evlist__mmap(evlist, pages) < 0) {
 		PyErr_SetFromErrno(PyExc_OSError);
 		return NULL;
 	}

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

* [tip:perf/core] perf evlist: Remove 'overwrite' parameter from perf_evlist__mmap_ex
  2017-12-03  2:00 ` [PATCH v2 2/8] perf tools: Remove 'overwrite' parameter from perf_evlist__mmap_ex Wang Nan
@ 2017-12-06 16:41   ` tip-bot for Wang Nan
  0 siblings, 0 replies; 17+ messages in thread
From: tip-bot for Wang Nan @ 2017-12-06 16:41 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, namhyung, jolsa, hpa, kan.liang, tglx, mingo, wangnan0,
	linux-kernel

Commit-ID:  7a276ff6c3202697c3c15cad757dec3bb07d14bf
Gitweb:     https://git.kernel.org/tip/7a276ff6c3202697c3c15cad757dec3bb07d14bf
Author:     Wang Nan <wangnan0@huawei.com>
AuthorDate: Sun, 3 Dec 2017 02:00:38 +0000
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 5 Dec 2017 15:43:53 -0300

perf evlist: Remove 'overwrite' parameter from perf_evlist__mmap_ex

All users of perf_evlist__mmap_ex set !overwrite. Remove it from its
arguments list.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Kan Liang <kan.liang@intel.com>
Link: http://lkml.kernel.org/r/20171203020044.81680-3-wangnan0@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-record.c | 2 +-
 tools/perf/util/evlist.c    | 8 ++++----
 tools/perf/util/evlist.h    | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index e304bc4..08070f8 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -301,7 +301,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) {
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 3c1778b..93272d9 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -1052,14 +1052,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)
@@ -1070,7 +1070,7 @@ 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->overwrite = false;
 	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;
@@ -1093,7 +1093,7 @@ int perf_evlist__mmap_ex(struct perf_evlist *evlist, unsigned int pages,
 
 int perf_evlist__mmap(struct perf_evlist *evlist, unsigned int pages)
 {
-	return perf_evlist__mmap_ex(evlist, pages, false, 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 f0f2c8b..424a3d6 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -169,7 +169,7 @@ 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);
 void perf_evlist__munmap(struct perf_evlist *evlist);

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

* [tip:perf/core] perf evlist: Remove evlist->overwrite
  2017-12-03  2:00 ` [PATCH v2 3/8] perf tools: Remove evlist->overwrite Wang Nan
@ 2017-12-06 16:42   ` tip-bot for Wang Nan
  0 siblings, 0 replies; 17+ messages in thread
From: tip-bot for Wang Nan @ 2017-12-06 16:42 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: hpa, wangnan0, tglx, namhyung, jolsa, acme, mingo, kan.liang,
	linux-kernel

Commit-ID:  144b9a4fc53039c09007b71a06640560a6e62140
Gitweb:     https://git.kernel.org/tip/144b9a4fc53039c09007b71a06640560a6e62140
Author:     Wang Nan <wangnan0@huawei.com>
AuthorDate: Sun, 3 Dec 2017 02:00:39 +0000
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 5 Dec 2017 15:43:54 -0300

perf evlist: Remove evlist->overwrite

evlist->overwrite is set to false in all users. It can be removed.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Kan Liang <kan.liang@intel.com>
Link: http://lkml.kernel.org/r/20171203020044.81680-4-wangnan0@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-record.c | 2 +-
 tools/perf/util/evlist.c    | 5 ++---
 tools/perf/util/evlist.h    | 1 -
 3 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 08070f8..3bc6cee 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -500,7 +500,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], false, backward, rec, record__pushfn) != 0) {
 				rc = -1;
 				goto out;
 			}
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 93272d9..a59134f 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)
@@ -1070,7 +1070,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 = false;
 	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;
diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
index 424a3d6..eec3377 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -31,7 +31,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;

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

* [tip:perf/core] perf mmap: Remove overwrite from arguments list of perf_mmap__push
  2017-12-03  2:00 ` [PATCH v2 4/8] perf tools: Remove overwrite from arguments list of perf_mmap__push Wang Nan
@ 2017-12-06 16:42   ` tip-bot for Wang Nan
  0 siblings, 0 replies; 17+ messages in thread
From: tip-bot for Wang Nan @ 2017-12-06 16:42 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: jolsa, acme, linux-kernel, mingo, tglx, namhyung, hpa, kan.liang,
	wangnan0

Commit-ID:  ca6a9a05391960be5e8161a59a9854b32325d901
Gitweb:     https://git.kernel.org/tip/ca6a9a05391960be5e8161a59a9854b32325d901
Author:     Wang Nan <wangnan0@huawei.com>
AuthorDate: Sun, 3 Dec 2017 02:00:40 +0000
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 5 Dec 2017 15:43:54 -0300

perf mmap: Remove overwrite from arguments list of perf_mmap__push

'overwrite' argument is always 'false'. Remove it from arguments list of
perf_mmap__push().

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Kan Liang <kan.liang@intel.com>
Link: http://lkml.kernel.org/r/20171203020044.81680-5-wangnan0@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-record.c | 2 +-
 tools/perf/util/mmap.c      | 6 +++---
 tools/perf/util/mmap.h      | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 3bc6cee..26b8571 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -500,7 +500,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], false, 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/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);

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

* [tip:perf/core] perf mmap: Remove overwrite and check_messup from mmap read
  2017-12-03  2:00 ` [PATCH v2 5/8] perf tools: Remove overwrite and check_messup from mmap read Wang Nan
@ 2017-12-06 16:42   ` tip-bot for Wang Nan
  0 siblings, 0 replies; 17+ messages in thread
From: tip-bot for Wang Nan @ 2017-12-06 16:42 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: jolsa, acme, kan.liang, mingo, namhyung, tglx, hpa, linux-kernel,
	wangnan0

Commit-ID:  8eb7a1fe31612fd3e8ae8042dd2ebaf7575504cb
Gitweb:     https://git.kernel.org/tip/8eb7a1fe31612fd3e8ae8042dd2ebaf7575504cb
Author:     Wang Nan <wangnan0@huawei.com>
AuthorDate: Sun, 3 Dec 2017 02:00:41 +0000
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 5 Dec 2017 15:43:54 -0300

perf mmap: Remove overwrite and check_messup from mmap read

All perf_mmap__read_forward() read from read-write ring buffer, so no
need check_messup. Reading from backward ring buffer doesn't require
check_messup because it never mess up. Cleanup arguments lists.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Kan Liang <kan.liang@intel.com>
Link: http://lkml.kernel.org/r/20171203020044.81680-6-wangnan0@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/evlist.c |  2 +-
 tools/perf/util/mmap.c   | 28 ++++------------------------
 tools/perf/util/mmap.h   |  2 +-
 3 files changed, 6 insertions(+), 26 deletions(-)

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index a59134f..68c1f95 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, false);
+	return perf_mmap__read_forward(md);
 }
 
 union perf_event *perf_evlist__mmap_read_backward(struct perf_evlist *evlist, int idx)
diff --git a/tools/perf/util/mmap.c b/tools/perf/util/mmap.c
index 703ed41..3f262e7 100644
--- a/tools/perf/util/mmap.c
+++ b/tools/perf/util/mmap.c
@@ -21,33 +21,13 @@ size_t perf_mmap__mmap_len(struct perf_mmap *map)
 }
 
 /* When check_messup is true, 'end' must points to a good entry */
-static union perf_event *perf_mmap__read(struct perf_mmap *map, bool check_messup,
+static union perf_event *perf_mmap__read(struct perf_mmap *map,
 					 u64 start, u64 end, u64 *prev)
 {
 	unsigned char *data = map->base + page_size;
 	union perf_event *event = NULL;
 	int diff = end - start;
 
-	if (check_messup) {
-		/*
-		 * If we're further behind than half the buffer, there's a chance
-		 * the writer will bite our tail and mess up the samples under us.
-		 *
-		 * If we somehow ended up ahead of the 'end', we got messed up.
-		 *
-		 * In either case, truncate and restart at 'end'.
-		 */
-		if (diff > map->mask / 2 || diff < 0) {
-			fprintf(stderr, "WARNING: failed to keep up with mmap data.\n");
-
-			/*
-			 * 'end' points to a known good entry, start there.
-			 */
-			start = end;
-			diff = 0;
-		}
-	}
-
 	if (diff >= (int)sizeof(event->header)) {
 		size_t size;
 
@@ -89,7 +69,7 @@ broken_event:
 	return event;
 }
 
-union perf_event *perf_mmap__read_forward(struct perf_mmap *map, bool check_messup)
+union perf_event *perf_mmap__read_forward(struct perf_mmap *map)
 {
 	u64 head;
 	u64 old = map->prev;
@@ -102,7 +82,7 @@ union perf_event *perf_mmap__read_forward(struct perf_mmap *map, bool check_mess
 
 	head = perf_mmap__read_head(map);
 
-	return perf_mmap__read(map, check_messup, old, head, &map->prev);
+	return perf_mmap__read(map, old, head, &map->prev);
 }
 
 union perf_event *perf_mmap__read_backward(struct perf_mmap *map)
@@ -138,7 +118,7 @@ union perf_event *perf_mmap__read_backward(struct perf_mmap *map)
 	else
 		end = head + map->mask + 1;
 
-	return perf_mmap__read(map, false, start, end, &map->prev);
+	return perf_mmap__read(map, start, end, &map->prev);
 }
 
 void perf_mmap__read_catchup(struct perf_mmap *map)
diff --git a/tools/perf/util/mmap.h b/tools/perf/util/mmap.h
index 2c3d291..d640273 100644
--- a/tools/perf/util/mmap.h
+++ b/tools/perf/util/mmap.h
@@ -86,7 +86,7 @@ static inline void perf_mmap__write_tail(struct perf_mmap *md, u64 tail)
 	pc->data_tail = tail;
 }
 
-union perf_event *perf_mmap__read_forward(struct perf_mmap *map, bool check_messup);
+union perf_event *perf_mmap__read_forward(struct perf_mmap *map);
 union perf_event *perf_mmap__read_backward(struct perf_mmap *map);
 
 int perf_mmap__push(struct perf_mmap *md, bool backward,

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

end of thread, other threads:[~2017-12-06 16:44 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-03  2:00 [PATCH v2 0/8] perf tools: perf tools: Clarify overwrite and backward, bugfix Wang Nan
2017-12-03  2:00 ` [PATCH v2 1/8] perf tools: Remove 'overwrite' parameter from perf_evlist__mmap Wang Nan
2017-12-06 16:41   ` [tip:perf/core] perf evlist: " tip-bot for Wang Nan
2017-12-03  2:00 ` [PATCH v2 2/8] perf tools: Remove 'overwrite' parameter from perf_evlist__mmap_ex Wang Nan
2017-12-06 16:41   ` [tip:perf/core] perf evlist: " tip-bot for Wang Nan
2017-12-03  2:00 ` [PATCH v2 3/8] perf tools: Remove evlist->overwrite Wang Nan
2017-12-06 16:42   ` [tip:perf/core] perf evlist: " tip-bot for Wang Nan
2017-12-03  2:00 ` [PATCH v2 4/8] perf tools: Remove overwrite from arguments list of perf_mmap__push Wang Nan
2017-12-06 16:42   ` [tip:perf/core] perf mmap: " tip-bot for Wang Nan
2017-12-03  2:00 ` [PATCH v2 5/8] perf tools: Remove overwrite and check_messup from mmap read Wang Nan
2017-12-06 16:42   ` [tip:perf/core] perf mmap: " tip-bot for Wang Nan
2017-12-03  2:00 ` [PATCH v2 6/8] perf mmap: Fix perf backward recording Wang Nan
2017-12-04  5:37   ` Namhyung Kim
2017-12-04 15:44     ` Arnaldo Carvalho de Melo
2017-12-03  2:00 ` [PATCH v2 7/8] perf tools: Don't discard prev in backward mode Wang Nan
2017-12-03  2:00 ` [PATCH v2 8/8] perf tools: Replace 'backward' to 'overwrite' in evlist, mmap and record Wang Nan
2017-12-04  5:39 ` [PATCH v2 0/8] perf tools: perf tools: Clarify overwrite and backward, bugfix Namhyung Kim

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.