Skip to content

Commit 4f8f382

Browse files
davem330acmel
davem330
authored and
acmel
committed
perf tools: Don't clone maps from parent when synthesizing forks
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>
1 parent ff27a06 commit 4f8f382

File tree

6 files changed

+29
-10
lines changed

6 files changed

+29
-10
lines changed

‎include/uapi/linux/perf_event.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -646,10 +646,12 @@ struct perf_event_mmap_page {
646646
*
647647
* PERF_RECORD_MISC_MMAP_DATA - PERF_RECORD_MMAP* events
648648
* PERF_RECORD_MISC_COMM_EXEC - PERF_RECORD_COMM event
649+
* PERF_RECORD_MISC_FORK_EXEC - PERF_RECORD_FORK event (perf internal)
649650
* PERF_RECORD_MISC_SWITCH_OUT - PERF_RECORD_SWITCH* events
650651
*/
651652
#define PERF_RECORD_MISC_MMAP_DATA (1 << 13)
652653
#define PERF_RECORD_MISC_COMM_EXEC (1 << 13)
654+
#define PERF_RECORD_MISC_FORK_EXEC (1 << 13)
653655
#define PERF_RECORD_MISC_SWITCH_OUT (1 << 13)
654656
/*
655657
* These PERF_RECORD_MISC_* flags below are safely reused

‎tools/include/uapi/linux/perf_event.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -646,10 +646,12 @@ struct perf_event_mmap_page {
646646
*
647647
* PERF_RECORD_MISC_MMAP_DATA - PERF_RECORD_MMAP* events
648648
* PERF_RECORD_MISC_COMM_EXEC - PERF_RECORD_COMM event
649+
* PERF_RECORD_MISC_FORK_EXEC - PERF_RECORD_FORK event (perf internal)
649650
* PERF_RECORD_MISC_SWITCH_OUT - PERF_RECORD_SWITCH* events
650651
*/
651652
#define PERF_RECORD_MISC_MMAP_DATA (1 << 13)
652653
#define PERF_RECORD_MISC_COMM_EXEC (1 << 13)
654+
#define PERF_RECORD_MISC_FORK_EXEC (1 << 13)
653655
#define PERF_RECORD_MISC_SWITCH_OUT (1 << 13)
654656
/*
655657
* These PERF_RECORD_MISC_* flags below are safely reused

‎tools/perf/util/event.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,7 @@ static int perf_event__synthesize_fork(struct perf_tool *tool,
308308
event->fork.pid = tgid;
309309
event->fork.tid = pid;
310310
event->fork.header.type = PERF_RECORD_FORK;
311+
event->fork.header.misc = PERF_RECORD_MISC_FORK_EXEC;
311312

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

‎tools/perf/util/machine.c

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1708,6 +1708,7 @@ int machine__process_fork_event(struct machine *machine, union perf_event *event
17081708
struct thread *parent = machine__findnew_thread(machine,
17091709
event->fork.ppid,
17101710
event->fork.ptid);
1711+
bool do_maps_clone = true;
17111712
int err = 0;
17121713

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

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

17401757
if (thread == NULL || parent == NULL ||
1741-
thread__fork(thread, parent, sample->time) < 0) {
1758+
thread__fork(thread, parent, sample->time, do_maps_clone) < 0) {
17421759
dump_printf("problem processing PERF_RECORD_FORK, skipping event.\n");
17431760
err = -1;
17441761
}

‎tools/perf/util/thread.c

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,8 @@ static int thread__prepare_access(struct thread *thread)
330330
}
331331

332332
static int thread__clone_map_groups(struct thread *thread,
333-
struct thread *parent)
333+
struct thread *parent,
334+
bool do_maps_clone)
334335
{
335336
/* This is new thread, we share map groups for process. */
336337
if (thread->pid_ == parent->pid_)
@@ -341,15 +342,11 @@ static int thread__clone_map_groups(struct thread *thread,
341342
thread->pid_, thread->tid, parent->pid_, parent->tid);
342343
return 0;
343344
}
344-
345345
/* But this one is new process, copy maps. */
346-
if (map_groups__clone(thread, parent->mg) < 0)
347-
return -ENOMEM;
348-
349-
return 0;
346+
return do_maps_clone ? map_groups__clone(thread, parent->mg) : 0;
350347
}
351348

352-
int thread__fork(struct thread *thread, struct thread *parent, u64 timestamp)
349+
int thread__fork(struct thread *thread, struct thread *parent, u64 timestamp, bool do_maps_clone)
353350
{
354351
if (parent->comm_set) {
355352
const char *comm = thread__comm_str(parent);
@@ -362,7 +359,7 @@ int thread__fork(struct thread *thread, struct thread *parent, u64 timestamp)
362359
}
363360

364361
thread->ppid = parent->tid;
365-
return thread__clone_map_groups(thread, parent);
362+
return thread__clone_map_groups(thread, parent, do_maps_clone);
366363
}
367364

368365
void thread__find_cpumode_addr_location(struct thread *thread, u64 addr,

‎tools/perf/util/thread.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ struct comm *thread__comm(const struct thread *thread);
8989
struct comm *thread__exec_comm(const struct thread *thread);
9090
const char *thread__comm_str(const struct thread *thread);
9191
int thread__insert_map(struct thread *thread, struct map *map);
92-
int thread__fork(struct thread *thread, struct thread *parent, u64 timestamp);
92+
int thread__fork(struct thread *thread, struct thread *parent, u64 timestamp, bool do_maps_clone);
9393
size_t thread__fprintf(struct thread *thread, FILE *fp);
9494

9595
struct thread *thread__main_thread(struct machine *machine, struct thread *thread);

0 commit comments

Comments
 (0)