* [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, ×tamp);
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, ×tamp);
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 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
* 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