[PATCH 1/3] perf record: Allocate area for sample_id_hdr in a synthesized comm event

[PATCH 1/3] perf record: Allocate area for sample_id_hdr in a synthesized comm event

[Namhyung Kim]

A previous patch added a synthesized comm event for forked child
process but it missed that the event should contain area for
sample_id_hdr at the end.  It worked by accident since the perf_event
union contains bigger event structs like mmap_events.

This patch fixes it by dynamically allocating event struct including
those area like in perf_event__synthesize_thread_map().

Reported-by: Arnaldo Carvalho de Melo <acme@kernel.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-record.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index a01c8ae1ee07..5e01c070dbf2 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -637,17 +637,25 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 	 * Let the child rip
 	 */
 	if (forks) {
-		union perf_event event;
+		union perf_event *event;
+
+		event = malloc(sizeof(event->comm) + machine->id_hdr_size);
+		if (event == NULL) {
+			err = -ENOMEM;
+			goto out_child;
+		}
+
 		/*
 		 * Some H/W events are generated before COMM event
 		 * which is emitted during exec(), so perf script
 		 * cannot see a correct process name for those events.
 		 * Synthesize COMM event to prevent it.
 		 */
-		perf_event__synthesize_comm(tool, &event,
+		perf_event__synthesize_comm(tool, event,
 					    rec->evlist->workload.pid,
 					    process_synthesized_event,
 					    machine);
+		free(event);
 
 		perf_evlist__start_workload(rec->evlist);
 	}
-- 
2.5.0

[PATCH 2/3] perf top: Fix unresolved comm when -s comm is used

[Namhyung Kim]

The perf top uses 'dso,symbol' sort keys by default so it overlooked a
problem in task's comm resolving.  When the sort key contains 'comm',
some task's comm is not shown properly.  This is because the
perf_top__mmap_read_idx() checks the cpumode value improperly.

The cpumode value of non-sample events are 0 (PERF_RECORD_MISC_CPUMODE_
UNKNOWN) so the events will be ignored by the switch statement.  This patch
allows it for non-sample events.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-top.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 38d4d6cac823..ae4c6420300b 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -857,9 +857,12 @@ static void perf_top__mmap_read_idx(struct perf_top *top, int idx)
 			 * TODO: we don't process guest user from host side
 			 * except simple counting.
 			 */
-			/* Fall thru */
-		default:
 			goto next_event;
+		default:
+			if (event->header.type == PERF_RECORD_SAMPLE)
+				goto next_event;
+			machine = &session->machines.host;
+			break;
 		}
 
 
-- 
2.5.0

[PATCH 3/3] perf top: Register idle thread

[Namhyung Kim]

The perf top didn't add the idle/swapper thread to the machine's
thread list and its comm was displayed as ':0'.  Fix it.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-top.c  | 3 +++
 tools/perf/util/session.c | 2 +-
 tools/perf/util/session.h | 2 ++
 3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index ae4c6420300b..6f641fd68296 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -964,6 +964,9 @@ static int __cmd_top(struct perf_top *top)
 	if (ret)
 		goto out_delete;
 
+	if (perf_session__register_idle_thread(top->session) == NULL)
+		goto out_delete;
+
 	machine__synthesize_threads(&top->session->machines.host, &opts->target,
 				    top->evlist->threads, false, opts->proc_map_timeout);
 
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 84a02eae4394..428149bc64d2 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1311,7 +1311,7 @@ struct thread *perf_session__findnew(struct perf_session *session, pid_t pid)
 	return machine__findnew_thread(&session->machines.host, -1, pid);
 }
 
-static struct thread *perf_session__register_idle_thread(struct perf_session *session)
+struct thread *perf_session__register_idle_thread(struct perf_session *session)
 {
 	struct thread *thread;
 
diff --git a/tools/perf/util/session.h b/tools/perf/util/session.h
index b44afc75d1cc..3e900c0efc73 100644
--- a/tools/perf/util/session.h
+++ b/tools/perf/util/session.h
@@ -89,6 +89,8 @@ struct machine *perf_session__findnew_machine(struct perf_session *session, pid_
 }
 
 struct thread *perf_session__findnew(struct perf_session *session, pid_t pid);
+struct thread *perf_session__register_idle_thread(struct perf_session *session);
+
 size_t perf_session__fprintf(struct perf_session *session, FILE *fp);
 
 size_t perf_session__fprintf_dsos(struct perf_session *session, FILE *fp);
-- 
2.5.0

Re: [PATCH 1/3] perf record: Allocate area for sample_id_hdr in a synthesized comm event

[Arnaldo Carvalho de Melo]

Em Wed, Sep 30, 2015 at 10:45:24AM +0900, Namhyung Kim escreveu:
> A previous patch added a synthesized comm event for forked child
> process but it missed that the event should contain area for
> sample_id_hdr at the end.  It worked by accident since the perf_event
> union contains bigger event structs like mmap_events.
> 
> This patch fixes it by dynamically allocating event struct including
> those area like in perf_event__synthesize_thread_map().

Thanks, applied the three patches in this series.

- Arnaldo

[tip:perf/core] perf record: Allocate area for sample_id_hdr in a synthesized comm event

[tip-bot for Namhyung Kim]

Commit-ID:  e5bed564485b340d87f2d8945643a393e55b8225
Gitweb:     http://git.kernel.org/tip/e5bed564485b340d87f2d8945643a393e55b8225
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Wed, 30 Sep 2015 10:45:24 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 1 Oct 2015 09:54:33 -0300

perf record: Allocate area for sample_id_hdr in a synthesized comm event

A previous patch added a synthesized comm event for forked child process
but it missed that the event should contain area for sample_id_hdr at
the end.  It worked by accident since the perf_event union contains
bigger event structs like mmap_events.

This patch fixes it by dynamically allocating event struct including
those area like in perf_event__synthesize_thread_map().

Reported-by: Arnaldo Carvalho de Melo <acme@kernel.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1443577526-3240-1-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-record.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index a01c8ae..5e01c07 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -637,17 +637,25 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 	 * Let the child rip
 	 */
 	if (forks) {
-		union perf_event event;
+		union perf_event *event;
+
+		event = malloc(sizeof(event->comm) + machine->id_hdr_size);
+		if (event == NULL) {
+			err = -ENOMEM;
+			goto out_child;
+		}
+
 		/*
 		 * Some H/W events are generated before COMM event
 		 * which is emitted during exec(), so perf script
 		 * cannot see a correct process name for those events.
 		 * Synthesize COMM event to prevent it.
 		 */
-		perf_event__synthesize_comm(tool, &event,
+		perf_event__synthesize_comm(tool, event,
 					    rec->evlist->workload.pid,
 					    process_synthesized_event,
 					    machine);
+		free(event);
 
 		perf_evlist__start_workload(rec->evlist);
 	}

[tip:perf/core] perf top: Fix unresolved comm when -s comm is used

[tip-bot for Namhyung Kim]

Commit-ID:  4b37af595742977f1bdd8c0fd0f3e6e55b36e7b7
Gitweb:     http://git.kernel.org/tip/4b37af595742977f1bdd8c0fd0f3e6e55b36e7b7
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Wed, 30 Sep 2015 10:45:25 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 1 Oct 2015 09:54:33 -0300

perf top: Fix unresolved comm when -s comm is used

The perf top uses 'dso,symbol' sort keys by default so it overlooked a
problem in task's comm resolving.  When the sort key contains 'comm',
some task's comm is not shown properly.  This is because the
perf_top__mmap_read_idx() checks the cpumode value improperly.

The cpumode value of non-sample events are 0 (PERF_RECORD_MISC_CPUMODE_
UNKNOWN) so the events will be ignored by the switch statement.  This patch
allows it for non-sample events.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1443577526-3240-2-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-top.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 38d4d6c..ae4c642 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -857,9 +857,12 @@ static void perf_top__mmap_read_idx(struct perf_top *top, int idx)
 			 * TODO: we don't process guest user from host side
 			 * except simple counting.
 			 */
-			/* Fall thru */
-		default:
 			goto next_event;
+		default:
+			if (event->header.type == PERF_RECORD_SAMPLE)
+				goto next_event;
+			machine = &session->machines.host;
+			break;
 		}
 
 

[tip:perf/core] perf top: Register idle thread

[tip-bot for Namhyung Kim]

Commit-ID:  c53d138d41a7f33cf085762c64b4b61e8d223e1c
Gitweb:     http://git.kernel.org/tip/c53d138d41a7f33cf085762c64b4b61e8d223e1c
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Wed, 30 Sep 2015 10:45:26 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 1 Oct 2015 09:54:33 -0300

perf top: Register idle thread

The perf top didn't add the idle/swapper thread to the machine's thread
list and its comm was displayed as ':0'.  Fix it.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1443577526-3240-3-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-top.c  | 3 +++
 tools/perf/util/session.c | 2 +-
 tools/perf/util/session.h | 2 ++
 3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index ae4c642..6f641fd 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -964,6 +964,9 @@ static int __cmd_top(struct perf_top *top)
 	if (ret)
 		goto out_delete;
 
+	if (perf_session__register_idle_thread(top->session) == NULL)
+		goto out_delete;
+
 	machine__synthesize_threads(&top->session->machines.host, &opts->target,
 				    top->evlist->threads, false, opts->proc_map_timeout);
 
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 84a02eae..428149b 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1311,7 +1311,7 @@ struct thread *perf_session__findnew(struct perf_session *session, pid_t pid)
 	return machine__findnew_thread(&session->machines.host, -1, pid);
 }
 
-static struct thread *perf_session__register_idle_thread(struct perf_session *session)
+struct thread *perf_session__register_idle_thread(struct perf_session *session)
 {
 	struct thread *thread;
 
diff --git a/tools/perf/util/session.h b/tools/perf/util/session.h
index b44afc7..3e900c0 100644
--- a/tools/perf/util/session.h
+++ b/tools/perf/util/session.h
@@ -89,6 +89,8 @@ struct machine *perf_session__findnew_machine(struct perf_session *session, pid_
 }
 
 struct thread *perf_session__findnew(struct perf_session *session, pid_t pid);
+struct thread *perf_session__register_idle_thread(struct perf_session *session);
+
 size_t perf_session__fprintf(struct perf_session *session, FILE *fp);
 
 size_t perf_session__fprintf_dsos(struct perf_session *session, FILE *fp);