2021-09-29 18:55:41

by Liang, Kan

[permalink] [raw]
Subject: [PATCH 1/2] perf script: Fix PERF_SAMPLE_WEIGHT_STRUCT support

From: Kan Liang <[email protected]>

-F weight in perf script is broken.

# ./perf mem record
# ./perf script -F weight
Samples for 'dummy:HG' event do not have WEIGHT attribute set. Cannot
print 'weight' field.

The sample type, PERF_SAMPLE_WEIGHT_STRUCT, is an alternative of the
PERF_SAMPLE_WEIGHT sample type. They share the same space, weight. The
lower 32 bits are exactly the same for both sample type. The higher 32
bits may be different for different architecture. For a new kernel on
x86, the PERF_SAMPLE_WEIGHT_STRUCT is used. For an old kernel or other
ARCHs, the PERF_SAMPLE_WEIGHT is used.

With -F weight, current perf script will only check the input string
"weight" with the PERF_SAMPLE_WEIGHT sample type. Because the commit
ea8d0ed6eae3 ("perf tools: Support PERF_SAMPLE_WEIGHT_STRUCT") didn't
update the PERF_SAMPLE_WEIGHT_STRUCT sample type for perf script. For a
new kernel on x86, the check fails.

Use PERF_SAMPLE_WEIGHT_TYPE, which supports both sample types, to
replace PERF_SAMPLE_WEIGHT.

Reported-by: Joe Mario <[email protected]>
Fixes: ea8d0ed6eae3 ("perf tools: Support PERF_SAMPLE_WEIGHT_STRUCT")
Signed-off-by: Kan Liang <[email protected]>
---
tools/perf/builtin-script.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 6211d0b..9f62ac6 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -459,7 +459,7 @@ static int evsel__check_attr(struct evsel *evsel, struct perf_session *session)
return -EINVAL;

if (PRINT_FIELD(WEIGHT) &&
- evsel__check_stype(evsel, PERF_SAMPLE_WEIGHT, "WEIGHT", PERF_OUTPUT_WEIGHT))
+ evsel__check_stype(evsel, PERF_SAMPLE_WEIGHT_TYPE, "WEIGHT", PERF_OUTPUT_WEIGHT))
return -EINVAL;

if (PRINT_FIELD(SYM) &&
--
2.7.4


2021-09-29 20:24:49

by Joe Mario

[permalink] [raw]
Subject: Re: [PATCH 1/2] perf script: Fix PERF_SAMPLE_WEIGHT_STRUCT support



On 9/29/21 2:42 PM, Jiri Olsa wrote:
> On Wed, Sep 29, 2021 at 01:54:39PM -0300, Arnaldo Carvalho de Melo wrote:
>> Em Wed, Sep 29, 2021 at 08:38:13AM -0700, [email protected] escreveu:
>>> From: Kan Liang <[email protected]>
>>>
>>> -F weight in perf script is broken.
>>>
>>> # ./perf mem record
>>> # ./perf script -F weight
>>> Samples for 'dummy:HG' event do not have WEIGHT attribute set. Cannot
>>> print 'weight' field.
>>>
>>> The sample type, PERF_SAMPLE_WEIGHT_STRUCT, is an alternative of the
>>> PERF_SAMPLE_WEIGHT sample type. They share the same space, weight. The
>>> lower 32 bits are exactly the same for both sample type. The higher 32
>>> bits may be different for different architecture. For a new kernel on
>>> x86, the PERF_SAMPLE_WEIGHT_STRUCT is used. For an old kernel or other
>>> ARCHs, the PERF_SAMPLE_WEIGHT is used.
>>>
>>> With -F weight, current perf script will only check the input string
>>> "weight" with the PERF_SAMPLE_WEIGHT sample type. Because the commit
>>> ea8d0ed6eae3 ("perf tools: Support PERF_SAMPLE_WEIGHT_STRUCT") didn't
>>> update the PERF_SAMPLE_WEIGHT_STRUCT sample type for perf script. For a
>>> new kernel on x86, the check fails.
>>>
>>> Use PERF_SAMPLE_WEIGHT_TYPE, which supports both sample types, to
>>> replace PERF_SAMPLE_WEIGHT.
>>>
>>> Reported-by: Joe Mario <[email protected]>
>>> Fixes: ea8d0ed6eae3 ("perf tools: Support PERF_SAMPLE_WEIGHT_STRUCT")
>>
>> Hey Joe, Jiri,
>>
>> Can I have your Tested-by?
>
> Acked/Tested-by: Jiri Olsa <[email protected]>
>
> thanks,
> jirka

Acked/Tested-by: Joe Mario <[email protected]>

The "perf script -F weight" command works correctly.

And I also verified that when just issuing "perf script", that the weight (cycle latency)
column that was missing with this bug, is now fixed and working properly.

Thanks,
Joe
>
>>
>> Thanks,
>>
>> - Arnaldo
>>
>>> Signed-off-by: Kan Liang <[email protected]>
>>> ---
>>> tools/perf/builtin-script.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
>>> index 6211d0b..9f62ac6 100644
>>> --- a/tools/perf/builtin-script.c
>>> +++ b/tools/perf/builtin-script.c
>>> @@ -459,7 +459,7 @@ static int evsel__check_attr(struct evsel *evsel, struct perf_session *session)
>>> return -EINVAL;
>>>
>>> if (PRINT_FIELD(WEIGHT) &&
>>> - evsel__check_stype(evsel, PERF_SAMPLE_WEIGHT, "WEIGHT", PERF_OUTPUT_WEIGHT))
>>> + evsel__check_stype(evsel, PERF_SAMPLE_WEIGHT_TYPE, "WEIGHT", PERF_OUTPUT_WEIGHT))
>>> return -EINVAL;
>>>
>>> if (PRINT_FIELD(SYM) &&
>>> --
>>> 2.7.4
>>
>> --
>>
>> - Arnaldo
>>
>

2021-09-29 20:58:27

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 1/2] perf script: Fix PERF_SAMPLE_WEIGHT_STRUCT support

Em Wed, Sep 29, 2021 at 08:38:13AM -0700, [email protected] escreveu:
> From: Kan Liang <[email protected]>
>
> -F weight in perf script is broken.
>
> # ./perf mem record
> # ./perf script -F weight
> Samples for 'dummy:HG' event do not have WEIGHT attribute set. Cannot
> print 'weight' field.
>
> The sample type, PERF_SAMPLE_WEIGHT_STRUCT, is an alternative of the
> PERF_SAMPLE_WEIGHT sample type. They share the same space, weight. The
> lower 32 bits are exactly the same for both sample type. The higher 32
> bits may be different for different architecture. For a new kernel on
> x86, the PERF_SAMPLE_WEIGHT_STRUCT is used. For an old kernel or other
> ARCHs, the PERF_SAMPLE_WEIGHT is used.
>
> With -F weight, current perf script will only check the input string
> "weight" with the PERF_SAMPLE_WEIGHT sample type. Because the commit
> ea8d0ed6eae3 ("perf tools: Support PERF_SAMPLE_WEIGHT_STRUCT") didn't
> update the PERF_SAMPLE_WEIGHT_STRUCT sample type for perf script. For a
> new kernel on x86, the check fails.
>
> Use PERF_SAMPLE_WEIGHT_TYPE, which supports both sample types, to
> replace PERF_SAMPLE_WEIGHT.
>
> Reported-by: Joe Mario <[email protected]>
> Fixes: ea8d0ed6eae3 ("perf tools: Support PERF_SAMPLE_WEIGHT_STRUCT")

Hey Joe, Jiri,

Can I have your Tested-by?

Thanks,

- Arnaldo

> Signed-off-by: Kan Liang <[email protected]>
> ---
> tools/perf/builtin-script.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index 6211d0b..9f62ac6 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -459,7 +459,7 @@ static int evsel__check_attr(struct evsel *evsel, struct perf_session *session)
> return -EINVAL;
>
> if (PRINT_FIELD(WEIGHT) &&
> - evsel__check_stype(evsel, PERF_SAMPLE_WEIGHT, "WEIGHT", PERF_OUTPUT_WEIGHT))
> + evsel__check_stype(evsel, PERF_SAMPLE_WEIGHT_TYPE, "WEIGHT", PERF_OUTPUT_WEIGHT))
> return -EINVAL;
>
> if (PRINT_FIELD(SYM) &&
> --
> 2.7.4

--

- Arnaldo

2021-09-29 21:01:43

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH 1/2] perf script: Fix PERF_SAMPLE_WEIGHT_STRUCT support

On Wed, Sep 29, 2021 at 01:54:39PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Wed, Sep 29, 2021 at 08:38:13AM -0700, [email protected] escreveu:
> > From: Kan Liang <[email protected]>
> >
> > -F weight in perf script is broken.
> >
> > # ./perf mem record
> > # ./perf script -F weight
> > Samples for 'dummy:HG' event do not have WEIGHT attribute set. Cannot
> > print 'weight' field.
> >
> > The sample type, PERF_SAMPLE_WEIGHT_STRUCT, is an alternative of the
> > PERF_SAMPLE_WEIGHT sample type. They share the same space, weight. The
> > lower 32 bits are exactly the same for both sample type. The higher 32
> > bits may be different for different architecture. For a new kernel on
> > x86, the PERF_SAMPLE_WEIGHT_STRUCT is used. For an old kernel or other
> > ARCHs, the PERF_SAMPLE_WEIGHT is used.
> >
> > With -F weight, current perf script will only check the input string
> > "weight" with the PERF_SAMPLE_WEIGHT sample type. Because the commit
> > ea8d0ed6eae3 ("perf tools: Support PERF_SAMPLE_WEIGHT_STRUCT") didn't
> > update the PERF_SAMPLE_WEIGHT_STRUCT sample type for perf script. For a
> > new kernel on x86, the check fails.
> >
> > Use PERF_SAMPLE_WEIGHT_TYPE, which supports both sample types, to
> > replace PERF_SAMPLE_WEIGHT.
> >
> > Reported-by: Joe Mario <[email protected]>
> > Fixes: ea8d0ed6eae3 ("perf tools: Support PERF_SAMPLE_WEIGHT_STRUCT")
>
> Hey Joe, Jiri,
>
> Can I have your Tested-by?

Acked/Tested-by: Jiri Olsa <[email protected]>

thanks,
jirka

>
> Thanks,
>
> - Arnaldo
>
> > Signed-off-by: Kan Liang <[email protected]>
> > ---
> > tools/perf/builtin-script.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> > index 6211d0b..9f62ac6 100644
> > --- a/tools/perf/builtin-script.c
> > +++ b/tools/perf/builtin-script.c
> > @@ -459,7 +459,7 @@ static int evsel__check_attr(struct evsel *evsel, struct perf_session *session)
> > return -EINVAL;
> >
> > if (PRINT_FIELD(WEIGHT) &&
> > - evsel__check_stype(evsel, PERF_SAMPLE_WEIGHT, "WEIGHT", PERF_OUTPUT_WEIGHT))
> > + evsel__check_stype(evsel, PERF_SAMPLE_WEIGHT_TYPE, "WEIGHT", PERF_OUTPUT_WEIGHT))
> > return -EINVAL;
> >
> > if (PRINT_FIELD(SYM) &&
> > --
> > 2.7.4
>
> --
>
> - Arnaldo
>

2021-09-30 13:06:26

by kajoljain

[permalink] [raw]
Subject: Re: [PATCH 1/2] perf script: Fix PERF_SAMPLE_WEIGHT_STRUCT support



On 9/29/21 9:08 PM, [email protected] wrote:
> From: Kan Liang <[email protected]>
>
> -F weight in perf script is broken.
>
> # ./perf mem record
> # ./perf script -F weight
> Samples for 'dummy:HG' event do not have WEIGHT attribute set. Cannot
> print 'weight' field.

Hi Kan,
This issue can also be reproduce in powerpc box.
Given patch fix the issue.

Reviewed-by: Kajol Jain<[email protected]>

Thanks,
Kajol Jain

>
> The sample type, PERF_SAMPLE_WEIGHT_STRUCT, is an alternative of the
> PERF_SAMPLE_WEIGHT sample type. They share the same space, weight. The
> lower 32 bits are exactly the same for both sample type. The higher 32
> bits may be different for different architecture. For a new kernel on
> x86, the PERF_SAMPLE_WEIGHT_STRUCT is used. For an old kernel or other
> ARCHs, the PERF_SAMPLE_WEIGHT is used.
>
> With -F weight, current perf script will only check the input string
> "weight" with the PERF_SAMPLE_WEIGHT sample type. Because the commit
> ea8d0ed6eae3 ("perf tools: Support PERF_SAMPLE_WEIGHT_STRUCT") didn't
> update the PERF_SAMPLE_WEIGHT_STRUCT sample type for perf script. For a
> new kernel on x86, the check fails.
>
> Use PERF_SAMPLE_WEIGHT_TYPE, which supports both sample types, to
> replace PERF_SAMPLE_WEIGHT.
>
> Reported-by: Joe Mario <[email protected]>
> Fixes: ea8d0ed6eae3 ("perf tools: Support PERF_SAMPLE_WEIGHT_STRUCT")
> Signed-off-by: Kan Liang <[email protected]>
> ---
> tools/perf/builtin-script.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index 6211d0b..9f62ac6 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -459,7 +459,7 @@ static int evsel__check_attr(struct evsel *evsel, struct perf_session *session)
> return -EINVAL;
>
> if (PRINT_FIELD(WEIGHT) &&
> - evsel__check_stype(evsel, PERF_SAMPLE_WEIGHT, "WEIGHT", PERF_OUTPUT_WEIGHT))
> + evsel__check_stype(evsel, PERF_SAMPLE_WEIGHT_TYPE, "WEIGHT", PERF_OUTPUT_WEIGHT))
> return -EINVAL;
>
> if (PRINT_FIELD(SYM) &&
>

2021-10-27 21:34:00

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 1/2] perf script: Fix PERF_SAMPLE_WEIGHT_STRUCT support

Em Wed, Sep 29, 2021 at 04:22:19PM -0400, Joe Mario escreveu:
>
>
> On 9/29/21 2:42 PM, Jiri Olsa wrote:
> > On Wed, Sep 29, 2021 at 01:54:39PM -0300, Arnaldo Carvalho de Melo wrote:
> >> Em Wed, Sep 29, 2021 at 08:38:13AM -0700, [email protected] escreveu:
> >>> From: Kan Liang <[email protected]>
> >>>
> >>> -F weight in perf script is broken.
> >>>
> >>> # ./perf mem record
> >>> # ./perf script -F weight
> >>> Samples for 'dummy:HG' event do not have WEIGHT attribute set. Cannot
> >>> print 'weight' field.
> >>>
> >>> The sample type, PERF_SAMPLE_WEIGHT_STRUCT, is an alternative of the
> >>> PERF_SAMPLE_WEIGHT sample type. They share the same space, weight. The
> >>> lower 32 bits are exactly the same for both sample type. The higher 32
> >>> bits may be different for different architecture. For a new kernel on
> >>> x86, the PERF_SAMPLE_WEIGHT_STRUCT is used. For an old kernel or other
> >>> ARCHs, the PERF_SAMPLE_WEIGHT is used.
> >>>
> >>> With -F weight, current perf script will only check the input string
> >>> "weight" with the PERF_SAMPLE_WEIGHT sample type. Because the commit
> >>> ea8d0ed6eae3 ("perf tools: Support PERF_SAMPLE_WEIGHT_STRUCT") didn't
> >>> update the PERF_SAMPLE_WEIGHT_STRUCT sample type for perf script. For a
> >>> new kernel on x86, the check fails.
> >>>
> >>> Use PERF_SAMPLE_WEIGHT_TYPE, which supports both sample types, to
> >>> replace PERF_SAMPLE_WEIGHT.
> >>>
> >>> Reported-by: Joe Mario <[email protected]>
> >>> Fixes: ea8d0ed6eae3 ("perf tools: Support PERF_SAMPLE_WEIGHT_STRUCT")
> >>
> >> Hey Joe, Jiri,
> >>
> >> Can I have your Tested-by?
> >
> > Acked/Tested-by: Jiri Olsa <[email protected]>
> >
> > thanks,
> > jirka
>
> Acked/Tested-by: Joe Mario <[email protected]>
>
> The "perf script -F weight" command works correctly.
>
> And I also verified that when just issuing "perf script", that the weight (cycle latency)
> column that was missing with this bug, is now fixed and working properly.

Thanks, applied.

- Arnaldo


> Thanks,
> Joe
> >
> >>
> >> Thanks,
> >>
> >> - Arnaldo
> >>
> >>> Signed-off-by: Kan Liang <[email protected]>
> >>> ---
> >>> tools/perf/builtin-script.c | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> >>> index 6211d0b..9f62ac6 100644
> >>> --- a/tools/perf/builtin-script.c
> >>> +++ b/tools/perf/builtin-script.c
> >>> @@ -459,7 +459,7 @@ static int evsel__check_attr(struct evsel *evsel, struct perf_session *session)
> >>> return -EINVAL;
> >>>
> >>> if (PRINT_FIELD(WEIGHT) &&
> >>> - evsel__check_stype(evsel, PERF_SAMPLE_WEIGHT, "WEIGHT", PERF_OUTPUT_WEIGHT))
> >>> + evsel__check_stype(evsel, PERF_SAMPLE_WEIGHT_TYPE, "WEIGHT", PERF_OUTPUT_WEIGHT))
> >>> return -EINVAL;
> >>>
> >>> if (PRINT_FIELD(SYM) &&
> >>> --
> >>> 2.7.4
> >>
> >> --
> >>
> >> - Arnaldo
> >>
> >

--

- Arnaldo