Re: [PATCH 1/2] perf cs-etm: Refactor timestamp variable names

2021-04-15 Thread Mathieu Poirier
On Wed, Apr 14, 2021 at 05:39:18PM +0300, James Clark wrote:
> Remove ambiguity in variable names relating to timestamps.
> A later commit will save the sample kernel timestamp in one
> of the etm structs, so name all elements appropriately to
> avoid confusion.
> 
> This is also removes some ambiguity arising from the fact
> that the --timestamp argument to perf record refers to
> sample kernel timestamps, and the /timestamp/ event modifier
> refers to etm timestamps, so the term is overloaded.
> 
> Signed-off-by: James Clark 
> ---
>  .../perf/util/cs-etm-decoder/cs-etm-decoder.c | 18 
>  tools/perf/util/cs-etm.c  | 42 +--
>  tools/perf/util/cs-etm.h  |  4 +-
>  3 files changed, 31 insertions(+), 33 deletions(-)
> 
> diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c 
> b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
> index 059bcec3f651..055cb93eca59 100644
> --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
> +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
> @@ -276,13 +276,13 @@ cs_etm_decoder__do_soft_timestamp(struct cs_etm_queue 
> *etmq,
> const uint8_t trace_chan_id)
>  {
>   /* No timestamp packet has been received, nothing to do */
> - if (!packet_queue->timestamp)
> + if (!packet_queue->etm_timestamp)
>   return OCSD_RESP_CONT;
>  
> - packet_queue->timestamp = packet_queue->next_timestamp;
> + packet_queue->etm_timestamp = packet_queue->next_etm_timestamp;
>  
>   /* Estimate the timestamp for the next range packet */
> - packet_queue->next_timestamp += packet_queue->instr_count;
> + packet_queue->next_etm_timestamp += packet_queue->instr_count;
>   packet_queue->instr_count = 0;
>  
>   /* Tell the front end which traceid_queue needs attention */
> @@ -308,8 +308,8 @@ cs_etm_decoder__do_hard_timestamp(struct cs_etm_queue 
> *etmq,
>* Function do_soft_timestamp() will report the value to the front end,
>* hence asking the decoder to keep decoding rather than stopping.
>*/
> - if (packet_queue->timestamp) {
> - packet_queue->next_timestamp = elem->timestamp;
> + if (packet_queue->etm_timestamp) {
> + packet_queue->next_etm_timestamp = elem->timestamp;
>   return OCSD_RESP_CONT;
>   }
>  
> @@ -320,8 +320,8 @@ cs_etm_decoder__do_hard_timestamp(struct cs_etm_queue 
> *etmq,
>* which instructions started by subtracting the number of instructions
>* executed to the timestamp.
>*/
> - packet_queue->timestamp = elem->timestamp - packet_queue->instr_count;
> - packet_queue->next_timestamp = elem->timestamp;
> + packet_queue->etm_timestamp = elem->timestamp - 
> packet_queue->instr_count;
> + packet_queue->next_etm_timestamp = elem->timestamp;
>   packet_queue->instr_count = 0;
>  
>   /* Tell the front end which traceid_queue needs attention */
> @@ -334,8 +334,8 @@ cs_etm_decoder__do_hard_timestamp(struct cs_etm_queue 
> *etmq,
>  static void
>  cs_etm_decoder__reset_timestamp(struct cs_etm_packet_queue *packet_queue)
>  {
> - packet_queue->timestamp = 0;
> - packet_queue->next_timestamp = 0;
> + packet_queue->etm_timestamp = 0;
> + packet_queue->next_etm_timestamp = 0;
>   packet_queue->instr_count = 0;
>  }
>  
> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 7e63e7dedc33..c25da2ffa8f3 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -38,8 +38,6 @@
>  #include 
>  #include "util/synthetic-events.h"
>  
> -#define MAX_TIMESTAMP (~0ULL)
> -
>  struct cs_etm_auxtrace {
>   struct auxtrace auxtrace;
>   struct auxtrace_queues queues;
> @@ -86,7 +84,7 @@ struct cs_etm_queue {
>   struct cs_etm_decoder *decoder;
>   struct auxtrace_buffer *buffer;
>   unsigned int queue_nr;
> - u8 pending_timestamp;
> + u8 pending_timestamp_chan_id;
>   u64 offset;
>   const unsigned char *buf;
>   size_t buf_len, buf_used;
> @@ -208,7 +206,7 @@ void cs_etm__etmq_set_traceid_queue_timestamp(struct 
> cs_etm_queue *etmq,
>* be more than one channel per cs_etm_queue, we need to specify
>* what traceID queue needs servicing.
>*/
> - etmq->pending_timestamp = trace_chan_id;
> + etmq->pending_timestamp_chan_id = trace_chan_id;
>  }
>  
>  static u64 cs_etm__etmq_get_timestamp(struct cs_etm_queue *etmq,
> @@ -216,22 +214,22 @@ static u64 cs_etm__etmq_get_timestamp(struct 
> cs_etm_queue *etmq,
>  {
>   struct cs_etm_packet_queue *packet_queue;
>  
> - if (!etmq->pending_timestamp)
> + if (!etmq->pending_timestamp_chan_id)
>   return 0;
>  
>   if (trace_chan_id)
> - *trace_chan_id = etmq->pending_timestamp;
> + *trace_chan_id = etmq->pending_timestamp_chan_id;
>  
>   packet_queue = cs_etm__etmq_get_packet_queue(etmq,
> - 

[PATCH 1/2] perf cs-etm: Refactor timestamp variable names

2021-04-14 Thread James Clark
Remove ambiguity in variable names relating to timestamps.
A later commit will save the sample kernel timestamp in one
of the etm structs, so name all elements appropriately to
avoid confusion.

This is also removes some ambiguity arising from the fact
that the --timestamp argument to perf record refers to
sample kernel timestamps, and the /timestamp/ event modifier
refers to etm timestamps, so the term is overloaded.

Signed-off-by: James Clark 
---
 .../perf/util/cs-etm-decoder/cs-etm-decoder.c | 18 
 tools/perf/util/cs-etm.c  | 42 +--
 tools/perf/util/cs-etm.h  |  4 +-
 3 files changed, 31 insertions(+), 33 deletions(-)

diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c 
b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
index 059bcec3f651..055cb93eca59 100644
--- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
+++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
@@ -276,13 +276,13 @@ cs_etm_decoder__do_soft_timestamp(struct cs_etm_queue 
*etmq,
  const uint8_t trace_chan_id)
 {
/* No timestamp packet has been received, nothing to do */
-   if (!packet_queue->timestamp)
+   if (!packet_queue->etm_timestamp)
return OCSD_RESP_CONT;
 
-   packet_queue->timestamp = packet_queue->next_timestamp;
+   packet_queue->etm_timestamp = packet_queue->next_etm_timestamp;
 
/* Estimate the timestamp for the next range packet */
-   packet_queue->next_timestamp += packet_queue->instr_count;
+   packet_queue->next_etm_timestamp += packet_queue->instr_count;
packet_queue->instr_count = 0;
 
/* Tell the front end which traceid_queue needs attention */
@@ -308,8 +308,8 @@ cs_etm_decoder__do_hard_timestamp(struct cs_etm_queue *etmq,
 * Function do_soft_timestamp() will report the value to the front end,
 * hence asking the decoder to keep decoding rather than stopping.
 */
-   if (packet_queue->timestamp) {
-   packet_queue->next_timestamp = elem->timestamp;
+   if (packet_queue->etm_timestamp) {
+   packet_queue->next_etm_timestamp = elem->timestamp;
return OCSD_RESP_CONT;
}
 
@@ -320,8 +320,8 @@ cs_etm_decoder__do_hard_timestamp(struct cs_etm_queue *etmq,
 * which instructions started by subtracting the number of instructions
 * executed to the timestamp.
 */
-   packet_queue->timestamp = elem->timestamp - packet_queue->instr_count;
-   packet_queue->next_timestamp = elem->timestamp;
+   packet_queue->etm_timestamp = elem->timestamp - 
packet_queue->instr_count;
+   packet_queue->next_etm_timestamp = elem->timestamp;
packet_queue->instr_count = 0;
 
/* Tell the front end which traceid_queue needs attention */
@@ -334,8 +334,8 @@ cs_etm_decoder__do_hard_timestamp(struct cs_etm_queue *etmq,
 static void
 cs_etm_decoder__reset_timestamp(struct cs_etm_packet_queue *packet_queue)
 {
-   packet_queue->timestamp = 0;
-   packet_queue->next_timestamp = 0;
+   packet_queue->etm_timestamp = 0;
+   packet_queue->next_etm_timestamp = 0;
packet_queue->instr_count = 0;
 }
 
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 7e63e7dedc33..c25da2ffa8f3 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -38,8 +38,6 @@
 #include 
 #include "util/synthetic-events.h"
 
-#define MAX_TIMESTAMP (~0ULL)
-
 struct cs_etm_auxtrace {
struct auxtrace auxtrace;
struct auxtrace_queues queues;
@@ -86,7 +84,7 @@ struct cs_etm_queue {
struct cs_etm_decoder *decoder;
struct auxtrace_buffer *buffer;
unsigned int queue_nr;
-   u8 pending_timestamp;
+   u8 pending_timestamp_chan_id;
u64 offset;
const unsigned char *buf;
size_t buf_len, buf_used;
@@ -208,7 +206,7 @@ void cs_etm__etmq_set_traceid_queue_timestamp(struct 
cs_etm_queue *etmq,
 * be more than one channel per cs_etm_queue, we need to specify
 * what traceID queue needs servicing.
 */
-   etmq->pending_timestamp = trace_chan_id;
+   etmq->pending_timestamp_chan_id = trace_chan_id;
 }
 
 static u64 cs_etm__etmq_get_timestamp(struct cs_etm_queue *etmq,
@@ -216,22 +214,22 @@ static u64 cs_etm__etmq_get_timestamp(struct cs_etm_queue 
*etmq,
 {
struct cs_etm_packet_queue *packet_queue;
 
-   if (!etmq->pending_timestamp)
+   if (!etmq->pending_timestamp_chan_id)
return 0;
 
if (trace_chan_id)
-   *trace_chan_id = etmq->pending_timestamp;
+   *trace_chan_id = etmq->pending_timestamp_chan_id;
 
packet_queue = cs_etm__etmq_get_packet_queue(etmq,
-etmq->pending_timestamp);
+
etmq->pending_timestamp_chan_id);
if (!packet_queue)
return