All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] perf: cs-etm: No-op refactor of synth opt usage
@ 2022-02-10 20:06 ` James Clark
  0 siblings, 0 replies; 14+ messages in thread
From: James Clark @ 2022-02-10 20:06 UTC (permalink / raw)
  To: acme, linux-perf-users, mathieu.poirier, coresight
  Cc: James Clark, Mike Leach, Leo Yan, John Garry, Will Deacon,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	linux-arm-kernel, linux-kernel

sample_branches and sample_instructions are already saved in the
synth_opts struct. Other usages like synth_opts.last_branch don't save
a value, so make this more consistent by always going through synth_opts
and not saving duplicate values.

Signed-off-by: James Clark <james.clark@arm.com>
---
 tools/perf/util/cs-etm.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 4f672f7d008c..796a065a500e 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -50,8 +50,6 @@ struct cs_etm_auxtrace {
 	u8 timeless_decoding;
 	u8 snapshot_mode;
 	u8 data_queued;
-	u8 sample_branches;
-	u8 sample_instructions;
 
 	int num_cpu;
 	u64 latest_kernel_timestamp;
@@ -410,8 +408,8 @@ static void cs_etm__packet_swap(struct cs_etm_auxtrace *etm,
 {
 	struct cs_etm_packet *tmp;
 
-	if (etm->sample_branches || etm->synth_opts.last_branch ||
-	    etm->sample_instructions) {
+	if (etm->synth_opts.branches || etm->synth_opts.last_branch ||
+	    etm->synth_opts.instructions) {
 		/*
 		 * Swap PACKET with PREV_PACKET: PACKET becomes PREV_PACKET for
 		 * the next incoming packet.
@@ -1365,7 +1363,6 @@ static int cs_etm__synth_events(struct cs_etm_auxtrace *etm,
 		err = cs_etm__synth_event(session, &attr, id);
 		if (err)
 			return err;
-		etm->sample_branches = true;
 		etm->branches_sample_type = attr.sample_type;
 		etm->branches_id = id;
 		id += 1;
@@ -1389,7 +1386,6 @@ static int cs_etm__synth_events(struct cs_etm_auxtrace *etm,
 		err = cs_etm__synth_event(session, &attr, id);
 		if (err)
 			return err;
-		etm->sample_instructions = true;
 		etm->instructions_sample_type = attr.sample_type;
 		etm->instructions_id = id;
 		id += 1;
@@ -1420,7 +1416,7 @@ static int cs_etm__sample(struct cs_etm_queue *etmq,
 	    tidq->prev_packet->last_instr_taken_branch)
 		cs_etm__update_last_branch_rb(etmq, tidq);
 
-	if (etm->sample_instructions &&
+	if (etm->synth_opts.instructions &&
 	    tidq->period_instructions >= etm->instructions_sample_period) {
 		/*
 		 * Emit instruction sample periodically
@@ -1503,7 +1499,7 @@ static int cs_etm__sample(struct cs_etm_queue *etmq,
 		}
 	}
 
-	if (etm->sample_branches) {
+	if (etm->synth_opts.branches) {
 		bool generate_sample = false;
 
 		/* Generate sample for tracing on packet */
@@ -1582,7 +1578,7 @@ static int cs_etm__flush(struct cs_etm_queue *etmq,
 
 	}
 
-	if (etm->sample_branches &&
+	if (etm->synth_opts.branches &&
 	    tidq->prev_packet->sample_type == CS_ETM_RANGE) {
 		err = cs_etm__synth_branch_sample(etmq, tidq);
 		if (err)
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread
* [PATCH 1/2] perf: cs-etm: No-op refactor of synth opt usage
@ 2022-02-10 20:06 ` James Clark
  0 siblings, 0 replies; 14+ messages in thread
From: James Clark @ 2022-02-10 20:06 UTC (permalink / raw)
  To: acme, linux-perf-users, mathieu.poirier, coresight
  Cc: James Clark, Mike Leach, Leo Yan, John Garry, Will Deacon,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	linux-arm-kernel, linux-kernel

sample_branches and sample_instructions are already saved in the
synth_opts struct. Other usages like synth_opts.last_branch don't save
a value, so make this more consistent by always going through synth_opts
and not saving duplicate values.

Signed-off-by: James Clark <james.clark@arm.com>
---
 tools/perf/util/cs-etm.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 4f672f7d008c..796a065a500e 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -50,8 +50,6 @@ struct cs_etm_auxtrace {
 	u8 timeless_decoding;
 	u8 snapshot_mode;
 	u8 data_queued;
-	u8 sample_branches;
-	u8 sample_instructions;
 
 	int num_cpu;
 	u64 latest_kernel_timestamp;
@@ -410,8 +408,8 @@ static void cs_etm__packet_swap(struct cs_etm_auxtrace *etm,
 {
 	struct cs_etm_packet *tmp;
 
-	if (etm->sample_branches || etm->synth_opts.last_branch ||
-	    etm->sample_instructions) {
+	if (etm->synth_opts.branches || etm->synth_opts.last_branch ||
+	    etm->synth_opts.instructions) {
 		/*
 		 * Swap PACKET with PREV_PACKET: PACKET becomes PREV_PACKET for
 		 * the next incoming packet.
@@ -1365,7 +1363,6 @@ static int cs_etm__synth_events(struct cs_etm_auxtrace *etm,
 		err = cs_etm__synth_event(session, &attr, id);
 		if (err)
 			return err;
-		etm->sample_branches = true;
 		etm->branches_sample_type = attr.sample_type;
 		etm->branches_id = id;
 		id += 1;
@@ -1389,7 +1386,6 @@ static int cs_etm__synth_events(struct cs_etm_auxtrace *etm,
 		err = cs_etm__synth_event(session, &attr, id);
 		if (err)
 			return err;
-		etm->sample_instructions = true;
 		etm->instructions_sample_type = attr.sample_type;
 		etm->instructions_id = id;
 		id += 1;
@@ -1420,7 +1416,7 @@ static int cs_etm__sample(struct cs_etm_queue *etmq,
 	    tidq->prev_packet->last_instr_taken_branch)
 		cs_etm__update_last_branch_rb(etmq, tidq);
 
-	if (etm->sample_instructions &&
+	if (etm->synth_opts.instructions &&
 	    tidq->period_instructions >= etm->instructions_sample_period) {
 		/*
 		 * Emit instruction sample periodically
@@ -1503,7 +1499,7 @@ static int cs_etm__sample(struct cs_etm_queue *etmq,
 		}
 	}
 
-	if (etm->sample_branches) {
+	if (etm->synth_opts.branches) {
 		bool generate_sample = false;
 
 		/* Generate sample for tracing on packet */
@@ -1582,7 +1578,7 @@ static int cs_etm__flush(struct cs_etm_queue *etmq,
 
 	}
 
-	if (etm->sample_branches &&
+	if (etm->synth_opts.branches &&
 	    tidq->prev_packet->sample_type == CS_ETM_RANGE) {
 		err = cs_etm__synth_branch_sample(etmq, tidq);
 		if (err)
-- 
2.28.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 14+ messages in thread
* [PATCH 2/2] perf: cs-etm: Fix corrupt inject files when only last branch option is enabled
  2022-02-10 20:06 ` James Clark
@ 2022-02-10 20:06   ` James Clark
  -1 siblings, 0 replies; 14+ messages in thread
From: James Clark @ 2022-02-10 20:06 UTC (permalink / raw)
  To: acme, linux-perf-users, mathieu.poirier, coresight
  Cc: James Clark, Mike Leach, Leo Yan, John Garry, Will Deacon,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	linux-arm-kernel, linux-kernel

Perf inject with Coresight data generates files that cannot be opened
when only the last branch option is specified:

  perf inject -i perf.data --itrace=l -o inject.data
  perf script -i inject.data
  0x33faa8 [0x8]: failed to process type: 9 [Bad address]

This is because cs_etm__synth_instruction_sample() is called even when
the sample type for instructions hasn't been setup. Last branch records
are attached to instruction samples so it doesn't make sense to generate
them when --itrace=i isn't specified anyway.

This change disables all calls of cs_etm__synth_instruction_sample()
unless --itrace=i is specified, resulting in a file with no samples if
only --itrace=l is provided, rather than a bad file.

Signed-off-by: James Clark <james.clark@arm.com>
---
 tools/perf/util/cs-etm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 796a065a500e..8b95fb3c4d7b 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -1553,6 +1553,7 @@ static int cs_etm__flush(struct cs_etm_queue *etmq,
 		goto swap_packet;
 
 	if (etmq->etm->synth_opts.last_branch &&
+	    etmq->etm->synth_opts.instructions &&
 	    tidq->prev_packet->sample_type == CS_ETM_RANGE) {
 		u64 addr;
 
@@ -1610,6 +1611,7 @@ static int cs_etm__end_block(struct cs_etm_queue *etmq,
 	 * the trace.
 	 */
 	if (etmq->etm->synth_opts.last_branch &&
+	    etmq->etm->synth_opts.instructions &&
 	    tidq->prev_packet->sample_type == CS_ETM_RANGE) {
 		u64 addr;
 
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread
* [PATCH 2/2] perf: cs-etm: Fix corrupt inject files when only last branch option is enabled
@ 2022-02-10 20:06   ` James Clark
  0 siblings, 0 replies; 14+ messages in thread
From: James Clark @ 2022-02-10 20:06 UTC (permalink / raw)
  To: acme, linux-perf-users, mathieu.poirier, coresight
  Cc: James Clark, Mike Leach, Leo Yan, John Garry, Will Deacon,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	linux-arm-kernel, linux-kernel

Perf inject with Coresight data generates files that cannot be opened
when only the last branch option is specified:

  perf inject -i perf.data --itrace=l -o inject.data
  perf script -i inject.data
  0x33faa8 [0x8]: failed to process type: 9 [Bad address]

This is because cs_etm__synth_instruction_sample() is called even when
the sample type for instructions hasn't been setup. Last branch records
are attached to instruction samples so it doesn't make sense to generate
them when --itrace=i isn't specified anyway.

This change disables all calls of cs_etm__synth_instruction_sample()
unless --itrace=i is specified, resulting in a file with no samples if
only --itrace=l is provided, rather than a bad file.

Signed-off-by: James Clark <james.clark@arm.com>
---
 tools/perf/util/cs-etm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 796a065a500e..8b95fb3c4d7b 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -1553,6 +1553,7 @@ static int cs_etm__flush(struct cs_etm_queue *etmq,
 		goto swap_packet;
 
 	if (etmq->etm->synth_opts.last_branch &&
+	    etmq->etm->synth_opts.instructions &&
 	    tidq->prev_packet->sample_type == CS_ETM_RANGE) {
 		u64 addr;
 
@@ -1610,6 +1611,7 @@ static int cs_etm__end_block(struct cs_etm_queue *etmq,
 	 * the trace.
 	 */
 	if (etmq->etm->synth_opts.last_branch &&
+	    etmq->etm->synth_opts.instructions &&
 	    tidq->prev_packet->sample_type == CS_ETM_RANGE) {
 		u64 addr;
 
-- 
2.28.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] perf: cs-etm: No-op refactor of synth opt usage
  2022-02-10 20:06 ` James Clark
@ 2022-02-11 15:20   ` Leo Yan
  -1 siblings, 0 replies; 14+ messages in thread
From: Leo Yan @ 2022-02-11 15:20 UTC (permalink / raw)
  To: James Clark
  Cc: acme, linux-perf-users, mathieu.poirier, coresight, Mike Leach,
	John Garry, Will Deacon, Mark Rutland, Alexander Shishkin,
	Jiri Olsa, Namhyung Kim, linux-arm-kernel, linux-kernel

On Thu, Feb 10, 2022 at 08:06:19PM +0000, James Clark wrote:
> sample_branches and sample_instructions are already saved in the
> synth_opts struct. Other usages like synth_opts.last_branch don't save
> a value, so make this more consistent by always going through synth_opts
> and not saving duplicate values.
> 
> Signed-off-by: James Clark <james.clark@arm.com>

The patch looks good to me:

Reviewed-by: Leo Yan <leo.yan@linaro.org>

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] perf: cs-etm: No-op refactor of synth opt usage
@ 2022-02-11 15:20   ` Leo Yan
  0 siblings, 0 replies; 14+ messages in thread
From: Leo Yan @ 2022-02-11 15:20 UTC (permalink / raw)
  To: James Clark
  Cc: acme, linux-perf-users, mathieu.poirier, coresight, Mike Leach,
	John Garry, Will Deacon, Mark Rutland, Alexander Shishkin,
	Jiri Olsa, Namhyung Kim, linux-arm-kernel, linux-kernel

On Thu, Feb 10, 2022 at 08:06:19PM +0000, James Clark wrote:
> sample_branches and sample_instructions are already saved in the
> synth_opts struct. Other usages like synth_opts.last_branch don't save
> a value, so make this more consistent by always going through synth_opts
> and not saving duplicate values.
> 
> Signed-off-by: James Clark <james.clark@arm.com>

The patch looks good to me:

Reviewed-by: Leo Yan <leo.yan@linaro.org>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] perf: cs-etm: Fix corrupt inject files when only last branch option is enabled
  2022-02-10 20:06   ` James Clark
@ 2022-02-11 16:05     ` Leo Yan
  -1 siblings, 0 replies; 14+ messages in thread
From: Leo Yan @ 2022-02-11 16:05 UTC (permalink / raw)
  To: James Clark
  Cc: acme, linux-perf-users, mathieu.poirier, coresight, Mike Leach,
	John Garry, Will Deacon, Mark Rutland, Alexander Shishkin,
	Jiri Olsa, Namhyung Kim, linux-arm-kernel, linux-kernel

On Thu, Feb 10, 2022 at 08:06:20PM +0000, James Clark wrote:
> Perf inject with Coresight data generates files that cannot be opened
> when only the last branch option is specified:
> 
>   perf inject -i perf.data --itrace=l -o inject.data
>   perf script -i inject.data
>   0x33faa8 [0x8]: failed to process type: 9 [Bad address]
> 
> This is because cs_etm__synth_instruction_sample() is called even when
> the sample type for instructions hasn't been setup. Last branch records
> are attached to instruction samples so it doesn't make sense to generate
> them when --itrace=i isn't specified anyway.
> 
> This change disables all calls of cs_etm__synth_instruction_sample()
> unless --itrace=i is specified, resulting in a file with no samples if
> only --itrace=l is provided, rather than a bad file.
> 
> Signed-off-by: James Clark <james.clark@arm.com>

Reviewed-by: Leo Yan <leo.yan@linaro.org>

> ---
>  tools/perf/util/cs-etm.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 796a065a500e..8b95fb3c4d7b 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -1553,6 +1553,7 @@ static int cs_etm__flush(struct cs_etm_queue *etmq,
>  		goto swap_packet;
>  
>  	if (etmq->etm->synth_opts.last_branch &&
> +	    etmq->etm->synth_opts.instructions &&
>  	    tidq->prev_packet->sample_type == CS_ETM_RANGE) {
>  		u64 addr;
>  
> @@ -1610,6 +1611,7 @@ static int cs_etm__end_block(struct cs_etm_queue *etmq,
>  	 * the trace.
>  	 */
>  	if (etmq->etm->synth_opts.last_branch &&
> +	    etmq->etm->synth_opts.instructions &&
>  	    tidq->prev_packet->sample_type == CS_ETM_RANGE) {
>  		u64 addr;
>  
> -- 
> 2.28.0
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] perf: cs-etm: Fix corrupt inject files when only last branch option is enabled
@ 2022-02-11 16:05     ` Leo Yan
  0 siblings, 0 replies; 14+ messages in thread
From: Leo Yan @ 2022-02-11 16:05 UTC (permalink / raw)
  To: James Clark
  Cc: acme, linux-perf-users, mathieu.poirier, coresight, Mike Leach,
	John Garry, Will Deacon, Mark Rutland, Alexander Shishkin,
	Jiri Olsa, Namhyung Kim, linux-arm-kernel, linux-kernel

On Thu, Feb 10, 2022 at 08:06:20PM +0000, James Clark wrote:
> Perf inject with Coresight data generates files that cannot be opened
> when only the last branch option is specified:
> 
>   perf inject -i perf.data --itrace=l -o inject.data
>   perf script -i inject.data
>   0x33faa8 [0x8]: failed to process type: 9 [Bad address]
> 
> This is because cs_etm__synth_instruction_sample() is called even when
> the sample type for instructions hasn't been setup. Last branch records
> are attached to instruction samples so it doesn't make sense to generate
> them when --itrace=i isn't specified anyway.
> 
> This change disables all calls of cs_etm__synth_instruction_sample()
> unless --itrace=i is specified, resulting in a file with no samples if
> only --itrace=l is provided, rather than a bad file.
> 
> Signed-off-by: James Clark <james.clark@arm.com>

Reviewed-by: Leo Yan <leo.yan@linaro.org>

> ---
>  tools/perf/util/cs-etm.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 796a065a500e..8b95fb3c4d7b 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -1553,6 +1553,7 @@ static int cs_etm__flush(struct cs_etm_queue *etmq,
>  		goto swap_packet;
>  
>  	if (etmq->etm->synth_opts.last_branch &&
> +	    etmq->etm->synth_opts.instructions &&
>  	    tidq->prev_packet->sample_type == CS_ETM_RANGE) {
>  		u64 addr;
>  
> @@ -1610,6 +1611,7 @@ static int cs_etm__end_block(struct cs_etm_queue *etmq,
>  	 * the trace.
>  	 */
>  	if (etmq->etm->synth_opts.last_branch &&
> +	    etmq->etm->synth_opts.instructions &&
>  	    tidq->prev_packet->sample_type == CS_ETM_RANGE) {
>  		u64 addr;
>  
> -- 
> 2.28.0
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] perf: cs-etm: Fix corrupt inject files when only last branch option is enabled
  2022-02-11 16:05     ` Leo Yan
@ 2022-02-15 15:07       ` Arnaldo Carvalho de Melo
  -1 siblings, 0 replies; 14+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-02-15 15:07 UTC (permalink / raw)
  To: Leo Yan
  Cc: James Clark, linux-perf-users, mathieu.poirier, coresight,
	Mike Leach, John Garry, Will Deacon, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-arm-kernel,
	linux-kernel

Em Sat, Feb 12, 2022 at 12:05:16AM +0800, Leo Yan escreveu:
> On Thu, Feb 10, 2022 at 08:06:20PM +0000, James Clark wrote:
> > Perf inject with Coresight data generates files that cannot be opened
> > when only the last branch option is specified:
> > 
> >   perf inject -i perf.data --itrace=l -o inject.data
> >   perf script -i inject.data
> >   0x33faa8 [0x8]: failed to process type: 9 [Bad address]
> > 
> > This is because cs_etm__synth_instruction_sample() is called even when
> > the sample type for instructions hasn't been setup. Last branch records
> > are attached to instruction samples so it doesn't make sense to generate
> > them when --itrace=i isn't specified anyway.
> > 
> > This change disables all calls of cs_etm__synth_instruction_sample()
> > unless --itrace=i is specified, resulting in a file with no samples if
> > only --itrace=l is provided, rather than a bad file.
> > 
> > Signed-off-by: James Clark <james.clark@arm.com>
> 
> Reviewed-by: Leo Yan <leo.yan@linaro.org>

Thanks, applied.

- Arnaldo

 
> > ---
> >  tools/perf/util/cs-etm.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> > index 796a065a500e..8b95fb3c4d7b 100644
> > --- a/tools/perf/util/cs-etm.c
> > +++ b/tools/perf/util/cs-etm.c
> > @@ -1553,6 +1553,7 @@ static int cs_etm__flush(struct cs_etm_queue *etmq,
> >  		goto swap_packet;
> >  
> >  	if (etmq->etm->synth_opts.last_branch &&
> > +	    etmq->etm->synth_opts.instructions &&
> >  	    tidq->prev_packet->sample_type == CS_ETM_RANGE) {
> >  		u64 addr;
> >  
> > @@ -1610,6 +1611,7 @@ static int cs_etm__end_block(struct cs_etm_queue *etmq,
> >  	 * the trace.
> >  	 */
> >  	if (etmq->etm->synth_opts.last_branch &&
> > +	    etmq->etm->synth_opts.instructions &&
> >  	    tidq->prev_packet->sample_type == CS_ETM_RANGE) {
> >  		u64 addr;
> >  
> > -- 
> > 2.28.0
> > 

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] perf: cs-etm: Fix corrupt inject files when only last branch option is enabled
@ 2022-02-15 15:07       ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 14+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-02-15 15:07 UTC (permalink / raw)
  To: Leo Yan
  Cc: James Clark, linux-perf-users, mathieu.poirier, coresight,
	Mike Leach, John Garry, Will Deacon, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-arm-kernel,
	linux-kernel

Em Sat, Feb 12, 2022 at 12:05:16AM +0800, Leo Yan escreveu:
> On Thu, Feb 10, 2022 at 08:06:20PM +0000, James Clark wrote:
> > Perf inject with Coresight data generates files that cannot be opened
> > when only the last branch option is specified:
> > 
> >   perf inject -i perf.data --itrace=l -o inject.data
> >   perf script -i inject.data
> >   0x33faa8 [0x8]: failed to process type: 9 [Bad address]
> > 
> > This is because cs_etm__synth_instruction_sample() is called even when
> > the sample type for instructions hasn't been setup. Last branch records
> > are attached to instruction samples so it doesn't make sense to generate
> > them when --itrace=i isn't specified anyway.
> > 
> > This change disables all calls of cs_etm__synth_instruction_sample()
> > unless --itrace=i is specified, resulting in a file with no samples if
> > only --itrace=l is provided, rather than a bad file.
> > 
> > Signed-off-by: James Clark <james.clark@arm.com>
> 
> Reviewed-by: Leo Yan <leo.yan@linaro.org>

Thanks, applied.

- Arnaldo

 
> > ---
> >  tools/perf/util/cs-etm.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> > index 796a065a500e..8b95fb3c4d7b 100644
> > --- a/tools/perf/util/cs-etm.c
> > +++ b/tools/perf/util/cs-etm.c
> > @@ -1553,6 +1553,7 @@ static int cs_etm__flush(struct cs_etm_queue *etmq,
> >  		goto swap_packet;
> >  
> >  	if (etmq->etm->synth_opts.last_branch &&
> > +	    etmq->etm->synth_opts.instructions &&
> >  	    tidq->prev_packet->sample_type == CS_ETM_RANGE) {
> >  		u64 addr;
> >  
> > @@ -1610,6 +1611,7 @@ static int cs_etm__end_block(struct cs_etm_queue *etmq,
> >  	 * the trace.
> >  	 */
> >  	if (etmq->etm->synth_opts.last_branch &&
> > +	    etmq->etm->synth_opts.instructions &&
> >  	    tidq->prev_packet->sample_type == CS_ETM_RANGE) {
> >  		u64 addr;
> >  
> > -- 
> > 2.28.0
> > 

-- 

- Arnaldo

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] perf: cs-etm: No-op refactor of synth opt usage
  2022-02-11 15:20   ` Leo Yan
@ 2022-02-15 15:07     ` Arnaldo Carvalho de Melo
  -1 siblings, 0 replies; 14+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-02-15 15:07 UTC (permalink / raw)
  To: Leo Yan
  Cc: James Clark, linux-perf-users, mathieu.poirier, coresight,
	Mike Leach, John Garry, Will Deacon, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-arm-kernel,
	linux-kernel

Em Fri, Feb 11, 2022 at 11:20:26PM +0800, Leo Yan escreveu:
> On Thu, Feb 10, 2022 at 08:06:19PM +0000, James Clark wrote:
> > sample_branches and sample_instructions are already saved in the
> > synth_opts struct. Other usages like synth_opts.last_branch don't save
> > a value, so make this more consistent by always going through synth_opts
> > and not saving duplicate values.
> > 
> > Signed-off-by: James Clark <james.clark@arm.com>
> 
> The patch looks good to me:
> 
> Reviewed-by: Leo Yan <leo.yan@linaro.org>


Thanks, applied.

- Arnaldo

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] perf: cs-etm: No-op refactor of synth opt usage
@ 2022-02-15 15:07     ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 14+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-02-15 15:07 UTC (permalink / raw)
  To: Leo Yan
  Cc: James Clark, linux-perf-users, mathieu.poirier, coresight,
	Mike Leach, John Garry, Will Deacon, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-arm-kernel,
	linux-kernel

Em Fri, Feb 11, 2022 at 11:20:26PM +0800, Leo Yan escreveu:
> On Thu, Feb 10, 2022 at 08:06:19PM +0000, James Clark wrote:
> > sample_branches and sample_instructions are already saved in the
> > synth_opts struct. Other usages like synth_opts.last_branch don't save
> > a value, so make this more consistent by always going through synth_opts
> > and not saving duplicate values.
> > 
> > Signed-off-by: James Clark <james.clark@arm.com>
> 
> The patch looks good to me:
> 
> Reviewed-by: Leo Yan <leo.yan@linaro.org>


Thanks, applied.

- Arnaldo

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] perf: cs-etm: Fix corrupt inject files when only last branch option is enabled
  2022-02-15 15:07       ` Arnaldo Carvalho de Melo
@ 2022-02-22 15:49         ` James Clark
  -1 siblings, 0 replies; 14+ messages in thread
From: James Clark @ 2022-02-22 15:49 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Leo Yan
  Cc: linux-perf-users, mathieu.poirier, coresight, Mike Leach,
	John Garry, Will Deacon, Mark Rutland, Alexander Shishkin,
	Jiri Olsa, Namhyung Kim, linux-arm-kernel, linux-kernel



On 15/02/2022 15:07, Arnaldo Carvalho de Melo wrote:
> Em Sat, Feb 12, 2022 at 12:05:16AM +0800, Leo Yan escreveu:
>> On Thu, Feb 10, 2022 at 08:06:20PM +0000, James Clark wrote:
>>> Perf inject with Coresight data generates files that cannot be opened
>>> when only the last branch option is specified:
>>>
>>>   perf inject -i perf.data --itrace=l -o inject.data
>>>   perf script -i inject.data
>>>   0x33faa8 [0x8]: failed to process type: 9 [Bad address]
>>>
>>> This is because cs_etm__synth_instruction_sample() is called even when
>>> the sample type for instructions hasn't been setup. Last branch records
>>> are attached to instruction samples so it doesn't make sense to generate
>>> them when --itrace=i isn't specified anyway.
>>>
>>> This change disables all calls of cs_etm__synth_instruction_sample()
>>> unless --itrace=i is specified, resulting in a file with no samples if
>>> only --itrace=l is provided, rather than a bad file.
>>>
>>> Signed-off-by: James Clark <james.clark@arm.com>
>>
>> Reviewed-by: Leo Yan <leo.yan@linaro.org>
> 
> Thanks, applied.
> 
> - Arnaldo
> 

Thanks Leo and Arnaldo!

>  
>>> ---
>>>  tools/perf/util/cs-etm.c | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
>>> index 796a065a500e..8b95fb3c4d7b 100644
>>> --- a/tools/perf/util/cs-etm.c
>>> +++ b/tools/perf/util/cs-etm.c
>>> @@ -1553,6 +1553,7 @@ static int cs_etm__flush(struct cs_etm_queue *etmq,
>>>  		goto swap_packet;
>>>  
>>>  	if (etmq->etm->synth_opts.last_branch &&
>>> +	    etmq->etm->synth_opts.instructions &&
>>>  	    tidq->prev_packet->sample_type == CS_ETM_RANGE) {
>>>  		u64 addr;
>>>  
>>> @@ -1610,6 +1611,7 @@ static int cs_etm__end_block(struct cs_etm_queue *etmq,
>>>  	 * the trace.
>>>  	 */
>>>  	if (etmq->etm->synth_opts.last_branch &&
>>> +	    etmq->etm->synth_opts.instructions &&
>>>  	    tidq->prev_packet->sample_type == CS_ETM_RANGE) {
>>>  		u64 addr;
>>>  
>>> -- 
>>> 2.28.0
>>>
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] perf: cs-etm: Fix corrupt inject files when only last branch option is enabled
@ 2022-02-22 15:49         ` James Clark
  0 siblings, 0 replies; 14+ messages in thread
From: James Clark @ 2022-02-22 15:49 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Leo Yan
  Cc: linux-perf-users, mathieu.poirier, coresight, Mike Leach,
	John Garry, Will Deacon, Mark Rutland, Alexander Shishkin,
	Jiri Olsa, Namhyung Kim, linux-arm-kernel, linux-kernel



On 15/02/2022 15:07, Arnaldo Carvalho de Melo wrote:
> Em Sat, Feb 12, 2022 at 12:05:16AM +0800, Leo Yan escreveu:
>> On Thu, Feb 10, 2022 at 08:06:20PM +0000, James Clark wrote:
>>> Perf inject with Coresight data generates files that cannot be opened
>>> when only the last branch option is specified:
>>>
>>>   perf inject -i perf.data --itrace=l -o inject.data
>>>   perf script -i inject.data
>>>   0x33faa8 [0x8]: failed to process type: 9 [Bad address]
>>>
>>> This is because cs_etm__synth_instruction_sample() is called even when
>>> the sample type for instructions hasn't been setup. Last branch records
>>> are attached to instruction samples so it doesn't make sense to generate
>>> them when --itrace=i isn't specified anyway.
>>>
>>> This change disables all calls of cs_etm__synth_instruction_sample()
>>> unless --itrace=i is specified, resulting in a file with no samples if
>>> only --itrace=l is provided, rather than a bad file.
>>>
>>> Signed-off-by: James Clark <james.clark@arm.com>
>>
>> Reviewed-by: Leo Yan <leo.yan@linaro.org>
> 
> Thanks, applied.
> 
> - Arnaldo
> 

Thanks Leo and Arnaldo!

>  
>>> ---
>>>  tools/perf/util/cs-etm.c | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
>>> index 796a065a500e..8b95fb3c4d7b 100644
>>> --- a/tools/perf/util/cs-etm.c
>>> +++ b/tools/perf/util/cs-etm.c
>>> @@ -1553,6 +1553,7 @@ static int cs_etm__flush(struct cs_etm_queue *etmq,
>>>  		goto swap_packet;
>>>  
>>>  	if (etmq->etm->synth_opts.last_branch &&
>>> +	    etmq->etm->synth_opts.instructions &&
>>>  	    tidq->prev_packet->sample_type == CS_ETM_RANGE) {
>>>  		u64 addr;
>>>  
>>> @@ -1610,6 +1611,7 @@ static int cs_etm__end_block(struct cs_etm_queue *etmq,
>>>  	 * the trace.
>>>  	 */
>>>  	if (etmq->etm->synth_opts.last_branch &&
>>> +	    etmq->etm->synth_opts.instructions &&
>>>  	    tidq->prev_packet->sample_type == CS_ETM_RANGE) {
>>>  		u64 addr;
>>>  
>>> -- 
>>> 2.28.0
>>>
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 14+ messages in thread
end of thread, other threads:[~2022-02-22 15:51 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-10 20:06 [PATCH 1/2] perf: cs-etm: No-op refactor of synth opt usage James Clark
2022-02-10 20:06 ` James Clark
2022-02-10 20:06 ` [PATCH 2/2] perf: cs-etm: Fix corrupt inject files when only last branch option is enabled James Clark
2022-02-10 20:06   ` James Clark
2022-02-11 16:05   ` Leo Yan
2022-02-11 16:05     ` Leo Yan
2022-02-15 15:07     ` Arnaldo Carvalho de Melo
2022-02-15 15:07       ` Arnaldo Carvalho de Melo
2022-02-22 15:49       ` James Clark
2022-02-22 15:49         ` James Clark
2022-02-11 15:20 ` [PATCH 1/2] perf: cs-etm: No-op refactor of synth opt usage Leo Yan
2022-02-11 15:20   ` Leo Yan
2022-02-15 15:07   ` Arnaldo Carvalho de Melo
2022-02-15 15:07     ` Arnaldo Carvalho de Melo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.