2022-09-08 07:21:04

by Xing Zhengjun

[permalink] [raw]
Subject: [PATCH] perf script: Fix Cannot print 'iregs' field for hybrid systems

From: Zhengjun Xing <[email protected]>

Commit b91e5492f9d7 ("perf record: Add a dummy event on hybrid systems to
collect metadata records") adds a dummy event on hybrid systems to fix the
symbol "unknown" issue when the workload is created in a P-core but runs
on an E-core. The added dummy event will cause "perf script -F iregs" to
fail. Dummy events do not have "iregs" attribute set, so when we do
evsel__check_attr, the "iregs" attribute check will fail, so the issue
happened.

The following commit [1] has fixed a similar issue by skipping the attr
check for the dummy event because it does not have any samples anyway. It
works okay for the normal mode, but the issue still happened when running
the test in the pipe mode. In the pipe mode, it calls process_attr() which
still checks the attr for the dummy event. This commit fixed the issue by
skipping the attr check for the dummy event in the API evsel__check_attr,
Otherwise, we have to patch everywhere when evsel__check_attr() is called.

Before:

#./perf record -o - --intr-regs=di,r8,dx,cx -e br_inst_retired.near_call:p -c 1000 --per-thread true 2>/dev/null|./perf script -F iregs |head -5
Samples for 'dummy:HG' event do not have IREGS attribute set. Cannot print 'iregs' field.
0x120 [0x90]: failed to process type: 64

After:

# ./perf record -o - --intr-regs=di,r8,dx,cx -e br_inst_retired.near_call:p -c 1000 --per-thread true 2>/dev/null|./perf script -F iregs |head -5
ABI:2 CX:0x55b8efa87000 DX:0x55b8efa7e000 DI:0xffffba5e625efbb0 R8:0xffff90e51f8ae100
ABI:2 CX:0x7f1dae1e4000 DX:0xd0 DI:0xffff90e18c675ac0 R8:0x71
ABI:2 CX:0xcc0 DX:0x1 DI:0xffff90e199880240 R8:0x0
ABI:2 CX:0xffff90e180dd7500 DX:0xffff90e180dd7500 DI:0xffff90e180043500 R8:0x1
ABI:2 CX:0x50 DX:0xffff90e18c583bd0 DI:0xffff90e1998803c0 R8:0x58

[1]https://lore.kernel.org/lkml/[email protected]/

Fixes: b91e5492f9d7 ("perf record: Add a dummy event on hybrid systems to collect metadata records")

Signed-off-by: Zhengjun Xing <[email protected]>
Suggested-by: Namhyung Kim <[email protected]>
---
tools/perf/builtin-script.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 585171479876..1a8790385eba 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -445,6 +445,9 @@ static int evsel__check_attr(struct evsel *evsel, struct perf_session *session)
struct perf_event_attr *attr = &evsel->core.attr;
bool allow_user_set;

+ if (evsel__is_dummy_event(evsel))
+ return 0;
+
if (perf_header__has_feat(&session->header, HEADER_STAT))
return 0;

--
2.25.1


2022-09-08 08:36:27

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH] perf script: Fix Cannot print 'iregs' field for hybrid systems

On Thu, Sep 08, 2022 at 03:00:30PM +0800, [email protected] wrote:
> From: Zhengjun Xing <[email protected]>
>
> Commit b91e5492f9d7 ("perf record: Add a dummy event on hybrid systems to
> collect metadata records") adds a dummy event on hybrid systems to fix the
> symbol "unknown" issue when the workload is created in a P-core but runs
> on an E-core. The added dummy event will cause "perf script -F iregs" to
> fail. Dummy events do not have "iregs" attribute set, so when we do
> evsel__check_attr, the "iregs" attribute check will fail, so the issue
> happened.
>
> The following commit [1] has fixed a similar issue by skipping the attr
> check for the dummy event because it does not have any samples anyway. It
> works okay for the normal mode, but the issue still happened when running
> the test in the pipe mode. In the pipe mode, it calls process_attr() which
> still checks the attr for the dummy event. This commit fixed the issue by
> skipping the attr check for the dummy event in the API evsel__check_attr,
> Otherwise, we have to patch everywhere when evsel__check_attr() is called.
>
> Before:
>
> #./perf record -o - --intr-regs=di,r8,dx,cx -e br_inst_retired.near_call:p -c 1000 --per-thread true 2>/dev/null|./perf script -F iregs |head -5
> Samples for 'dummy:HG' event do not have IREGS attribute set. Cannot print 'iregs' field.
> 0x120 [0x90]: failed to process type: 64
>
> After:
>
> # ./perf record -o - --intr-regs=di,r8,dx,cx -e br_inst_retired.near_call:p -c 1000 --per-thread true 2>/dev/null|./perf script -F iregs |head -5
> ABI:2 CX:0x55b8efa87000 DX:0x55b8efa7e000 DI:0xffffba5e625efbb0 R8:0xffff90e51f8ae100
> ABI:2 CX:0x7f1dae1e4000 DX:0xd0 DI:0xffff90e18c675ac0 R8:0x71
> ABI:2 CX:0xcc0 DX:0x1 DI:0xffff90e199880240 R8:0x0
> ABI:2 CX:0xffff90e180dd7500 DX:0xffff90e180dd7500 DI:0xffff90e180043500 R8:0x1
> ABI:2 CX:0x50 DX:0xffff90e18c583bd0 DI:0xffff90e1998803c0 R8:0x58
>
> [1]https://lore.kernel.org/lkml/[email protected]/
>
> Fixes: b91e5492f9d7 ("perf record: Add a dummy event on hybrid systems to collect metadata records")
>
> Signed-off-by: Zhengjun Xing <[email protected]>
> Suggested-by: Namhyung Kim <[email protected]>

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

thanks,
jirka

> ---
> tools/perf/builtin-script.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index 585171479876..1a8790385eba 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -445,6 +445,9 @@ static int evsel__check_attr(struct evsel *evsel, struct perf_session *session)
> struct perf_event_attr *attr = &evsel->core.attr;
> bool allow_user_set;
>
> + if (evsel__is_dummy_event(evsel))
> + return 0;
> +
> if (perf_header__has_feat(&session->header, HEADER_STAT))
> return 0;
>
> --
> 2.25.1
>

2022-09-08 19:03:17

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH] perf script: Fix Cannot print 'iregs' field for hybrid systems

Em Thu, Sep 08, 2022 at 10:24:27AM +0200, Jiri Olsa escreveu:
> On Thu, Sep 08, 2022 at 03:00:30PM +0800, [email protected] wrote:
> > From: Zhengjun Xing <[email protected]>
> >
> > Commit b91e5492f9d7 ("perf record: Add a dummy event on hybrid systems to
> > collect metadata records") adds a dummy event on hybrid systems to fix the
> > symbol "unknown" issue when the workload is created in a P-core but runs
> > on an E-core. The added dummy event will cause "perf script -F iregs" to
> > fail. Dummy events do not have "iregs" attribute set, so when we do
> > evsel__check_attr, the "iregs" attribute check will fail, so the issue
> > happened.
> >
> > The following commit [1] has fixed a similar issue by skipping the attr
> > check for the dummy event because it does not have any samples anyway. It
> > works okay for the normal mode, but the issue still happened when running
> > the test in the pipe mode. In the pipe mode, it calls process_attr() which
> > still checks the attr for the dummy event. This commit fixed the issue by
> > skipping the attr check for the dummy event in the API evsel__check_attr,
> > Otherwise, we have to patch everywhere when evsel__check_attr() is called.
> >
> > Before:
> >
> > #./perf record -o - --intr-regs=di,r8,dx,cx -e br_inst_retired.near_call:p -c 1000 --per-thread true 2>/dev/null|./perf script -F iregs |head -5
> > Samples for 'dummy:HG' event do not have IREGS attribute set. Cannot print 'iregs' field.
> > 0x120 [0x90]: failed to process type: 64
> >
> > After:
> >
> > # ./perf record -o - --intr-regs=di,r8,dx,cx -e br_inst_retired.near_call:p -c 1000 --per-thread true 2>/dev/null|./perf script -F iregs |head -5
> > ABI:2 CX:0x55b8efa87000 DX:0x55b8efa7e000 DI:0xffffba5e625efbb0 R8:0xffff90e51f8ae100
> > ABI:2 CX:0x7f1dae1e4000 DX:0xd0 DI:0xffff90e18c675ac0 R8:0x71
> > ABI:2 CX:0xcc0 DX:0x1 DI:0xffff90e199880240 R8:0x0
> > ABI:2 CX:0xffff90e180dd7500 DX:0xffff90e180dd7500 DI:0xffff90e180043500 R8:0x1
> > ABI:2 CX:0x50 DX:0xffff90e18c583bd0 DI:0xffff90e1998803c0 R8:0x58
> >
> > [1]https://lore.kernel.org/lkml/[email protected]/
> >
> > Fixes: b91e5492f9d7 ("perf record: Add a dummy event on hybrid systems to collect metadata records")
> >
> > Signed-off-by: Zhengjun Xing <[email protected]>
> > Suggested-by: Namhyung Kim <[email protected]>
>
> Acked-by: Jiri Olsa <[email protected]>

Thanks, applied.

- Arnaldo