perf intel-pt: Fix error with config term pt=0

PATCH Applied Adrian Hunter 3 participants 6 messages Nov 27, 2018 – Dec 18, 2018 perf events

lore

Share & tools

lore

Adrian Hunter

author reviewer

2018-11-27 08:45 UTC #0 lore raw

PATCH 1/1 perf intel-pt: Fix error with config term pt=0

Users should never use 'pt=0', but if they do it may give a meaningless error:

	$ perf record -e intel_pt/pt=0/u uname
	Error:
	The sys_perf_event_open() syscall returned with 22 (Invalid argument) for
	event (intel_pt/pt=0/u).

Fix that by forcing 'pt=1'.

Signed-off-by Adrian Hunter <[email protected]>

---
 tools/perf/arch/x86/util/intel-pt.c | 8 ++++++++
 1 file changed, 8 insertions(+)

tools/perf/arch/x86/util/intel-pt.c +8 -0

index db0ba8caf5a2..af25a7824ee0 100644
@@ -524,10 +524,18 @@ static int intel_pt_validate_config(struct perf_pmu *intel_pt_pmu,
 				    struct perf_evsel *evsel)
 {
 	int err;
+	char c;
 
 	if (!evsel)
 		return 0;
 
+	/*
+	 * If supported, force pass-through config term (pt=1) even if user
+	 * sets pt=0, which avoids senseless kernel errors.
+	 */
+	if (perf_pmu__scan_file(intel_pt_pmu, "format/pt", "%c", &c) == 1)
+		evsel->attr.config |= 1;
+
 	err = intel_pt_val_config_term(intel_pt_pmu, "caps/cycle_thresholds",
 				       "cyc_thresh", "caps/psb_cyc",
 				       evsel->attr.config);

Signature · 1 line

2.17.1

Arnaldo Carvalho de Melo

maintainer

2018-11-27 12:39 UTC #1 lore raw

Em Tue, Nov 27, 2018 at 10:43:36AM +0200, Adrian Hunter escreveu:

Quoted 24 lines

Users should never use 'pt=0', but if they do it may give a meaningless error:

	$ perf record -e intel_pt/pt=0/u uname
	Error:
	The sys_perf_event_open() syscall returned with 22 (Invalid argument) for
	event (intel_pt/pt=0/u).

Fix that by forcing 'pt=1'.

Signed-off-by Adrian Hunter <[email protected]>

---
 tools/perf/arch/x86/util/intel-pt.c | 8 ++++++++
 1 file changed, 8 insertions(+)

tools/perf/arch/x86/util/intel-pt.c +1 -0

index db0ba8caf5a2..af25a7824ee0 100644
@@ -524,10 +524,18 @@ static int intel_pt_validate_config(struct perf_pmu *intel_pt_pmu,
 				    struct perf_evsel *evsel)
 {
 	int err;
+	char c;

tools/perf/arch/x86/util/intel-pt.c static int intel_pt_validate_config(struct perf_pmu *intel_pt_pmu, · line 537

Quoted 10 lines

tools/perf/arch/x86/util/intel-pt.c +6 -0

 
 	if (!evsel)
 		return 0;
 
+	/*
+	 * If supported, force pass-through config term (pt=1) even if user
+	 * sets pt=0, which avoids senseless kernel errors.
+	 */
+	if (perf_pmu__scan_file(intel_pt_pmu, "format/pt", "%c", &c) == 1)
+		evsel->attr.config |= 1;

shouldn't we have a warning like:

pr_warning("pt=0 doesn't make sense, forcing pt=1")

Instead of silently doing something the user, mistakenly, did explicitely?

- Arnaldo

Quoted 6 lines

tools/perf/arch/x86/util/intel-pt.c +1 -0

+
 	err = intel_pt_val_config_term(intel_pt_pmu, "caps/cycle_thresholds",
 				       "cyc_thresh", "caps/psb_cyc",
 				       evsel->attr.config);

Signature · 1 line

2.17.1

Signature · 1 line

- Arnaldo

Adrian Hunter

author reviewer

2018-11-28 12:39 UTC #2 lore raw

Quoted 38 lines · Arnaldo Carvalho de Melo

Em Tue, Nov 27, 2018 at 10:43:36AM +0200, Adrian Hunter escreveu:
Quoted 24 lines
Users should never use 'pt=0', but if they do it may give a meaningless error:
	$ perf record -e intel_pt/pt=0/u uname
	Error:
	The sys_perf_event_open() syscall returned with 22 (Invalid argument) for
	event (intel_pt/pt=0/u).
Fix that by forcing 'pt=1'.
Signed-off-by Adrian Hunter <[email protected]>
---
 tools/perf/arch/x86/util/intel-pt.c | 8 ++++++++
 1 file changed, 8 insertions(+)
tools/perf/arch/x86/util/intel-pt.c +1 -0
index db0ba8caf5a2..af25a7824ee0 100644
@@ -524,10 +524,18 @@ static int intel_pt_validate_config(struct perf_pmu *intel_pt_pmu,
 				    struct perf_evsel *evsel)
 {
 	int err;
+	char c;
tools/perf/arch/x86/util/intel-pt.c static int intel_pt_validate_config(struct perf_pmu *intel_pt_pmu, · line 537
Quoted 10 lines
tools/perf/arch/x86/util/intel-pt.c +6 -0
 
 	if (!evsel)
 		return 0;
 
+	/*
+	 * If supported, force pass-through config term (pt=1) even if user
+	 * sets pt=0, which avoids senseless kernel errors.
+	 */
+	if (perf_pmu__scan_file(intel_pt_pmu, "format/pt", "%c", &c) == 1)
+		evsel->attr.config |= 1;
shouldn't we have a warning like:

Quoted 5 lines

pr_warning("pt=0 doesn't make sense, forcing pt=1")
Instead of silently doing something the user, mistakenly, did explicitely?

Sure, here it is:

From: Adrian Hunter <[email protected]>
Date: Mon, 26 Nov 2018 14:12:52 +0200
Subject: [PATCH] perf intel-pt: Fix error with config term pt=0

Users should never use 'pt=0', but if they do it may give a meaningless error:

	$ perf record -e intel_pt/pt=0/u uname
	Error:
	The sys_perf_event_open() syscall returned with 22 (Invalid argument) for
	event (intel_pt/pt=0/u).

Fix that by forcing 'pt=1'.

Signed-off-by Adrian Hunter <[email protected]>

---
 tools/perf/arch/x86/util/intel-pt.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

tools/perf/arch/x86/util/intel-pt.c +11 -0

index db0ba8caf5a2..ba8ecaf52200 100644
@@ -524,10 +524,21 @@ static int intel_pt_validate_config(struct perf_pmu *intel_pt_pmu,
 				    struct perf_evsel *evsel)
 {
 	int err;
+	char c;
 
 	if (!evsel)
 		return 0;
 
+	/*
+	 * If supported, force pass-through config term (pt=1) even if user
+	 * sets pt=0, which avoids senseless kernel errors.
+	 */
+	if (perf_pmu__scan_file(intel_pt_pmu, "format/pt", "%c", &c) == 1 &&
+	    !(evsel->attr.config & 1)) {
+		pr_warning("pt=0 doesn't make sense, forcing pt=1\n");
+		evsel->attr.config |= 1;
+	}
+
 	err = intel_pt_val_config_term(intel_pt_pmu, "caps/cycle_thresholds",
 				       "cyc_thresh", "caps/psb_cyc",
 				       evsel->attr.config);

Signature · 1 line

2.17.1

Arnaldo Carvalho de Melo

maintainer Applied

2018-11-28 13:09 UTC #3 lore raw

Em Wed, Nov 28, 2018 at 02:38:13PM +0200, Adrian Hunter escreveu:

Quoted 39 lines · Arnaldo Carvalho de Melo

Quoted 38 lines
Em Tue, Nov 27, 2018 at 10:43:36AM +0200, Adrian Hunter escreveu:
Quoted 24 lines
Users should never use 'pt=0', but if they do it may give a meaningless error:
	$ perf record -e intel_pt/pt=0/u uname
	Error:
	The sys_perf_event_open() syscall returned with 22 (Invalid argument) for
	event (intel_pt/pt=0/u).
Fix that by forcing 'pt=1'.
Signed-off-by Adrian Hunter <[email protected]>
---
 tools/perf/arch/x86/util/intel-pt.c | 8 ++++++++
 1 file changed, 8 insertions(+)
tools/perf/arch/x86/util/intel-pt.c +1 -0
index db0ba8caf5a2..af25a7824ee0 100644
@@ -524,10 +524,18 @@ static int intel_pt_validate_config(struct perf_pmu *intel_pt_pmu,
 				    struct perf_evsel *evsel)
 {
 	int err;
+	char c;
tools/perf/arch/x86/util/intel-pt.c static int intel_pt_validate_config(struct perf_pmu *intel_pt_pmu, · line 537
Quoted 10 lines
tools/perf/arch/x86/util/intel-pt.c +6 -0
 
 	if (!evsel)
 		return 0;
 
+	/*
+	 * If supported, force pass-through config term (pt=1) even if user
+	 * sets pt=0, which avoids senseless kernel errors.
+	 */
+	if (perf_pmu__scan_file(intel_pt_pmu, "format/pt", "%c", &c) == 1)
+		evsel->attr.config |= 1;
shouldn't we have a warning like:

Quoted 7 lines

Quoted 5 lines
pr_warning("pt=0 doesn't make sense, forcing pt=1")
Instead of silently doing something the user, mistakenly, did explicitely?
Sure, here it is:

Thanks, sticked this to the cset:

------------------------

Committer testing:

  # perf record -e intel_pt/pt=0/u uname
  Error:
  The sys_perf_event_open() syscall returned with 22 (Invalid argument) for event (intel_pt/pt=0/u).
  /bin/dmesg | grep -i perf may provide additional information.

  # perf record -e intel_pt/pt=0/u uname
  pt=0 doesn't make sense, forcing pt=1
  Linux
  [ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 0.020 MB perf.data ]
  #
------------------------

Thanks, applied.

- Arnaldo

tip-bot for Adrian Hunter

merge bot

2018-12-14 20:27 UTC #4 lore raw

[tip:perf/core] perf intel-pt: Fix error with config term "pt=0"

In reply to Adrian Hunter

perf intel-pt: Fix error with config term "pt=0"

Commit-ID:  932d0166b1dbc2a631ac718615efc4917412ecf3
Gitweb:     https://git.kernel.org/tip/932d0166b1dbc2a631ac718615efc4917412ecf3
Author:     Adrian Hunter <[email protected]>
AuthorDate: Mon, 26 Nov 2018 14:12:52 +0200
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Thu, 29 Nov 2018 20:42:49 -0300

perf intel-pt: Fix error with config term "pt=0"

Users should never use 'pt=0', but if they do it may give a meaningless error:

	$ perf record -e intel_pt/pt=0/u uname
	Error:
	The sys_perf_event_open() syscall returned with 22 (Invalid argument) for
	event (intel_pt/pt=0/u).

Fix that by forcing 'pt=1'.

Committer testing:

  # perf record -e intel_pt/pt=0/u uname
  Error:
  The sys_perf_event_open() syscall returned with 22 (Invalid argument) for event (intel_pt/pt=0/u).
  /bin/dmesg | grep -i perf may provide additional information.

  # perf record -e intel_pt/pt=0/u uname
  pt=0 doesn't make sense, forcing pt=1
  Linux
  [ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 0.020 MB perf.data ]
  #

Signed-off-by Adrian Hunter <[email protected]>

Tested-by Arnaldo Carvalho de Melo <[email protected]>

Cc Jiri Olsa <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 tools/perf/arch/x86/util/intel-pt.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

tools/perf/arch/x86/util/intel-pt.c +11 -0

index db0ba8caf5a2..ba8ecaf52200 100644
@@ -524,10 +524,21 @@ static int intel_pt_validate_config(struct perf_pmu *intel_pt_pmu,
 				    struct perf_evsel *evsel)
 {
 	int err;
+	char c;
 
 	if (!evsel)
 		return 0;
 
+	/*
+	 * If supported, force pass-through config term (pt=1) even if user
+	 * sets pt=0, which avoids senseless kernel errors.
+	 */
+	if (perf_pmu__scan_file(intel_pt_pmu, "format/pt", "%c", &c) == 1 &&
+	    !(evsel->attr.config & 1)) {
+		pr_warning("pt=0 doesn't make sense, forcing pt=1\n");
+		evsel->attr.config |= 1;
+	}
+
 	err = intel_pt_val_config_term(intel_pt_pmu, "caps/cycle_thresholds",
 				       "cyc_thresh", "caps/psb_cyc",
 				       evsel->attr.config);

tip-bot for Adrian Hunter

merge bot

2018-12-18 13:54 UTC #5 lore raw

[tip:perf/core] perf intel-pt: Fix error with config term "pt=0"

In reply to Adrian Hunter

perf intel-pt: Fix error with config term "pt=0"

Commit-ID:  1c6f709b9f96366cc47af23c05ecec9b8c0c392d
Gitweb:     https://git.kernel.org/tip/1c6f709b9f96366cc47af23c05ecec9b8c0c392d
Author:     Adrian Hunter <[email protected]>
AuthorDate: Mon, 26 Nov 2018 14:12:52 +0200
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Mon, 17 Dec 2018 14:54:51 -0300

perf intel-pt: Fix error with config term "pt=0"

Users should never use 'pt=0', but if they do it may give a meaningless error:

	$ perf record -e intel_pt/pt=0/u uname
	Error:
	The sys_perf_event_open() syscall returned with 22 (Invalid argument) for
	event (intel_pt/pt=0/u).

Fix that by forcing 'pt=1'.

Committer testing:

  # perf record -e intel_pt/pt=0/u uname
  Error:
  The sys_perf_event_open() syscall returned with 22 (Invalid argument) for event (intel_pt/pt=0/u).
  /bin/dmesg | grep -i perf may provide additional information.

  # perf record -e intel_pt/pt=0/u uname
  pt=0 doesn't make sense, forcing pt=1
  Linux
  [ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 0.020 MB perf.data ]
  #

Signed-off-by Adrian Hunter <[email protected]>

Tested-by Arnaldo Carvalho de Melo <[email protected]>

Cc Jiri Olsa <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 tools/perf/arch/x86/util/intel-pt.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

tools/perf/arch/x86/util/intel-pt.c +11 -0

index db0ba8caf5a2..ba8ecaf52200 100644
@@ -524,10 +524,21 @@ static int intel_pt_validate_config(struct perf_pmu *intel_pt_pmu,
 				    struct perf_evsel *evsel)
 {
 	int err;
+	char c;
 
 	if (!evsel)
 		return 0;
 
+	/*
+	 * If supported, force pass-through config term (pt=1) even if user
+	 * sets pt=0, which avoids senseless kernel errors.
+	 */
+	if (perf_pmu__scan_file(intel_pt_pmu, "format/pt", "%c", &c) == 1 &&
+	    !(evsel->attr.config & 1)) {
+		pr_warning("pt=0 doesn't make sense, forcing pt=1\n");
+		evsel->attr.config |= 1;
+	}
+
 	err = intel_pt_val_config_term(intel_pt_pmu, "caps/cycle_thresholds",
 				       "cyc_thresh", "caps/psb_cyc",
 				       evsel->attr.config);