[PATCH v3] perf parse-events: Set exclude_guest=1 for user-space counting

[PATCH v3] perf parse-events: Set exclude_guest=1 for user-space counting

[Jin Yao]

Currently if we run 'perf record -e cycles:u', exclude_guest=0.

But it doesn't make sense in most cases that we request for
user-space counting but we also get the guest report.

Of course, we also need to consider perf kvm usage case that
authorized perf users on the host may only want to count guest
user space events. For example,

perf kvm --guest record -e cycles:u

When we have 'exclude_guest=1' for perf kvm usage, we may get
nothing from guest events.

To keep perf semantics consistent and clear, this patch sets
exclude_guest=1 for user-space counting but except for perf
kvm usage.

Before:
  perf record -e cycles:u ./div
  perf evlist -v
  cycles:u: ..., exclude_kernel: 1, exclude_hv: 1, ...

After:
  perf record -e cycles:u ./div
  perf evlist -v
  cycles:u: ..., exclude_kernel: 1, exclude_hv: 1,  exclude_guest: 1, ...

Before:
  perf kvm --guest record -e cycles:u -vvv

perf_event_attr:
  size                             120
  { sample_period, sample_freq }   4000
  sample_type                      IP|TID|TIME|ID|CPU|PERIOD
  read_format                      ID
  disabled                         1
  inherit                          1
  exclude_kernel                   1
  exclude_hv                       1
  freq                             1
  sample_id_all                    1

After:
  perf kvm --guest record -e cycles:u -vvv

perf_event_attr:
  size                             120
  { sample_period, sample_freq }   4000
  sample_type                      IP|TID|TIME|ID|CPU|PERIOD
  read_format                      ID
  disabled                         1
  inherit                          1
  exclude_kernel                   1
  exclude_hv                       1
  freq                             1
  sample_id_all                    1

For Before/After, exclude_guest are both 0 for perf kvm usage.

perf test 6
 6: Parse event definition strings             : Ok

Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
---
 v3:
 ---
 For perf kvm, if we have 'exclude_guest=1', we can't get guest
 events. So we don't set 'exclude_guest=1' for perf kvm.

 v2:
 ---
 Fix the 'perf test 6' failure.

 tools/perf/tests/parse-events.c | 4 ++--
 tools/perf/util/parse-events.c  | 3 +++
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
index 7f9f87a470c3..aae0fd9045c1 100644
--- a/tools/perf/tests/parse-events.c
+++ b/tools/perf/tests/parse-events.c
@@ -719,7 +719,7 @@ static int test__group2(struct evlist *evlist)
 	TEST_ASSERT_VAL("wrong exclude_user", !evsel->core.attr.exclude_user);
 	TEST_ASSERT_VAL("wrong exclude_kernel", !evsel->core.attr.exclude_kernel);
 	TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv);
-	TEST_ASSERT_VAL("wrong exclude guest", !evsel->core.attr.exclude_guest);
+	TEST_ASSERT_VAL("wrong exclude guest", evsel->core.attr.exclude_guest);
 	TEST_ASSERT_VAL("wrong exclude host", !evsel->core.attr.exclude_host);
 	TEST_ASSERT_VAL("wrong precise_ip", !evsel->core.attr.precise_ip);
 	TEST_ASSERT_VAL("wrong leader", evsel__is_group_leader(evsel));
@@ -842,7 +842,7 @@ static int test__group3(struct evlist *evlist __maybe_unused)
 	TEST_ASSERT_VAL("wrong exclude_user", !evsel->core.attr.exclude_user);
 	TEST_ASSERT_VAL("wrong exclude_kernel", evsel->core.attr.exclude_kernel);
 	TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv);
-	TEST_ASSERT_VAL("wrong exclude guest", !evsel->core.attr.exclude_guest);
+	TEST_ASSERT_VAL("wrong exclude guest", evsel->core.attr.exclude_guest);
 	TEST_ASSERT_VAL("wrong exclude host", !evsel->core.attr.exclude_host);
 	TEST_ASSERT_VAL("wrong precise_ip", !evsel->core.attr.precise_ip);
 	TEST_ASSERT_VAL("wrong leader", evsel__is_group_leader(evsel));
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 9f7260e69113..ff4c23d2a0f3 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -37,6 +37,7 @@
 #include "util/evsel_config.h"
 #include "util/event.h"
 #include "util/pfm.h"
+#include "perf.h"
 
 #define MAX_NAME_LEN 100
 
@@ -1794,6 +1795,8 @@ static int get_event_modifier(struct event_modifier *mod, char *str,
 		if (*str == 'u') {
 			if (!exclude)
 				exclude = eu = ek = eh = 1;
+			if (!exclude_GH && !perf_guest)
+				eG = 1;
 			eu = 0;
 		} else if (*str == 'k') {
 			if (!exclude)
-- 
2.17.1

Re: [PATCH v3] perf parse-events: Set exclude_guest=1 for user-space counting

[Arnaldo Carvalho de Melo]

Em Fri, Aug 14, 2020 at 09:21:20AM +0800, Jin Yao escreveu:
> Currently if we run 'perf record -e cycles:u', exclude_guest=0.
> 
> But it doesn't make sense in most cases that we request for
> user-space counting but we also get the guest report.
> 
> Of course, we also need to consider perf kvm usage case that
> authorized perf users on the host may only want to count guest
> user space events. For example,
> 
> perf kvm --guest record -e cycles:u

Ok, probably this works, but what if I want to record guest user samples
without using 'perf kvm'?

Can we have a 'U' modifier, perhaps, for that?

I.e.

perf record -e cycles:uU would not set exclude_host not exclude_guest,
cycles:u excludes guest user, cycles:U excludes host user, would that be
possible?

Anyway, I think that with what we have, your patch makes sense, having a
way to, without using 'perf kvm' still be able to sample the guest can
be done on top. of this.

Xu, can we get your Reviewed-by if this addresses your concerns?

- Arnaldo
 
> When we have 'exclude_guest=1' for perf kvm usage, we may get
> nothing from guest events.
> 
> To keep perf semantics consistent and clear, this patch sets
> exclude_guest=1 for user-space counting but except for perf
> kvm usage.
> 
> Before:
>   perf record -e cycles:u ./div
>   perf evlist -v
>   cycles:u: ..., exclude_kernel: 1, exclude_hv: 1, ...
> 
> After:
>   perf record -e cycles:u ./div
>   perf evlist -v
>   cycles:u: ..., exclude_kernel: 1, exclude_hv: 1,  exclude_guest: 1, ...
> 
> Before:
>   perf kvm --guest record -e cycles:u -vvv
> 
> perf_event_attr:
>   size                             120
>   { sample_period, sample_freq }   4000
>   sample_type                      IP|TID|TIME|ID|CPU|PERIOD
>   read_format                      ID
>   disabled                         1
>   inherit                          1
>   exclude_kernel                   1
>   exclude_hv                       1
>   freq                             1
>   sample_id_all                    1
> 
> After:
>   perf kvm --guest record -e cycles:u -vvv
> 
> perf_event_attr:
>   size                             120
>   { sample_period, sample_freq }   4000
>   sample_type                      IP|TID|TIME|ID|CPU|PERIOD
>   read_format                      ID
>   disabled                         1
>   inherit                          1
>   exclude_kernel                   1
>   exclude_hv                       1
>   freq                             1
>   sample_id_all                    1
> 
> For Before/After, exclude_guest are both 0 for perf kvm usage.
> 
> perf test 6
>  6: Parse event definition strings             : Ok
> 
> Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
> ---
>  v3:
>  ---
>  For perf kvm, if we have 'exclude_guest=1', we can't get guest
>  events. So we don't set 'exclude_guest=1' for perf kvm.
> 
>  v2:
>  ---
>  Fix the 'perf test 6' failure.
> 
>  tools/perf/tests/parse-events.c | 4 ++--
>  tools/perf/util/parse-events.c  | 3 +++
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
> index 7f9f87a470c3..aae0fd9045c1 100644
> --- a/tools/perf/tests/parse-events.c
> +++ b/tools/perf/tests/parse-events.c
> @@ -719,7 +719,7 @@ static int test__group2(struct evlist *evlist)
>  	TEST_ASSERT_VAL("wrong exclude_user", !evsel->core.attr.exclude_user);
>  	TEST_ASSERT_VAL("wrong exclude_kernel", !evsel->core.attr.exclude_kernel);
>  	TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv);
> -	TEST_ASSERT_VAL("wrong exclude guest", !evsel->core.attr.exclude_guest);
> +	TEST_ASSERT_VAL("wrong exclude guest", evsel->core.attr.exclude_guest);
>  	TEST_ASSERT_VAL("wrong exclude host", !evsel->core.attr.exclude_host);
>  	TEST_ASSERT_VAL("wrong precise_ip", !evsel->core.attr.precise_ip);
>  	TEST_ASSERT_VAL("wrong leader", evsel__is_group_leader(evsel));
> @@ -842,7 +842,7 @@ static int test__group3(struct evlist *evlist __maybe_unused)
>  	TEST_ASSERT_VAL("wrong exclude_user", !evsel->core.attr.exclude_user);
>  	TEST_ASSERT_VAL("wrong exclude_kernel", evsel->core.attr.exclude_kernel);
>  	TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv);
> -	TEST_ASSERT_VAL("wrong exclude guest", !evsel->core.attr.exclude_guest);
> +	TEST_ASSERT_VAL("wrong exclude guest", evsel->core.attr.exclude_guest);
>  	TEST_ASSERT_VAL("wrong exclude host", !evsel->core.attr.exclude_host);
>  	TEST_ASSERT_VAL("wrong precise_ip", !evsel->core.attr.precise_ip);
>  	TEST_ASSERT_VAL("wrong leader", evsel__is_group_leader(evsel));
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 9f7260e69113..ff4c23d2a0f3 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -37,6 +37,7 @@
>  #include "util/evsel_config.h"
>  #include "util/event.h"
>  #include "util/pfm.h"
> +#include "perf.h"
>  
>  #define MAX_NAME_LEN 100
>  
> @@ -1794,6 +1795,8 @@ static int get_event_modifier(struct event_modifier *mod, char *str,
>  		if (*str == 'u') {
>  			if (!exclude)
>  				exclude = eu = ek = eh = 1;
> +			if (!exclude_GH && !perf_guest)
> +				eG = 1;
>  			eu = 0;
>  		} else if (*str == 'k') {
>  			if (!exclude)
> -- 
> 2.17.1
> 

-- 

- Arnaldo

Re: [PATCH v3] perf parse-events: Set exclude_guest=1 for user-space counting

[Jin, Yao]

Hi Arnaldo,

On 8/14/2020 8:47 PM, Arnaldo Carvalho de Melo wrote:
> Em Fri, Aug 14, 2020 at 09:21:20AM +0800, Jin Yao escreveu:
>> Currently if we run 'perf record -e cycles:u', exclude_guest=0.
>>
>> But it doesn't make sense in most cases that we request for
>> user-space counting but we also get the guest report.
>>
>> Of course, we also need to consider perf kvm usage case that
>> authorized perf users on the host may only want to count guest
>> user space events. For example,
>>
>> perf kvm --guest record -e cycles:u
> Ok, probably this works, but what if I want to record guest user samples
> without using 'perf kvm'?
> 
> Can we have a 'U' modifier, perhaps, for that?
> 
> I.e.
> 
> perf record -e cycles:uU would not set exclude_host not exclude_guest,
> cycles:u excludes guest user, cycles:U excludes host user, would that be
> possible?
> 
> Anyway, I think that with what we have, your patch makes sense, having a
> way to, without using 'perf kvm' still be able to sample the guest can
> be done on top. of this.
> 
> Xu, can we get your Reviewed-by if this addresses your concerns?
> 
> - Arnaldo
>   

Do you suggest we will create a new modifier 'U' as a follow-up patch?

cycles:u - Count host user space but it doesn't count the guest user space (exclude_host = 0, 
exclude_guest = 1, exclude_kernel = 1, exclude_user = 0, exclude_hv = 1)

cycles:U - Count the guest user space but it doesn't count the host user space (exclude_host = 1, 
exclude_guest = 0, exclude_kernel = 1, exclude_user = 0, exclude_hv = 1)

Is above understanding correct?

Thanks
Jin Yao

Re: [PATCH v3] perf parse-events: Set exclude_guest=1 for user-space counting

[Like Xu]

Hi Arnaldo,

On 2020/8/17 10:32, Jin, Yao wrote:
> Hi Arnaldo,
> 
> On 8/14/2020 8:47 PM, Arnaldo Carvalho de Melo wrote:
>> Em Fri, Aug 14, 2020 at 09:21:20AM +0800, Jin Yao escreveu:
>>> Currently if we run 'perf record -e cycles:u', exclude_guest=0.
>>>
>>> But it doesn't make sense in most cases that we request for
>>> user-space counting but we also get the guest report.
>>>
>>> Of course, we also need to consider perf kvm usage case that
>>> authorized perf users on the host may only want to count guest
>>> user space events. For example,
>>>
>>> perf kvm --guest record -e cycles:u
>> Ok, probably this works, but what if I want to record guest user samples
>> without using 'perf kvm'?
>>
>> Can we have a 'U' modifier, perhaps, for that?
>>
>> I.e.
>>
>> perf record -e cycles:uU would not set exclude_host not exclude_guest,
>> cycles:u excludes guest user, cycles:U excludes host user, would that be
>> possible?
>>
>> Anyway, I think that with what we have, your patch makes sense, having a
>> way to, without using 'perf kvm' still be able to sample the guest can
>> be done on top. of this.
>>
>> Xu, can we get your Reviewed-by if this addresses your concerns?

My concern is about do not break the perf kvm usages and Yao did it in v3.

Tested-by: Like Xu <like.xu@linux.intel.com>

>>
>> - Arnaldo
> 
> Do you suggest we will create a new modifier 'U' as a follow-up patch?
> 
> cycles:u - Count host user space but it doesn't count the guest user space 
> (exclude_host = 0, exclude_guest = 1, exclude_kernel = 1, exclude_user = 0, 
> exclude_hv = 1)
> 
> cycles:U - Count the guest user space but it doesn't count the host user 
> space (exclude_host = 1, exclude_guest = 0, exclude_kernel = 1, 
> exclude_user = 0, exclude_hv = 1)
> 
> Is above understanding correct?

Do we really need to support sample guests without using perf kvm?

If it's true, I would prefer "cycles:uG" to cover guest user space
events only, "cycles:kG" to cover guest kernel space events only,
and only host events are sampled without the ":G" modifier by default.

I am not sure if ":G" has been used in perf.

At the very least, this v3 patch may not
block the follow-up patch from Yao (if any).

Thanks,
Like Xu

> 
> Thanks
> Jin Yao

Re: [PATCH v3] perf parse-events: Set exclude_guest=1 for user-space counting

[Jin, Yao]

Hi Arnaldo,

On 8/19/2020 11:05 AM, Like Xu wrote:
> Hi Arnaldo,
> 
> On 2020/8/17 10:32, Jin, Yao wrote:
>> Hi Arnaldo,
>>
>> On 8/14/2020 8:47 PM, Arnaldo Carvalho de Melo wrote:
>>> Em Fri, Aug 14, 2020 at 09:21:20AM +0800, Jin Yao escreveu:
>>>> Currently if we run 'perf record -e cycles:u', exclude_guest=0.
>>>>
>>>> But it doesn't make sense in most cases that we request for
>>>> user-space counting but we also get the guest report.
>>>>
>>>> Of course, we also need to consider perf kvm usage case that
>>>> authorized perf users on the host may only want to count guest
>>>> user space events. For example,
>>>>
>>>> perf kvm --guest record -e cycles:u
>>> Ok, probably this works, but what if I want to record guest user samples
>>> without using 'perf kvm'?
>>>
>>> Can we have a 'U' modifier, perhaps, for that?
>>>
>>> I.e.
>>>
>>> perf record -e cycles:uU would not set exclude_host not exclude_guest,
>>> cycles:u excludes guest user, cycles:U excludes host user, would that be
>>> possible?
>>>
>>> Anyway, I think that with what we have, your patch makes sense, having a
>>> way to, without using 'perf kvm' still be able to sample the guest can
>>> be done on top. of this.
>>>
>>> Xu, can we get your Reviewed-by if this addresses your concerns?
> 
> My concern is about do not break the perf kvm usages and Yao did it in v3.
> 
> Tested-by: Like Xu <like.xu@linux.intel.com>
> 

Can this patch be accepted?

>>>
>>> - Arnaldo
>>
>> Do you suggest we will create a new modifier 'U' as a follow-up patch?
>>
>> cycles:u - Count host user space but it doesn't count the guest user space (exclude_host = 0, 
>> exclude_guest = 1, exclude_kernel = 1, exclude_user = 0, exclude_hv = 1)
>>
>> cycles:U - Count the guest user space but it doesn't count the host user space (exclude_host = 1, 
>> exclude_guest = 0, exclude_kernel = 1, exclude_user = 0, exclude_hv = 1)
>>
>> Is above understanding correct?
> 
> Do we really need to support sample guests without using perf kvm?
> 
> If it's true, I would prefer "cycles:uG" to cover guest user space
> events only, "cycles:kG" to cover guest kernel space events only,
> and only host events are sampled without the ":G" modifier by default.
> 
> I am not sure if ":G" has been used in perf.
>

That may be another good idea I think. :)

Thanks
Jin Yao

> At the very least, this v3 patch may not
> block the follow-up patch from Yao (if any).
> 
> Thanks,
> Like Xu
> 
>>
>> Thanks
>> Jin Yao
> 

Re: [PATCH v3] perf parse-events: Set exclude_guest=1 for user-space counting

[Arnaldo Carvalho de Melo]

Em Tue, Aug 25, 2020 at 03:54:37PM +0800, Jin, Yao escreveu:
> Hi Arnaldo,
> 
> On 8/19/2020 11:05 AM, Like Xu wrote:
> > Hi Arnaldo,
> > 
> > On 2020/8/17 10:32, Jin, Yao wrote:
> > > Hi Arnaldo,
> > > 
> > > On 8/14/2020 8:47 PM, Arnaldo Carvalho de Melo wrote:
> > > > Em Fri, Aug 14, 2020 at 09:21:20AM +0800, Jin Yao escreveu:
> > > > > Currently if we run 'perf record -e cycles:u', exclude_guest=0.
> > > > > 
> > > > > But it doesn't make sense in most cases that we request for
> > > > > user-space counting but we also get the guest report.
> > > > > 
> > > > > Of course, we also need to consider perf kvm usage case that
> > > > > authorized perf users on the host may only want to count guest
> > > > > user space events. For example,
> > > > > 
> > > > > perf kvm --guest record -e cycles:u
> > > > Ok, probably this works, but what if I want to record guest user samples
> > > > without using 'perf kvm'?
> > > > 
> > > > Can we have a 'U' modifier, perhaps, for that?
> > > > 
> > > > I.e.
> > > > 
> > > > perf record -e cycles:uU would not set exclude_host not exclude_guest,
> > > > cycles:u excludes guest user, cycles:U excludes host user, would that be
> > > > possible?
> > > > 
> > > > Anyway, I think that with what we have, your patch makes sense, having a
> > > > way to, without using 'perf kvm' still be able to sample the guest can
> > > > be done on top. of this.
> > > > 
> > > > Xu, can we get your Reviewed-by if this addresses your concerns?
> > 
> > My concern is about do not break the perf kvm usages and Yao did it in v3.
> > 
> > Tested-by: Like Xu <like.xu@linux.intel.com>
> > 
> 
> Can this patch be accepted?

Sure.

> > > > 
> > > > - Arnaldo
> > > 
> > > Do you suggest we will create a new modifier 'U' as a follow-up patch?
> > > 
> > > cycles:u - Count host user space but it doesn't count the guest user
> > > space (exclude_host = 0, exclude_guest = 1, exclude_kernel = 1,
> > > exclude_user = 0, exclude_hv = 1)
> > > 
> > > cycles:U - Count the guest user space but it doesn't count the host
> > > user space (exclude_host = 1, exclude_guest = 0, exclude_kernel = 1,
> > > exclude_user = 0, exclude_hv = 1)
> > > 
> > > Is above understanding correct?
> > 
> > Do we really need to support sample guests without using perf kvm?
> > 
> > If it's true, I would prefer "cycles:uG" to cover guest user space
> > events only, "cycles:kG" to cover guest kernel space events only,
> > and only host events are sampled without the ":G" modifier by default.
> > 
> > I am not sure if ":G" has been used in perf.
> > 
> 
> That may be another good idea I think. :)

I have to check, but I don't think that :G is used.

- Arnaldo