* [PATCH] perf parse-events: Set exclude_guest for user-space counting
@ 2020-08-12 6:59 Jin Yao
2020-08-12 12:15 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 9+ messages in thread
From: Jin Yao @ 2020-08-12 6:59 UTC (permalink / raw)
To: acme, jolsa, peterz, mingo, alexander.shishkin
Cc: Linux-kernel, ak, kan.liang, yao.jin, like.xu, Jin Yao
Currently if we run 'perf record -e cycles:u', exclude_guest is 0.
But it doesn't make sense that we request for user-space counting
but we also get the guest report.
To keep perf semantics consistent and clear, this patch sets
exclude_guest for user-space counting.
Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
---
tools/perf/util/parse-events.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 9f7260e69113..4d809f1fe269 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1794,6 +1794,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)
+ eG = 1;
eu = 0;
} else if (*str == 'k') {
if (!exclude)
--
2.17.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] perf parse-events: Set exclude_guest for user-space counting
2020-08-12 6:59 [PATCH] perf parse-events: Set exclude_guest for user-space counting Jin Yao
@ 2020-08-12 12:15 ` Arnaldo Carvalho de Melo
2020-08-12 12:55 ` Arnaldo Carvalho de Melo
2020-08-12 13:02 ` Like Xu
0 siblings, 2 replies; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-08-12 12:15 UTC (permalink / raw)
To: Jin Yao
Cc: jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
kan.liang, yao.jin, like.xu
Em Wed, Aug 12, 2020 at 02:59:53PM +0800, Jin Yao escreveu:
> Currently if we run 'perf record -e cycles:u', exclude_guest is 0.
>
> But it doesn't make sense that we request for user-space counting
> but we also get the guest report.
>
> To keep perf semantics consistent and clear, this patch sets
> exclude_guest for user-space counting.
Applied, and also added this, that you should consider doing in the
future (modulo the "Committer testing:" header :) ):
Committer testing:
Before:
# perf record -e cycles:u
^C[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 1.231 MB perf.data (91 samples) ]
#
# perf evlist -v
cycles:u: 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
<SNIP>
#
After:
# perf record -e cycles:u
^C[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 1.263 MB perf.data (403 samples) ]
#
# perf evlist -v
cycles:u: 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, exclude_guest: 1
#
----
I.e. show actual command output before and after that demonstrates the
problem and then the solution.
> Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
> ---
> tools/perf/util/parse-events.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 9f7260e69113..4d809f1fe269 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -1794,6 +1794,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)
> + eG = 1;
> eu = 0;
> } else if (*str == 'k') {
> if (!exclude)
> --
> 2.17.1
>
--
- Arnaldo
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] perf parse-events: Set exclude_guest for user-space counting
2020-08-12 12:15 ` Arnaldo Carvalho de Melo
@ 2020-08-12 12:55 ` Arnaldo Carvalho de Melo
2020-08-13 5:09 ` Jin, Yao
2020-08-12 13:02 ` Like Xu
1 sibling, 1 reply; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-08-12 12:55 UTC (permalink / raw)
To: Jin Yao
Cc: jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
kan.liang, yao.jin, like.xu
Em Wed, Aug 12, 2020 at 09:15:04AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Wed, Aug 12, 2020 at 02:59:53PM +0800, Jin Yao escreveu:
> > Currently if we run 'perf record -e cycles:u', exclude_guest is 0.
> >
> > But it doesn't make sense that we request for user-space counting
> > but we also get the guest report.
> >
> > To keep perf semantics consistent and clear, this patch sets
> > exclude_guest for user-space counting.
>
> Applied, and also added this, that you should consider doing in the
> future (modulo the "Committer testing:" header :) ):
>
> Committer testing:
>
> Before:
>
> # perf record -e cycles:u
> ^C[ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 1.231 MB perf.data (91 samples) ]
> #
> # perf evlist -v
> cycles:u: 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
> <SNIP>
> #
>
> After:
>
> # perf record -e cycles:u
> ^C[ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 1.263 MB perf.data (403 samples) ]
> #
> # perf evlist -v
> cycles:u: 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, exclude_guest: 1
> #
Also, please run 'perf test', as this will require changes to some
expected perf_event_attr setups:
[root@quaco ~]# perf test "event definition"
6: Parse event definition strings : FAILED!
[root@quaco ~]#
- Arnaldo
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] perf parse-events: Set exclude_guest for user-space counting
2020-08-12 12:15 ` Arnaldo Carvalho de Melo
2020-08-12 12:55 ` Arnaldo Carvalho de Melo
@ 2020-08-12 13:02 ` Like Xu
2020-08-13 3:11 ` Jin, Yao
1 sibling, 1 reply; 9+ messages in thread
From: Like Xu @ 2020-08-12 13:02 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Jin Yao
Cc: jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
kan.liang, yao.jin
On 2020/8/12 20:15, Arnaldo Carvalho de Melo wrote:
> Em Wed, Aug 12, 2020 at 02:59:53PM +0800, Jin Yao escreveu:
>> Currently if we run 'perf record -e cycles:u', exclude_guest is 0.
>>
>> But it doesn't make sense that we request for user-space counting
>> but we also get the guest report.
>>
Please hold the horse and allow this possibility.
Some authorized perf users on the host may
only want to count (KVM) guest user space events.
Thanks,
Like Xu
>> To keep perf semantics consistent and clear, this patch sets
>> exclude_guest for user-space counting.
>
> Applied, and also added this, that you should consider doing in the
> future (modulo the "Committer testing:" header :) ):
>
> Committer testing:
>
> Before:
>
> # perf record -e cycles:u
> ^C[ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 1.231 MB perf.data (91 samples) ]
> #
> # perf evlist -v
> cycles:u: 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
> <SNIP>
> #
>
> After:
>
> # perf record -e cycles:u
> ^C[ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 1.263 MB perf.data (403 samples) ]
> #
> # perf evlist -v
> cycles:u: 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, exclude_guest: 1
> #
>
> ----
>
> I.e. show actual command output before and after that demonstrates the
> problem and then the solution.
>
>> Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
>> ---
>> tools/perf/util/parse-events.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
>> index 9f7260e69113..4d809f1fe269 100644
>> --- a/tools/perf/util/parse-events.c
>> +++ b/tools/perf/util/parse-events.c
>> @@ -1794,6 +1794,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)
>> + eG = 1;
>> eu = 0;
>> } else if (*str == 'k') {
>> if (!exclude)
>> --
>> 2.17.1
>>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] perf parse-events: Set exclude_guest for user-space counting
2020-08-12 13:02 ` Like Xu
@ 2020-08-13 3:11 ` Jin, Yao
2020-08-13 6:57 ` Like Xu
0 siblings, 1 reply; 9+ messages in thread
From: Jin, Yao @ 2020-08-13 3:11 UTC (permalink / raw)
To: Like Xu, Arnaldo Carvalho de Melo
Cc: jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
kan.liang, yao.jin
Hi Like,
On 8/12/2020 9:02 PM, Like Xu wrote:
> On 2020/8/12 20:15, Arnaldo Carvalho de Melo wrote:
>> Em Wed, Aug 12, 2020 at 02:59:53PM +0800, Jin Yao escreveu:
>>> Currently if we run 'perf record -e cycles:u', exclude_guest is 0.
>>>
>>> But it doesn't make sense that we request for user-space counting
>>> but we also get the guest report.
>>>
>
> Please hold the horse and allow this possibility.
>
> Some authorized perf users on the host may
> only want to count (KVM) guest user space events.
>
> Thanks,
> Like Xu
>
Without this patch, if we don't set the ":u" modifier, exclude_guest = 1.
perf record -e cycles ./div
perf evlist -v
cycles: size: 120, { sample_period, sample_freq }: 4000, sample_type: IP|TID|TIME|PERIOD,
read_format: ID, disabled: 1, inherit: 1, mmap: 1, comm: 1, freq: 1, enable_on_exec: 1, task: 1,
sample_id_all: 1, exclude_guest: 1, mmap2: 1, comm_exec: 1, ksymbol: 1, bpf_event: 1
So this patch doesn't change perf's original behavior.
Thanks
Jin Yao
>>> To keep perf semantics consistent and clear, this patch sets
>>> exclude_guest for user-space counting.
>>
>> Applied, and also added this, that you should consider doing in the
>> future (modulo the "Committer testing:" header :) ):
>>
>> Committer testing:
>>
>> Before:
>>
>> # perf record -e cycles:u
>> ^C[ perf record: Woken up 1 times to write data ]
>> [ perf record: Captured and wrote 1.231 MB perf.data (91 samples) ]
>> #
>> # perf evlist -v
>> cycles:u: 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
>> <SNIP>
>> #
>>
>> After:
>>
>> # perf record -e cycles:u
>> ^C[ perf record: Woken up 1 times to write data ]
>> [ perf record: Captured and wrote 1.263 MB perf.data (403 samples) ]
>> #
>> # perf evlist -v
>> cycles:u: 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, exclude_guest: 1
>> #
>>
>> ----
>>
>> I.e. show actual command output before and after that demonstrates the
>> problem and then the solution.
>>
>>> Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
>>> ---
>>> tools/perf/util/parse-events.c | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
>>> index 9f7260e69113..4d809f1fe269 100644
>>> --- a/tools/perf/util/parse-events.c
>>> +++ b/tools/perf/util/parse-events.c
>>> @@ -1794,6 +1794,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)
>>> + eG = 1;
>>> eu = 0;
>>> } else if (*str == 'k') {
>>> if (!exclude)
>>> --
>>> 2.17.1
>>>
>>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] perf parse-events: Set exclude_guest for user-space counting
2020-08-12 12:55 ` Arnaldo Carvalho de Melo
@ 2020-08-13 5:09 ` Jin, Yao
0 siblings, 0 replies; 9+ messages in thread
From: Jin, Yao @ 2020-08-13 5:09 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
kan.liang, yao.jin, like.xu
Hi Arnaldo,
On 8/12/2020 8:55 PM, Arnaldo Carvalho de Melo wrote:
> Em Wed, Aug 12, 2020 at 09:15:04AM -0300, Arnaldo Carvalho de Melo escreveu:
>> Em Wed, Aug 12, 2020 at 02:59:53PM +0800, Jin Yao escreveu:
>>> Currently if we run 'perf record -e cycles:u', exclude_guest is 0.
>>>
>>> But it doesn't make sense that we request for user-space counting
>>> but we also get the guest report.
>>>
>>> To keep perf semantics consistent and clear, this patch sets
>>> exclude_guest for user-space counting.
>>
>> Applied, and also added this, that you should consider doing in the
>> future (modulo the "Committer testing:" header :) ):
>>
>> Committer testing:
>>
>> Before:
>>
>> # perf record -e cycles:u
>> ^C[ perf record: Woken up 1 times to write data ]
>> [ perf record: Captured and wrote 1.231 MB perf.data (91 samples) ]
>> #
>> # perf evlist -v
>> cycles:u: 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
>> <SNIP>
>> #
>>
>> After:
>>
>> # perf record -e cycles:u
>> ^C[ perf record: Woken up 1 times to write data ]
>> [ perf record: Captured and wrote 1.263 MB perf.data (403 samples) ]
>> #
>> # perf evlist -v
>> cycles:u: 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, exclude_guest: 1
>> #
>
> Also, please run 'perf test', as this will require changes to some
> expected perf_event_attr setups:
>
> [root@quaco ~]# perf test "event definition"
> 6: Parse event definition strings : FAILED!
> [root@quaco ~]#
>
> - Arnaldo
>
Sorry for the perf test failure! I will post v2 to fix this issue.
Thanks
Jin Yao
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] perf parse-events: Set exclude_guest for user-space counting
2020-08-13 3:11 ` Jin, Yao
@ 2020-08-13 6:57 ` Like Xu
2020-08-13 7:08 ` Jin, Yao
0 siblings, 1 reply; 9+ messages in thread
From: Like Xu @ 2020-08-13 6:57 UTC (permalink / raw)
To: Jin, Yao, Arnaldo Carvalho de Melo
Cc: jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
kan.liang, yao.jin
Hi Yao,
On 2020/8/13 11:11, Jin, Yao wrote:
> Hi Like,
>
> On 8/12/2020 9:02 PM, Like Xu wrote:
>> On 2020/8/12 20:15, Arnaldo Carvalho de Melo wrote:
>>> Em Wed, Aug 12, 2020 at 02:59:53PM +0800, Jin Yao escreveu:
>>>> Currently if we run 'perf record -e cycles:u', exclude_guest is 0.
>>>>
>>>> But it doesn't make sense that we request for user-space counting
>>>> but we also get the guest report.
>>>>
>>
>> Please hold the horse and allow this possibility.
>>
>> Some authorized perf users on the host may
>> only want to count (KVM) guest user space events.
>>
>> Thanks,
>> Like Xu
>>
>
> Without this patch, if we don't set the ":u" modifier, exclude_guest = 1.
It's true for the non ":u" case.
>
> perf record -e cycles ./div
> perf evlist -v
> cycles: size: 120, { sample_period, sample_freq }: 4000, sample_type:
> IP|TID|TIME|PERIOD, read_format: ID, disabled: 1, inherit: 1, mmap: 1,
> comm: 1, freq: 1, enable_on_exec: 1, task: 1, sample_id_all: 1,
> exclude_guest: 1, mmap2: 1, comm_exec: 1, ksymbol: 1, bpf_event: 1
>
> So this patch doesn't change perf's original behavior.
The patch changes the "perf kvm" original behavior.
Testcase: perf kvm --host --guest --guestkallsyms=guest-kallsyms \
--guestmodules=guest-modules record -e cycles:u ...
From:
cycles:u: 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
dummy:HG: type: 1, size: 120, config: 0x9, { sample_period, sample_freq }:
4000, sample_type: IP|TID|TIME|ID|CPU|PERIOD, read_format: ID, inherit: 1,
mmap: 1, comm: 1, freq: 1, task: 1, sample_id_all: 1, mmap2: 1, comm_exec:
1, ksymbol: 1, bpf_event: 1
To:
cycles:u: 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,
exclude_guest: 1
dummy:HG: type: 1, size: 120, config: 0x9, { sample_period, sample_freq }:
4000, sample_type: IP|TID|TIME|ID|CPU|PERIOD, read_format: ID, inherit: 1,
mmap: 1, comm: 1, freq: 1, task: 1, sample_id_all: 1, mmap2: 1, comm_exec:
1, ksymbol: 1, bpf_event: 1
Thanks,
Like Xu
>
> Thanks
> Jin Yao
>
>>>> To keep perf semantics consistent and clear, this patch sets
>>>> exclude_guest for user-space counting.
>>>
>>> Applied, and also added this, that you should consider doing in the
>>> future (modulo the "Committer testing:" header :) ):
>>>
>>> Committer testing:
>>>
>>> Before:
>>>
>>> # perf record -e cycles:u
>>> ^C[ perf record: Woken up 1 times to write data ]
>>> [ perf record: Captured and wrote 1.231 MB perf.data (91 samples) ]
>>> #
>>> # perf evlist -v
>>> cycles:u: 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
>>> <SNIP>
>>> #
>>>
>>> After:
>>>
>>> # perf record -e cycles:u
>>> ^C[ perf record: Woken up 1 times to write data ]
>>> [ perf record: Captured and wrote 1.263 MB perf.data (403 samples) ]
>>> #
>>> # perf evlist -v
>>> cycles:u: 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,
>>> exclude_guest: 1
>>> #
>>>
>>> ----
>>>
>>> I.e. show actual command output before and after that demonstrates the
>>> problem and then the solution.
>>>
>>>> Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
>>>> ---
>>>> tools/perf/util/parse-events.c | 2 ++
>>>> 1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/tools/perf/util/parse-events.c
>>>> b/tools/perf/util/parse-events.c
>>>> index 9f7260e69113..4d809f1fe269 100644
>>>> --- a/tools/perf/util/parse-events.c
>>>> +++ b/tools/perf/util/parse-events.c
>>>> @@ -1794,6 +1794,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)
>>>> + eG = 1;
>>>> eu = 0;
>>>> } else if (*str == 'k') {
>>>> if (!exclude)
>>>> --
>>>> 2.17.1
>>>>
>>>
>>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] perf parse-events: Set exclude_guest for user-space counting
2020-08-13 6:57 ` Like Xu
@ 2020-08-13 7:08 ` Jin, Yao
2020-08-13 7:21 ` Like Xu
0 siblings, 1 reply; 9+ messages in thread
From: Jin, Yao @ 2020-08-13 7:08 UTC (permalink / raw)
To: Like Xu, Arnaldo Carvalho de Melo
Cc: jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
kan.liang, yao.jin
On 8/13/2020 2:57 PM, Like Xu wrote:
> Hi Yao,
>
> On 2020/8/13 11:11, Jin, Yao wrote:
>> Hi Like,
>>
>> On 8/12/2020 9:02 PM, Like Xu wrote:
>>> On 2020/8/12 20:15, Arnaldo Carvalho de Melo wrote:
>>>> Em Wed, Aug 12, 2020 at 02:59:53PM +0800, Jin Yao escreveu:
>>>>> Currently if we run 'perf record -e cycles:u', exclude_guest is 0.
>>>>>
>>>>> But it doesn't make sense that we request for user-space counting
>>>>> but we also get the guest report.
>>>>>
>>>
>>> Please hold the horse and allow this possibility.
>>>
>>> Some authorized perf users on the host may
>>> only want to count (KVM) guest user space events.
>>>
>>> Thanks,
>>> Like Xu
>>>
>>
>> Without this patch, if we don't set the ":u" modifier, exclude_guest = 1.
>
> It's true for the non ":u" case.
>
>>
>> perf record -e cycles ./div
>> perf evlist -v
>> cycles: size: 120, { sample_period, sample_freq }: 4000, sample_type: IP|TID|TIME|PERIOD,
>> read_format: ID, disabled: 1, inherit: 1, mmap: 1, comm: 1, freq: 1, enable_on_exec: 1, task: 1,
>> sample_id_all: 1, exclude_guest: 1, mmap2: 1, comm_exec: 1, ksymbol: 1, bpf_event: 1
>>
>> So this patch doesn't change perf's original behavior.
>
> The patch changes the "perf kvm" original behavior.
>
> Testcase: perf kvm --host --guest --guestkallsyms=guest-kallsyms \
> --guestmodules=guest-modules record -e cycles:u ...
>
> From:
>
> cycles:u: 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
> dummy:HG: type: 1, size: 120, config: 0x9, { sample_period, sample_freq }: 4000, sample_type:
> IP|TID|TIME|ID|CPU|PERIOD, read_format: ID, inherit: 1, mmap: 1, comm: 1, freq: 1, task: 1,
> sample_id_all: 1, mmap2: 1, comm_exec: 1, ksymbol: 1, bpf_event: 1
>
> To:
>
> cycles:u: 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,
>
> exclude_guest: 1
>
> dummy:HG: type: 1, size: 120, config: 0x9, { sample_period, sample_freq }: 4000, sample_type:
> IP|TID|TIME|ID|CPU|PERIOD, read_format: ID, inherit: 1, mmap: 1, comm: 1, freq: 1, task: 1,
> sample_id_all: 1, mmap2: 1, comm_exec: 1, ksymbol: 1, bpf_event: 1
>
> Thanks,
> Like Xu
The behavior is similar as native kernel. The change of exclude_guest (0 -> 1) is expected.
On native kernel,
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, ...
Thanks
Jin Yao
>>
>> Thanks
>> Jin Yao
>>
>>>>> To keep perf semantics consistent and clear, this patch sets
>>>>> exclude_guest for user-space counting.
>>>>
>>>> Applied, and also added this, that you should consider doing in the
>>>> future (modulo the "Committer testing:" header :) ):
>>>>
>>>> Committer testing:
>>>>
>>>> Before:
>>>>
>>>> # perf record -e cycles:u
>>>> ^C[ perf record: Woken up 1 times to write data ]
>>>> [ perf record: Captured and wrote 1.231 MB perf.data (91 samples) ]
>>>> #
>>>> # perf evlist -v
>>>> cycles:u: 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
>>>> <SNIP>
>>>> #
>>>>
>>>> After:
>>>>
>>>> # perf record -e cycles:u
>>>> ^C[ perf record: Woken up 1 times to write data ]
>>>> [ perf record: Captured and wrote 1.263 MB perf.data (403 samples) ]
>>>> #
>>>> # perf evlist -v
>>>> cycles:u: 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, exclude_guest: 1
>>>> #
>>>>
>>>> ----
>>>>
>>>> I.e. show actual command output before and after that demonstrates the
>>>> problem and then the solution.
>>>>
>>>>> Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
>>>>> ---
>>>>> tools/perf/util/parse-events.c | 2 ++
>>>>> 1 file changed, 2 insertions(+)
>>>>>
>>>>> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
>>>>> index 9f7260e69113..4d809f1fe269 100644
>>>>> --- a/tools/perf/util/parse-events.c
>>>>> +++ b/tools/perf/util/parse-events.c
>>>>> @@ -1794,6 +1794,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)
>>>>> + eG = 1;
>>>>> eu = 0;
>>>>> } else if (*str == 'k') {
>>>>> if (!exclude)
>>>>> --
>>>>> 2.17.1
>>>>>
>>>>
>>>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] perf parse-events: Set exclude_guest for user-space counting
2020-08-13 7:08 ` Jin, Yao
@ 2020-08-13 7:21 ` Like Xu
0 siblings, 0 replies; 9+ messages in thread
From: Like Xu @ 2020-08-13 7:21 UTC (permalink / raw)
To: Jin, Yao, Arnaldo Carvalho de Melo
Cc: jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
kan.liang, yao.jin
On 2020/8/13 15:08, Jin, Yao wrote:
>
>
> On 8/13/2020 2:57 PM, Like Xu wrote:
>> Hi Yao,
>>
>> On 2020/8/13 11:11, Jin, Yao wrote:
>>> Hi Like,
>>>
>>> On 8/12/2020 9:02 PM, Like Xu wrote:
>>>> On 2020/8/12 20:15, Arnaldo Carvalho de Melo wrote:
>>>>> Em Wed, Aug 12, 2020 at 02:59:53PM +0800, Jin Yao escreveu:
>>>>>> Currently if we run 'perf record -e cycles:u', exclude_guest is 0.
>>>>>>
>>>>>> But it doesn't make sense that we request for user-space counting
>>>>>> but we also get the guest report.
>>>>>>
>>>>
>>>> Please hold the horse and allow this possibility.
>>>>
>>>> Some authorized perf users on the host may
>>>> only want to count (KVM) guest user space events.
>>>>
>>>> Thanks,
>>>> Like Xu
>>>>
>>>
>>> Without this patch, if we don't set the ":u" modifier, exclude_guest = 1.
>>
>> It's true for the non ":u" case.
>>
>>>
>>> perf record -e cycles ./div
>>> perf evlist -v
>>> cycles: size: 120, { sample_period, sample_freq }: 4000, sample_type:
>>> IP|TID|TIME|PERIOD, read_format: ID, disabled: 1, inherit: 1, mmap: 1,
>>> comm: 1, freq: 1, enable_on_exec: 1, task: 1, sample_id_all: 1,
>>> exclude_guest: 1, mmap2: 1, comm_exec: 1, ksymbol: 1, bpf_event: 1
>>>
>>> So this patch doesn't change perf's original behavior.
>>
>> The patch changes the "perf kvm" original behavior.
>>
>> Testcase: perf kvm --host --guest --guestkallsyms=guest-kallsyms \
>> --guestmodules=guest-modules record -e cycles:u ...
>>
>> From:
>>
>> cycles:u: 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
>> dummy:HG: type: 1, size: 120, config: 0x9, { sample_period, sample_freq
>> }: 4000, sample_type: IP|TID|TIME|ID|CPU|PERIOD, read_format: ID,
>> inherit: 1, mmap: 1, comm: 1, freq: 1, task: 1, sample_id_all: 1, mmap2:
>> 1, comm_exec: 1, ksymbol: 1, bpf_event: 1
>>
>> To:
>>
>> cycles:u: 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,
>>
>> exclude_guest: 1
>>
>> dummy:HG: type: 1, size: 120, config: 0x9, { sample_period, sample_freq
>> }: 4000, sample_type: IP|TID|TIME|ID|CPU|PERIOD, read_format: ID,
>> inherit: 1, mmap: 1, comm: 1, freq: 1, task: 1, sample_id_all: 1, mmap2:
>> 1, comm_exec: 1, ksymbol: 1, bpf_event: 1
>>
>> Thanks,
>> Like Xu
>
> The behavior is similar as native kernel. The change of exclude_guest (0 ->
> 1) is expected.
When we have 'exclude_guest: 1' for perf kvm usage,
we may get nothing from guest events.
The perf kvm user couldn't count (KVM) guest
user space events only with your patch.
It's unexpected from the pure ':u' semantics.
>
> On native kernel,
>
> 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, ...
>
> Thanks
> Jin Yao
>
>>>
>>> Thanks
>>> Jin Yao
>>>
>>>>>> To keep perf semantics consistent and clear, this patch sets
>>>>>> exclude_guest for user-space counting.
>>>>>
>>>>> Applied, and also added this, that you should consider doing in the
>>>>> future (modulo the "Committer testing:" header :) ):
>>>>>
>>>>> Committer testing:
>>>>>
>>>>> Before:
>>>>>
>>>>> # perf record -e cycles:u
>>>>> ^C[ perf record: Woken up 1 times to write data ]
>>>>> [ perf record: Captured and wrote 1.231 MB perf.data (91 samples) ]
>>>>> #
>>>>> # perf evlist -v
>>>>> cycles:u: 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
>>>>> <SNIP>
>>>>> #
>>>>>
>>>>> After:
>>>>>
>>>>> # perf record -e cycles:u
>>>>> ^C[ perf record: Woken up 1 times to write data ]
>>>>> [ perf record: Captured and wrote 1.263 MB perf.data (403 samples) ]
>>>>> #
>>>>> # perf evlist -v
>>>>> cycles:u: 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, exclude_guest: 1
>>>>> #
>>>>>
>>>>> ----
>>>>>
>>>>> I.e. show actual command output before and after that demonstrates the
>>>>> problem and then the solution.
>>>>>
>>>>>> Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
>>>>>> ---
>>>>>> tools/perf/util/parse-events.c | 2 ++
>>>>>> 1 file changed, 2 insertions(+)
>>>>>>
>>>>>> diff --git a/tools/perf/util/parse-events.c
>>>>>> b/tools/perf/util/parse-events.c
>>>>>> index 9f7260e69113..4d809f1fe269 100644
>>>>>> --- a/tools/perf/util/parse-events.c
>>>>>> +++ b/tools/perf/util/parse-events.c
>>>>>> @@ -1794,6 +1794,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)
>>>>>> + eG = 1;
>>>>>> eu = 0;
>>>>>> } else if (*str == 'k') {
>>>>>> if (!exclude)
>>>>>> --
>>>>>> 2.17.1
>>>>>>
>>>>>
>>>>
>>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-08-13 7:21 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-12 6:59 [PATCH] perf parse-events: Set exclude_guest for user-space counting Jin Yao
2020-08-12 12:15 ` Arnaldo Carvalho de Melo
2020-08-12 12:55 ` Arnaldo Carvalho de Melo
2020-08-13 5:09 ` Jin, Yao
2020-08-12 13:02 ` Like Xu
2020-08-13 3:11 ` Jin, Yao
2020-08-13 6:57 ` Like Xu
2020-08-13 7:08 ` Jin, Yao
2020-08-13 7:21 ` Like Xu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).