2020-08-14 01:25:42

by Jin Yao

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

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 <[email protected]>
---
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


2020-08-14 13:37:59

by Arnaldo Carvalho de Melo

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

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 <[email protected]>
> ---
> 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

2020-08-17 02:34:32

by Jin Yao

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

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

2020-08-19 03:42:57

by Like Xu

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

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 <[email protected]>

>>
>> - 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

2020-08-25 07:58:44

by Jin Yao

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

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 <[email protected]>
>

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
>

2020-08-25 16:45:25

by Arnaldo Carvalho de Melo

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

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 <[email protected]>
> >
>
> 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