Just a small collection of fixes noticed while hacking up the c2c tool
Don Zickus (3):
perf, machine: Use map as success in ip__resolve_ams
perf, session: Change header.misc dump from decimal to hex
perf: fix synthesizing mmaps for threads
tools/perf/util/event.c | 15 +++++++++++----
tools/perf/util/machine.c | 6 +++---
tools/perf/util/session.c | 2 +-
3 files changed, 15 insertions(+), 8 deletions(-)
--
1.7.11.7
When trying to map a bunch of instruction addresses to their respective
threads, I kept getting a lot of bogus entries [I forget the exact reason
as I patched my code months ago].
Looking through ip__resovle_ams, I noticed the check for
if (al.sym)
and realized, most times I have an al.map definition but sometimes an
al.sym is undefined. In the cases where al.sym is undefined, the loop
keeps going even though a valid al.map exists.
Modify this check to use the more reliable al.map. This fixed my bogus
entries.
Signed-off-by: Don Zickus <[email protected]>
---
tools/perf/util/machine.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index ac37d78..813e94e 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -1213,7 +1213,7 @@ static void ip__resolve_ams(struct machine *machine, struct thread *thread,
*/
thread__find_addr_location(thread, machine, m, MAP__FUNCTION,
ip, &al);
- if (al.sym)
+ if (al.map)
goto found;
}
found:
--
1.7.11.7
When printing the raw dump of a data file, the header.misc is
printed as a decimal. Unfortunately, that field is a bit mask, so
it is hard to interpret as a decimal.
Print in hex, so the user can easily see what bits are set and more
importantly what type of info it is conveying.
V2: add 0x in front per Jiri Olsa
Signed-off-by: Don Zickus <[email protected]>
---
tools/perf/util/session.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 1d555d6..55960f2 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -794,7 +794,7 @@ static void dump_sample(struct perf_evsel *evsel, union perf_event *event,
if (!dump_trace)
return;
- printf("(IP, %d): %d/%d: %#" PRIx64 " period: %" PRIu64 " addr: %#" PRIx64 "\n",
+ printf("(IP, 0x%x): %d/%d: %#" PRIx64 " period: %" PRIu64 " addr: %#" PRIx64 "\n",
event->header.misc, sample->pid, sample->tid, sample->ip,
sample->period, sample->addr);
--
1.7.11.7
Currently if a process creates a bunch of threads using pthread_create
and then perf is run in system_wide mode, the mmaps for those threads
are not captured with a synthesized mmap event.
The reason is those threads are not visible when walking the /proc/
directory looking for /proc/<pid>/maps files. Instead they are discovered
using the /proc/<pid>/tasks file (which the synthesized comm event uses).
This causes problems when a program is trying to map a data address to a
tid. Because the tid has no maps, the event is dropped. Changing the program
to look up using the pid instead of the tid, finds the correct maps but creates
ugly hacks in the program to carry the correct tid around.
Fix this by synthesizing mmap events for each tid found in the /proc/<pid>/tasks
file.
This may not be entirely clean but it seems to work.
Signed-off-by: Don Zickus <[email protected]>
---
tools/perf/util/event.c | 15 +++++++++++----
tools/perf/util/machine.c | 4 ++--
2 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 086c7c8..09c53bb 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -93,10 +93,13 @@ static pid_t perf_event__get_comm_tgid(pid_t pid, char *comm, size_t len)
}
static pid_t perf_event__synthesize_comm(struct perf_tool *tool,
- union perf_event *event, pid_t pid,
+ union perf_event *event,
+ union perf_event *mmap_event,
+ pid_t pid,
int full,
perf_event__handler_t process,
- struct machine *machine)
+ struct machine *machine,
+ bool mmap_data)
{
char filename[PATH_MAX];
size_t size;
@@ -168,6 +171,10 @@ static pid_t perf_event__synthesize_comm(struct perf_tool *tool,
tgid = -1;
break;
}
+
+ /* process the thread's maps too */
+ perf_event__synthesize_mmap_events(tool, mmap_event, pid, tgid,
+ process, machine, mmap_data);
}
closedir(tasks);
@@ -331,8 +338,8 @@ static int __event__synthesize_thread(union perf_event *comm_event,
struct perf_tool *tool,
struct machine *machine, bool mmap_data)
{
- pid_t tgid = perf_event__synthesize_comm(tool, comm_event, pid, full,
- process, machine);
+ pid_t tgid = perf_event__synthesize_comm(tool, comm_event, mmap_event, pid,
+ full, process, machine, mmap_data);
if (tgid == -1)
return -1;
return perf_event__synthesize_mmap_events(tool, mmap_event, pid, tgid,
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 813e94e..eb26544 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -1026,7 +1026,7 @@ int machine__process_mmap2_event(struct machine *machine,
}
thread = machine__findnew_thread(machine, event->mmap2.pid,
- event->mmap2.pid);
+ event->mmap2.tid);
if (thread == NULL)
goto out_problem;
@@ -1074,7 +1074,7 @@ int machine__process_mmap_event(struct machine *machine, union perf_event *event
}
thread = machine__findnew_thread(machine, event->mmap.pid,
- event->mmap.pid);
+ event->mmap.tid);
if (thread == NULL)
goto out_problem;
--
1.7.11.7
Em Tue, Feb 25, 2014 at 10:43:47PM -0500, Don Zickus escreveu:
> Currently if a process creates a bunch of threads using pthread_create
> and then perf is run in system_wide mode, the mmaps for those threads
> are not captured with a synthesized mmap event.
>
> The reason is those threads are not visible when walking the /proc/
> directory looking for /proc/<pid>/maps files. Instead they are discovered
> using the /proc/<pid>/tasks file (which the synthesized comm event uses).
>
> This causes problems when a program is trying to map a data address to a
> tid. Because the tid has no maps, the event is dropped. Changing the program
> to look up using the pid instead of the tid, finds the correct maps but creates
> ugly hacks in the program to carry the correct tid around.
>
> Fix this by synthesizing mmap events for each tid found in the /proc/<pid>/tasks
> file.
This seems to cover two problems, the first is for mmap/mmap2 event
processing to lookup pid/tid instead of pid/pid, the other one is to
iterate thru /proc/pid/tasks/, so this needes spliting up.
Now looking at the /tasks/ part...
> This may not be entirely clean but it seems to work.
>
> Signed-off-by: Don Zickus <[email protected]>
> ---
> tools/perf/util/event.c | 15 +++++++++++----
> tools/perf/util/machine.c | 4 ++--
> 2 files changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
> index 086c7c8..09c53bb 100644
> --- a/tools/perf/util/event.c
> +++ b/tools/perf/util/event.c
> @@ -93,10 +93,13 @@ static pid_t perf_event__get_comm_tgid(pid_t pid, char *comm, size_t len)
> }
>
> static pid_t perf_event__synthesize_comm(struct perf_tool *tool,
> - union perf_event *event, pid_t pid,
> + union perf_event *event,
> + union perf_event *mmap_event,
> + pid_t pid,
> int full,
> perf_event__handler_t process,
> - struct machine *machine)
> + struct machine *machine,
> + bool mmap_data)
> {
> char filename[PATH_MAX];
> size_t size;
> @@ -168,6 +171,10 @@ static pid_t perf_event__synthesize_comm(struct perf_tool *tool,
> tgid = -1;
> break;
> }
> +
> + /* process the thread's maps too */
> + perf_event__synthesize_mmap_events(tool, mmap_event, pid, tgid,
> + process, machine, mmap_data);
> }
>
> closedir(tasks);
> @@ -331,8 +338,8 @@ static int __event__synthesize_thread(union perf_event *comm_event,
> struct perf_tool *tool,
> struct machine *machine, bool mmap_data)
> {
> - pid_t tgid = perf_event__synthesize_comm(tool, comm_event, pid, full,
> - process, machine);
> + pid_t tgid = perf_event__synthesize_comm(tool, comm_event, mmap_event, pid,
> + full, process, machine, mmap_data);
> if (tgid == -1)
> return -1;
> return perf_event__synthesize_mmap_events(tool, mmap_event, pid, tgid,
> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> index 813e94e..eb26544 100644
> --- a/tools/perf/util/machine.c
> +++ b/tools/perf/util/machine.c
> @@ -1026,7 +1026,7 @@ int machine__process_mmap2_event(struct machine *machine,
> }
>
> thread = machine__findnew_thread(machine, event->mmap2.pid,
> - event->mmap2.pid);
> + event->mmap2.tid);
> if (thread == NULL)
> goto out_problem;
>
> @@ -1074,7 +1074,7 @@ int machine__process_mmap_event(struct machine *machine, union perf_event *event
> }
>
> thread = machine__findnew_thread(machine, event->mmap.pid,
> - event->mmap.pid);
> + event->mmap.tid);
> if (thread == NULL)
> goto out_problem;
>
> --
> 1.7.11.7
Em Wed, Feb 26, 2014 at 11:17:26AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Tue, Feb 25, 2014 at 10:43:47PM -0500, Don Zickus escreveu:
> > Currently if a process creates a bunch of threads using pthread_create
> > and then perf is run in system_wide mode, the mmaps for those threads
> > are not captured with a synthesized mmap event.
> >
> > The reason is those threads are not visible when walking the /proc/
> > directory looking for /proc/<pid>/maps files. Instead they are discovered
> > using the /proc/<pid>/tasks file (which the synthesized comm event uses).
> >
> > This causes problems when a program is trying to map a data address to a
> > tid. Because the tid has no maps, the event is dropped. Changing the program
> > to look up using the pid instead of the tid, finds the correct maps but creates
> > ugly hacks in the program to carry the correct tid around.
> >
> > Fix this by synthesizing mmap events for each tid found in the /proc/<pid>/tasks
> > file.
>
> This seems to cover two problems, the first is for mmap/mmap2 event
> processing to lookup pid/tid instead of pid/pid, the other one is to
> iterate thru /proc/pid/tasks/, so this needes spliting up.
>
> Now looking at the /tasks/ part...
Agreed we need to synthesize the mmap events for the threads in
/proc/pid/task/, but that need to be done in
perf_event__synthesize_mmap_events, not in
perf_event__synthesize_comm_events, that should remain just for
synthesizing comm events .
I.e. we should move that /tasks/ traversal from synthesize_comm to its
caller and from there synthesize the mmaps too.
I can do that if you don't beat me to it. :-)
- Arnaldo
> > This may not be entirely clean but it seems to work.
> >
> > Signed-off-by: Don Zickus <[email protected]>
> > ---
> > tools/perf/util/event.c | 15 +++++++++++----
> > tools/perf/util/machine.c | 4 ++--
> > 2 files changed, 13 insertions(+), 6 deletions(-)
> >
> > diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
> > index 086c7c8..09c53bb 100644
> > --- a/tools/perf/util/event.c
> > +++ b/tools/perf/util/event.c
> > @@ -93,10 +93,13 @@ static pid_t perf_event__get_comm_tgid(pid_t pid, char *comm, size_t len)
> > }
> >
> > static pid_t perf_event__synthesize_comm(struct perf_tool *tool,
> > - union perf_event *event, pid_t pid,
> > + union perf_event *event,
> > + union perf_event *mmap_event,
> > + pid_t pid,
> > int full,
> > perf_event__handler_t process,
> > - struct machine *machine)
> > + struct machine *machine,
> > + bool mmap_data)
> > {
> > char filename[PATH_MAX];
> > size_t size;
> > @@ -168,6 +171,10 @@ static pid_t perf_event__synthesize_comm(struct perf_tool *tool,
> > tgid = -1;
> > break;
> > }
> > +
> > + /* process the thread's maps too */
> > + perf_event__synthesize_mmap_events(tool, mmap_event, pid, tgid,
> > + process, machine, mmap_data);
> > }
> >
> > closedir(tasks);
> > @@ -331,8 +338,8 @@ static int __event__synthesize_thread(union perf_event *comm_event,
> > struct perf_tool *tool,
> > struct machine *machine, bool mmap_data)
> > {
> > - pid_t tgid = perf_event__synthesize_comm(tool, comm_event, pid, full,
> > - process, machine);
> > + pid_t tgid = perf_event__synthesize_comm(tool, comm_event, mmap_event, pid,
> > + full, process, machine, mmap_data);
> > if (tgid == -1)
> > return -1;
> > return perf_event__synthesize_mmap_events(tool, mmap_event, pid, tgid,
> > diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> > index 813e94e..eb26544 100644
> > --- a/tools/perf/util/machine.c
> > +++ b/tools/perf/util/machine.c
> > @@ -1026,7 +1026,7 @@ int machine__process_mmap2_event(struct machine *machine,
> > }
> >
> > thread = machine__findnew_thread(machine, event->mmap2.pid,
> > - event->mmap2.pid);
> > + event->mmap2.tid);
> > if (thread == NULL)
> > goto out_problem;
> >
> > @@ -1074,7 +1074,7 @@ int machine__process_mmap_event(struct machine *machine, union perf_event *event
> > }
> >
> > thread = machine__findnew_thread(machine, event->mmap.pid,
> > - event->mmap.pid);
> > + event->mmap.tid);
> > if (thread == NULL)
> > goto out_problem;
> >
> > --
> > 1.7.11.7
On Wed, Feb 26, 2014 at 11:17:26AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Feb 25, 2014 at 10:43:47PM -0500, Don Zickus escreveu:
> > Currently if a process creates a bunch of threads using pthread_create
> > and then perf is run in system_wide mode, the mmaps for those threads
> > are not captured with a synthesized mmap event.
> >
> > The reason is those threads are not visible when walking the /proc/
> > directory looking for /proc/<pid>/maps files. Instead they are discovered
> > using the /proc/<pid>/tasks file (which the synthesized comm event uses).
> >
> > This causes problems when a program is trying to map a data address to a
> > tid. Because the tid has no maps, the event is dropped. Changing the program
> > to look up using the pid instead of the tid, finds the correct maps but creates
> > ugly hacks in the program to carry the correct tid around.
> >
> > Fix this by synthesizing mmap events for each tid found in the /proc/<pid>/tasks
> > file.
>
> This seems to cover two problems, the first is for mmap/mmap2 event
> processing to lookup pid/tid instead of pid/pid, the other one is to
> iterate thru /proc/pid/tasks/, so this needes spliting up.
>
> Now looking at the /tasks/ part...
Fair enough. :-)
Cheers,
Don
>
> > This may not be entirely clean but it seems to work.
> >
> > Signed-off-by: Don Zickus <[email protected]>
> > ---
> > tools/perf/util/event.c | 15 +++++++++++----
> > tools/perf/util/machine.c | 4 ++--
> > 2 files changed, 13 insertions(+), 6 deletions(-)
> >
> > diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
> > index 086c7c8..09c53bb 100644
> > --- a/tools/perf/util/event.c
> > +++ b/tools/perf/util/event.c
> > @@ -93,10 +93,13 @@ static pid_t perf_event__get_comm_tgid(pid_t pid, char *comm, size_t len)
> > }
> >
> > static pid_t perf_event__synthesize_comm(struct perf_tool *tool,
> > - union perf_event *event, pid_t pid,
> > + union perf_event *event,
> > + union perf_event *mmap_event,
> > + pid_t pid,
> > int full,
> > perf_event__handler_t process,
> > - struct machine *machine)
> > + struct machine *machine,
> > + bool mmap_data)
> > {
> > char filename[PATH_MAX];
> > size_t size;
> > @@ -168,6 +171,10 @@ static pid_t perf_event__synthesize_comm(struct perf_tool *tool,
> > tgid = -1;
> > break;
> > }
> > +
> > + /* process the thread's maps too */
> > + perf_event__synthesize_mmap_events(tool, mmap_event, pid, tgid,
> > + process, machine, mmap_data);
> > }
> >
> > closedir(tasks);
> > @@ -331,8 +338,8 @@ static int __event__synthesize_thread(union perf_event *comm_event,
> > struct perf_tool *tool,
> > struct machine *machine, bool mmap_data)
> > {
> > - pid_t tgid = perf_event__synthesize_comm(tool, comm_event, pid, full,
> > - process, machine);
> > + pid_t tgid = perf_event__synthesize_comm(tool, comm_event, mmap_event, pid,
> > + full, process, machine, mmap_data);
> > if (tgid == -1)
> > return -1;
> > return perf_event__synthesize_mmap_events(tool, mmap_event, pid, tgid,
> > diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> > index 813e94e..eb26544 100644
> > --- a/tools/perf/util/machine.c
> > +++ b/tools/perf/util/machine.c
> > @@ -1026,7 +1026,7 @@ int machine__process_mmap2_event(struct machine *machine,
> > }
> >
> > thread = machine__findnew_thread(machine, event->mmap2.pid,
> > - event->mmap2.pid);
> > + event->mmap2.tid);
> > if (thread == NULL)
> > goto out_problem;
> >
> > @@ -1074,7 +1074,7 @@ int machine__process_mmap_event(struct machine *machine, union perf_event *event
> > }
> >
> > thread = machine__findnew_thread(machine, event->mmap.pid,
> > - event->mmap.pid);
> > + event->mmap.tid);
> > if (thread == NULL)
> > goto out_problem;
> >
> > --
> > 1.7.11.7