[RFC PATCH 3/3 v2] perf: Update perf tool to monitor uncore events

[RFC PATCH 3/3 v2] perf: Update perf tool to monitor uncore events

[Lin Ming]

Uncore events are monitored with raw events with "ru" prefix("u" for uncore).

$ ./perf stat -C 0 -e ru0101 -- ls

 Performance counter stats for 'ls':

            1033791  raw 0x101               

        0.003027583  seconds time elapsed

$ sudo ./perf top -e ru0101

--------------------------------------------------------------------------------------------------------------------
   PerfTop:      22 irqs/sec  kernel:68.2%  exact:  0.0% [1000Hz raw 0x101],  (all, 8 CPUs)
--------------------------------------------------------------------------------------------------------------------

             samples  pcnt function               DSO
             _______ _____ ______________________ ____________________________________

                8.00 18.6% kallsyms_expand_symbol [kernel.kallsyms]                   
                5.00 11.6% rb_next                /home/mlin/linux-2.6/tools/perf/perf
                5.00 11.6% clear_page_c           [kernel.kallsyms]

Signed-off-by: Lin Ming <ming.m.lin@intel.com>
---
 tools/perf/util/parse-events.c |   14 ++++++++++----
 1 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 898aa94..b03457a 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -278,7 +278,7 @@ const char *__event_name(int type, u64 config)
 {
 	static char buf[32];
 
-	if (type == PERF_TYPE_RAW) {
+	if (type == PERF_TYPE_RAW || type == PERF_TYPE_UNCORE) {
 		sprintf(buf, "raw 0x%llx", config);
 		return buf;
 	}
@@ -655,14 +655,20 @@ parse_raw_event(const char **strp, struct perf_event_attr *attr)
 {
 	const char *str = *strp;
 	u64 config;
+	int uncore = 0;
 	int n;
 
 	if (*str != 'r')
 		return EVT_FAILED;
-	n = hex2u64(str + 1, &config);
+	if (*(str+1) == 'u')
+		uncore = 1;
+	n = hex2u64(str + 1 + uncore, &config);
 	if (n > 0) {
-		*strp = str + n + 1;
-		attr->type = PERF_TYPE_RAW;
+		*strp = str + n + 1 + uncore;
+		if (!uncore)
+			attr->type = PERF_TYPE_RAW;
+		else
+			attr->type = PERF_TYPE_UNCORE;
 		attr->config = config;
 		return EVT_HANDLED;
 	}
-- 
1.7.2.3

Re: [RFC PATCH 3/3 v2] perf: Update perf tool to monitor uncore events

[Andi Kleen]

>
>              samples  pcnt function               DSO
>              _______ _____ ______________________
> ____________________________________
>
>                 8.00 18.6% kallsyms_expand_symbol [kernel.kallsyms]

Reporting a symbol for an uncore event seems highly misleading.
After all the uncore counter has no idea for which core the event was,
so there isn't really any instruction pointer to report.
The event could be event caused by a PCI device or similar.

For per function monitoring of uncore events one has to use
OFFCORE_RESPONSE, like I implemented recently.

I would suggest to not report any symbol names for uncore events.
Doing so just will confuse users.

In fact I suspect uncore events are only really useful
with "stat", but not with "top", or if they are used in top
then the symbol reporting should be disabled.

-Andi

Re: [RFC PATCH 3/3 v2] perf: Update perf tool to monitor uncore events

[Stephane Eranian]

On Sun, Nov 21, 2010 at 1:09 PM, Andi Kleen <andi@firstfloor.org> wrote:
>
>>
>>              samples  pcnt function               DSO
>>              _______ _____ ______________________
>> ____________________________________
>>
>>                 8.00 18.6% kallsyms_expand_symbol [kernel.kallsyms]
>
> Reporting a symbol for an uncore event seems highly misleading.
> After all the uncore counter has no idea for which core the event was,
> so there isn't really any instruction pointer to report.
> The event could be event caused by a PCI device or similar.
>
> For per function monitoring of uncore events one has to use
> OFFCORE_RESPONSE, like I implemented recently.
>
> I would suggest to not report any symbol names for uncore events.
> Doing so just will confuse users.
>
> In fact I suspect uncore events are only really useful
> with "stat", but not with "top", or if they are used in top
> then the symbol reporting should be disabled.
>
I agree, uncore should only be used for counting on a
per-cpu basis. You can leave the perf tool as is, but
that opens up the risk of misinterpretation by many users,
or you restrict this in the tool directly which is the better
solution in my mind.

Re: [RFC PATCH 3/3 v2] perf: Update perf tool to monitor uncore events

[Lin Ming]

On Sun, 2010-11-21 at 20:09 +0800, Andi Kleen wrote:
> >
> >              samples  pcnt function               DSO
> >              _______ _____ ______________________
> > ____________________________________
> >
> >                 8.00 18.6% kallsyms_expand_symbol [kernel.kallsyms]
> 
> Reporting a symbol for an uncore event seems highly misleading.
> After all the uncore counter has no idea for which core the event was,
> so there isn't really any instruction pointer to report.
> The event could be event caused by a PCI device or similar.
> 
> For per function monitoring of uncore events one has to use
> OFFCORE_RESPONSE, like I implemented recently.

Let me look at this and then get back to you.

Thanks.

> 
> I would suggest to not report any symbol names for uncore events.
> Doing so just will confuse users.
> 
> In fact I suspect uncore events are only really useful
> with "stat", but not with "top", or if they are used in top
> then the symbol reporting should be disabled.
> 
> -Andi
> 
> 

Re: [RFC PATCH 3/3 v2] perf: Update perf tool to monitor uncore events

[Lin Ming]

On Sun, 2010-11-21 at 20:22 +0800, Stephane Eranian wrote:
> On Sun, Nov 21, 2010 at 1:09 PM, Andi Kleen <andi@firstfloor.org> wrote:
> >
> >>
> >>              samples  pcnt function               DSO
> >>              _______ _____ ______________________
> >> ____________________________________
> >>
> >>                 8.00 18.6% kallsyms_expand_symbol [kernel.kallsyms]
> >
> > Reporting a symbol for an uncore event seems highly misleading.
> > After all the uncore counter has no idea for which core the event was,
> > so there isn't really any instruction pointer to report.
> > The event could be event caused by a PCI device or similar.
> >
> > For per function monitoring of uncore events one has to use
> > OFFCORE_RESPONSE, like I implemented recently.
> >
> > I would suggest to not report any symbol names for uncore events.
> > Doing so just will confuse users.
> >
> > In fact I suspect uncore events are only really useful
> > with "stat", but not with "top", or if they are used in top
> > then the symbol reporting should be disabled.
> >
> I agree, uncore should only be used for counting on a
> per-cpu basis. You can leave the perf tool as is, but
> that opens up the risk of misinterpretation by many users,
> or you restrict this in the tool directly which is the better
> solution in my mind.

I agree restricting the tool is a better idea.
How about below?

#one cpu stat is allowed
perf stat -e ruNNNN -C <cpu>
perf stat -e ruNNNN -C <cpu> -p <pid>
perf stat -e ruNNNN -C <cpu> -- <command>

#per thread is NOT allowed
perf stat -e ruNNNN -p <pid>
perf stat -e ruNNNN -- <command>

#all cpus stat is NOT allowed, because it will mess the results
perf stat -e ruNNNN -a

Re: [RFC PATCH 3/3 v2] perf: Update perf tool to monitor uncore events

[Andi Kleen]

>>
>> In fact I suspect uncore events are only really useful
>> with "stat", but not with "top", or if they are used in top
>> then the symbol reporting should be disabled.
>>
> I agree, uncore should only be used for counting on a
> per-cpu basis. You can leave the perf tool as is, but

I think you mean per socket base, right? per cpu (= cpu core thread)
doesn't make sense for the uncore either.

Per socket would need a new command line mode. The new mode should
never report any symbols or processes, just events per socket.

-Andi

Re: [RFC PATCH 3/3 v2] perf: Update perf tool to monitor uncore events

[Peter Zijlstra]

On Sun, 2010-11-21 at 13:22 +0100, Stephane Eranian wrote:
> On Sun, Nov 21, 2010 at 1:09 PM, Andi Kleen <andi@firstfloor.org> wrote:
> >
> >>
> >>              samples  pcnt function               DSO
> >>              _______ _____ ______________________
> >> ____________________________________
> >>
> >>                 8.00 18.6% kallsyms_expand_symbol [kernel.kallsyms]
> >
> > Reporting a symbol for an uncore event seems highly misleading.
> > After all the uncore counter has no idea for which core the event was,
> > so there isn't really any instruction pointer to report.
> > The event could be event caused by a PCI device or similar.
> >
> > For per function monitoring of uncore events one has to use
> > OFFCORE_RESPONSE, like I implemented recently.
> >
> > I would suggest to not report any symbol names for uncore events.
> > Doing so just will confuse users.
> >
> > In fact I suspect uncore events are only really useful
> > with "stat", but not with "top", or if they are used in top
> > then the symbol reporting should be disabled.
> >
> I agree, uncore should only be used for counting on a
> per-cpu basis. You can leave the perf tool as is, but
> that opens up the risk of misinterpretation by many users,
> or you restrict this in the tool directly which is the better
> solution in my mind.

I would argue against restricting the tool, print a warning perhaps. I
mean, give the user all the rope he needs and tell him how to tie the
knot is the unix way, right? :-)

Re: [RFC PATCH 3/3 v2] perf: Update perf tool to monitor uncore events

[Andi Kleen]

> I would argue against restricting the tool, print a warning perhaps. I
> mean, give the user all the rope he needs and tell him how to tie the
> knot is the unix way, right? :-)

WARNING: the following information is completely useless and wrong

Basically reporting rip here is lying and don't think lying is the
"Unix way".

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

Re: [RFC PATCH 3/3 v2] perf: Update perf tool to monitor uncore events

[Peter Zijlstra]

On Sun, 2010-11-21 at 19:28 +0100, Andi Kleen wrote:
> > I would argue against restricting the tool, print a warning perhaps. I
> > mean, give the user all the rope he needs and tell him how to tie the
> > knot is the unix way, right? :-)
> 
> WARNING: the following information is completely useless and wrong
> 
> Basically reporting rip here is lying and don't think lying is the
> "Unix way".

Yeah, unless there's only one task running on that socket.. Dude,
really, lighten up.

Re: [RFC PATCH 3/3 v2] perf: Update perf tool to monitor uncore events

[Stephane Eranian]

On Sun, Nov 21, 2010 at 5:52 PM, Andi Kleen <andi@firstfloor.org> wrote:
>
>>>
>>> In fact I suspect uncore events are only really useful
>>> with "stat", but not with "top", or if they are used in top
>>> then the symbol reporting should be disabled.
>>>
>> I agree, uncore should only be used for counting on a
>> per-cpu basis. You can leave the perf tool as is, but
>

I meant one cpu per socket: perf stat -a -C 0 -e unc_clk_unhalted.
One could envision the tool picking one CPU per socket. It can
figure this out from the topology information in sysfs.

If you do perf stat -a -e unc_*, then you'll get into multiplexing very
quickly AND it is useless especially because auto aggregation will
yield bogus counts.

> I think you mean per socket base, right? per cpu (= cpu core thread)
> doesn't make sense for the uncore either.
>
> Per socket would need a new command line mode. The new mode should
> never report any symbols or processes, just events per socket.
>
> -Andi
>
>