[RFC] perf_sample_id::idx

[RFC] perf_sample_id::idx

[Jiri Olsa]

hi,
what's the perf_sample_id::idx for? It was added in here:
  3c659eedada2 perf tools: Add id index

but I dont see any practical usage of it in the sources,
when I remove it like below, I get clean build

any idea?

thanks,
jirka


---
diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
index 70841d115349..24b90f68d616 100644
--- a/tools/perf/util/event.h
+++ b/tools/perf/util/event.h
@@ -498,7 +498,7 @@ struct tracing_data_event {
 
 struct id_index_entry {
 	u64 id;
-	u64 idx;
+	u64 idx; /* deprecated */
 	u64 cpu;
 	u64 tid;
 };
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index c4489a1ad6bc..e55133cacb64 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -519,11 +519,11 @@ int perf_evlist__id_add_fd(struct evlist *evlist,
 }
 
 static void perf_evlist__set_sid_idx(struct evlist *evlist,
-				     struct evsel *evsel, int idx, int cpu,
+				     struct evsel *evsel, int cpu,
 				     int thread)
 {
 	struct perf_sample_id *sid = SID(evsel, cpu, thread);
-	sid->idx = idx;
+
 	if (evlist->core.cpus && cpu >= 0)
 		sid->cpu = evlist->core.cpus->map[cpu];
 	else
@@ -795,8 +795,7 @@ static int perf_evlist__mmap_per_evsel(struct evlist *evlist, int idx,
 			if (perf_evlist__id_add_fd(evlist, evsel, cpu, thread,
 						   fd) < 0)
 				return -1;
-			perf_evlist__set_sid_idx(evlist, evsel, idx, cpu,
-						 thread);
+			perf_evlist__set_sid_idx(evlist, evsel, cpu, thread);
 		}
 	}
 
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 3cf35aa782b9..b9d864933d75 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -23,7 +23,6 @@ struct perf_sample_id {
 	struct hlist_node 	node;
 	u64		 	id;
 	struct evsel		*evsel;
-	int			idx;
 	int			cpu;
 	pid_t			tid;
 
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index b9fe71d11bf6..2642d60aa875 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -2394,7 +2394,6 @@ int perf_event__process_id_index(struct perf_session *session,
 		sid = perf_evlist__id2sid(evlist, e->id);
 		if (!sid)
 			return -ENOENT;
-		sid->idx = e->idx;
 		sid->cpu = e->cpu;
 		sid->tid = e->tid;
 	}
@@ -2454,7 +2453,7 @@ int perf_event__synthesize_id_index(struct perf_tool *tool,
 				return -ENOENT;
 			}
 
-			e->idx = sid->idx;
+			e->idx = -1;
 			e->cpu = sid->cpu;
 			e->tid = sid->tid;
 		}

RE: [RFC] perf_sample_id::idx

[Hunter, Adrian]

It will be used for AUX area sampling.  A sample will have AUX area data that will be queued for decoding, where there are separate queues for each CPU (per-cpu tracing) or task (per-thread tracing).  The sample ID can be used to lookup 'idx' which is effectively the queue number.

> -----Original Message-----
> From: Jiri Olsa [mailto:jolsa@redhat.com]
> Sent: Friday, August 9, 2019 12:28 PM
> To: Hunter, Adrian <adrian.hunter@intel.com>
> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>; Ingo Molnar
> <mingo@kernel.org>; Namhyung Kim <namhyung@kernel.org>; Alexander
> Shishkin <alexander.shishkin@linux.intel.com>; Peter Zijlstra
> <a.p.zijlstra@chello.nl>; Michael Petlan <mpetlan@redhat.com>; linux-
> kernel@vger.kernel.org
> Subject: [RFC] perf_sample_id::idx
> 
> hi,
> what's the perf_sample_id::idx for? It was added in here:
>   3c659eedada2 perf tools: Add id index
> 
> but I dont see any practical usage of it in the sources, when I remove it like
> below, I get clean build
> 
> any idea?
> 
> thanks,
> jirka
> 
> 
> ---
> diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h index
> 70841d115349..24b90f68d616 100644
> --- a/tools/perf/util/event.h
> +++ b/tools/perf/util/event.h
> @@ -498,7 +498,7 @@ struct tracing_data_event {
> 
>  struct id_index_entry {
>  	u64 id;
> -	u64 idx;
> +	u64 idx; /* deprecated */
>  	u64 cpu;
>  	u64 tid;
>  };
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c index
> c4489a1ad6bc..e55133cacb64 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -519,11 +519,11 @@ int perf_evlist__id_add_fd(struct evlist *evlist,  }
> 
>  static void perf_evlist__set_sid_idx(struct evlist *evlist,
> -				     struct evsel *evsel, int idx, int cpu,
> +				     struct evsel *evsel, int cpu,
>  				     int thread)
>  {
>  	struct perf_sample_id *sid = SID(evsel, cpu, thread);
> -	sid->idx = idx;
> +
>  	if (evlist->core.cpus && cpu >= 0)
>  		sid->cpu = evlist->core.cpus->map[cpu];
>  	else
> @@ -795,8 +795,7 @@ static int perf_evlist__mmap_per_evsel(struct evlist
> *evlist, int idx,
>  			if (perf_evlist__id_add_fd(evlist, evsel, cpu, thread,
>  						   fd) < 0)
>  				return -1;
> -			perf_evlist__set_sid_idx(evlist, evsel, idx, cpu,
> -						 thread);
> +			perf_evlist__set_sid_idx(evlist, evsel, cpu, thread);
>  		}
>  	}
> 
> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h index
> 3cf35aa782b9..b9d864933d75 100644
> --- a/tools/perf/util/evsel.h
> +++ b/tools/perf/util/evsel.h
> @@ -23,7 +23,6 @@ struct perf_sample_id {
>  	struct hlist_node 	node;
>  	u64		 	id;
>  	struct evsel		*evsel;
> -	int			idx;
>  	int			cpu;
>  	pid_t			tid;
> 
> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c index
> b9fe71d11bf6..2642d60aa875 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -2394,7 +2394,6 @@ int perf_event__process_id_index(struct
> perf_session *session,
>  		sid = perf_evlist__id2sid(evlist, e->id);
>  		if (!sid)
>  			return -ENOENT;
> -		sid->idx = e->idx;
>  		sid->cpu = e->cpu;
>  		sid->tid = e->tid;
>  	}
> @@ -2454,7 +2453,7 @@ int perf_event__synthesize_id_index(struct
> perf_tool *tool,
>  				return -ENOENT;
>  			}
> 
> -			e->idx = sid->idx;
> +			e->idx = -1;
>  			e->cpu = sid->cpu;
>  			e->tid = sid->tid;
>  		}

Re: [RFC] perf_sample_id::idx

[Arnaldo Carvalho de Melo]

Em Fri, Aug 09, 2019 at 03:20:14PM +0000, Hunter, Adrian escreveu:

> It will be used for AUX area sampling.  A sample will have AUX area
> data that will be queued for decoding, where there are separate queues
> for each CPU (per-cpu tracing) or task (per-thread tracing).  The
> sample ID can be used to lookup 'idx' which is effectively the queue
> number.

Would be good to have this as a comment in the perf_sample_id struct
definition :-)

- Arnaldo
 
> > -----Original Message-----
> > From: Jiri Olsa [mailto:jolsa@redhat.com]
> > Sent: Friday, August 9, 2019 12:28 PM
> > To: Hunter, Adrian <adrian.hunter@intel.com>
> > Cc: Arnaldo Carvalho de Melo <acme@kernel.org>; Ingo Molnar
> > <mingo@kernel.org>; Namhyung Kim <namhyung@kernel.org>; Alexander
> > Shishkin <alexander.shishkin@linux.intel.com>; Peter Zijlstra
> > <a.p.zijlstra@chello.nl>; Michael Petlan <mpetlan@redhat.com>; linux-
> > kernel@vger.kernel.org
> > Subject: [RFC] perf_sample_id::idx
> > 
> > hi,
> > what's the perf_sample_id::idx for? It was added in here:
> >   3c659eedada2 perf tools: Add id index
> > 
> > but I dont see any practical usage of it in the sources, when I remove it like
> > below, I get clean build
> > 
> > any idea?
> > 
> > thanks,
> > jirka
> > 
> > 
> > ---
> > diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h index
> > 70841d115349..24b90f68d616 100644
> > --- a/tools/perf/util/event.h
> > +++ b/tools/perf/util/event.h
> > @@ -498,7 +498,7 @@ struct tracing_data_event {
> > 
> >  struct id_index_entry {
> >  	u64 id;
> > -	u64 idx;
> > +	u64 idx; /* deprecated */
> >  	u64 cpu;
> >  	u64 tid;
> >  };
> > diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c index
> > c4489a1ad6bc..e55133cacb64 100644
> > --- a/tools/perf/util/evlist.c
> > +++ b/tools/perf/util/evlist.c
> > @@ -519,11 +519,11 @@ int perf_evlist__id_add_fd(struct evlist *evlist,  }
> > 
> >  static void perf_evlist__set_sid_idx(struct evlist *evlist,
> > -				     struct evsel *evsel, int idx, int cpu,
> > +				     struct evsel *evsel, int cpu,
> >  				     int thread)
> >  {
> >  	struct perf_sample_id *sid = SID(evsel, cpu, thread);
> > -	sid->idx = idx;
> > +
> >  	if (evlist->core.cpus && cpu >= 0)
> >  		sid->cpu = evlist->core.cpus->map[cpu];
> >  	else
> > @@ -795,8 +795,7 @@ static int perf_evlist__mmap_per_evsel(struct evlist
> > *evlist, int idx,
> >  			if (perf_evlist__id_add_fd(evlist, evsel, cpu, thread,
> >  						   fd) < 0)
> >  				return -1;
> > -			perf_evlist__set_sid_idx(evlist, evsel, idx, cpu,
> > -						 thread);
> > +			perf_evlist__set_sid_idx(evlist, evsel, cpu, thread);
> >  		}
> >  	}
> > 
> > diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h index
> > 3cf35aa782b9..b9d864933d75 100644
> > --- a/tools/perf/util/evsel.h
> > +++ b/tools/perf/util/evsel.h
> > @@ -23,7 +23,6 @@ struct perf_sample_id {
> >  	struct hlist_node 	node;
> >  	u64		 	id;
> >  	struct evsel		*evsel;
> > -	int			idx;
> >  	int			cpu;
> >  	pid_t			tid;
> > 
> > diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c index
> > b9fe71d11bf6..2642d60aa875 100644
> > --- a/tools/perf/util/session.c
> > +++ b/tools/perf/util/session.c
> > @@ -2394,7 +2394,6 @@ int perf_event__process_id_index(struct
> > perf_session *session,
> >  		sid = perf_evlist__id2sid(evlist, e->id);
> >  		if (!sid)
> >  			return -ENOENT;
> > -		sid->idx = e->idx;
> >  		sid->cpu = e->cpu;
> >  		sid->tid = e->tid;
> >  	}
> > @@ -2454,7 +2453,7 @@ int perf_event__synthesize_id_index(struct
> > perf_tool *tool,
> >  				return -ENOENT;
> >  			}
> > 
> > -			e->idx = sid->idx;
> > +			e->idx = -1;
> >  			e->cpu = sid->cpu;
> >  			e->tid = sid->tid;
> >  		}

-- 

- Arnaldo

Re: [RFC] perf_sample_id::idx

[Adrian Hunter]

On 9/08/19 7:04 PM, Arnaldo Carvalho de Melo wrote:
> Em Fri, Aug 09, 2019 at 03:20:14PM +0000, Hunter, Adrian escreveu:
> 
>> It will be used for AUX area sampling.  A sample will have AUX area
>> data that will be queued for decoding, where there are separate queues
>> for each CPU (per-cpu tracing) or task (per-thread tracing).  The
>> sample ID can be used to lookup 'idx' which is effectively the queue
>> number.
> 
> Would be good to have this as a comment in the perf_sample_id struct
> definition :-)


From 45d57bd7b25c9864f21e25534274ea461ff83d9d Mon Sep 17 00:00:00 2001
From: Adrian Hunter <adrian.hunter@intel.com>
Date: Mon, 12 Aug 2019 12:06:31 +0300
Subject: [PATCH] perf tools: Add comment for idx in struct perf_sample_id

'idx' was added as preparation for AUX area sampling. Add a comment to
describe why.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/evsel.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index cad54e8ba522..ba13eb771775 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -22,6 +22,13 @@ struct perf_sample_id {
 	struct hlist_node 	node;
 	u64		 	id;
 	struct perf_evsel	*evsel;
+	/*
+	 * 'idx' will be used for AUX area sampling. A sample will have AUX area
+	 * data that will be queued for decoding, where there are separate
+	 * queues for each CPU (per-cpu tracing) or task (per-thread tracing).
+	 * The sample ID can be used to lookup 'idx' which is effectively the
+	 * queue number.
+	 */
 	int			idx;
 	int			cpu;
 	pid_t			tid;
-- 
2.17.1

Re: [RFC] perf_sample_id::idx

[Arnaldo Carvalho de Melo]

Em Mon, Aug 12, 2019 at 12:09:35PM +0300, Adrian Hunter escreveu:
> On 9/08/19 7:04 PM, Arnaldo Carvalho de Melo wrote:
> > Em Fri, Aug 09, 2019 at 03:20:14PM +0000, Hunter, Adrian escreveu:
> > 
> >> It will be used for AUX area sampling.  A sample will have AUX area
> >> data that will be queued for decoding, where there are separate queues
> >> for each CPU (per-cpu tracing) or task (per-thread tracing).  The
> >> sample ID can be used to lookup 'idx' which is effectively the queue
> >> number.
> > 
> > Would be good to have this as a comment in the perf_sample_id struct
> > definition :-)

Thanks, applied to perf/core.
 
> 
> >From 45d57bd7b25c9864f21e25534274ea461ff83d9d Mon Sep 17 00:00:00 2001
> From: Adrian Hunter <adrian.hunter@intel.com>
> Date: Mon, 12 Aug 2019 12:06:31 +0300
> Subject: [PATCH] perf tools: Add comment for idx in struct perf_sample_id
> 
> 'idx' was added as preparation for AUX area sampling. Add a comment to
> describe why.
> 
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>  tools/perf/util/evsel.h | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> index cad54e8ba522..ba13eb771775 100644
> --- a/tools/perf/util/evsel.h
> +++ b/tools/perf/util/evsel.h
> @@ -22,6 +22,13 @@ struct perf_sample_id {
>  	struct hlist_node 	node;
>  	u64		 	id;
>  	struct perf_evsel	*evsel;
> +	/*
> +	 * 'idx' will be used for AUX area sampling. A sample will have AUX area
> +	 * data that will be queued for decoding, where there are separate
> +	 * queues for each CPU (per-cpu tracing) or task (per-thread tracing).
> +	 * The sample ID can be used to lookup 'idx' which is effectively the
> +	 * queue number.
> +	 */
>  	int			idx;
>  	int			cpu;
>  	pid_t			tid;
> -- 
> 2.17.1

-- 

- Arnaldo

[tip: perf/core] perf evsel: Add comment for 'idx' member in 'struct perf_sample_id

[tip-bot2 for Adrian Hunter]

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     3c84e65a533dbaa1a29bfd847deca73704b675eb
Gitweb:        https://git.kernel.org/tip/3c84e65a533dbaa1a29bfd847deca73704b675eb
Author:        Adrian Hunter <adrian.hunter@intel.com>
AuthorDate:    Mon, 12 Aug 2019 12:09:35 +03:00
Committer:     Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Tue, 20 Aug 2019 12:17:45 -03:00

perf evsel: Add comment for 'idx' member in 'struct perf_sample_id

The 'idx' member was added as preparation for AUX area sampling. Add a
comment to describe why.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Michael Petlan <mpetlan@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/83ff264f-84c3-5372-8976-dd9293d20c6f@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/evsel.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 9cd6e3a..efe0806 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -23,6 +23,13 @@ struct perf_sample_id {
 	struct hlist_node 	node;
 	u64		 	id;
 	struct evsel		*evsel;
+       /*
+	* 'idx' will be used for AUX area sampling. A sample will have AUX area
+	* data that will be queued for decoding, where there are separate
+	* queues for each CPU (per-cpu tracing) or task (per-thread tracing).
+	* The sample ID can be used to lookup 'idx' which is effectively the
+	* queue number.
+	*/
 	int			idx;
 	int			cpu;
 	pid_t			tid;