Skip to content

Commit

Permalink
perf tools: Don't clone maps from parent when synthesizing forks
Browse files Browse the repository at this point in the history
When synthesizing FORK events, we are trying to create thread objects
for the already running tasks on the machine.

Normally, for a kernel FORK event, we want to clone the parent's maps
because that is what the kernel just did.

But when synthesizing, this should not be done.  If we do, we end up
with overlapping maps as we process the sythesized MMAP2 events that
get delivered shortly thereafter.

Use the FORK event misc flags in an internal way to signal this
situation, so we can elide the map clone when appropriate.

Signed-off-by: David S. Miller <davem@davemloft.net>
Cc: Don Zickus <dzickus@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Joe Mario <jmario@redhat.com>
Link: http://lkml.kernel.org/r/20181030.222404.2085088822877051075.davem@davemloft.net
[ Added comment about flag use in machine__process_fork_event(),
  use ternary op in thread__clone_map_groups() as suggested by Jiri ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
  • Loading branch information
David Miller authored and Arnaldo Carvalho de Melo committed Oct 31, 2018
1 parent ff27a06 commit 4f8f382
Show file tree
Hide file tree
Showing 6 changed files with 29 additions and 10 deletions.
2 changes: 2 additions & 0 deletions include/uapi/linux/perf_event.h
Original file line number Diff line number Diff line change
Expand Up @@ -646,10 +646,12 @@ struct perf_event_mmap_page {
*
* PERF_RECORD_MISC_MMAP_DATA - PERF_RECORD_MMAP* events
* PERF_RECORD_MISC_COMM_EXEC - PERF_RECORD_COMM event
* PERF_RECORD_MISC_FORK_EXEC - PERF_RECORD_FORK event (perf internal)
* PERF_RECORD_MISC_SWITCH_OUT - PERF_RECORD_SWITCH* events
*/
#define PERF_RECORD_MISC_MMAP_DATA (1 << 13)
#define PERF_RECORD_MISC_COMM_EXEC (1 << 13)
#define PERF_RECORD_MISC_FORK_EXEC (1 << 13)
#define PERF_RECORD_MISC_SWITCH_OUT (1 << 13)
/*
* These PERF_RECORD_MISC_* flags below are safely reused
Expand Down
2 changes: 2 additions & 0 deletions tools/include/uapi/linux/perf_event.h
Original file line number Diff line number Diff line change
Expand Up @@ -646,10 +646,12 @@ struct perf_event_mmap_page {
*
* PERF_RECORD_MISC_MMAP_DATA - PERF_RECORD_MMAP* events
* PERF_RECORD_MISC_COMM_EXEC - PERF_RECORD_COMM event
* PERF_RECORD_MISC_FORK_EXEC - PERF_RECORD_FORK event (perf internal)
* PERF_RECORD_MISC_SWITCH_OUT - PERF_RECORD_SWITCH* events
*/
#define PERF_RECORD_MISC_MMAP_DATA (1 << 13)
#define PERF_RECORD_MISC_COMM_EXEC (1 << 13)
#define PERF_RECORD_MISC_FORK_EXEC (1 << 13)
#define PERF_RECORD_MISC_SWITCH_OUT (1 << 13)
/*
* These PERF_RECORD_MISC_* flags below are safely reused
Expand Down
1 change: 1 addition & 0 deletions tools/perf/util/event.c
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,7 @@ static int perf_event__synthesize_fork(struct perf_tool *tool,
event->fork.pid = tgid;
event->fork.tid = pid;
event->fork.header.type = PERF_RECORD_FORK;
event->fork.header.misc = PERF_RECORD_MISC_FORK_EXEC;

event->fork.header.size = (sizeof(event->fork) + machine->id_hdr_size);

Expand Down
19 changes: 18 additions & 1 deletion tools/perf/util/machine.c
Original file line number Diff line number Diff line change
Expand Up @@ -1708,6 +1708,7 @@ int machine__process_fork_event(struct machine *machine, union perf_event *event
struct thread *parent = machine__findnew_thread(machine,
event->fork.ppid,
event->fork.ptid);
bool do_maps_clone = true;
int err = 0;

if (dump_trace)
Expand Down Expand Up @@ -1736,9 +1737,25 @@ int machine__process_fork_event(struct machine *machine, union perf_event *event

thread = machine__findnew_thread(machine, event->fork.pid,
event->fork.tid);
/*
* When synthesizing FORK events, we are trying to create thread
* objects for the already running tasks on the machine.
*
* Normally, for a kernel FORK event, we want to clone the parent's
* maps because that is what the kernel just did.
*
* But when synthesizing, this should not be done. If we do, we end up
* with overlapping maps as we process the sythesized MMAP2 events that
* get delivered shortly thereafter.
*
* Use the FORK event misc flags in an internal way to signal this
* situation, so we can elide the map clone when appropriate.
*/
if (event->fork.header.misc & PERF_RECORD_MISC_FORK_EXEC)
do_maps_clone = false;

if (thread == NULL || parent == NULL ||
thread__fork(thread, parent, sample->time) < 0) {
thread__fork(thread, parent, sample->time, do_maps_clone) < 0) {
dump_printf("problem processing PERF_RECORD_FORK, skipping event.\n");
err = -1;
}
Expand Down
13 changes: 5 additions & 8 deletions tools/perf/util/thread.c
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,8 @@ static int thread__prepare_access(struct thread *thread)
}

static int thread__clone_map_groups(struct thread *thread,
struct thread *parent)
struct thread *parent,
bool do_maps_clone)
{
/* This is new thread, we share map groups for process. */
if (thread->pid_ == parent->pid_)
Expand All @@ -341,15 +342,11 @@ static int thread__clone_map_groups(struct thread *thread,
thread->pid_, thread->tid, parent->pid_, parent->tid);
return 0;
}

/* But this one is new process, copy maps. */
if (map_groups__clone(thread, parent->mg) < 0)
return -ENOMEM;

return 0;
return do_maps_clone ? map_groups__clone(thread, parent->mg) : 0;
}

int thread__fork(struct thread *thread, struct thread *parent, u64 timestamp)
int thread__fork(struct thread *thread, struct thread *parent, u64 timestamp, bool do_maps_clone)
{
if (parent->comm_set) {
const char *comm = thread__comm_str(parent);
Expand All @@ -362,7 +359,7 @@ int thread__fork(struct thread *thread, struct thread *parent, u64 timestamp)
}

thread->ppid = parent->tid;
return thread__clone_map_groups(thread, parent);
return thread__clone_map_groups(thread, parent, do_maps_clone);
}

void thread__find_cpumode_addr_location(struct thread *thread, u64 addr,
Expand Down
2 changes: 1 addition & 1 deletion tools/perf/util/thread.h
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ struct comm *thread__comm(const struct thread *thread);
struct comm *thread__exec_comm(const struct thread *thread);
const char *thread__comm_str(const struct thread *thread);
int thread__insert_map(struct thread *thread, struct map *map);
int thread__fork(struct thread *thread, struct thread *parent, u64 timestamp);
int thread__fork(struct thread *thread, struct thread *parent, u64 timestamp, bool do_maps_clone);
size_t thread__fprintf(struct thread *thread, FILE *fp);

struct thread *thread__main_thread(struct machine *machine, struct thread *thread);
Expand Down

0 comments on commit 4f8f382

Please sign in to comment.