linux-kernel.vger.kernel.org archive mirror
 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).