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

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH] perf intel-pt: Fix error with config term pt=0
@ 2018-11-27  8:43 Adrian Hunter
  2018-11-27 12:39 ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 6+ messages in thread
From: Adrian Hunter @ 2018-11-27  8:43 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo; +Cc: Jiri Olsa, linux-kernel

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 <adrian.hunter@intel.com>
---
 tools/perf/arch/x86/util/intel-pt.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tools/perf/arch/x86/util/intel-pt.c b/tools/perf/arch/x86/util/intel-pt.c
index db0ba8caf5a2..af25a7824ee0 100644
--- a/tools/perf/arch/x86/util/intel-pt.c
+++ b/tools/perf/arch/x86/util/intel-pt.c
@@ -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);
-- 
2.17.1


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

* Re: [PATCH] perf intel-pt: Fix error with config term pt=0
  2018-11-27  8:43 [PATCH] perf intel-pt: Fix error with config term pt=0 Adrian Hunter
@ 2018-11-27 12:39 ` Arnaldo Carvalho de Melo
  2018-11-28 12:38   ` Adrian Hunter
  0 siblings, 1 reply; 6+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-11-27 12:39 UTC (permalink / raw)
  To: Adrian Hunter; +Cc: Jiri Olsa, linux-kernel

Em Tue, Nov 27, 2018 at 10:43:36AM +0200, Adrian Hunter escreveu:
> 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 <adrian.hunter@intel.com>
> ---
>  tools/perf/arch/x86/util/intel-pt.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/tools/perf/arch/x86/util/intel-pt.c b/tools/perf/arch/x86/util/intel-pt.c
> index db0ba8caf5a2..af25a7824ee0 100644
> --- a/tools/perf/arch/x86/util/intel-pt.c
> +++ b/tools/perf/arch/x86/util/intel-pt.c
> @@ -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;

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
  

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

-- 

- Arnaldo

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

* Re: [PATCH] perf intel-pt: Fix error with config term pt=0
  2018-11-27 12:39 ` Arnaldo Carvalho de Melo
@ 2018-11-28 12:38   ` Adrian Hunter
  2018-11-28 13:09     ` Arnaldo Carvalho de Melo
                       ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Adrian Hunter @ 2018-11-28 12:38 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo; +Cc: Jiri Olsa, linux-kernel

On 27/11/18 2:39 PM, Arnaldo Carvalho de Melo wrote:
> Em Tue, Nov 27, 2018 at 10:43:36AM +0200, Adrian Hunter escreveu:
>> 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 <adrian.hunter@intel.com>
>> ---
>>  tools/perf/arch/x86/util/intel-pt.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/tools/perf/arch/x86/util/intel-pt.c b/tools/perf/arch/x86/util/intel-pt.c
>> index db0ba8caf5a2..af25a7824ee0 100644
>> --- a/tools/perf/arch/x86/util/intel-pt.c
>> +++ b/tools/perf/arch/x86/util/intel-pt.c
>> @@ -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;
> 
> 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?

Sure, here it is:

From: Adrian Hunter <adrian.hunter@intel.com>
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 <adrian.hunter@intel.com>
---
 tools/perf/arch/x86/util/intel-pt.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/tools/perf/arch/x86/util/intel-pt.c b/tools/perf/arch/x86/util/intel-pt.c
index db0ba8caf5a2..ba8ecaf52200 100644
--- a/tools/perf/arch/x86/util/intel-pt.c
+++ b/tools/perf/arch/x86/util/intel-pt.c
@@ -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);
-- 
2.17.1


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

* Re: [PATCH] perf intel-pt: Fix error with config term pt=0
  2018-11-28 12:38   ` Adrian Hunter
@ 2018-11-28 13:09     ` Arnaldo Carvalho de Melo
  2018-12-14 20:27     ` [tip:perf/core] perf intel-pt: Fix error with config term "pt=0" tip-bot for Adrian Hunter
  2018-12-18 13:54     ` tip-bot for Adrian Hunter
  2 siblings, 0 replies; 6+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-11-28 13:09 UTC (permalink / raw)
  To: Adrian Hunter; +Cc: Jiri Olsa, linux-kernel

Em Wed, Nov 28, 2018 at 02:38:13PM +0200, Adrian Hunter escreveu:
> On 27/11/18 2:39 PM, Arnaldo Carvalho de Melo wrote:
> > Em Tue, Nov 27, 2018 at 10:43:36AM +0200, Adrian Hunter escreveu:
> >> 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 <adrian.hunter@intel.com>
> >> ---
> >>  tools/perf/arch/x86/util/intel-pt.c | 8 ++++++++
> >>  1 file changed, 8 insertions(+)
> >>
> >> diff --git a/tools/perf/arch/x86/util/intel-pt.c b/tools/perf/arch/x86/util/intel-pt.c
> >> index db0ba8caf5a2..af25a7824ee0 100644
> >> --- a/tools/perf/arch/x86/util/intel-pt.c
> >> +++ b/tools/perf/arch/x86/util/intel-pt.c
> >> @@ -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;
> > 
> > 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?
> 
> 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

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

* [tip:perf/core] perf intel-pt: Fix error with config term "pt=0"
  2018-11-28 12:38   ` Adrian Hunter
  2018-11-28 13:09     ` Arnaldo Carvalho de Melo
@ 2018-12-14 20:27     ` tip-bot for Adrian Hunter
  2018-12-18 13:54     ` tip-bot for Adrian Hunter
  2 siblings, 0 replies; 6+ messages in thread
From: tip-bot for Adrian Hunter @ 2018-12-14 20:27 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: jolsa, adrian.hunter, tglx, hpa, acme, linux-kernel, mingo

Commit-ID:  932d0166b1dbc2a631ac718615efc4917412ecf3
Gitweb:     https://git.kernel.org/tip/932d0166b1dbc2a631ac718615efc4917412ecf3
Author:     Adrian Hunter <adrian.hunter@intel.com>
AuthorDate: Mon, 26 Nov 2018 14:12:52 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
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 <adrian.hunter@intel.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Link: http://lkml.kernel.org/r/b7c5b4e5-9497-10e5-fd43-5f3e4a0fe51d@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/arch/x86/util/intel-pt.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/tools/perf/arch/x86/util/intel-pt.c b/tools/perf/arch/x86/util/intel-pt.c
index db0ba8caf5a2..ba8ecaf52200 100644
--- a/tools/perf/arch/x86/util/intel-pt.c
+++ b/tools/perf/arch/x86/util/intel-pt.c
@@ -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);

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

* [tip:perf/core] perf intel-pt: Fix error with config term "pt=0"
  2018-11-28 12:38   ` Adrian Hunter
  2018-11-28 13:09     ` Arnaldo Carvalho de Melo
  2018-12-14 20:27     ` [tip:perf/core] perf intel-pt: Fix error with config term "pt=0" tip-bot for Adrian Hunter
@ 2018-12-18 13:54     ` tip-bot for Adrian Hunter
  2 siblings, 0 replies; 6+ messages in thread
From: tip-bot for Adrian Hunter @ 2018-12-18 13:54 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: tglx, acme, mingo, linux-kernel, jolsa, adrian.hunter, hpa

Commit-ID:  1c6f709b9f96366cc47af23c05ecec9b8c0c392d
Gitweb:     https://git.kernel.org/tip/1c6f709b9f96366cc47af23c05ecec9b8c0c392d
Author:     Adrian Hunter <adrian.hunter@intel.com>
AuthorDate: Mon, 26 Nov 2018 14:12:52 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
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 <adrian.hunter@intel.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Link: http://lkml.kernel.org/r/b7c5b4e5-9497-10e5-fd43-5f3e4a0fe51d@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/arch/x86/util/intel-pt.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/tools/perf/arch/x86/util/intel-pt.c b/tools/perf/arch/x86/util/intel-pt.c
index db0ba8caf5a2..ba8ecaf52200 100644
--- a/tools/perf/arch/x86/util/intel-pt.c
+++ b/tools/perf/arch/x86/util/intel-pt.c
@@ -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);

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

end of thread, other threads:[~2018-12-18 13:54 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-11-27  8:43 [PATCH] perf intel-pt: Fix error with config term pt=0 Adrian Hunter
2018-11-27 12:39 ` Arnaldo Carvalho de Melo
2018-11-28 12:38   ` Adrian Hunter
2018-11-28 13:09     ` Arnaldo Carvalho de Melo
2018-12-14 20:27     ` [tip:perf/core] perf intel-pt: Fix error with config term "pt=0" tip-bot for Adrian Hunter
2018-12-18 13:54     ` tip-bot for Adrian Hunter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.