Re: [PATCH v1 0/2] perf stat: Support --all-kernel and --all-user

From: Jin, Yao
Date: Fri Oct 11 2019 - 21:49:21 EST

On 10/11/2019 3:21 PM, Jiri Olsa wrote:
On Fri, Oct 11, 2019 at 10:50:35AM +0800, Jin, Yao wrote:

On 10/10/2019 8:33 PM, Arnaldo Carvalho de Melo wrote:
Em Thu, Oct 10, 2019 at 04:33:57PM +0800, Jin, Yao escreveu:

On 10/10/2019 4:00 PM, Jiri Olsa wrote:
On Thu, Oct 10, 2019 at 02:46:36PM +0800, Jin, Yao wrote:

On 10/1/2019 10:17 AM, Andi Kleen wrote:
I think it's useful. Makes it easy to do kernel/user break downs.
perf record should support the same.

Don't we have this already with:

[root@quaco ~]# perf stat -e cycles:u,instructions:u,cycles:k,instructions:k -a -- sleep 1

This only works for simple cases. Try it for --topdown or multiple -M metrics.


Hi Arnaldo, Jiri,

We think it should be very useful if --all-user / --all-kernel can be
specified together, so that we can get a break down between user and kernel

But yes, the patches for supporting this new semantics is much complicated
than the patch which just follows original perf-record behavior. I fully
understand this concern.

So if this new semantics can be accepted, that would be very good. But if
you think the new semantics is too complicated, I'm also fine for posting a
new patch which just follows the perf-record behavior.

I still need to think a bit more about this.. did you consider
other options like cloning of the perf_evlist/perf_evsel and
changing just the exclude* bits? might be event worse actualy ;-)

That should be another approach, but it might be a bit more complicated than
just appending ":u"/":k" modifiers to the event name string.

or maybe if we add modifier we could add extra events/groups
within the parser.. like:


but that might be still more complicated then what you did

Yes agree.

also please add the perf record changes so we have same code
and logic for both if we are going to change it
If this new semantics can be accepted, I'd like to add perf record
supporting as well. :)

Changes in semantics should be avoided, when we add an option already
present in some other tool, we should strive to keep the semantics, so
that people can reuse their knowledge and just switch tools to go from
sampling to counting, say.

Yes, that makes sense. We need to try our best to keep the original
semantics. I will post a patch for perf-stat which just follows the
semantics in perf-record.

So if at all possible, and without having really looked deep in this
specific case, I would prefer that new semantics come with a new syntax,
would that be possible?

Yes, that's possible. Maybe we can use a new option for automatically adding
two copies of the events (one copy for user and the other copy for kernel).
The option something like "--all-space"?

some other ideas:

-e {cycles,cache-misses}:A,cycles,instructions:A
-e {cycles,cache-misses}:B,cycles,instructions:B
--duplicate-every-event-or-group-of-events-for-each-address-space ;-)


I like '--uk'. :)

Jin Yao