linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/7] perf mmap: Store mmap scope and type in struct perf_mmap
@ 2018-03-05 19:10 kan.liang
  2018-03-05 19:10 ` [PATCH 2/7] perf mmap: Using the stored scope data in perf_mmap__push kan.liang
                   ` (6 more replies)
  0 siblings, 7 replies; 12+ messages in thread
From: kan.liang @ 2018-03-05 19:10 UTC (permalink / raw)
  To: acme, mingo, linux-kernel; +Cc: jolsa, namhyung, wangnan0, ak, Kan Liang

From: Kan Liang <kan.liang@linux.intel.com>

There are too many boilerplates for the perf_mmap__read*() interfaces.

Some of the data (e.g. 'start', 'end', 'overwrite') should be stored in
struct perf_mmap at initialization. They will be used later.

No functional change.

Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
---
 tools/perf/util/mmap.c | 11 ++++++++---
 tools/perf/util/mmap.h |  3 +++
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/mmap.c b/tools/perf/util/mmap.c
index 4f27c46..642b479 100644
--- a/tools/perf/util/mmap.c
+++ b/tools/perf/util/mmap.c
@@ -250,11 +250,14 @@ int perf_mmap__read_init(struct perf_mmap *md, bool overwrite,
 
 	*startp = overwrite ? head : old;
 	*endp = overwrite ? old : head;
+	md->overwrite = overwrite;
+	md->start = overwrite ? head : old;
+	md->end = overwrite ? old : head;
 
-	if (*startp == *endp)
+	if (md->start == md->end)
 		return -EAGAIN;
 
-	size = *endp - *startp;
+	size = md->end - md->start;
 	if (size > (unsigned long)(md->mask) + 1) {
 		if (!overwrite) {
 			WARN_ONCE(1, "failed to keep up with mmap data. (warn only once)\n");
@@ -268,8 +271,10 @@ int perf_mmap__read_init(struct perf_mmap *md, bool overwrite,
 		 * Backward ring buffer is full. We still have a chance to read
 		 * most of data from it.
 		 */
-		if (overwrite_rb_find_range(data, md->mask, head, startp, endp))
+		if (overwrite_rb_find_range(data, md->mask, head, &md->start, &md->end))
 			return -EINVAL;
+		*startp = md->start;
+		*endp = md->end;
 	}
 
 	return 0;
diff --git a/tools/perf/util/mmap.h b/tools/perf/util/mmap.h
index ec7d3a24..9359e93 100644
--- a/tools/perf/util/mmap.h
+++ b/tools/perf/util/mmap.h
@@ -20,6 +20,9 @@ struct perf_mmap {
 	int		 fd;
 	refcount_t	 refcnt;
 	u64		 prev;
+	u64		 start;
+	u64		 end;
+	bool		 overwrite;
 	struct auxtrace_mmap auxtrace_mmap;
 	char		 event_copy[PERF_SAMPLE_MAX_SIZE] __aligned(8);
 };
-- 
2.4.11

^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [PATCH 2/7] perf mmap: Using the stored scope data in perf_mmap__push
  2018-03-05 19:10 [PATCH 1/7] perf mmap: Store mmap scope and type in struct perf_mmap kan.liang
@ 2018-03-05 19:10 ` kan.liang
  2018-03-05 19:10 ` [PATCH 3/7] perf mmap: Using the stored data in perf_mmap__read_event kan.liang
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: kan.liang @ 2018-03-05 19:10 UTC (permalink / raw)
  To: acme, mingo, linux-kernel; +Cc: jolsa, namhyung, wangnan0, ak, Kan Liang

From: Kan Liang <kan.liang@linux.intel.com>

Using the 'start' and 'end' which are stored in struct perf_mmap to
replace the temporary 'start' and 'end'.
The temporary variables will be discarded later.

No functional change.

Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
---
 tools/perf/util/mmap.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/tools/perf/util/mmap.c b/tools/perf/util/mmap.c
index 642b479..4cb3614 100644
--- a/tools/perf/util/mmap.c
+++ b/tools/perf/util/mmap.c
@@ -294,12 +294,12 @@ int perf_mmap__push(struct perf_mmap *md, bool overwrite,
 	if (rc < 0)
 		return (rc == -EAGAIN) ? 0 : -1;
 
-	size = end - start;
+	size = md->end - md->start;
 
-	if ((start & md->mask) + size != (end & md->mask)) {
-		buf = &data[start & md->mask];
-		size = md->mask + 1 - (start & md->mask);
-		start += size;
+	if ((md->start & md->mask) + size != (md->end & md->mask)) {
+		buf = &data[md->start & md->mask];
+		size = md->mask + 1 - (md->start & md->mask);
+		md->start += size;
 
 		if (push(to, buf, size) < 0) {
 			rc = -1;
@@ -307,9 +307,9 @@ int perf_mmap__push(struct perf_mmap *md, bool overwrite,
 		}
 	}
 
-	buf = &data[start & md->mask];
-	size = end - start;
-	start += size;
+	buf = &data[md->start & md->mask];
+	size = md->end - md->start;
+	md->start += size;
 
 	if (push(to, buf, size) < 0) {
 		rc = -1;
-- 
2.4.11

^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [PATCH 3/7] perf mmap: Using the stored data in perf_mmap__read_event
  2018-03-05 19:10 [PATCH 1/7] perf mmap: Store mmap scope and type in struct perf_mmap kan.liang
  2018-03-05 19:10 ` [PATCH 2/7] perf mmap: Using the stored scope data in perf_mmap__push kan.liang
@ 2018-03-05 19:10 ` kan.liang
  2018-03-05 19:10 ` [PATCH 4/7] perf mmap: Using stored 'overwrite' in perf_mmap__consume kan.liang
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: kan.liang @ 2018-03-05 19:10 UTC (permalink / raw)
  To: acme, mingo, linux-kernel; +Cc: jolsa, namhyung, wangnan0, ak, Kan Liang

From: Kan Liang <kan.liang@linux.intel.com>

Using the 'start', 'end' and 'overwrite' which are stored in
struct perf_mmap to replace the parameters of perf_mmap__read_event().
The parameters will be discarded later.

No functional change.

Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
---
 tools/perf/util/mmap.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/tools/perf/util/mmap.c b/tools/perf/util/mmap.c
index 4cb3614..da9e68b 100644
--- a/tools/perf/util/mmap.c
+++ b/tools/perf/util/mmap.c
@@ -76,8 +76,8 @@ static union perf_event *perf_mmap__read(struct perf_mmap *map,
  * perf_mmap__read_done()
  */
 union perf_event *perf_mmap__read_event(struct perf_mmap *map,
-					bool overwrite,
-					u64 *startp, u64 end)
+					bool overwrite __maybe_unused,
+					u64 *startp, u64 end __maybe_unused)
 {
 	union perf_event *event;
 
@@ -91,13 +91,14 @@ union perf_event *perf_mmap__read_event(struct perf_mmap *map,
 		return NULL;
 
 	/* non-overwirte doesn't pause the ringbuffer */
-	if (!overwrite)
-		end = perf_mmap__read_head(map);
+	if (!map->overwrite)
+		map->end = perf_mmap__read_head(map);
 
-	event = perf_mmap__read(map, startp, end);
+	event = perf_mmap__read(map, &map->start, map->end);
+	*startp = map->start;
 
-	if (!overwrite)
-		map->prev = *startp;
+	if (!map->overwrite)
+		map->prev = map->start;
 
 	return event;
 }
-- 
2.4.11

^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [PATCH 4/7] perf mmap: Using stored 'overwrite' in perf_mmap__consume
  2018-03-05 19:10 [PATCH 1/7] perf mmap: Store mmap scope and type in struct perf_mmap kan.liang
  2018-03-05 19:10 ` [PATCH 2/7] perf mmap: Using the stored scope data in perf_mmap__push kan.liang
  2018-03-05 19:10 ` [PATCH 3/7] perf mmap: Using the stored data in perf_mmap__read_event kan.liang
@ 2018-03-05 19:10 ` kan.liang
  2018-03-05 19:10 ` [PATCH 5/7] perf tools: Refine perf_mmap__consume kan.liang
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: kan.liang @ 2018-03-05 19:10 UTC (permalink / raw)
  To: acme, mingo, linux-kernel; +Cc: jolsa, namhyung, wangnan0, ak, Kan Liang

From: Kan Liang <kan.liang@linux.intel.com>

The 'overwrite' is set at initialization. It will not be changed.
Using it to replace the parameter of perf_mmap__consume().
The parameters will be discarded later.

No functional change.

Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
---
 tools/perf/util/mmap.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/mmap.c b/tools/perf/util/mmap.c
index da9e68b..c4e41d2 100644
--- a/tools/perf/util/mmap.c
+++ b/tools/perf/util/mmap.c
@@ -121,9 +121,9 @@ void perf_mmap__put(struct perf_mmap *map)
 		perf_mmap__munmap(map);
 }
 
-void perf_mmap__consume(struct perf_mmap *map, bool overwrite)
+void perf_mmap__consume(struct perf_mmap *map, bool overwrite __maybe_unused)
 {
-	if (!overwrite) {
+	if (!map->overwrite) {
 		u64 old = map->prev;
 
 		perf_mmap__write_tail(map, old);
-- 
2.4.11

^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [PATCH 5/7] perf tools: Refine perf_mmap__consume
  2018-03-05 19:10 [PATCH 1/7] perf mmap: Store mmap scope and type in struct perf_mmap kan.liang
                   ` (2 preceding siblings ...)
  2018-03-05 19:10 ` [PATCH 4/7] perf mmap: Using stored 'overwrite' in perf_mmap__consume kan.liang
@ 2018-03-05 19:10 ` kan.liang
  2018-03-05 19:10 ` [PATCH 6/7] perf tools: Refine perf_mmap__read_event kan.liang
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: kan.liang @ 2018-03-05 19:10 UTC (permalink / raw)
  To: acme, mingo, linux-kernel; +Cc: jolsa, namhyung, wangnan0, ak, Kan Liang

From: Kan Liang <kan.liang@linux.intel.com>

It doesn't need to pass the 'overwrite' to perf_mmap__consume().
Discard the parameter.

Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
---
 tools/perf/arch/x86/tests/perf-time-to-tsc.c | 2 +-
 tools/perf/builtin-kvm.c                     | 4 ++--
 tools/perf/builtin-top.c                     | 2 +-
 tools/perf/builtin-trace.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/mmap.c                       | 6 +++---
 tools/perf/util/mmap.h                       | 2 +-
 tools/perf/util/python.c                     | 2 +-
 15 files changed, 18 insertions(+), 18 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 7f82d91..a9bc77d 100644
--- a/tools/perf/arch/x86/tests/perf-time-to-tsc.c
+++ b/tools/perf/arch/x86/tests/perf-time-to-tsc.c
@@ -134,7 +134,7 @@ int test__perf_time_to_tsc(struct test *test __maybe_unused, int subtest __maybe
 				comm2_time = sample.time;
 			}
 next_event:
-			perf_mmap__consume(md, false);
+			perf_mmap__consume(md);
 		}
 		perf_mmap__read_done(md);
 	}
diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
index d2703d3b..165c0446 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -760,7 +760,7 @@ static s64 perf_kvm__mmap_read_idx(struct perf_kvm_stat *kvm, int idx,
 	while ((event = perf_mmap__read_event(md, false, &start, end)) != NULL) {
 		err = perf_evlist__parse_sample_timestamp(evlist, event, &timestamp);
 		if (err) {
-			perf_mmap__consume(md, false);
+			perf_mmap__consume(md);
 			pr_err("Failed to parse sample\n");
 			return -1;
 		}
@@ -770,7 +770,7 @@ static s64 perf_kvm__mmap_read_idx(struct perf_kvm_stat *kvm, int idx,
 		 * FIXME: Here we can't consume the event, as perf_session__queue_event will
 		 *        point to it, and it'll get possibly overwritten by the kernel.
 		 */
-		perf_mmap__consume(md, false);
+		perf_mmap__consume(md);
 
 		if (err) {
 			pr_err("Failed to enqueue sample: %d\n", err);
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index bb4f9fa..11b4a41 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -879,7 +879,7 @@ static void perf_top__mmap_read_idx(struct perf_top *top, int idx)
 		} else
 			++session->evlist->stats.nr_unknown_events;
 next_event:
-		perf_mmap__consume(md, opts->overwrite);
+		perf_mmap__consume(md);
 	}
 
 	perf_mmap__read_done(md);
diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index 1a93deb..abc855d 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -2492,7 +2492,7 @@ static int trace__run(struct trace *trace, int argc, const char **argv)
 
 			trace__handle_event(trace, event, &sample);
 next_event:
-			perf_mmap__consume(md, false);
+			perf_mmap__consume(md);
 
 			if (interrupted)
 				goto out_disable;
diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c
index 03ed8c7..f7c199a 100644
--- a/tools/perf/tests/code-reading.c
+++ b/tools/perf/tests/code-reading.c
@@ -420,7 +420,7 @@ static int process_events(struct machine *machine, struct perf_evlist *evlist,
 
 		while ((event = perf_mmap__read_event(md, false, &start, end)) != NULL) {
 			ret = process_event(machine, evlist, event, state);
-			perf_mmap__consume(md, false);
+			perf_mmap__consume(md);
 			if (ret < 0)
 				return ret;
 		}
diff --git a/tools/perf/tests/keep-tracking.c b/tools/perf/tests/keep-tracking.c
index 4590d8f..1f1db59 100644
--- a/tools/perf/tests/keep-tracking.c
+++ b/tools/perf/tests/keep-tracking.c
@@ -42,7 +42,7 @@ static int find_comm(struct perf_evlist *evlist, const char *comm)
 			    (pid_t)event->comm.tid == getpid() &&
 			    strcmp(event->comm.comm, comm) == 0)
 				found += 1;
-			perf_mmap__consume(md, false);
+			perf_mmap__consume(md);
 		}
 		perf_mmap__read_done(md);
 	}
diff --git a/tools/perf/tests/mmap-basic.c b/tools/perf/tests/mmap-basic.c
index 44c58d6..f473e10 100644
--- a/tools/perf/tests/mmap-basic.c
+++ b/tools/perf/tests/mmap-basic.c
@@ -135,7 +135,7 @@ int test__basic_mmap(struct test *test __maybe_unused, int subtest __maybe_unuse
 			goto out_delete_evlist;
 		}
 		nr_events[evsel->idx]++;
-		perf_mmap__consume(md, false);
+		perf_mmap__consume(md);
 	}
 	perf_mmap__read_done(md);
 
diff --git a/tools/perf/tests/openat-syscall-tp-fields.c b/tools/perf/tests/openat-syscall-tp-fields.c
index 620b210..7837ae9 100644
--- a/tools/perf/tests/openat-syscall-tp-fields.c
+++ b/tools/perf/tests/openat-syscall-tp-fields.c
@@ -101,7 +101,7 @@ int test__syscall_openat_tp_fields(struct test *test __maybe_unused, int subtest
 				++nr_events;
 
 				if (type != PERF_RECORD_SAMPLE) {
-					perf_mmap__consume(md, false);
+					perf_mmap__consume(md);
 					continue;
 				}
 
diff --git a/tools/perf/tests/perf-record.c b/tools/perf/tests/perf-record.c
index 31f3f70..6ff5f99 100644
--- a/tools/perf/tests/perf-record.c
+++ b/tools/perf/tests/perf-record.c
@@ -272,7 +272,7 @@ int test__PERF_RECORD(struct test *test __maybe_unused, int subtest __maybe_unus
 					++errs;
 				}
 
-				perf_mmap__consume(md, false);
+				perf_mmap__consume(md);
 			}
 			perf_mmap__read_done(md);
 		}
diff --git a/tools/perf/tests/sw-clock.c b/tools/perf/tests/sw-clock.c
index e6320e2..b58297b 100644
--- a/tools/perf/tests/sw-clock.c
+++ b/tools/perf/tests/sw-clock.c
@@ -114,7 +114,7 @@ static int __test__sw_clock_freq(enum perf_sw_ids clock_id)
 		total_periods += sample.period;
 		nr_samples++;
 next_event:
-		perf_mmap__consume(md, false);
+		perf_mmap__consume(md);
 	}
 	perf_mmap__read_done(md);
 
diff --git a/tools/perf/tests/switch-tracking.c b/tools/perf/tests/switch-tracking.c
index 10c4dcd..dbf9e20 100644
--- a/tools/perf/tests/switch-tracking.c
+++ b/tools/perf/tests/switch-tracking.c
@@ -270,7 +270,7 @@ static int process_events(struct perf_evlist *evlist,
 		while ((event = perf_mmap__read_event(md, false, &start, end)) != NULL) {
 			cnt += 1;
 			ret = add_event(evlist, &events, event);
-			 perf_mmap__consume(md, false);
+			 perf_mmap__consume(md);
 			if (ret < 0)
 				goto out_free_nodes;
 		}
diff --git a/tools/perf/tests/task-exit.c b/tools/perf/tests/task-exit.c
index 02b0888..5aa2e68 100644
--- a/tools/perf/tests/task-exit.c
+++ b/tools/perf/tests/task-exit.c
@@ -120,7 +120,7 @@ int test__task_exit(struct test *test __maybe_unused, int subtest __maybe_unused
 		if (event->header.type == PERF_RECORD_EXIT)
 			nr_exit++;
 
-		perf_mmap__consume(md, false);
+		perf_mmap__consume(md);
 	}
 	perf_mmap__read_done(md);
 
diff --git a/tools/perf/util/mmap.c b/tools/perf/util/mmap.c
index c4e41d2..a56c66b 100644
--- a/tools/perf/util/mmap.c
+++ b/tools/perf/util/mmap.c
@@ -121,7 +121,7 @@ void perf_mmap__put(struct perf_mmap *map)
 		perf_mmap__munmap(map);
 }
 
-void perf_mmap__consume(struct perf_mmap *map, bool overwrite __maybe_unused)
+void perf_mmap__consume(struct perf_mmap *map)
 {
 	if (!map->overwrite) {
 		u64 old = map->prev;
@@ -264,7 +264,7 @@ int perf_mmap__read_init(struct perf_mmap *md, bool overwrite,
 			WARN_ONCE(1, "failed to keep up with mmap data. (warn only once)\n");
 
 			md->prev = head;
-			perf_mmap__consume(md, overwrite);
+			perf_mmap__consume(md);
 			return -EAGAIN;
 		}
 
@@ -318,7 +318,7 @@ int perf_mmap__push(struct perf_mmap *md, bool overwrite,
 	}
 
 	md->prev = head;
-	perf_mmap__consume(md, overwrite);
+	perf_mmap__consume(md);
 out:
 	return rc;
 }
diff --git a/tools/perf/util/mmap.h b/tools/perf/util/mmap.h
index 9359e93..d098b47 100644
--- a/tools/perf/util/mmap.h
+++ b/tools/perf/util/mmap.h
@@ -66,7 +66,7 @@ void perf_mmap__munmap(struct perf_mmap *map);
 void perf_mmap__get(struct perf_mmap *map);
 void perf_mmap__put(struct perf_mmap *map);
 
-void perf_mmap__consume(struct perf_mmap *map, bool overwrite);
+void perf_mmap__consume(struct perf_mmap *map);
 
 static inline u64 perf_mmap__read_head(struct perf_mmap *mm)
 {
diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
index 35fb5ef..ca077f8 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -1013,7 +1013,7 @@ static PyObject *pyrf_evlist__read_on_cpu(struct pyrf_evlist *pevlist,
 		err = perf_evsel__parse_sample(evsel, event, &pevent->sample);
 
 		/* Consume the even only after we parsed it out. */
-		perf_mmap__consume(md, false);
+		perf_mmap__consume(md);
 
 		if (err)
 			return PyErr_Format(PyExc_OSError,
-- 
2.4.11

^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [PATCH 6/7] perf tools: Refine perf_mmap__read_event
  2018-03-05 19:10 [PATCH 1/7] perf mmap: Store mmap scope and type in struct perf_mmap kan.liang
                   ` (3 preceding siblings ...)
  2018-03-05 19:10 ` [PATCH 5/7] perf tools: Refine perf_mmap__consume kan.liang
@ 2018-03-05 19:10 ` kan.liang
  2018-03-05 19:10 ` [PATCH 7/7] perf tools: Refine perf_mmap__read_init kan.liang
  2018-03-05 20:20 ` [PATCH 1/7] perf mmap: Store mmap scope and type in struct perf_mmap Arnaldo Carvalho de Melo
  6 siblings, 0 replies; 12+ messages in thread
From: kan.liang @ 2018-03-05 19:10 UTC (permalink / raw)
  To: acme, mingo, linux-kernel; +Cc: jolsa, namhyung, wangnan0, ak, Kan Liang

From: Kan Liang <kan.liang@linux.intel.com>

It doesn't need to pass the 'overwrite', 'start' and 'end' to
perf_mmap__read_event.
Discard the parameters.

Signed-off-by: Kan Liang <kan.liang@linux.intel.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/mmap.c                       | 8 +-------
 tools/perf/util/mmap.h                       | 4 +---
 tools/perf/util/python.c                     | 2 +-
 17 files changed, 17 insertions(+), 25 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 a9bc77d..17cf7fc 100644
--- a/tools/perf/arch/x86/tests/perf-time-to-tsc.c
+++ b/tools/perf/arch/x86/tests/perf-time-to-tsc.c
@@ -115,7 +115,7 @@ int test__perf_time_to_tsc(struct test *test __maybe_unused, int subtest __maybe
 		if (perf_mmap__read_init(md, false, &start, &end) < 0)
 			continue;
 
-		while ((event = perf_mmap__read_event(md, false, &start, end)) != NULL) {
+		while ((event = perf_mmap__read_event(md)) != NULL) {
 			struct perf_sample sample;
 
 			if (event->header.type != PERF_RECORD_COMM ||
diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
index 165c0446..e9f69b8 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -757,7 +757,7 @@ static s64 perf_kvm__mmap_read_idx(struct perf_kvm_stat *kvm, int idx,
 	if (err < 0)
 		return (err == -EAGAIN) ? 0 : -1;
 
-	while ((event = perf_mmap__read_event(md, false, &start, end)) != NULL) {
+	while ((event = perf_mmap__read_event(md)) != NULL) {
 		err = perf_evlist__parse_sample_timestamp(evlist, event, &timestamp);
 		if (err) {
 			perf_mmap__consume(md);
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 11b4a41..eb19cf9 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -824,7 +824,7 @@ static void perf_top__mmap_read_idx(struct perf_top *top, int idx)
 	if (perf_mmap__read_init(md, opts->overwrite, &start, &end) < 0)
 		return;
 
-	while ((event = perf_mmap__read_event(md, opts->overwrite, &start, end)) != NULL) {
+	while ((event = perf_mmap__read_event(md)) != NULL) {
 		ret = perf_evlist__parse_sample(evlist, event, &sample);
 		if (ret) {
 			pr_err("Can't parse sample, err = %d\n", ret);
diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index abc855d..c71ef7ba 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -2479,7 +2479,7 @@ static int trace__run(struct trace *trace, int argc, const char **argv)
 		if (perf_mmap__read_init(md, false, &start, &end) < 0)
 			continue;
 
-		while ((event = perf_mmap__read_event(md, false, &start, end)) != NULL) {
+		while ((event = perf_mmap__read_event(md)) != NULL) {
 			struct perf_sample sample;
 
 			++trace->nr_events;
diff --git a/tools/perf/tests/backward-ring-buffer.c b/tools/perf/tests/backward-ring-buffer.c
index e0b1b41..e0eae10 100644
--- a/tools/perf/tests/backward-ring-buffer.c
+++ b/tools/perf/tests/backward-ring-buffer.c
@@ -36,7 +36,7 @@ static int count_samples(struct perf_evlist *evlist, int *sample_count,
 		u64 start, end;
 
 		perf_mmap__read_init(map, true, &start, &end);
-		while ((event = perf_mmap__read_event(map, true, &start, end)) != NULL) {
+		while ((event = perf_mmap__read_event(map)) != NULL) {
 			const u32 type = event->header.type;
 
 			switch (type) {
diff --git a/tools/perf/tests/bpf.c b/tools/perf/tests/bpf.c
index 09c9c9f..384c20f 100644
--- a/tools/perf/tests/bpf.c
+++ b/tools/perf/tests/bpf.c
@@ -183,7 +183,7 @@ static int do_test(struct bpf_object *obj, int (*func)(void),
 		if (perf_mmap__read_init(md, false, &start, &end) < 0)
 			continue;
 
-		while ((event = perf_mmap__read_event(md, false, &start, end)) != NULL) {
+		while ((event = perf_mmap__read_event(md)) != NULL) {
 			const u32 type = event->header.type;
 
 			if (type == PERF_RECORD_SAMPLE)
diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c
index f7c199a..f791966 100644
--- a/tools/perf/tests/code-reading.c
+++ b/tools/perf/tests/code-reading.c
@@ -418,7 +418,7 @@ static int process_events(struct machine *machine, struct perf_evlist *evlist,
 		if (perf_mmap__read_init(md, false, &start, &end) < 0)
 			continue;
 
-		while ((event = perf_mmap__read_event(md, false, &start, end)) != NULL) {
+		while ((event = perf_mmap__read_event(md)) != NULL) {
 			ret = process_event(machine, evlist, event, state);
 			perf_mmap__consume(md);
 			if (ret < 0)
diff --git a/tools/perf/tests/keep-tracking.c b/tools/perf/tests/keep-tracking.c
index 1f1db59..ad477b7 100644
--- a/tools/perf/tests/keep-tracking.c
+++ b/tools/perf/tests/keep-tracking.c
@@ -36,7 +36,7 @@ static int find_comm(struct perf_evlist *evlist, const char *comm)
 		md = &evlist->mmap[i];
 		if (perf_mmap__read_init(md, false, &start, &end) < 0)
 			continue;
-		while ((event = perf_mmap__read_event(md, false, &start, end)) != NULL) {
+		while ((event = perf_mmap__read_event(md)) != NULL) {
 			if (event->header.type == PERF_RECORD_COMM &&
 			    (pid_t)event->comm.pid == getpid() &&
 			    (pid_t)event->comm.tid == getpid() &&
diff --git a/tools/perf/tests/mmap-basic.c b/tools/perf/tests/mmap-basic.c
index f473e10..7790eb3 100644
--- a/tools/perf/tests/mmap-basic.c
+++ b/tools/perf/tests/mmap-basic.c
@@ -112,7 +112,7 @@ int test__basic_mmap(struct test *test __maybe_unused, int subtest __maybe_unuse
 	if (perf_mmap__read_init(md, false, &start, &end) < 0)
 		goto out_init;
 
-	while ((event = perf_mmap__read_event(md, false, &start, end)) != NULL) {
+	while ((event = perf_mmap__read_event(md)) != NULL) {
 		struct perf_sample sample;
 
 		if (event->header.type != PERF_RECORD_SAMPLE) {
diff --git a/tools/perf/tests/openat-syscall-tp-fields.c b/tools/perf/tests/openat-syscall-tp-fields.c
index 7837ae9..b0be2da 100644
--- a/tools/perf/tests/openat-syscall-tp-fields.c
+++ b/tools/perf/tests/openat-syscall-tp-fields.c
@@ -93,7 +93,7 @@ int test__syscall_openat_tp_fields(struct test *test __maybe_unused, int subtest
 			if (perf_mmap__read_init(md, false, &start, &end) < 0)
 				continue;
 
-			while ((event = perf_mmap__read_event(md, false, &start, end)) != NULL) {
+			while ((event = perf_mmap__read_event(md)) != NULL) {
 				const u32 type = event->header.type;
 				int tp_flags;
 				struct perf_sample sample;
diff --git a/tools/perf/tests/perf-record.c b/tools/perf/tests/perf-record.c
index 6ff5f99..59be094 100644
--- a/tools/perf/tests/perf-record.c
+++ b/tools/perf/tests/perf-record.c
@@ -171,7 +171,7 @@ int test__PERF_RECORD(struct test *test __maybe_unused, int subtest __maybe_unus
 			if (perf_mmap__read_init(md, false, &start, &end) < 0)
 				continue;
 
-			while ((event = perf_mmap__read_event(md, false, &start, end)) != NULL) {
+			while ((event = perf_mmap__read_event(md)) != NULL) {
 				const u32 type = event->header.type;
 				const char *name = perf_event__name(type);
 
diff --git a/tools/perf/tests/sw-clock.c b/tools/perf/tests/sw-clock.c
index b58297b..403f2d9 100644
--- a/tools/perf/tests/sw-clock.c
+++ b/tools/perf/tests/sw-clock.c
@@ -99,7 +99,7 @@ static int __test__sw_clock_freq(enum perf_sw_ids clock_id)
 	if (perf_mmap__read_init(md, false, &start, &end) < 0)
 		goto out_init;
 
-	while ((event = perf_mmap__read_event(md, false, &start, end)) != NULL) {
+	while ((event = perf_mmap__read_event(md)) != NULL) {
 		struct perf_sample sample;
 
 		if (event->header.type != PERF_RECORD_SAMPLE)
diff --git a/tools/perf/tests/switch-tracking.c b/tools/perf/tests/switch-tracking.c
index dbf9e20..99839de 100644
--- a/tools/perf/tests/switch-tracking.c
+++ b/tools/perf/tests/switch-tracking.c
@@ -267,7 +267,7 @@ static int process_events(struct perf_evlist *evlist,
 		if (perf_mmap__read_init(md, false, &start, &end) < 0)
 			continue;
 
-		while ((event = perf_mmap__read_event(md, false, &start, end)) != NULL) {
+		while ((event = perf_mmap__read_event(md)) != NULL) {
 			cnt += 1;
 			ret = add_event(evlist, &events, event);
 			 perf_mmap__consume(md);
diff --git a/tools/perf/tests/task-exit.c b/tools/perf/tests/task-exit.c
index 5aa2e68..2df0c05 100644
--- a/tools/perf/tests/task-exit.c
+++ b/tools/perf/tests/task-exit.c
@@ -116,7 +116,7 @@ int test__task_exit(struct test *test __maybe_unused, int subtest __maybe_unused
 	if (perf_mmap__read_init(md, false, &start, &end) < 0)
 		goto out_init;
 
-	while ((event = perf_mmap__read_event(md, false, &start, end)) != NULL) {
+	while ((event = perf_mmap__read_event(md)) != NULL) {
 		if (event->header.type == PERF_RECORD_EXIT)
 			nr_exit++;
 
diff --git a/tools/perf/util/mmap.c b/tools/perf/util/mmap.c
index a56c66b..e3921ed 100644
--- a/tools/perf/util/mmap.c
+++ b/tools/perf/util/mmap.c
@@ -75,9 +75,7 @@ static union perf_event *perf_mmap__read(struct perf_mmap *map,
  * }
  * perf_mmap__read_done()
  */
-union perf_event *perf_mmap__read_event(struct perf_mmap *map,
-					bool overwrite __maybe_unused,
-					u64 *startp, u64 end __maybe_unused)
+union perf_event *perf_mmap__read_event(struct perf_mmap *map)
 {
 	union perf_event *event;
 
@@ -87,15 +85,11 @@ union perf_event *perf_mmap__read_event(struct perf_mmap *map,
 	if (!refcount_read(&map->refcnt))
 		return NULL;
 
-	if (startp == NULL)
-		return NULL;
-
 	/* non-overwirte doesn't pause the ringbuffer */
 	if (!map->overwrite)
 		map->end = perf_mmap__read_head(map);
 
 	event = perf_mmap__read(map, &map->start, map->end);
-	*startp = map->start;
 
 	if (!map->overwrite)
 		map->prev = map->start;
diff --git a/tools/perf/util/mmap.h b/tools/perf/util/mmap.h
index d098b47..c09b4e7 100644
--- a/tools/perf/util/mmap.h
+++ b/tools/perf/util/mmap.h
@@ -89,9 +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);
 
-union perf_event *perf_mmap__read_event(struct perf_mmap *map,
-					bool overwrite,
-					u64 *startp, u64 end);
+union perf_event *perf_mmap__read_event(struct perf_mmap *map);
 
 int perf_mmap__push(struct perf_mmap *md, bool backward,
 		    void *to, int push(void *to, void *buf, size_t size));
diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
index ca077f8..4798db9 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -995,7 +995,7 @@ static PyObject *pyrf_evlist__read_on_cpu(struct pyrf_evlist *pevlist,
 	if (perf_mmap__read_init(md, false, &start, &end) < 0)
 		goto end;
 
-	event = perf_mmap__read_event(md, false, &start, end);
+	event = perf_mmap__read_event(md);
 	if (event != NULL) {
 		PyObject *pyevent = pyrf_event__new(event);
 		struct pyrf_event *pevent = (struct pyrf_event *)pyevent;
-- 
2.4.11

^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [PATCH 7/7] perf tools: Refine perf_mmap__read_init
  2018-03-05 19:10 [PATCH 1/7] perf mmap: Store mmap scope and type in struct perf_mmap kan.liang
                   ` (4 preceding siblings ...)
  2018-03-05 19:10 ` [PATCH 6/7] perf tools: Refine perf_mmap__read_event kan.liang
@ 2018-03-05 19:10 ` kan.liang
  2018-03-05 22:25   ` Jiri Olsa
  2018-03-05 20:20 ` [PATCH 1/7] perf mmap: Store mmap scope and type in struct perf_mmap Arnaldo Carvalho de Melo
  6 siblings, 1 reply; 12+ messages in thread
From: kan.liang @ 2018-03-05 19:10 UTC (permalink / raw)
  To: acme, mingo, linux-kernel; +Cc: jolsa, namhyung, wangnan0, ak, Kan Liang

From: Kan Liang <kan.liang@linux.intel.com>

It doesn't need to pass the 'start' and 'end' boilerplate to
perf_mmap__read_init().
The data will be stored in the struct perf_mmap.

Discard the parameters.

Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
---
 tools/perf/arch/x86/tests/perf-time-to-tsc.c |  3 +--
 tools/perf/builtin-kvm.c                     |  3 +--
 tools/perf/builtin-top.c                     |  3 +--
 tools/perf/builtin-trace.c                   |  3 +--
 tools/perf/tests/backward-ring-buffer.c      |  3 +--
 tools/perf/tests/bpf.c                       |  3 +--
 tools/perf/tests/code-reading.c              |  3 +--
 tools/perf/tests/keep-tracking.c             |  3 +--
 tools/perf/tests/mmap-basic.c                |  3 +--
 tools/perf/tests/openat-syscall-tp-fields.c  |  3 +--
 tools/perf/tests/perf-record.c               |  3 +--
 tools/perf/tests/sw-clock.c                  |  3 +--
 tools/perf/tests/switch-tracking.c           |  3 +--
 tools/perf/tests/task-exit.c                 |  3 +--
 tools/perf/util/mmap.c                       | 10 ++--------
 tools/perf/util/mmap.h                       |  3 +--
 tools/perf/util/python.c                     |  3 +--
 17 files changed, 18 insertions(+), 40 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 17cf7fc..a7c9f60 100644
--- a/tools/perf/arch/x86/tests/perf-time-to-tsc.c
+++ b/tools/perf/arch/x86/tests/perf-time-to-tsc.c
@@ -61,7 +61,6 @@ int test__perf_time_to_tsc(struct test *test __maybe_unused, int subtest __maybe
 	u64 test_tsc, comm1_tsc, comm2_tsc;
 	u64 test_time, comm1_time = 0, comm2_time = 0;
 	struct perf_mmap *md;
-	u64 end, start;
 
 	threads = thread_map__new(-1, getpid(), UINT_MAX);
 	CHECK_NOT_NULL__(threads);
@@ -112,7 +111,7 @@ int test__perf_time_to_tsc(struct test *test __maybe_unused, int subtest __maybe
 
 	for (i = 0; i < evlist->nr_mmaps; i++) {
 		md = &evlist->mmap[i];
-		if (perf_mmap__read_init(md, false, &start, &end) < 0)
+		if (perf_mmap__read_init(md, false) < 0)
 			continue;
 
 		while ((event = perf_mmap__read_event(md)) != NULL) {
diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
index e9f69b8..2bcc599 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -746,14 +746,13 @@ static s64 perf_kvm__mmap_read_idx(struct perf_kvm_stat *kvm, int idx,
 	struct perf_evlist *evlist = kvm->evlist;
 	union perf_event *event;
 	struct perf_mmap *md;
-	u64 end, start;
 	u64 timestamp;
 	s64 n = 0;
 	int err;
 
 	*mmap_time = ULLONG_MAX;
 	md = &evlist->mmap[idx];
-	err = perf_mmap__read_init(md, false, &start, &end);
+	err = perf_mmap__read_init(md, false);
 	if (err < 0)
 		return (err == -EAGAIN) ? 0 : -1;
 
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index eb19cf9..9fc13c4 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -817,11 +817,10 @@ static void perf_top__mmap_read_idx(struct perf_top *top, int idx)
 	struct perf_session *session = top->session;
 	union perf_event *event;
 	struct machine *machine;
-	u64 end, start;
 	int ret;
 
 	md = opts->overwrite ? &evlist->overwrite_mmap[idx] : &evlist->mmap[idx];
-	if (perf_mmap__read_init(md, opts->overwrite, &start, &end) < 0)
+	if (perf_mmap__read_init(md, opts->overwrite) < 0)
 		return;
 
 	while ((event = perf_mmap__read_event(md)) != NULL) {
diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index c71ef7ba..86cdc98 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -2473,10 +2473,9 @@ static int trace__run(struct trace *trace, int argc, const char **argv)
 	for (i = 0; i < evlist->nr_mmaps; i++) {
 		union perf_event *event;
 		struct perf_mmap *md;
-		u64 end, start;
 
 		md = &evlist->mmap[i];
-		if (perf_mmap__read_init(md, false, &start, &end) < 0)
+		if (perf_mmap__read_init(md, false) < 0)
 			continue;
 
 		while ((event = perf_mmap__read_event(md)) != NULL) {
diff --git a/tools/perf/tests/backward-ring-buffer.c b/tools/perf/tests/backward-ring-buffer.c
index e0eae10..15b80b2 100644
--- a/tools/perf/tests/backward-ring-buffer.c
+++ b/tools/perf/tests/backward-ring-buffer.c
@@ -33,9 +33,8 @@ static int count_samples(struct perf_evlist *evlist, int *sample_count,
 	for (i = 0; i < evlist->nr_mmaps; i++) {
 		struct perf_mmap *map = &evlist->overwrite_mmap[i];
 		union perf_event *event;
-		u64 start, end;
 
-		perf_mmap__read_init(map, true, &start, &end);
+		perf_mmap__read_init(map, true);
 		while ((event = perf_mmap__read_event(map)) != NULL) {
 			const u32 type = event->header.type;
 
diff --git a/tools/perf/tests/bpf.c b/tools/perf/tests/bpf.c
index 384c20f..192c37f 100644
--- a/tools/perf/tests/bpf.c
+++ b/tools/perf/tests/bpf.c
@@ -177,10 +177,9 @@ static int do_test(struct bpf_object *obj, int (*func)(void),
 	for (i = 0; i < evlist->nr_mmaps; i++) {
 		union perf_event *event;
 		struct perf_mmap *md;
-		u64 end, start;
 
 		md = &evlist->mmap[i];
-		if (perf_mmap__read_init(md, false, &start, &end) < 0)
+		if (perf_mmap__read_init(md, false) < 0)
 			continue;
 
 		while ((event = perf_mmap__read_event(md)) != NULL) {
diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c
index f791966..b494837 100644
--- a/tools/perf/tests/code-reading.c
+++ b/tools/perf/tests/code-reading.c
@@ -410,12 +410,11 @@ static int process_events(struct machine *machine, struct perf_evlist *evlist,
 {
 	union perf_event *event;
 	struct perf_mmap *md;
-	u64 end, start;
 	int i, ret;
 
 	for (i = 0; i < evlist->nr_mmaps; i++) {
 		md = &evlist->mmap[i];
-		if (perf_mmap__read_init(md, false, &start, &end) < 0)
+		if (perf_mmap__read_init(md, false) < 0)
 			continue;
 
 		while ((event = perf_mmap__read_event(md)) != NULL) {
diff --git a/tools/perf/tests/keep-tracking.c b/tools/perf/tests/keep-tracking.c
index ad477b7..9ddd914 100644
--- a/tools/perf/tests/keep-tracking.c
+++ b/tools/perf/tests/keep-tracking.c
@@ -28,13 +28,12 @@ static int find_comm(struct perf_evlist *evlist, const char *comm)
 {
 	union perf_event *event;
 	struct perf_mmap *md;
-	u64 end, start;
 	int i, found;
 
 	found = 0;
 	for (i = 0; i < evlist->nr_mmaps; i++) {
 		md = &evlist->mmap[i];
-		if (perf_mmap__read_init(md, false, &start, &end) < 0)
+		if (perf_mmap__read_init(md, false) < 0)
 			continue;
 		while ((event = perf_mmap__read_event(md)) != NULL) {
 			if (event->header.type == PERF_RECORD_COMM &&
diff --git a/tools/perf/tests/mmap-basic.c b/tools/perf/tests/mmap-basic.c
index 7790eb3..102a704 100644
--- a/tools/perf/tests/mmap-basic.c
+++ b/tools/perf/tests/mmap-basic.c
@@ -39,7 +39,6 @@ int test__basic_mmap(struct test *test __maybe_unused, int subtest __maybe_unuse
 	struct perf_evsel *evsels[nsyscalls], *evsel;
 	char sbuf[STRERR_BUFSIZE];
 	struct perf_mmap *md;
-	u64 end, start;
 
 	threads = thread_map__new(-1, getpid(), UINT_MAX);
 	if (threads == NULL) {
@@ -109,7 +108,7 @@ int test__basic_mmap(struct test *test __maybe_unused, int subtest __maybe_unuse
 		}
 
 	md = &evlist->mmap[0];
-	if (perf_mmap__read_init(md, false, &start, &end) < 0)
+	if (perf_mmap__read_init(md, false) < 0)
 		goto out_init;
 
 	while ((event = perf_mmap__read_event(md)) != NULL) {
diff --git a/tools/perf/tests/openat-syscall-tp-fields.c b/tools/perf/tests/openat-syscall-tp-fields.c
index b0be2da..917f7f5b 100644
--- a/tools/perf/tests/openat-syscall-tp-fields.c
+++ b/tools/perf/tests/openat-syscall-tp-fields.c
@@ -87,10 +87,9 @@ int test__syscall_openat_tp_fields(struct test *test __maybe_unused, int subtest
 		for (i = 0; i < evlist->nr_mmaps; i++) {
 			union perf_event *event;
 			struct perf_mmap *md;
-			u64 end, start;
 
 			md = &evlist->mmap[i];
-			if (perf_mmap__read_init(md, false, &start, &end) < 0)
+			if (perf_mmap__read_init(md, false) < 0)
 				continue;
 
 			while ((event = perf_mmap__read_event(md)) != NULL) {
diff --git a/tools/perf/tests/perf-record.c b/tools/perf/tests/perf-record.c
index 59be094..4ff6262 100644
--- a/tools/perf/tests/perf-record.c
+++ b/tools/perf/tests/perf-record.c
@@ -165,10 +165,9 @@ int test__PERF_RECORD(struct test *test __maybe_unused, int subtest __maybe_unus
 		for (i = 0; i < evlist->nr_mmaps; i++) {
 			union perf_event *event;
 			struct perf_mmap *md;
-			u64 end, start;
 
 			md = &evlist->mmap[i];
-			if (perf_mmap__read_init(md, false, &start, &end) < 0)
+			if (perf_mmap__read_init(md, false) < 0)
 				continue;
 
 			while ((event = perf_mmap__read_event(md)) != NULL) {
diff --git a/tools/perf/tests/sw-clock.c b/tools/perf/tests/sw-clock.c
index 403f2d9..9244b74 100644
--- a/tools/perf/tests/sw-clock.c
+++ b/tools/perf/tests/sw-clock.c
@@ -40,7 +40,6 @@ static int __test__sw_clock_freq(enum perf_sw_ids clock_id)
 	struct cpu_map *cpus;
 	struct thread_map *threads;
 	struct perf_mmap *md;
-	u64 end, start;
 
 	attr.sample_freq = 500;
 
@@ -96,7 +95,7 @@ static int __test__sw_clock_freq(enum perf_sw_ids clock_id)
 	perf_evlist__disable(evlist);
 
 	md = &evlist->mmap[0];
-	if (perf_mmap__read_init(md, false, &start, &end) < 0)
+	if (perf_mmap__read_init(md, false) < 0)
 		goto out_init;
 
 	while ((event = perf_mmap__read_event(md)) != NULL) {
diff --git a/tools/perf/tests/switch-tracking.c b/tools/perf/tests/switch-tracking.c
index 99839de..bdf5d1d 100644
--- a/tools/perf/tests/switch-tracking.c
+++ b/tools/perf/tests/switch-tracking.c
@@ -259,12 +259,11 @@ static int process_events(struct perf_evlist *evlist,
 	LIST_HEAD(events);
 	struct event_node *events_array, *node;
 	struct perf_mmap *md;
-	u64 end, start;
 	int i, ret;
 
 	for (i = 0; i < evlist->nr_mmaps; i++) {
 		md = &evlist->mmap[i];
-		if (perf_mmap__read_init(md, false, &start, &end) < 0)
+		if (perf_mmap__read_init(md, false) < 0)
 			continue;
 
 		while ((event = perf_mmap__read_event(md)) != NULL) {
diff --git a/tools/perf/tests/task-exit.c b/tools/perf/tests/task-exit.c
index 2df0c05..31954a4 100644
--- a/tools/perf/tests/task-exit.c
+++ b/tools/perf/tests/task-exit.c
@@ -48,7 +48,6 @@ int test__task_exit(struct test *test __maybe_unused, int subtest __maybe_unused
 	struct cpu_map *cpus;
 	struct thread_map *threads;
 	struct perf_mmap *md;
-	u64 end, start;
 
 	signal(SIGCHLD, sig_handler);
 
@@ -113,7 +112,7 @@ int test__task_exit(struct test *test __maybe_unused, int subtest __maybe_unused
 
 retry:
 	md = &evlist->mmap[0];
-	if (perf_mmap__read_init(md, false, &start, &end) < 0)
+	if (perf_mmap__read_init(md, false) < 0)
 		goto out_init;
 
 	while ((event = perf_mmap__read_event(md)) != NULL) {
diff --git a/tools/perf/util/mmap.c b/tools/perf/util/mmap.c
index e3921ed..403c5e6 100644
--- a/tools/perf/util/mmap.c
+++ b/tools/perf/util/mmap.c
@@ -235,16 +235,13 @@ static int overwrite_rb_find_range(void *buf, int mask, u64 head, u64 *start, u6
 /*
  * Report the start and end of the available data in ringbuffer
  */
-int perf_mmap__read_init(struct perf_mmap *md, bool overwrite,
-			 u64 *startp, u64 *endp)
+int perf_mmap__read_init(struct perf_mmap *md, bool overwrite)
 {
 	u64 head = perf_mmap__read_head(md);
 	u64 old = md->prev;
 	unsigned char *data = md->base + page_size;
 	unsigned long size;
 
-	*startp = overwrite ? head : old;
-	*endp = overwrite ? old : head;
 	md->overwrite = overwrite;
 	md->start = overwrite ? head : old;
 	md->end = overwrite ? old : head;
@@ -268,8 +265,6 @@ int perf_mmap__read_init(struct perf_mmap *md, bool overwrite,
 		 */
 		if (overwrite_rb_find_range(data, md->mask, head, &md->start, &md->end))
 			return -EINVAL;
-		*startp = md->start;
-		*endp = md->end;
 	}
 
 	return 0;
@@ -279,13 +274,12 @@ 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);
-	u64 end, start;
 	unsigned char *data = md->base + page_size;
 	unsigned long size;
 	void *buf;
 	int rc = 0;
 
-	rc = perf_mmap__read_init(md, overwrite, &start, &end);
+	rc = perf_mmap__read_init(md, overwrite);
 	if (rc < 0)
 		return (rc == -EAGAIN) ? 0 : -1;
 
diff --git a/tools/perf/util/mmap.h b/tools/perf/util/mmap.h
index c09b4e7..6442025 100644
--- a/tools/perf/util/mmap.h
+++ b/tools/perf/util/mmap.h
@@ -96,7 +96,6 @@ int perf_mmap__push(struct perf_mmap *md, bool backward,
 
 size_t perf_mmap__mmap_len(struct perf_mmap *map);
 
-int perf_mmap__read_init(struct perf_mmap *md, bool overwrite,
-			 u64 *startp, u64 *endp);
+int perf_mmap__read_init(struct perf_mmap *md, bool overwrite);
 void perf_mmap__read_done(struct perf_mmap *map);
 #endif /*__PERF_MMAP_H */
diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
index 4798db9..2055399 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -984,7 +984,6 @@ static PyObject *pyrf_evlist__read_on_cpu(struct pyrf_evlist *pevlist,
 	int sample_id_all = 1, cpu;
 	static char *kwlist[] = { "cpu", "sample_id_all", NULL };
 	struct perf_mmap *md;
-	u64 end, start;
 	int err;
 
 	if (!PyArg_ParseTupleAndKeywords(args, kwargs, "i|i", kwlist,
@@ -992,7 +991,7 @@ static PyObject *pyrf_evlist__read_on_cpu(struct pyrf_evlist *pevlist,
 		return NULL;
 
 	md = &evlist->mmap[cpu];
-	if (perf_mmap__read_init(md, false, &start, &end) < 0)
+	if (perf_mmap__read_init(md, false) < 0)
 		goto end;
 
 	event = perf_mmap__read_event(md);
-- 
2.4.11

^ permalink raw reply related	[flat|nested] 12+ messages in thread
* Re: [PATCH 1/7] perf mmap: Store mmap scope and type in struct perf_mmap
  2018-03-05 19:10 [PATCH 1/7] perf mmap: Store mmap scope and type in struct perf_mmap kan.liang
                   ` (5 preceding siblings ...)
  2018-03-05 19:10 ` [PATCH 7/7] perf tools: Refine perf_mmap__read_init kan.liang
@ 2018-03-05 20:20 ` Arnaldo Carvalho de Melo
  2018-03-05 20:55   ` Liang, Kan
  6 siblings, 1 reply; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-03-05 20:20 UTC (permalink / raw)
  To: kan.liang; +Cc: mingo, linux-kernel, jolsa, namhyung, wangnan0, ak

Em Mon, Mar 05, 2018 at 02:10:53PM -0500, kan.liang@linux.intel.com escreveu:
> From: Kan Liang <kan.liang@linux.intel.com>
> 
> There are too many boilerplates for the perf_mmap__read*() interfaces.
> 
> Some of the data (e.g. 'start', 'end', 'overwrite') should be stored in
> struct perf_mmap at initialization. They will be used later.
> 
> No functional change.
> 
> Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
> ---
>  tools/perf/util/mmap.c | 11 ++++++++---
>  tools/perf/util/mmap.h |  3 +++
>  2 files changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/util/mmap.c b/tools/perf/util/mmap.c
> index 4f27c46..642b479 100644
> --- a/tools/perf/util/mmap.c
> +++ b/tools/perf/util/mmap.c
> @@ -250,11 +250,14 @@ int perf_mmap__read_init(struct perf_mmap *md, bool overwrite,
>  
>  	*startp = overwrite ? head : old;
>  	*endp = overwrite ? old : head;

Why did you keep the above and haven't removed the startp/endp args? to
ease the conversion somehow? I'll look at the other patches, but I think
we should change the function signature here and in the callers all at
once.

Sometimes this is ok, and probably this is one case.

- Arnaldo

> +	md->overwrite = overwrite;
> +	md->start = overwrite ? head : old;
> +	md->end = overwrite ? old : head;
>  
> -	if (*startp == *endp)
> +	if (md->start == md->end)
>  		return -EAGAIN;
>  
> -	size = *endp - *startp;
> +	size = md->end - md->start;
>  	if (size > (unsigned long)(md->mask) + 1) {
>  		if (!overwrite) {
>  			WARN_ONCE(1, "failed to keep up with mmap data. (warn only once)\n");
> @@ -268,8 +271,10 @@ int perf_mmap__read_init(struct perf_mmap *md, bool overwrite,
>  		 * Backward ring buffer is full. We still have a chance to read
>  		 * most of data from it.
>  		 */
> -		if (overwrite_rb_find_range(data, md->mask, head, startp, endp))
> +		if (overwrite_rb_find_range(data, md->mask, head, &md->start, &md->end))
>  			return -EINVAL;
> +		*startp = md->start;
> +		*endp = md->end;
>  	}
>  
>  	return 0;
> diff --git a/tools/perf/util/mmap.h b/tools/perf/util/mmap.h
> index ec7d3a24..9359e93 100644
> --- a/tools/perf/util/mmap.h
> +++ b/tools/perf/util/mmap.h
> @@ -20,6 +20,9 @@ struct perf_mmap {
>  	int		 fd;
>  	refcount_t	 refcnt;
>  	u64		 prev;
> +	u64		 start;
> +	u64		 end;
> +	bool		 overwrite;
>  	struct auxtrace_mmap auxtrace_mmap;
>  	char		 event_copy[PERF_SAMPLE_MAX_SIZE] __aligned(8);
>  };
> -- 
> 2.4.11

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH 1/7] perf mmap: Store mmap scope and type in struct perf_mmap
  2018-03-05 20:20 ` [PATCH 1/7] perf mmap: Store mmap scope and type in struct perf_mmap Arnaldo Carvalho de Melo
@ 2018-03-05 20:55   ` Liang, Kan
  0 siblings, 0 replies; 12+ messages in thread
From: Liang, Kan @ 2018-03-05 20:55 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: mingo, linux-kernel, jolsa, namhyung, wangnan0, ak



On 3/5/2018 3:20 PM, Arnaldo Carvalho de Melo wrote:
> Em Mon, Mar 05, 2018 at 02:10:53PM -0500, kan.liang@linux.intel.com escreveu:
>> From: Kan Liang <kan.liang@linux.intel.com>
>>
>> There are too many boilerplates for the perf_mmap__read*() interfaces.
>>
>> Some of the data (e.g. 'start', 'end', 'overwrite') should be stored in
>> struct perf_mmap at initialization. They will be used later.
>>
>> No functional change.
>>
>> Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
>> ---
>>   tools/perf/util/mmap.c | 11 ++++++++---
>>   tools/perf/util/mmap.h |  3 +++
>>   2 files changed, 11 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/perf/util/mmap.c b/tools/perf/util/mmap.c
>> index 4f27c46..642b479 100644
>> --- a/tools/perf/util/mmap.c
>> +++ b/tools/perf/util/mmap.c
>> @@ -250,11 +250,14 @@ int perf_mmap__read_init(struct perf_mmap *md, bool overwrite,
>>   
>>   	*startp = overwrite ? head : old;
>>   	*endp = overwrite ? old : head;
> 
> Why did you keep the above and haven't removed the startp/endp args? 

The startp/endp are used by perf_mmap__read_event().
I don't want to make all the changes in a single patch. I think it's 
hard to review.
So I modified the interface one by one.
Only changing one interface will break the usage.
So the old 'startp/endp' and new 'md->start/md->end' will exist 
simultaneously in the first few patches. They have the same value.

Patch 1,3,4 add the new 'md->start/md->end' for the three interfaces.
Patch 2 uses the new 'md->start/md->end' for perf record.
Patch 5,6,7 remove the old 'startp/endp' for other perf tools/test and 
refine the interfaces.

Thanks,
Kan
> to
> ease the conversion somehow? I'll look at the other patches, but I think
> we should change the function signature here and in the callers all at
> once.
> 
> Sometimes this is ok, and probably this is one case.
> 
> - Arnaldo
> 
>> +	md->overwrite = overwrite;
>> +	md->start = overwrite ? head : old;
>> +	md->end = overwrite ? old : head;
>>   
>> -	if (*startp == *endp)
>> +	if (md->start == md->end)
>>   		return -EAGAIN;
>>   
>> -	size = *endp - *startp;
>> +	size = md->end - md->start;
>>   	if (size > (unsigned long)(md->mask) + 1) {
>>   		if (!overwrite) {
>>   			WARN_ONCE(1, "failed to keep up with mmap data. (warn only once)\n");
>> @@ -268,8 +271,10 @@ int perf_mmap__read_init(struct perf_mmap *md, bool overwrite,
>>   		 * Backward ring buffer is full. We still have a chance to read
>>   		 * most of data from it.
>>   		 */
>> -		if (overwrite_rb_find_range(data, md->mask, head, startp, endp))
>> +		if (overwrite_rb_find_range(data, md->mask, head, &md->start, &md->end))
>>   			return -EINVAL;
>> +		*startp = md->start;
>> +		*endp = md->end;
>>   	}
>>   
>>   	return 0;
>> diff --git a/tools/perf/util/mmap.h b/tools/perf/util/mmap.h
>> index ec7d3a24..9359e93 100644
>> --- a/tools/perf/util/mmap.h
>> +++ b/tools/perf/util/mmap.h
>> @@ -20,6 +20,9 @@ struct perf_mmap {
>>   	int		 fd;
>>   	refcount_t	 refcnt;
>>   	u64		 prev;
>> +	u64		 start;
>> +	u64		 end;
>> +	bool		 overwrite;
>>   	struct auxtrace_mmap auxtrace_mmap;
>>   	char		 event_copy[PERF_SAMPLE_MAX_SIZE] __aligned(8);
>>   };
>> -- 
>> 2.4.11

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH 7/7] perf tools: Refine perf_mmap__read_init
  2018-03-05 19:10 ` [PATCH 7/7] perf tools: Refine perf_mmap__read_init kan.liang
@ 2018-03-05 22:25   ` Jiri Olsa
  2018-03-06 13:29     ` Liang, Kan
  0 siblings, 1 reply; 12+ messages in thread
From: Jiri Olsa @ 2018-03-05 22:25 UTC (permalink / raw)
  To: kan.liang; +Cc: acme, mingo, linux-kernel, namhyung, wangnan0, ak

On Mon, Mar 05, 2018 at 02:10:59PM -0500, kan.liang@linux.intel.com wrote:

SNIP

> diff --git a/tools/perf/util/mmap.c b/tools/perf/util/mmap.c
> index e3921ed..403c5e6 100644
> --- a/tools/perf/util/mmap.c
> +++ b/tools/perf/util/mmap.c
> @@ -235,16 +235,13 @@ static int overwrite_rb_find_range(void *buf, int mask, u64 head, u64 *start, u6
>  /*
>   * Report the start and end of the available data in ringbuffer
>   */
> -int perf_mmap__read_init(struct perf_mmap *md, bool overwrite,
> -			 u64 *startp, u64 *endp)
> +int perf_mmap__read_init(struct perf_mmap *md, bool overwrite)
>  {
>  	u64 head = perf_mmap__read_head(md);
>  	u64 old = md->prev;
>  	unsigned char *data = md->base + page_size;
>  	unsigned long size;
>  
> -	*startp = overwrite ? head : old;
> -	*endp = overwrite ? old : head;
>  	md->overwrite = overwrite;

hum, can't we set 'overwrite' at the moment we create struct perf_mmap?

it's not changing during the mmap's lifetime.. maybe we could do it in
perf_evlist__alloc_mmap function

really nice cleanup!

thanks,
jirka

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH 7/7] perf tools: Refine perf_mmap__read_init
  2018-03-05 22:25   ` Jiri Olsa
@ 2018-03-06 13:29     ` Liang, Kan
  2018-03-06 14:01       ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 12+ messages in thread
From: Liang, Kan @ 2018-03-06 13:29 UTC (permalink / raw)
  To: Jiri Olsa; +Cc: acme, mingo, linux-kernel, namhyung, wangnan0, ak



On 3/5/2018 5:25 PM, Jiri Olsa wrote:
> On Mon, Mar 05, 2018 at 02:10:59PM -0500, kan.liang@linux.intel.com wrote:
> 
> SNIP
> 
>> diff --git a/tools/perf/util/mmap.c b/tools/perf/util/mmap.c
>> index e3921ed..403c5e6 100644
>> --- a/tools/perf/util/mmap.c
>> +++ b/tools/perf/util/mmap.c
>> @@ -235,16 +235,13 @@ static int overwrite_rb_find_range(void *buf, int mask, u64 head, u64 *start, u6
>>   /*
>>    * Report the start and end of the available data in ringbuffer
>>    */
>> -int perf_mmap__read_init(struct perf_mmap *md, bool overwrite,
>> -			 u64 *startp, u64 *endp)
>> +int perf_mmap__read_init(struct perf_mmap *md, bool overwrite)
>>   {
>>   	u64 head = perf_mmap__read_head(md);
>>   	u64 old = md->prev;
>>   	unsigned char *data = md->base + page_size;
>>   	unsigned long size;
>>   
>> -	*startp = overwrite ? head : old;
>> -	*endp = overwrite ? old : head;
>>   	md->overwrite = overwrite;
> 
> hum, can't we set 'overwrite' at the moment we create struct perf_mmap?
> 
> it's not changing during the mmap's lifetime.. maybe we could do it in
> perf_evlist__alloc_mmap function

Yes, we know that the __alloc_mmap() is for mmap or overwrite_mmap.
I think we can set the 'overwrite' there.
I will do the change in V2.

Thanks,
Kan

> 
> really nice cleanup!
> 
> thanks,
> jirka
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH 7/7] perf tools: Refine perf_mmap__read_init
  2018-03-06 13:29     ` Liang, Kan
@ 2018-03-06 14:01       ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-03-06 14:01 UTC (permalink / raw)
  To: Liang, Kan; +Cc: Jiri Olsa, mingo, linux-kernel, namhyung, wangnan0, ak

Em Tue, Mar 06, 2018 at 08:29:03AM -0500, Liang, Kan escreveu:
> 
> 
> On 3/5/2018 5:25 PM, Jiri Olsa wrote:
> > On Mon, Mar 05, 2018 at 02:10:59PM -0500, kan.liang@linux.intel.com wrote:
> > 
> > SNIP
> > 
> > > diff --git a/tools/perf/util/mmap.c b/tools/perf/util/mmap.c
> > > index e3921ed..403c5e6 100644
> > > --- a/tools/perf/util/mmap.c
> > > +++ b/tools/perf/util/mmap.c
> > > @@ -235,16 +235,13 @@ static int overwrite_rb_find_range(void *buf, int mask, u64 head, u64 *start, u6
> > >   /*
> > >    * Report the start and end of the available data in ringbuffer
> > >    */
> > > -int perf_mmap__read_init(struct perf_mmap *md, bool overwrite,
> > > -			 u64 *startp, u64 *endp)
> > > +int perf_mmap__read_init(struct perf_mmap *md, bool overwrite)
> > >   {
> > >   	u64 head = perf_mmap__read_head(md);
> > >   	u64 old = md->prev;
> > >   	unsigned char *data = md->base + page_size;
> > >   	unsigned long size;
> > > -	*startp = overwrite ? head : old;
> > > -	*endp = overwrite ? old : head;
> > >   	md->overwrite = overwrite;
> > 
> > hum, can't we set 'overwrite' at the moment we create struct perf_mmap?
> > 
> > it's not changing during the mmap's lifetime.. maybe we could do it in
> > perf_evlist__alloc_mmap function
> 
> Yes, we know that the __alloc_mmap() is for mmap or overwrite_mmap.
> I think we can set the 'overwrite' there.
> I will do the change in V2.

That is even better!

- Arnaldo

^ permalink raw reply	[flat|nested] 12+ messages in thread
end of thread, other threads:[~2018-03-06 14:02 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-05 19:10 [PATCH 1/7] perf mmap: Store mmap scope and type in struct perf_mmap kan.liang
2018-03-05 19:10 ` [PATCH 2/7] perf mmap: Using the stored scope data in perf_mmap__push kan.liang
2018-03-05 19:10 ` [PATCH 3/7] perf mmap: Using the stored data in perf_mmap__read_event kan.liang
2018-03-05 19:10 ` [PATCH 4/7] perf mmap: Using stored 'overwrite' in perf_mmap__consume kan.liang
2018-03-05 19:10 ` [PATCH 5/7] perf tools: Refine perf_mmap__consume kan.liang
2018-03-05 19:10 ` [PATCH 6/7] perf tools: Refine perf_mmap__read_event kan.liang
2018-03-05 19:10 ` [PATCH 7/7] perf tools: Refine perf_mmap__read_init kan.liang
2018-03-05 22:25   ` Jiri Olsa
2018-03-06 13:29     ` Liang, Kan
2018-03-06 14:01       ` Arnaldo Carvalho de Melo
2018-03-05 20:20 ` [PATCH 1/7] perf mmap: Store mmap scope and type in struct perf_mmap Arnaldo Carvalho de Melo
2018-03-05 20:55   ` Liang, Kan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).