Filter option should follow a tracer option

Filter option should follow a tracer option

[Jack Henschel]

Hi everyone,

I'm experimenting with Intel PT and perf, but I am currently stuck at getting filters to work with `perf record`.

I have the following command:
> perf record -e intel_pt// --filter 'filter main @ /bin/sleep' /bin/sleep 1
But it throws this error message:
> --filter option should follow a -e tracepoint or HW tracer option

Maybe this is a trivial mistake, but I don't understand the error, because as far as I'm concerned the --filter options *is* following a -e HW tracer option.

I'm using perf version 4.9.30 with kernel 4.9.30-2+deb9u3 on Debian 9 Stretch.

Best Regards,
Jack Henschel

Re: Filter option should follow a tracer option

[Arnaldo Carvalho de Melo]

Em Fri, Aug 18, 2017 at 11:43:51AM +0200, Jack Henschel escreveu:
> Hi everyone,
> 
> I'm experimenting with Intel PT and perf, but I am currently stuck at getting filters to work with `perf record`.
> 
> I have the following command:
> > perf record -e intel_pt// --filter 'filter main @ /bin/sleep' /bin/sleep 1
> But it throws this error message:
> > --filter option should follow a -e tracepoint or HW tracer option
> 
> Maybe this is a trivial mistake, but I don't understand the error, because as far as I'm concerned the --filter options *is* following a -e HW tracer option.
> 
> I'm using perf version 4.9.30 with kernel 4.9.30-2+deb9u3 on Debian 9 Stretch.

Adrian,

	Are you subscribed to linux-perf-users?

- Arnaldo

Re: Filter option should follow a tracer option

[Adrian Hunter]

On 22/08/17 22:00, Arnaldo Carvalho de Melo wrote:
> Em Fri, Aug 18, 2017 at 11:43:51AM +0200, Jack Henschel escreveu:
>> Hi everyone,
>>
>> I'm experimenting with Intel PT and perf, but I am currently stuck at getting filters to work with `perf record`.
>>
>> I have the following command:
>>> perf record -e intel_pt// --filter 'filter main @ /bin/sleep' /bin/sleep 1
>> But it throws this error message:
>>> --filter option should follow a -e tracepoint or HW tracer option
>>
>> Maybe this is a trivial mistake, but I don't understand the error, because as far as I'm concerned the --filter options *is* following a -e HW tracer option.
>>
>> I'm using perf version 4.9.30 with kernel 4.9.30-2+deb9u3 on Debian 9 Stretch.

It looks like either your CPU does not support Intel PT at all, or it does not support address filtering.

What CPU is it?

Have you checked the value of /sys/bus/event_source/devices/intel_pt/nr_addr_filters ?

Re: Filter option should follow a tracer option

[Jack Henschel]

> Adrian Hunter <adrian.hunter@intel.com> hat am 23. August 2017 um 08:06 geschrieben:
> 
> 
> On 22/08/17 22:00, Arnaldo Carvalho de Melo wrote:
> > Em Fri, Aug 18, 2017 at 11:43:51AM +0200, Jack Henschel escreveu:
> >> Hi everyone,
> >>
> >> I'm experimenting with Intel PT and perf, but I am currently stuck at getting filters to work with `perf record`.
> >>
> >> I have the following command:
> >>> perf record -e intel_pt// --filter 'filter main @ /bin/sleep' /bin/sleep 1
> >> But it throws this error message:
> >>> --filter option should follow a -e tracepoint or HW tracer option
> >>
> >> Maybe this is a trivial mistake, but I don't understand the error, because as far as I'm concerned the --filter options *is* following a -e HW tracer option.
> >>
> >> I'm using perf version 4.9.30 with kernel 4.9.30-2+deb9u3 on Debian 9 Stretch.
> 
> It looks like either your CPU does not support Intel PT at all, or it does not support address filtering.

My CPU (Xeon CPU E5-2680 v4) supports Intel PT (intel_pt flag is present in /proc/cpuinfo), but this is not an issue with Intel PT. Here are some more examples (with various syntaxes):

> $ perf list | grep -E 'intel_pt|branch-loads|cache-misses'
>  cache-misses                                       [Hardware event]
>  branch-loads                                       [Hardware cache event]
>  cache-misses OR cpu/cache-misses/                  [Kernel PMU event]
>  intel_pt//                                         [Kernel PMU event]
> $ perf record --event intel_pt// --exclude-perf -a
> --exclude-perf option should follow a -e tracepoint option
> $ perf record --event=branch-loads --filter 'filter main @ /bin/sleep' -- /bin/sleep 1
> --filter option should follow a -e tracepoint or HW tracer option
> $ perf record -e cache-misses --filter 'filter main @ /bin/sleep' -- /bin/sleep 1
> --filter option should follow a -e tracepoint or HW tracer option

> Have you checked the value of /sys/bus/event_source/devices/intel_pt/nr_addr_filters ?
I don't have that file:
> $ ls -l /sys/bus/event_source/devices/intel_pt/   
> total 0
> drwxr-xr-x 2 root root    0 Aug 22 06:32 caps
> drwxr-xr-x 2 root root    0 Aug 22 06:32 format
> -r--r--r-- 1 root root 4096 Aug 23 04:47 max_nonturbo_ratio
> -rw-r--r-- 1 root root 4096 Aug 23 04:47 perf_event_mux_interval_ms
> drwxr-xr-x 2 root root    0 Aug 23 04:47 power
> lrwxrwxrwx 1 root root    0 Aug 22 06:30 subsystem -> ../../bus/event_source
> -r--r--r-- 1 root root 4096 Aug 23 04:47 tsc_art_ratio
> -r--r--r-- 1 root root 4096 Aug 22 06:32 type
> -rw-r--r-- 1 root root 4096 Aug 22 06:30 uevent


The above commands were ran on linux kernel 4.13-rc6 along with perf 4.13 (both compiled from git):
> $ uname -r
> 4.13.0-rc6+
> $ perf version
> perf version 4.13.rc6.g647081

Greetings
Jack

Re: Filter option should follow a tracer option

[Adrian Hunter]

On 23/08/17 11:51, Jack Henschel wrote:
>> Adrian Hunter <adrian.hunter@intel.com> hat am 23. August 2017 um 08:06 geschrieben:
>>
>>
>> On 22/08/17 22:00, Arnaldo Carvalho de Melo wrote:
>>> Em Fri, Aug 18, 2017 at 11:43:51AM +0200, Jack Henschel escreveu:
>>>> Hi everyone,
>>>>
>>>> I'm experimenting with Intel PT and perf, but I am currently stuck at getting filters to work with `perf record`.
>>>>
>>>> I have the following command:
>>>>> perf record -e intel_pt// --filter 'filter main @ /bin/sleep' /bin/sleep 1
>>>> But it throws this error message:
>>>>> --filter option should follow a -e tracepoint or HW tracer option
>>>>
>>>> Maybe this is a trivial mistake, but I don't understand the error, because as far as I'm concerned the --filter options *is* following a -e HW tracer option.
>>>>
>>>> I'm using perf version 4.9.30 with kernel 4.9.30-2+deb9u3 on Debian 9 Stretch.
>>
>> It looks like either your CPU does not support Intel PT at all, or it does not support address filtering.
> 
> My CPU (Xeon CPU E5-2680 v4) supports Intel PT (intel_pt flag is present in /proc/cpuinfo), but this is not an issue with Intel PT. Here are some more examples (with various syntaxes):

I think that is Broadwell which doesn't have address filtering.

> 
>> $ perf list | grep -E 'intel_pt|branch-loads|cache-misses'
>>  cache-misses                                       [Hardware event]
>>  branch-loads                                       [Hardware cache event]
>>  cache-misses OR cpu/cache-misses/                  [Kernel PMU event]
>>  intel_pt//                                         [Kernel PMU event]
>> $ perf record --event intel_pt// --exclude-perf -a
>> --exclude-perf option should follow a -e tracepoint option
>> $ perf record --event=branch-loads --filter 'filter main @ /bin/sleep' -- /bin/sleep 1
>> --filter option should follow a -e tracepoint or HW tracer option
>> $ perf record -e cache-misses --filter 'filter main @ /bin/sleep' -- /bin/sleep 1
>> --filter option should follow a -e tracepoint or HW tracer option
> 
>> Have you checked the value of /sys/bus/event_source/devices/intel_pt/nr_addr_filters ?
> I don't have that file:

Which definitely means address filtering is not supported.  That is
mentioned in the man page for 'perf record'.

Re: Filter option should follow a tracer option

[Jack Henschel]

> Adrian Hunter <adrian.hunter@intel.com> hat am 23. August 2017 um 12:33 geschrieben:
> 
> 
> On 23/08/17 11:51, Jack Henschel wrote:
> >> Adrian Hunter <adrian.hunter@intel.com> hat am 23. August 2017 um 08:06 geschrieben:
> >>
> >>
> >> On 22/08/17 22:00, Arnaldo Carvalho de Melo wrote:
> >>> Em Fri, Aug 18, 2017 at 11:43:51AM +0200, Jack Henschel escreveu:
> >>>> Hi everyone,
> >>>>
> >>>> I'm experimenting with Intel PT and perf, but I am currently stuck at getting filters to work with `perf record`.
> >>>>
> >>>> I have the following command:
> >>>>> perf record -e intel_pt// --filter 'filter main @ /bin/sleep' /bin/sleep 1
> >>>> But it throws this error message:
> >>>>> --filter option should follow a -e tracepoint or HW tracer option
> >>>>
> >>>> Maybe this is a trivial mistake, but I don't understand the error, because as far as I'm concerned the --filter options *is* following a -e HW tracer option.
> >>>>
> >>>> I'm using perf version 4.9.30 with kernel 4.9.30-2+deb9u3 on Debian 9 Stretch.
> >>
> >> It looks like either your CPU does not support Intel PT at all, or it does not support address filtering.
> > 
> > My CPU (Xeon CPU E5-2680 v4) supports Intel PT (intel_pt flag is present in /proc/cpuinfo), but this is not an issue with Intel PT. Here are some more examples (with various syntaxes):
> 
> I think that is Broadwell which doesn't have address filtering.
Indeed, it is Broadwell.

> > 
> >> $ perf list | grep -E 'intel_pt|branch-loads|cache-misses'
> >>  cache-misses                                       [Hardware event]
> >>  branch-loads                                       [Hardware cache event]
> >>  cache-misses OR cpu/cache-misses/                  [Kernel PMU event]
> >>  intel_pt//                                         [Kernel PMU event]
> >> $ perf record --event intel_pt// --exclude-perf -a
> >> --exclude-perf option should follow a -e tracepoint option
> >> $ perf record --event=branch-loads --filter 'filter main @ /bin/sleep' -- /bin/sleep 1
> >> --filter option should follow a -e tracepoint or HW tracer option
> >> $ perf record -e cache-misses --filter 'filter main @ /bin/sleep' -- /bin/sleep 1
> >> --filter option should follow a -e tracepoint or HW tracer option
> > 
> >> Have you checked the value of /sys/bus/event_source/devices/intel_pt/nr_addr_filters ?
> > I don't have that file:
> 
> Which definitely means address filtering is not supported.  That is
> mentioned in the man page for 'perf record'.
Ok, thanks for the clarification. It is mentioned in the perf record man-page, but the perf error message could be a little bit more specific for this issue. (e.g. /sys/bus/event_source/devices/intel_pt/nr_addr_filters not present -> "CPU does not support address filtering").

Thanks for your help!

Greetings
Jack

Re: Filter option should follow a tracer option

[Arnaldo Carvalho de Melo]

Em Wed, Aug 23, 2017 at 02:11:16PM +0200, Jack Henschel escreveu:
> > Adrian Hunter <adrian.hunter@intel.com> hat am 23. August 2017 um 12:33 geschrieben:
> > On 23/08/17 11:51, Jack Henschel wrote:
> > >> Adrian Hunter <adrian.hunter@intel.com> hat am 23. August 2017 um 08:06 geschrieben:
> > >> On 22/08/17 22:00, Arnaldo Carvalho de Melo wrote:
> > >>> Em Fri, Aug 18, 2017 at 11:43:51AM +0200, Jack Henschel escreveu:
> > >>>> I'm using perf version 4.9.30 with kernel 4.9.30-2+deb9u3 on Debian 9 Stretch.

> > >> It looks like either your CPU does not support Intel PT at all, or it does not support address filtering.

> > > My CPU (Xeon CPU E5-2680 v4) supports Intel PT (intel_pt flag is
> > > present in /proc/cpuinfo), but this is not an issue with Intel PT.
> > > Here are some more examples (with various syntaxes):

> > I think that is Broadwell which doesn't have address filtering.
> Indeed, it is Broadwell.
 
> > >> Have you checked the value of /sys/bus/event_source/devices/intel_pt/nr_addr_filters ?
> > > I don't have that file:
> > 
> > Which definitely means address filtering is not supported.  That is
> > mentioned in the man page for 'perf record'.

> Ok, thanks for the clarification. It is mentioned in the perf record
> man-page, but the perf error message could be a little bit more
> specific for this issue. (e.g.
> /sys/bus/event_source/devices/intel_pt/nr_addr_filters not present ->
> "CPU does not support address filtering").

Right, I'll get that into some __strerror() function. Will CC you when
that is done.

E.g. __strerror() function: perf_evsel__open_strerror(), that helps the
user wrt errors returned by the perf_evsel__open() function:

https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/tree/tools/perf/util/evsel.c?h=perf/core#n2669

- Arnaldo

Re: Filter option should follow a tracer option

[Jack Henschel]

[-- Attachment #1.1: Type: text/plain, Size: 7012 bytes --]

On 08/23/2017 09:07 PM, Arnaldo Carvalho de Melo wrote:
> Em Wed, Aug 23, 2017 at 03:40:41PM +0200, Jack Henschel escreveu:
>> On 08/23/2017 03:22 PM, Arnaldo Carvalho de Melo wrote:
>>> Em Wed, Aug 23, 2017 at 02:11:16PM +0200, Jack Henschel escreveu:
>>>>> Adrian Hunter <adrian.hunter@intel.com> hat am 23. August 2017 um 12:33 geschrieben:
>>>>> On 23/08/17 11:51, Jack Henschel wrote:
>>>>>>> Adrian Hunter <adrian.hunter@intel.com> hat am 23. August 2017 um 08:06 geschrieben:
>>>>>>> On 22/08/17 22:00, Arnaldo Carvalho de Melo wrote:
>>>>>>>> Em Fri, Aug 18, 2017 at 11:43:51AM +0200, Jack Henschel escreveu:
>>>>>>>>> I'm using perf version 4.9.30 with kernel 4.9.30-2+deb9u3 on Debian 9 Stretch.
>>>
>>>>>>> It looks like either your CPU does not support Intel PT at all, or it does not support address filtering.
>>>
>>>>>> My CPU (Xeon CPU E5-2680 v4) supports Intel PT (intel_pt flag is
>>>>>> present in /proc/cpuinfo), but this is not an issue with Intel PT.
>>>>>> Here are some more examples (with various syntaxes):
>>>
>>>>> I think that is Broadwell which doesn't have address filtering.
>>>> Indeed, it is Broadwell.
>>>  
>>>>>>> Have you checked the value of /sys/bus/event_source/devices/intel_pt/nr_addr_filters ?
>>>>>> I don't have that file:
>>>>>
>>>>> Which definitely means address filtering is not supported.  That is
>>>>> mentioned in the man page for 'perf record'.
>>>
>>>> Ok, thanks for the clarification. It is mentioned in the perf record
>>>> man-page, but the perf error message could be a little bit more
>>>> specific for this issue. (e.g.
>>>> /sys/bus/event_source/devices/intel_pt/nr_addr_filters not present ->
>>>> "CPU does not support address filtering").
>>>
>>> Right, I'll get that into some __strerror() function. Will CC you when
>>> that is done.
>>>
>>> E.g. __strerror() function: perf_evsel__open_strerror(), that helps the
>>> user wrt errors returned by the perf_evsel__open() function:
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/tree/tools/perf/util/evsel.c?h=perf/core#n2669
>>
>> Hi,
>>
>> I just hacked together a quick patch for this, too :-)
>>
>> Since you are 
>> a) the maintainer of affected files (as I have just found out with the get_maintainer.pl script) 
>> and 
>> b) this is my first kernel patch
>> maybe you can give me some feedback on it. I did my best to follow all kernel coding and contributing guidelines.
> 
> Ok, thank you in advance.
> 
> Even in these cases, its always good to CC the development mailing list,
> which in this case is linux-kernel@vger.kernel.org, because even I'm
> being the maintainer of tools/perf/ in general there are areas where
> other people, like Jiri Olsa, here CCed have more experience or are the
> primary authors.
Ok, thanks for the hint. :-)

> Having said that, from a quick look, I was expecting a even more precise
> explanation, one tied to intel PT, where that file Adrian pointed out,
> would have its existence checked
> (/sys/bus/event_source/devices/intel_pt/nr_addr_filters).
I would have made the error message more explicit, but as far as I can see this does not necessarily have anything to with intel_pt (other hardware such as Intel RTIT may provide address filtering, too):

set_filter (util/parse-events.c) calls perf_pmu__scan to find available PMUs. 
perf_pmu__scan (util/pmu.c) is in this case (pmu == NULL) just a wrapper over pmu_read_sysfs.
pmu_read_sysfs (util/pmu.c) in turn just reads all available entries from EVENT_SOURCE_DEVICE_PATH (/sys/bus/event_source/devices/).

=> Not necessarily Intel PT specific.

> Because people that know that their CPU has Intel PT, say having a
> broadwell, like you, would maybe think that they could use hw filters,
> when only some later chips have that feature.
> 
> Adrian, is this something that started from some specific revision that
> we could point out in the message?

I spent yesterdays afternoon digging through the Intel Webpage to find out which chips / micro-architecture supports which feature, but so far the only real resource I have found is Andi Kleen's blog post on Intel PT:

CPU 	                                    Support
Broadwell (5th generation Core, Xeon v4)    More overhead. No fine grained timing.
Skylake (6th generation Core, Xeon v5) 	    Fine grained timing. Address filtering.
Goldmont (Apollo Lake, Denverton) 	    Fine grained timing. Address filtering.

http://halobates.de/blog/p/410

Greetings
Jack


> - Arnaldo
>  
>> (see attached)
>>
>> Greetings
>> Jack
> 
>> From 3143fb373b16eb6e8f674cca7127801e2809e4c0 Mon Sep 17 00:00:00 2001
>> From: Jack Henschel <jackdev@mailbox.org>
>> Date: Wed, 23 Aug 2017 15:20:40 +0200
>> Subject: [PATCH] perf events parse: Improve error message for address filters
>>
>> This patch improves the error message of the perf events parser
>> when the hardware does not support address filters.
>>
>> Previously, the perf returned the following error:
>>> --filter option should follow a -e tracepoint or HW tracer option
>> This implies there is some syntax error present in the command line,
>> which is not true. Rather, notify the user that the CPU does not have
>> support for this feature.
>>
>> Signed-off-by: Jack Henschel <jackdev@mailbox.org>
>> ---
>>  tools/perf/util/parse-events.c | 20 ++++++++++----------
>>  1 file changed, 10 insertions(+), 10 deletions(-)
>>
>> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
>> index 01e779b91c8e..7f9e3906fb81 100644
>> --- a/tools/perf/util/parse-events.c
>> +++ b/tools/perf/util/parse-events.c
>> @@ -1833,8 +1833,11 @@ static int set_filter(struct perf_evsel *evsel, const void *arg)
>>  	int nr_addr_filters = 0;
>>  	struct perf_pmu *pmu = NULL;
>>  
>> -	if (evsel == NULL)
>> -		goto err;
>> +	if (evsel == NULL) {
>> +		fprintf(stderr,
>> +			"--filter option should follow a -e tracepoint or HW tracer option\n");
>> +		return -1;
>> +	}
>>  
>>  	if (evsel->attr.type == PERF_TYPE_TRACEPOINT) {
>>  		if (perf_evsel__append_tp_filter(evsel, str) < 0) {
>> @@ -1856,8 +1859,11 @@ static int set_filter(struct perf_evsel *evsel, const void *arg)
>>  		perf_pmu__scan_file(pmu, "nr_addr_filters",
>>  				    "%d", &nr_addr_filters);
>>  
>> -	if (!nr_addr_filters)
>> -		goto err;
>> +	if (!nr_addr_filters) {
>> +		fprintf(stderr,
>> +			"This CPU does not support address filtering\n");
>> +		return -1;
>> +	}
>>  
>>  	if (perf_evsel__append_addr_filter(evsel, str) < 0) {
>>  		fprintf(stderr,
>> @@ -1866,12 +1872,6 @@ static int set_filter(struct perf_evsel *evsel, const void *arg)
>>  	}
>>  
>>  	return 0;
>> -
>> -err:
>> -	fprintf(stderr,
>> -		"--filter option should follow a -e tracepoint or HW tracer option\n");
>> -
>> -	return -1;
>>  }
>>  
>>  int parse_filter(const struct option *opt, const char *str,
>> -- 
>> 2.14.1
>>
> 
> 
> 
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 862 bytes --]

Re: Filter option should follow a tracer option

[Kim Phillips]

On Wed, 23 Aug 2017 10:22:51 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

Hi Arnaldo,

[adding tools/perf/util/evsel.c maintainers to cc]

> Em Wed, Aug 23, 2017 at 02:11:16PM +0200, Jack Henschel escreveu:
> > > Adrian Hunter <adrian.hunter@intel.com> hat am 23. August 2017 um 12:33 geschrieben:
> > > On 23/08/17 11:51, Jack Henschel wrote:
> > > >> Adrian Hunter <adrian.hunter@intel.com> hat am 23. August 2017 um 08:06 geschrieben:
> > > >> On 22/08/17 22:00, Arnaldo Carvalho de Melo wrote:
> > > >>> Em Fri, Aug 18, 2017 at 11:43:51AM +0200, Jack Henschel escreveu:
> > > >> Have you checked the value of /sys/bus/event_source/devices/intel_pt/nr_addr_filters ?
> > > > I don't have that file:
> > > 
> > > Which definitely means address filtering is not supported.  That is
> > > mentioned in the man page for 'perf record'.
> 
> > Ok, thanks for the clarification. It is mentioned in the perf record
> > man-page, but the perf error message could be a little bit more
> > specific for this issue. (e.g.
> > /sys/bus/event_source/devices/intel_pt/nr_addr_filters not present ->
> > "CPU does not support address filtering").
> 
> Right, I'll get that into some __strerror() function. Will CC you when
> that is done.
> 
> E.g. __strerror() function: perf_evsel__open_strerror(), that helps the
> user wrt errors returned by the perf_evsel__open() function:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/tree/tools/perf/util/evsel.c?h=perf/core#n2669

I don't know if you've noticed, but I too am not very happy with the
way perf and/or various PMU drivers cannot report exact error
information back to the user.

I tried to inquire about alternatives to pr_err/dmesg, and gave David
Howell's supplemental error facility "errorf" a shot [1], which worked
nicely I thought until I chased David Howells down at Plumbers, where
he said Linus Torvalds had nacked it (on a mailing list whose archives
aren't publically accessible, linux-summit I think).

So, for any PMU drivers that can return a single error code for multiple
reasons, see e.g. [2], we're effectively back to square one, i.e., the
suboptimal pr_err/dmesg.

In this email thread's Intel-PT case, the fix is to add additional
error message text to perf userspace's perf_evsel__open_strerror() for
the additional possible condition(s) that can make the driver fail its
event_init(), and tell the user what the problem is, based on the
specific error code returned by the PMU driver.

In Arm arch, however, there is a greater variety of on- and off-core
(also called SoC) PMUs that do not have an as-consistent look and
feel as in x86, power, or s390 arches.  I'm also seeing a trend in x86
uncore drivers to have more error conditions than the on-core ones [3].

Back on Arm, though, the naive perf user usually gets confusing
messages, e.g., perf saying:

"PMU Hardware doesn't support sampling/overflow-interrupts."

if the user didn't supply an event sample period to record via -c.

Or "The arm_spe_0// event is not supported." instead of something like
"spe-pmu@0: not supported on CPU 4. Supported CPU list: 0-3"

I can - and intend on - cleaning up the most general Arm cases, and
aligning the PMU driver exit codes with new userspace text in evsel.c,
which may or may not agree with the existing x86 content.

Would you agree with this type of change, and if so, would you
prefer an ifdef on $ARCH, or a dynamic/runtime arch check?

I haven't attempted it yet, but imagine things will still be unclear
for the user in cases where there are too many PMU driver conditions
per single error code.  In that case, should the perf userspace report
the error, after having gained knowledge about the specific PMU in
use?  IOW, should userspace perf customize its error messages to the
PMU in the record -e parameter?

If yes, then consider the case of the Arm SPE driver, where the h/w
will only accept 256, 512, 768, 1024, 1536, 2048, 3072, 4096 values for
the event sample period (currently specified via "perf record -c <n>").
In order to inform the user of the possible values for the particular
version of the h/w, should the driver be modified to expose this
numeric list to the perf tool via /sys/bus/event_source/devices/?  It
could then emit something like this:

"spe-pmu@0: invalid sample period.  Valid values are 256, 512, 768,
1024, 1536, 2048, 3072, 4096"

Thanks for your input into these important perf usage case matters.

Kim

[1] https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1472313.html

[2] https://github.com/torvalds/linux/blob/master/drivers/perf/xgene_pmu.c#L899
https://github.com/torvalds/linux/blob/master/drivers/perf/qcom_l3_pmu.c#L486
https://github.com/torvalds/linux/blob/master/drivers/perf/qcom_l2_pmu.c#L473

[3] https://github.com/torvalds/linux/blob/master/arch/x86/events/intel/uncore_snb.c#L342
https://github.com/torvalds/linux/blob/master/arch/x86/events/amd/uncore.c#L184

Re: Filter option should follow a tracer option

[Kim Phillips]

On Wed, 23 Aug 2017 10:22:51 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

Hi Arnaldo,

[adding tools/perf/util/evsel.c maintainers to cc]

> Em Wed, Aug 23, 2017 at 02:11:16PM +0200, Jack Henschel escreveu:
> > > Adrian Hunter <adrian.hunter@intel.com> hat am 23. August 2017 um 12:33 geschrieben:
> > > On 23/08/17 11:51, Jack Henschel wrote:
> > > >> Adrian Hunter <adrian.hunter@intel.com> hat am 23. August 2017 um 08:06 geschrieben:
> > > >> On 22/08/17 22:00, Arnaldo Carvalho de Melo wrote:
> > > >>> Em Fri, Aug 18, 2017 at 11:43:51AM +0200, Jack Henschel escreveu:
> > > >> Have you checked the value of /sys/bus/event_source/devices/intel_pt/nr_addr_filters ?
> > > > I don't have that file:
> > > 
> > > Which definitely means address filtering is not supported.  That is
> > > mentioned in the man page for 'perf record'.
> 
> > Ok, thanks for the clarification. It is mentioned in the perf record
> > man-page, but the perf error message could be a little bit more
> > specific for this issue. (e.g.
> > /sys/bus/event_source/devices/intel_pt/nr_addr_filters not present ->
> > "CPU does not support address filtering").
> 
> Right, I'll get that into some __strerror() function. Will CC you when
> that is done.
> 
> E.g. __strerror() function: perf_evsel__open_strerror(), that helps the
> user wrt errors returned by the perf_evsel__open() function:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/tree/tools/perf/util/evsel.c?h=perf/core#n2669

I don't know if you've noticed, but I too am not very happy with the
way perf and/or various PMU drivers cannot report exact error
information back to the user.

I tried to inquire about alternatives to pr_err/dmesg, and gave David
Howell's supplemental error facility "errorf" a shot [1], which worked
nicely I thought until I chased David Howells down at Plumbers, where
he said Linus Torvalds had nacked it (on a mailing list whose archives
aren't publically accessible, linux-summit I think).

So, for any PMU drivers that can return a single error code for multiple
reasons, see e.g. [2], we're effectively back to square one, i.e., the
suboptimal pr_err/dmesg.

In this email thread's Intel-PT case, the fix is to add additional
error message text to perf userspace's perf_evsel__open_strerror() for
the additional possible condition(s) that can make the driver fail its
event_init(), and tell the user what the problem is, based on the
specific error code returned by the PMU driver.

In Arm arch, however, there is a greater variety of on- and off-core
(also called SoC) PMUs that do not have an as-consistent look and
feel as in x86, power, or s390 arches.  I'm also seeing a trend in x86
uncore drivers to have more error conditions than the on-core ones [3].

Back on Arm, though, the naive perf user usually gets confusing
messages, e.g., perf saying:

"PMU Hardware doesn't support sampling/overflow-interrupts."

if the user didn't supply an event sample period to record via -c.

Or "The arm_spe_0// event is not supported." instead of something like
"spe-pmu@0: not supported on CPU 4. Supported CPU list: 0-3"

I can - and intend on - cleaning up the most general Arm cases, and
aligning the PMU driver exit codes with new userspace text in evsel.c,
which may or may not agree with the existing x86 content.

Would you agree with this type of change, and if so, would you
prefer an ifdef on $ARCH, or a dynamic/runtime arch check?

I haven't attempted it yet, but imagine things will still be unclear
for the user in cases where there are too many PMU driver conditions
per single error code.  In that case, should the perf userspace report
the error, after having gained knowledge about the specific PMU in
use?  IOW, should userspace perf customize its error messages to the
PMU in the record -e parameter?

If yes, then consider the case of the Arm SPE driver, where the h/w
will only accept 256, 512, 768, 1024, 1536, 2048, 3072, 4096 values for
the event sample period (currently specified via "perf record -c <n>").
In order to inform the user of the possible values for the particular
version of the h/w, should the driver be modified to expose this
numeric list to the perf tool via /sys/bus/event_source/devices/?  It
could then emit something like this:

"spe-pmu@0: invalid sample period.  Valid values are 256, 512, 768,
1024, 1536, 2048, 3072, 4096"

Thanks for your input into these important perf usage case matters.

Kim

[1] https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1472313.html

[2] https://github.com/torvalds/linux/blob/master/drivers/perf/xgene_pmu.c#L899
https://github.com/torvalds/linux/blob/master/drivers/perf/qcom_l3_pmu.c#L486
https://github.com/torvalds/linux/blob/master/drivers/perf/qcom_l2_pmu.c#L473

[3] https://github.com/torvalds/linux/blob/master/arch/x86/events/intel/uncore_snb.c#L342
https://github.com/torvalds/linux/blob/master/arch/x86/events/amd/uncore.c#L184