[PATCH v2 1/2] perf cs-etm: Support unknown_thread in cs_etm_auxtrace

[PATCH v2 1/2] perf cs-etm: Support unknown_thread in cs_etm_auxtrace

[Leo Yan]

CoreSight doesn't allocate thread structure for unknown_thread in etm
auxtrace, so unknown_thread is NULL pointer.  If the perf data doesn't
contain valid tid and then cs_etm__mem_access() uses unknown_thread
instead as thread handler, this results in segmentation fault when
thread__find_addr_map() accesses thread handler.

This commit creates new thread data which is used by unknown_thread, so
CoreSight tracing can roll back to use unknown_thread if perf data
doesn't include valid thread info.  This commit also releases thread
data for initialization failure case and for normal auxtrace free flow.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
Acked-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 tools/perf/util/cs-etm.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 6533b1a..69f21de 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -239,6 +239,7 @@ static void cs_etm__free(struct perf_session *session)
 	for (i = 0; i < aux->num_cpu; i++)
 		zfree(&aux->metadata[i]);
 
+	thread__zput(aux->unknown_thread);
 	zfree(&aux->metadata);
 	zfree(&aux);
 }
@@ -1355,6 +1356,23 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
 	etm->auxtrace.free = cs_etm__free;
 	session->auxtrace = &etm->auxtrace;
 
+	etm->unknown_thread = thread__new(999999999, 999999999);
+	if (!etm->unknown_thread)
+		goto err_free_queues;
+
+	/*
+	 * Initialize list node so that at thread__zput() we can avoid
+	 * segmentation fault at list_del_init().
+	 */
+	INIT_LIST_HEAD(&etm->unknown_thread->node);
+
+	err = thread__set_comm(etm->unknown_thread, "unknown", 0);
+	if (err)
+		goto err_delete_thread;
+
+	if (thread__init_map_groups(etm->unknown_thread, etm->machine))
+		goto err_delete_thread;
+
 	if (dump_trace) {
 		cs_etm__print_auxtrace_info(auxtrace_info->priv, num_cpu);
 		return 0;
@@ -1369,16 +1387,18 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
 
 	err = cs_etm__synth_events(etm, session);
 	if (err)
-		goto err_free_queues;
+		goto err_delete_thread;
 
 	err = auxtrace_queues__process_index(&etm->queues, session);
 	if (err)
-		goto err_free_queues;
+		goto err_delete_thread;
 
 	etm->data_queued = etm->queues.populated;
 
 	return 0;
 
+err_delete_thread:
+	thread__zput(etm->unknown_thread);
 err_free_queues:
 	auxtrace_queues__free(&etm->queues);
 	session->auxtrace = NULL;
-- 
2.7.4

[PATCH v2 1/2] perf cs-etm: Support unknown_thread in cs_etm_auxtrace

[Leo Yan]

CoreSight doesn't allocate thread structure for unknown_thread in etm
auxtrace, so unknown_thread is NULL pointer.  If the perf data doesn't
contain valid tid and then cs_etm__mem_access() uses unknown_thread
instead as thread handler, this results in segmentation fault when
thread__find_addr_map() accesses thread handler.

This commit creates new thread data which is used by unknown_thread, so
CoreSight tracing can roll back to use unknown_thread if perf data
doesn't include valid thread info.  This commit also releases thread
data for initialization failure case and for normal auxtrace free flow.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
Acked-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 tools/perf/util/cs-etm.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 6533b1a..69f21de 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -239,6 +239,7 @@ static void cs_etm__free(struct perf_session *session)
 	for (i = 0; i < aux->num_cpu; i++)
 		zfree(&aux->metadata[i]);
 
+	thread__zput(aux->unknown_thread);
 	zfree(&aux->metadata);
 	zfree(&aux);
 }
@@ -1355,6 +1356,23 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
 	etm->auxtrace.free = cs_etm__free;
 	session->auxtrace = &etm->auxtrace;
 
+	etm->unknown_thread = thread__new(999999999, 999999999);
+	if (!etm->unknown_thread)
+		goto err_free_queues;
+
+	/*
+	 * Initialize list node so that at thread__zput() we can avoid
+	 * segmentation fault at list_del_init().
+	 */
+	INIT_LIST_HEAD(&etm->unknown_thread->node);
+
+	err = thread__set_comm(etm->unknown_thread, "unknown", 0);
+	if (err)
+		goto err_delete_thread;
+
+	if (thread__init_map_groups(etm->unknown_thread, etm->machine))
+		goto err_delete_thread;
+
 	if (dump_trace) {
 		cs_etm__print_auxtrace_info(auxtrace_info->priv, num_cpu);
 		return 0;
@@ -1369,16 +1387,18 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
 
 	err = cs_etm__synth_events(etm, session);
 	if (err)
-		goto err_free_queues;
+		goto err_delete_thread;
 
 	err = auxtrace_queues__process_index(&etm->queues, session);
 	if (err)
-		goto err_free_queues;
+		goto err_delete_thread;
 
 	etm->data_queued = etm->queues.populated;
 
 	return 0;
 
+err_delete_thread:
+	thread__zput(etm->unknown_thread);
 err_free_queues:
 	auxtrace_queues__free(&etm->queues);
 	session->auxtrace = NULL;
-- 
2.7.4

[PATCH v2 2/2] perf cs-etm: Remove redundant space

[Leo Yan]

There have two spaces ahead function name cs_etm__set_pid_tid_cpu(), so
remove one space and correct indentation.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
Acked-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 tools/perf/util/cs-etm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 69f21de..822ba91 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -611,8 +611,8 @@ cs_etm__get_trace(struct cs_etm_buffer *buff, struct cs_etm_queue *etmq)
 	return buff->len;
 }
 
-static void  cs_etm__set_pid_tid_cpu(struct cs_etm_auxtrace *etm,
-				     struct auxtrace_queue *queue)
+static void cs_etm__set_pid_tid_cpu(struct cs_etm_auxtrace *etm,
+				    struct auxtrace_queue *queue)
 {
 	struct cs_etm_queue *etmq = queue->priv;
 
-- 
2.7.4

[PATCH v2 2/2] perf cs-etm: Remove redundant space

[Leo Yan]

There have two spaces ahead function name cs_etm__set_pid_tid_cpu(), so
remove one space and correct indentation.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
Acked-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 tools/perf/util/cs-etm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 69f21de..822ba91 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -611,8 +611,8 @@ cs_etm__get_trace(struct cs_etm_buffer *buff, struct cs_etm_queue *etmq)
 	return buff->len;
 }
 
-static void  cs_etm__set_pid_tid_cpu(struct cs_etm_auxtrace *etm,
-				     struct auxtrace_queue *queue)
+static void cs_etm__set_pid_tid_cpu(struct cs_etm_auxtrace *etm,
+				    struct auxtrace_queue *queue)
 {
 	struct cs_etm_queue *etmq = queue->priv;
 
-- 
2.7.4

Re: [PATCH v2 1/2] perf cs-etm: Support unknown_thread in cs_etm_auxtrace

[Mathieu Poirier]

On Wed, May 09, 2018 at 12:15:11PM +0800, Leo Yan wrote:
> CoreSight doesn't allocate thread structure for unknown_thread in etm
> auxtrace, so unknown_thread is NULL pointer.  If the perf data doesn't
> contain valid tid and then cs_etm__mem_access() uses unknown_thread
> instead as thread handler, this results in segmentation fault when
> thread__find_addr_map() accesses thread handler.
> 
> This commit creates new thread data which is used by unknown_thread, so
> CoreSight tracing can roll back to use unknown_thread if perf data
> doesn't include valid thread info.  This commit also releases thread
> data for initialization failure case and for normal auxtrace free flow.

Hi Leo,

Arnaldo maintains the perf tools and he receives dozens of patches every day.
If his name is not the first one to appear on the "To:" list, it is unlikely
that his email filters will catch your patches.

Mathieu

> 
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> Acked-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> ---
>  tools/perf/util/cs-etm.c | 24 ++++++++++++++++++++++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 6533b1a..69f21de 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -239,6 +239,7 @@ static void cs_etm__free(struct perf_session *session)
>  	for (i = 0; i < aux->num_cpu; i++)
>  		zfree(&aux->metadata[i]);
>  
> +	thread__zput(aux->unknown_thread);
>  	zfree(&aux->metadata);
>  	zfree(&aux);
>  }
> @@ -1355,6 +1356,23 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
>  	etm->auxtrace.free = cs_etm__free;
>  	session->auxtrace = &etm->auxtrace;
>  
> +	etm->unknown_thread = thread__new(999999999, 999999999);
> +	if (!etm->unknown_thread)
> +		goto err_free_queues;
> +
> +	/*
> +	 * Initialize list node so that at thread__zput() we can avoid
> +	 * segmentation fault at list_del_init().
> +	 */
> +	INIT_LIST_HEAD(&etm->unknown_thread->node);
> +
> +	err = thread__set_comm(etm->unknown_thread, "unknown", 0);
> +	if (err)
> +		goto err_delete_thread;
> +
> +	if (thread__init_map_groups(etm->unknown_thread, etm->machine))
> +		goto err_delete_thread;
> +
>  	if (dump_trace) {
>  		cs_etm__print_auxtrace_info(auxtrace_info->priv, num_cpu);
>  		return 0;
> @@ -1369,16 +1387,18 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
>  
>  	err = cs_etm__synth_events(etm, session);
>  	if (err)
> -		goto err_free_queues;
> +		goto err_delete_thread;
>  
>  	err = auxtrace_queues__process_index(&etm->queues, session);
>  	if (err)
> -		goto err_free_queues;
> +		goto err_delete_thread;
>  
>  	etm->data_queued = etm->queues.populated;
>  
>  	return 0;
>  
> +err_delete_thread:
> +	thread__zput(etm->unknown_thread);
>  err_free_queues:
>  	auxtrace_queues__free(&etm->queues);
>  	session->auxtrace = NULL;
> -- 
> 2.7.4
> 

[PATCH v2 1/2] perf cs-etm: Support unknown_thread in cs_etm_auxtrace

[Mathieu Poirier]

On Wed, May 09, 2018 at 12:15:11PM +0800, Leo Yan wrote:
> CoreSight doesn't allocate thread structure for unknown_thread in etm
> auxtrace, so unknown_thread is NULL pointer.  If the perf data doesn't
> contain valid tid and then cs_etm__mem_access() uses unknown_thread
> instead as thread handler, this results in segmentation fault when
> thread__find_addr_map() accesses thread handler.
> 
> This commit creates new thread data which is used by unknown_thread, so
> CoreSight tracing can roll back to use unknown_thread if perf data
> doesn't include valid thread info.  This commit also releases thread
> data for initialization failure case and for normal auxtrace free flow.

Hi Leo,

Arnaldo maintains the perf tools and he receives dozens of patches every day.
If his name is not the first one to appear on the "To:" list, it is unlikely
that his email filters will catch your patches.

Mathieu

> 
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> Acked-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> ---
>  tools/perf/util/cs-etm.c | 24 ++++++++++++++++++++++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 6533b1a..69f21de 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -239,6 +239,7 @@ static void cs_etm__free(struct perf_session *session)
>  	for (i = 0; i < aux->num_cpu; i++)
>  		zfree(&aux->metadata[i]);
>  
> +	thread__zput(aux->unknown_thread);
>  	zfree(&aux->metadata);
>  	zfree(&aux);
>  }
> @@ -1355,6 +1356,23 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
>  	etm->auxtrace.free = cs_etm__free;
>  	session->auxtrace = &etm->auxtrace;
>  
> +	etm->unknown_thread = thread__new(999999999, 999999999);
> +	if (!etm->unknown_thread)
> +		goto err_free_queues;
> +
> +	/*
> +	 * Initialize list node so that at thread__zput() we can avoid
> +	 * segmentation fault at list_del_init().
> +	 */
> +	INIT_LIST_HEAD(&etm->unknown_thread->node);
> +
> +	err = thread__set_comm(etm->unknown_thread, "unknown", 0);
> +	if (err)
> +		goto err_delete_thread;
> +
> +	if (thread__init_map_groups(etm->unknown_thread, etm->machine))
> +		goto err_delete_thread;
> +
>  	if (dump_trace) {
>  		cs_etm__print_auxtrace_info(auxtrace_info->priv, num_cpu);
>  		return 0;
> @@ -1369,16 +1387,18 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
>  
>  	err = cs_etm__synth_events(etm, session);
>  	if (err)
> -		goto err_free_queues;
> +		goto err_delete_thread;
>  
>  	err = auxtrace_queues__process_index(&etm->queues, session);
>  	if (err)
> -		goto err_free_queues;
> +		goto err_delete_thread;
>  
>  	etm->data_queued = etm->queues.populated;
>  
>  	return 0;
>  
> +err_delete_thread:
> +	thread__zput(etm->unknown_thread);
>  err_free_queues:
>  	auxtrace_queues__free(&etm->queues);
>  	session->auxtrace = NULL;
> -- 
> 2.7.4
> 

Re: [PATCH v2 1/2] perf cs-etm: Support unknown_thread in cs_etm_auxtrace

[Leo Yan]

On Wed, May 09, 2018 at 09:51:50AM -0600, Mathieu Poirier wrote:
> On Wed, May 09, 2018 at 12:15:11PM +0800, Leo Yan wrote:
> > CoreSight doesn't allocate thread structure for unknown_thread in etm
> > auxtrace, so unknown_thread is NULL pointer.  If the perf data doesn't
> > contain valid tid and then cs_etm__mem_access() uses unknown_thread
> > instead as thread handler, this results in segmentation fault when
> > thread__find_addr_map() accesses thread handler.
> > 
> > This commit creates new thread data which is used by unknown_thread, so
> > CoreSight tracing can roll back to use unknown_thread if perf data
> > doesn't include valid thread info.  This commit also releases thread
> > data for initialization failure case and for normal auxtrace free flow.
> 
> Hi Leo,
> 
> Arnaldo maintains the perf tools and he receives dozens of patches every day.
> If his name is not the first one to appear on the "To:" list, it is unlikely
> that his email filters will catch your patches.

Thanks for reminding, Mathieu.  Will resend patches for this.

Thanks,
Leo Yan

[PATCH v2 1/2] perf cs-etm: Support unknown_thread in cs_etm_auxtrace

[Leo Yan]

On Wed, May 09, 2018 at 09:51:50AM -0600, Mathieu Poirier wrote:
> On Wed, May 09, 2018 at 12:15:11PM +0800, Leo Yan wrote:
> > CoreSight doesn't allocate thread structure for unknown_thread in etm
> > auxtrace, so unknown_thread is NULL pointer.  If the perf data doesn't
> > contain valid tid and then cs_etm__mem_access() uses unknown_thread
> > instead as thread handler, this results in segmentation fault when
> > thread__find_addr_map() accesses thread handler.
> > 
> > This commit creates new thread data which is used by unknown_thread, so
> > CoreSight tracing can roll back to use unknown_thread if perf data
> > doesn't include valid thread info.  This commit also releases thread
> > data for initialization failure case and for normal auxtrace free flow.
> 
> Hi Leo,
> 
> Arnaldo maintains the perf tools and he receives dozens of patches every day.
> If his name is not the first one to appear on the "To:" list, it is unlikely
> that his email filters will catch your patches.

Thanks for reminding, Mathieu.  Will resend patches for this.

Thanks,
Leo Yan