* [PATCH] perf: cs-etm: Fix the assert() to handle captured and unprocessed cpu trace
@ 2024-09-24 23:39 Ilkka Koskinen
2024-09-25 9:54 ` James Clark
0 siblings, 1 reply; 4+ messages in thread
From: Ilkka Koskinen @ 2024-09-24 23:39 UTC (permalink / raw)
To: James Clark, John Garry, Will Deacon, Mike Leach, Leo Yan,
Suzuki K Poulose, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
Liang, Kan
Cc: linux-arm-kernel, coresight, linux-perf-users, linux-kernel,
Ilkka Koskinen
If one builds perf with DEBUG=1, captures data on multiple CPUs and
finally runs 'perf report -C <cpu>' for only one of the cpus, assert()
aborts the program. This happens because there are empty queues with
format set. This patch changes the condition to abort only if a queue
is not empty and if the format is unset.
$ make -C tools/perf DEBUG=1 CORESIGHT=1 CSLIBS=/usr/lib CSINCLUDES=/usr/include install
$ perf record -o kcore --kcore -e cs_etm/timestamp/k -s -C 0-1 dd if=/dev/zero of=/dev/null bs=1M count=1
$ perf report --input kcore/data --vmlinux=/home/ikoskine/projects/linux/vmlinux -C 1
Aborted (core dumped)
Fixes: 57880a7966be ("perf: cs-etm: Allocate queues for all CPUs")
Signed-off-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>
---
tools/perf/util/cs-etm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 90f32f327b9b..40f047baef81 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -3323,7 +3323,7 @@ static int cs_etm__create_decoders(struct cs_etm_auxtrace *etm)
* Don't create decoders for empty queues, mainly because
* etmq->format is unknown for empty queues.
*/
- assert(empty == (etmq->format == UNSET));
+ assert(empty || etmq->format != UNSET);
if (empty)
continue;
--
2.42.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] perf: cs-etm: Fix the assert() to handle captured and unprocessed cpu trace
2024-09-24 23:39 [PATCH] perf: cs-etm: Fix the assert() to handle captured and unprocessed cpu trace Ilkka Koskinen
@ 2024-09-25 9:54 ` James Clark
2024-09-25 23:04 ` Namhyung Kim
0 siblings, 1 reply; 4+ messages in thread
From: James Clark @ 2024-09-25 9:54 UTC (permalink / raw)
To: Ilkka Koskinen
Cc: linux-arm-kernel, coresight, linux-perf-users, linux-kernel,
John Garry, Will Deacon, Mike Leach, Leo Yan, Suzuki K Poulose,
Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Liang, Kan
On 25/09/2024 12:39 am, Ilkka Koskinen wrote:
> If one builds perf with DEBUG=1, captures data on multiple CPUs and
> finally runs 'perf report -C <cpu>' for only one of the cpus, assert()
> aborts the program. This happens because there are empty queues with
> format set. This patch changes the condition to abort only if a queue
> is not empty and if the format is unset.
>
> $ make -C tools/perf DEBUG=1 CORESIGHT=1 CSLIBS=/usr/lib CSINCLUDES=/usr/include install
> $ perf record -o kcore --kcore -e cs_etm/timestamp/k -s -C 0-1 dd if=/dev/zero of=/dev/null bs=1M count=1
> $ perf report --input kcore/data --vmlinux=/home/ikoskine/projects/linux/vmlinux -C 1
> Aborted (core dumped)
>
> Fixes: 57880a7966be ("perf: cs-etm: Allocate queues for all CPUs")
> Signed-off-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>
> ---
> tools/perf/util/cs-etm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 90f32f327b9b..40f047baef81 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -3323,7 +3323,7 @@ static int cs_etm__create_decoders(struct cs_etm_auxtrace *etm)
> * Don't create decoders for empty queues, mainly because
> * etmq->format is unknown for empty queues.
> */
> - assert(empty == (etmq->format == UNSET));
> + assert(empty || etmq->format != UNSET);
> if (empty)
> continue;
>
Oops I didn't realize you could filter on CPU in report mode. Thanks for
the fix. Adding a test to the end of test_arm_coresight.sh might be
quite useful. Either way:
Reviewed-by: James Clark <james.clark@linaro.org>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] perf: cs-etm: Fix the assert() to handle captured and unprocessed cpu trace
2024-09-25 9:54 ` James Clark
@ 2024-09-25 23:04 ` Namhyung Kim
2024-10-01 19:06 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 4+ messages in thread
From: Namhyung Kim @ 2024-09-25 23:04 UTC (permalink / raw)
To: James Clark, Arnaldo Carvalho de Melo
Cc: Ilkka Koskinen, linux-arm-kernel, coresight, linux-perf-users,
linux-kernel, John Garry, Will Deacon, Mike Leach, Leo Yan,
Suzuki K Poulose, Peter Zijlstra, Ingo Molnar, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
Liang, Kan
On Wed, Sep 25, 2024 at 10:54:31AM +0100, James Clark wrote:
>
>
> On 25/09/2024 12:39 am, Ilkka Koskinen wrote:
> > If one builds perf with DEBUG=1, captures data on multiple CPUs and
> > finally runs 'perf report -C <cpu>' for only one of the cpus, assert()
> > aborts the program. This happens because there are empty queues with
> > format set. This patch changes the condition to abort only if a queue
> > is not empty and if the format is unset.
> >
> > $ make -C tools/perf DEBUG=1 CORESIGHT=1 CSLIBS=/usr/lib CSINCLUDES=/usr/include install
> > $ perf record -o kcore --kcore -e cs_etm/timestamp/k -s -C 0-1 dd if=/dev/zero of=/dev/null bs=1M count=1
> > $ perf report --input kcore/data --vmlinux=/home/ikoskine/projects/linux/vmlinux -C 1
> > Aborted (core dumped)
> >
> > Fixes: 57880a7966be ("perf: cs-etm: Allocate queues for all CPUs")
> > Signed-off-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>
> > ---
> > tools/perf/util/cs-etm.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> > index 90f32f327b9b..40f047baef81 100644
> > --- a/tools/perf/util/cs-etm.c
> > +++ b/tools/perf/util/cs-etm.c
> > @@ -3323,7 +3323,7 @@ static int cs_etm__create_decoders(struct cs_etm_auxtrace *etm)
> > * Don't create decoders for empty queues, mainly because
> > * etmq->format is unknown for empty queues.
> > */
> > - assert(empty == (etmq->format == UNSET));
> > + assert(empty || etmq->format != UNSET);
> > if (empty)
> > continue;
>
> Oops I didn't realize you could filter on CPU in report mode. Thanks for the
> fix. Adding a test to the end of test_arm_coresight.sh might be quite
> useful. Either way:
>
> Reviewed-by: James Clark <james.clark@linaro.org>
Thanks, it should go to the perf-tool. Arnaldo, please pick up.
Namhyung
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] perf: cs-etm: Fix the assert() to handle captured and unprocessed cpu trace
2024-09-25 23:04 ` Namhyung Kim
@ 2024-10-01 19:06 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 4+ messages in thread
From: Arnaldo Carvalho de Melo @ 2024-10-01 19:06 UTC (permalink / raw)
To: Namhyung Kim
Cc: James Clark, Ilkka Koskinen, linux-arm-kernel, coresight,
linux-perf-users, linux-kernel, John Garry, Will Deacon,
Mike Leach, Leo Yan, Suzuki K Poulose, Peter Zijlstra,
Ingo Molnar, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Liang, Kan
On Wed, Sep 25, 2024 at 04:04:55PM -0700, Namhyung Kim wrote:
> On Wed, Sep 25, 2024 at 10:54:31AM +0100, James Clark wrote:
> >
> >
> > On 25/09/2024 12:39 am, Ilkka Koskinen wrote:
> > > If one builds perf with DEBUG=1, captures data on multiple CPUs and
> > > finally runs 'perf report -C <cpu>' for only one of the cpus, assert()
> > > aborts the program. This happens because there are empty queues with
> > > format set. This patch changes the condition to abort only if a queue
> > > is not empty and if the format is unset.
> > >
> > > $ make -C tools/perf DEBUG=1 CORESIGHT=1 CSLIBS=/usr/lib CSINCLUDES=/usr/include install
> > > $ perf record -o kcore --kcore -e cs_etm/timestamp/k -s -C 0-1 dd if=/dev/zero of=/dev/null bs=1M count=1
> > > $ perf report --input kcore/data --vmlinux=/home/ikoskine/projects/linux/vmlinux -C 1
> > > Aborted (core dumped)
> > >
> > > Fixes: 57880a7966be ("perf: cs-etm: Allocate queues for all CPUs")
> > > Signed-off-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>
> > > ---
> > > tools/perf/util/cs-etm.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> > > index 90f32f327b9b..40f047baef81 100644
> > > --- a/tools/perf/util/cs-etm.c
> > > +++ b/tools/perf/util/cs-etm.c
> > > @@ -3323,7 +3323,7 @@ static int cs_etm__create_decoders(struct cs_etm_auxtrace *etm)
> > > * Don't create decoders for empty queues, mainly because
> > > * etmq->format is unknown for empty queues.
> > > */
> > > - assert(empty == (etmq->format == UNSET));
> > > + assert(empty || etmq->format != UNSET);
> > > if (empty)
> > > continue;
> >
> > Oops I didn't realize you could filter on CPU in report mode. Thanks for the
> > fix. Adding a test to the end of test_arm_coresight.sh might be quite
> > useful. Either way:
> >
> > Reviewed-by: James Clark <james.clark@linaro.org>
>
> Thanks, it should go to the perf-tool. Arnaldo, please pick up.
Right, picking it now.
Thanks,
- Arnaldo
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-10-01 19:07 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-24 23:39 [PATCH] perf: cs-etm: Fix the assert() to handle captured and unprocessed cpu trace Ilkka Koskinen
2024-09-25 9:54 ` James Clark
2024-09-25 23:04 ` Namhyung Kim
2024-10-01 19:06 ` Arnaldo Carvalho de Melo
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).