Skip to contentSkip to conversation

perf script: Fix --reltime with --time

PATCH Applied Andi Kleen 3 participants 6 messages Oct 11, 2019 – Oct 22, 2019 perf events

lore
Share & tools
lore

Andi Kleen

author
#0 lore raw

PATCH 1/2 perf script: Fix --reltime with --time

From: Andi Kleen <[email protected]>

My earlier patch to just enable --reltime with --time was a little too optimistic. The --time parsing would accept absolute time, which is very confusing to the user.

Support relative time in --time parsing too. This only works with recent perf record that records the first sample time. Otherwise we error out.

Fixes 3714437d3fcc ("perf script: Allow --time with --reltime")
---
 tools/perf/builtin-script.c  |  5 +++--
 tools/perf/util/time-utils.c | 27 ++++++++++++++++++++++++---
 tools/perf/util/time-utils.h |  5 +++++
 3 files changed, 32 insertions(+), 5 deletions(-)
tools/perf/builtin-script.c +3 -2
index 1c797a948ada..f86c5cce5b2c 100644
@@ -3864,10 +3864,11 @@ int cmd_script(int argc, const char **argv)
 		goto out_delete;
 
 	if (script.time_str) {
-		err = perf_time__parse_for_ranges(script.time_str, session,
+		err = perf_time__parse_for_ranges_reltime(script.time_str, session,
 						  &script.ptime_range,
 						  &script.range_size,
-						  &script.range_num);
+						  &script.range_num,
+						  reltime);
 		if (err < 0)
 			goto out_delete;
 
tools/perf/util/time-utils.c +24 -3
index 9796a2e43f67..302443921681 100644
@@ -458,10 +458,11 @@ bool perf_time__ranges_skip_sample(struct perf_time_interval *ptime_buf,
 	return true;
 }
 
-int perf_time__parse_for_ranges(const char *time_str,
+int perf_time__parse_for_ranges_reltime(const char *time_str,
 				struct perf_session *session,
 				struct perf_time_interval **ranges,
-				int *range_size, int *range_num)
+				int *range_size, int *range_num,
+				bool reltime)
 {
 	bool has_percent = strchr(time_str, '%');
 	struct perf_time_interval *ptime_range;
@@ -471,7 +472,7 @@ int perf_time__parse_for_ranges(const char *time_str,
 	if (!ptime_range)
 		return -ENOMEM;
 
-	if (has_percent) {
+	if (has_percent || reltime) {
 		if (session->evlist->first_sample_time == 0 &&
 		    session->evlist->last_sample_time == 0) {
 			pr_err("HINT: no first/last sample time found in perf data.\n"
@@ -479,7 +480,9 @@ int perf_time__parse_for_ranges(const char *time_str,
 			       "(if '--buildid-all' is enabled, please set '--timestamp-boundary').\n");
 			goto error;
 		}
+	}
 
+	if (has_percent) {
 		num = perf_time__percent_parse_str(
 				ptime_range, size,
 				time_str,
@@ -492,6 +495,15 @@ int perf_time__parse_for_ranges(const char *time_str,
 	if (num < 0)
 		goto error_invalid;
 
+	if (reltime) {
+		int i;
+
+		for (i = 0; i < num; i++) {
+			ptime_range[i].start += session->evlist->first_sample_time;
+			ptime_range[i].end += session->evlist->first_sample_time;
+		}
+	}
+
 	*range_size = size;
 	*range_num = num;
 	*ranges = ptime_range;
@@ -504,6 +516,15 @@ int perf_time__parse_for_ranges(const char *time_str,
 	return ret;
 }
 
+int perf_time__parse_for_ranges(const char *time_str,
+				struct perf_session *session,
+				struct perf_time_interval **ranges,
+				int *range_size, int *range_num)
+{
+	return perf_time__parse_for_ranges_reltime(time_str, session, ranges,
+					range_size, range_num, false);
+}
+
 int timestamp__scnprintf_usec(u64 timestamp, char *buf, size_t sz)
 {
 	u64  sec = timestamp / NSEC_PER_SEC;
tools/perf/util/time-utils.h +5 -0
index 4f42988eb2f7..1142b0bddd5e 100644
@@ -26,6 +26,11 @@ bool perf_time__ranges_skip_sample(struct perf_time_interval *ptime_buf,
 
 struct perf_session;
 
+int perf_time__parse_for_ranges_reltime(const char *str, struct perf_session *session,
+				struct perf_time_interval **ranges,
+				int *range_size, int *range_num,
+				bool reltime);
+
 int perf_time__parse_for_ranges(const char *str, struct perf_session *session,
 				struct perf_time_interval **ranges,
 				int *range_size, int *range_num);
Signature · 1 line
2.21.0
author
#1 lore raw

PATCH 2/2 perf evlist: Fix fix for freed id arrays

From: Andi Kleen <[email protected]>

In the earlier fix for the memory overrun of id arrays I managed to typo the wrong event in the fix.

Of course we need to close the current event in the loop, not the original failing event.

The same test case as in the original patch still passes.

Fixes 7834fa948beb ("perf evlist: Fix access of freed id arrays")
---
 tools/perf/util/evlist.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tools/perf/util/evlist.c +1 -1
index e33b46aca5cb..2f8ac60af76b 100644
@@ -1728,7 +1728,7 @@ struct evsel *perf_evlist__reset_weak_group(struct evlist *evsel_list,
 			is_open = false;
 		if (c2->leader == leader) {
 			if (is_open)
-				perf_evsel__close(&evsel->core);
+				perf_evsel__close(&c2->core);
 			c2->leader = c2;
 			c2->core.nr_members = 0;
 		}
Signature · 1 line
2.21.0
merge bot Applied
#2 lore raw

[tip: perf/urgent] perf evlist: Fix fix for freed id arrays

The following commit has been merged into the perf/urgent branch of tip:

The following commit has been merged into the perf/urgent branch of tip:

Commit-ID:     98a8b2e60c69927b1f405c3b001a1de3f4e53901
Gitweb:        https://git.kernel.org/tip/98a8b2e60c69927b1f405c3b001a1de3f4e53901
Author:        Andi Kleen <[email protected]>
AuthorDate:    Fri, 11 Oct 2019 11:21:40 -07:00
Committer:     Arnaldo Carvalho de Melo <[email protected]>
CommitterDate: Tue, 15 Oct 2019 11:51:33 -03:00

perf evlist: Fix fix for freed id arrays

In the earlier fix for the memory overrun of id arrays I managed to typo the wrong event in the fix.

Of course we need to close the current event in the loop, not the original failing event.

The same test case as in the original patch still passes.

Fixes 7834fa948beb ("perf evlist: Fix access of freed id arrays")
Cc Jiri Olsa <[email protected]>
---
 tools/perf/util/evlist.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tools/perf/util/evlist.c +1 -1
index d277a98..de79c73 100644
@@ -1659,7 +1659,7 @@ struct evsel *perf_evlist__reset_weak_group(struct evlist *evsel_list,
 			is_open = false;
 		if (c2->leader == leader) {
 			if (is_open)
-				perf_evsel__close(&evsel->core);
+				perf_evsel__close(&c2->core);
 			c2->leader = c2;
 			c2->core.nr_members = 0;
 		}
merge bot Applied
#3 lore raw

[tip: perf/core] perf evlist: Fix fix for freed id arrays

In reply to Andi Kleen
The following commit has been merged into the perf/core branch of tip:

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     5a40e1994815ab09c59614c6a13d94eef55d1a7f
Gitweb:        https://git.kernel.org/tip/5a40e1994815ab09c59614c6a13d94eef55d1a7f
Author:        Andi Kleen <[email protected]>
AuthorDate:    Fri, 11 Oct 2019 11:21:40 -07:00
Committer:     Arnaldo Carvalho de Melo <[email protected]>
CommitterDate: Tue, 15 Oct 2019 08:36:22 -03:00

perf evlist: Fix fix for freed id arrays

In the earlier fix for the memory overrun of id arrays I managed to typo the wrong event in the fix.

Of course we need to close the current event in the loop, not the original failing event.

The same test case as in the original patch still passes.

Fixes 7834fa948beb ("perf evlist: Fix access of freed id arrays")
Cc Jiri Olsa <[email protected]>
---
 tools/perf/util/evlist.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tools/perf/util/evlist.c +1 -1
index 21b77ef..8793b4e 100644
@@ -1599,7 +1599,7 @@ struct evsel *perf_evlist__reset_weak_group(struct evlist *evsel_list,
 			is_open = false;
 		if (c2->leader == leader) {
 			if (is_open)
-				perf_evsel__close(&evsel->core);
+				perf_evsel__close(&c2->core);
 			c2->leader = c2;
 			c2->core.nr_members = 0;
 		}
#4 lore raw
In reply to Andi Kleen

Em Fri, Oct 11, 2019 at 11:21:39AM -0700, Andi Kleen escreveu:

Quoted 7 lines

From: Andi Kleen <[email protected]>

My earlier patch to just enable --reltime with --time was a little too optimistic. The --time parsing would accept absolute time, which is very confusing to the user.

Support relative time in --time parsing too. This only works with recent perf record that records the first sample time. Otherwise we error out.

Thanks, applied both patches.

- Arnaldo

Quoted 113 lines
Fixes 3714437d3fcc ("perf script: Allow --time with --reltime")
---
 tools/perf/builtin-script.c  |  5 +++--
 tools/perf/util/time-utils.c | 27 ++++++++++++++++++++++++---
 tools/perf/util/time-utils.h |  5 +++++
 3 files changed, 32 insertions(+), 5 deletions(-)
tools/perf/builtin-script.c +3 -2
index 1c797a948ada..f86c5cce5b2c 100644
@@ -3864,10 +3864,11 @@ int cmd_script(int argc, const char **argv)
 		goto out_delete;
 
 	if (script.time_str) {
-		err = perf_time__parse_for_ranges(script.time_str, session,
+		err = perf_time__parse_for_ranges_reltime(script.time_str, session,
 						  &script.ptime_range,
 						  &script.range_size,
-						  &script.range_num);
+						  &script.range_num,
+						  reltime);
 		if (err < 0)
 			goto out_delete;
 
tools/perf/util/time-utils.c +24 -3
index 9796a2e43f67..302443921681 100644
@@ -458,10 +458,11 @@ bool perf_time__ranges_skip_sample(struct perf_time_interval *ptime_buf,
 	return true;
 }
 
-int perf_time__parse_for_ranges(const char *time_str,
+int perf_time__parse_for_ranges_reltime(const char *time_str,
 				struct perf_session *session,
 				struct perf_time_interval **ranges,
-				int *range_size, int *range_num)
+				int *range_size, int *range_num,
+				bool reltime)
 {
 	bool has_percent = strchr(time_str, '%');
 	struct perf_time_interval *ptime_range;
@@ -471,7 +472,7 @@ int perf_time__parse_for_ranges(const char *time_str,
 	if (!ptime_range)
 		return -ENOMEM;
 
-	if (has_percent) {
+	if (has_percent || reltime) {
 		if (session->evlist->first_sample_time == 0 &&
 		    session->evlist->last_sample_time == 0) {
 			pr_err("HINT: no first/last sample time found in perf data.\n"
@@ -479,7 +480,9 @@ int perf_time__parse_for_ranges(const char *time_str,
 			       "(if '--buildid-all' is enabled, please set '--timestamp-boundary').\n");
 			goto error;
 		}
+	}
 
+	if (has_percent) {
 		num = perf_time__percent_parse_str(
 				ptime_range, size,
 				time_str,
@@ -492,6 +495,15 @@ int perf_time__parse_for_ranges(const char *time_str,
 	if (num < 0)
 		goto error_invalid;
 
+	if (reltime) {
+		int i;
+
+		for (i = 0; i < num; i++) {
+			ptime_range[i].start += session->evlist->first_sample_time;
+			ptime_range[i].end += session->evlist->first_sample_time;
+		}
+	}
+
 	*range_size = size;
 	*range_num = num;
 	*ranges = ptime_range;
@@ -504,6 +516,15 @@ int perf_time__parse_for_ranges(const char *time_str,
 	return ret;
 }
 
+int perf_time__parse_for_ranges(const char *time_str,
+				struct perf_session *session,
+				struct perf_time_interval **ranges,
+				int *range_size, int *range_num)
+{
+	return perf_time__parse_for_ranges_reltime(time_str, session, ranges,
+					range_size, range_num, false);
+}
+
 int timestamp__scnprintf_usec(u64 timestamp, char *buf, size_t sz)
 {
 	u64  sec = timestamp / NSEC_PER_SEC;
tools/perf/util/time-utils.h +5 -0
index 4f42988eb2f7..1142b0bddd5e 100644
@@ -26,6 +26,11 @@ bool perf_time__ranges_skip_sample(struct perf_time_interval *ptime_buf,
 
 struct perf_session;
 
+int perf_time__parse_for_ranges_reltime(const char *str, struct perf_session *session,
+				struct perf_time_interval **ranges,
+				int *range_size, int *range_num,
+				bool reltime);
+
 int perf_time__parse_for_ranges(const char *str, struct perf_session *session,
 				struct perf_time_interval **ranges,
 				int *range_size, int *range_num);
Signature · 1 line
2.21.0
Signature · 1 line
- Arnaldo
merge bot Applied
#5 lore raw
In reply to Andi Kleen
The following commit has been merged into the perf/core branch of tip:

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     b3509b6ed7a79ec49f6b64e4f3b780f259a2a468
Gitweb:        https://git.kernel.org/tip/b3509b6ed7a79ec49f6b64e4f3b780f259a2a468
Author:        Andi Kleen <[email protected]>
AuthorDate:    Fri, 11 Oct 2019 11:21:39 -07:00
Committer:     Arnaldo Carvalho de Melo <[email protected]>
CommitterDate: Tue, 15 Oct 2019 08:36:22 -03:00

perf script: Fix --reltime with --time

My earlier patch to just enable --reltime with --time was a little too optimistic. The --time parsing would accept absolute time, which is very confusing to the user.

Support relative time in --time parsing too. This only works with recent perf record that records the first sample time. Otherwise we error out.

Fixes 3714437d3fcc ("perf script: Allow --time with --reltime")
Cc Jiri Olsa <[email protected]>
---
 tools/perf/builtin-script.c  |  5 +++--
 tools/perf/util/time-utils.c | 27 ++++++++++++++++++++++++---
 tools/perf/util/time-utils.h |  5 +++++
 3 files changed, 32 insertions(+), 5 deletions(-)
tools/perf/builtin-script.c +3 -2
index 1c797a9..f86c5cc 100644
@@ -3864,10 +3864,11 @@ int cmd_script(int argc, const char **argv)
 		goto out_delete;
 
 	if (script.time_str) {
-		err = perf_time__parse_for_ranges(script.time_str, session,
+		err = perf_time__parse_for_ranges_reltime(script.time_str, session,
 						  &script.ptime_range,
 						  &script.range_size,
-						  &script.range_num);
+						  &script.range_num,
+						  reltime);
 		if (err < 0)
 			goto out_delete;
 
tools/perf/util/time-utils.c +24 -3
index 9796a2e..3024439 100644
@@ -458,10 +458,11 @@ bool perf_time__ranges_skip_sample(struct perf_time_interval *ptime_buf,
 	return true;
 }
 
-int perf_time__parse_for_ranges(const char *time_str,
+int perf_time__parse_for_ranges_reltime(const char *time_str,
 				struct perf_session *session,
 				struct perf_time_interval **ranges,
-				int *range_size, int *range_num)
+				int *range_size, int *range_num,
+				bool reltime)
 {
 	bool has_percent = strchr(time_str, '%');
 	struct perf_time_interval *ptime_range;
@@ -471,7 +472,7 @@ int perf_time__parse_for_ranges(const char *time_str,
 	if (!ptime_range)
 		return -ENOMEM;
 
-	if (has_percent) {
+	if (has_percent || reltime) {
 		if (session->evlist->first_sample_time == 0 &&
 		    session->evlist->last_sample_time == 0) {
 			pr_err("HINT: no first/last sample time found in perf data.\n"
@@ -479,7 +480,9 @@ int perf_time__parse_for_ranges(const char *time_str,
 			       "(if '--buildid-all' is enabled, please set '--timestamp-boundary').\n");
 			goto error;
 		}
+	}
 
+	if (has_percent) {
 		num = perf_time__percent_parse_str(
 				ptime_range, size,
 				time_str,
@@ -492,6 +495,15 @@ int perf_time__parse_for_ranges(const char *time_str,
 	if (num < 0)
 		goto error_invalid;
 
+	if (reltime) {
+		int i;
+
+		for (i = 0; i < num; i++) {
+			ptime_range[i].start += session->evlist->first_sample_time;
+			ptime_range[i].end += session->evlist->first_sample_time;
+		}
+	}
+
 	*range_size = size;
 	*range_num = num;
 	*ranges = ptime_range;
@@ -504,6 +516,15 @@ error:
 	return ret;
 }
 
+int perf_time__parse_for_ranges(const char *time_str,
+				struct perf_session *session,
+				struct perf_time_interval **ranges,
+				int *range_size, int *range_num)
+{
+	return perf_time__parse_for_ranges_reltime(time_str, session, ranges,
+					range_size, range_num, false);
+}
+
 int timestamp__scnprintf_usec(u64 timestamp, char *buf, size_t sz)
 {
 	u64  sec = timestamp / NSEC_PER_SEC;
tools/perf/util/time-utils.h +5 -0
index 4f42988..1142b0b 100644
@@ -26,6 +26,11 @@ bool perf_time__ranges_skip_sample(struct perf_time_interval *ptime_buf,
 
 struct perf_session;
 
+int perf_time__parse_for_ranges_reltime(const char *str, struct perf_session *session,
+				struct perf_time_interval **ranges,
+				int *range_size, int *range_num,
+				bool reltime);
+
 int perf_time__parse_for_ranges(const char *str, struct perf_session *session,
 				struct perf_time_interval **ranges,
 				int *range_size, int *range_num);