Skip to content

Commit 599c193

Browse files
captain5050acmel
authored andcommitted
perf callchain: Fix stitch LBR memory leaks
The 'struct callchain_cursor_node' has a 'struct map_symbol' whose maps and map members are reference counted. Ensure these values use a _get routine to increment the reference counts and use map_symbol__exit() to release the reference counts. Do similar for 'struct thread's prev_lbr_cursor, but save the size of the prev_lbr_cursor array so that it may be iterated. Ensure that when stitch_nodes are placed on the free list the map_symbols are exited. Fix resolve_lbr_callchain_sample() by replacing list_replace_init() to list_splice_init(), so the whole list is moved and nodes aren't leaked. A reproduction of the memory leaks is possible with a leak sanitizer build in the perf report command of: ``` $ perf record -e cycles --call-graph lbr perf test -w thloop $ perf report --stitch-lbr ``` Reviewed-by: Kan Liang <[email protected]> Fixes: ff16562 ("perf callchain: Stitch LBR call stack") Signed-off-by: Ian Rogers <[email protected]> [ Basic tests after applying the patch, repeating the example above ] Tested-by: Arnaldo Carvalho de Melo <[email protected]> Cc: Adrian Hunter <[email protected]> Cc: Alexander Shishkin <[email protected]> Cc: Andi Kleen <[email protected]> Cc: Anne Macedo <[email protected]> Cc: Changbin Du <[email protected]> Cc: Ingo Molnar <[email protected]> Cc: Jiri Olsa <[email protected]> Cc: Mark Rutland <[email protected]> Cc: Namhyung Kim <[email protected]> Cc: Peter Zijlstra <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
1 parent 37e2a19 commit 599c193

File tree

3 files changed

+20
-2
lines changed

3 files changed

+20
-2
lines changed

tools/perf/util/machine.c

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2270,8 +2270,12 @@ static void save_lbr_cursor_node(struct thread *thread,
22702270
cursor->curr = cursor->first;
22712271
else
22722272
cursor->curr = cursor->curr->next;
2273+
2274+
map_symbol__exit(&lbr_stitch->prev_lbr_cursor[idx].ms);
22732275
memcpy(&lbr_stitch->prev_lbr_cursor[idx], cursor->curr,
22742276
sizeof(struct callchain_cursor_node));
2277+
lbr_stitch->prev_lbr_cursor[idx].ms.maps = maps__get(cursor->curr->ms.maps);
2278+
lbr_stitch->prev_lbr_cursor[idx].ms.map = map__get(cursor->curr->ms.map);
22752279

22762280
lbr_stitch->prev_lbr_cursor[idx].valid = true;
22772281
cursor->pos++;
@@ -2482,6 +2486,9 @@ static bool has_stitched_lbr(struct thread *thread,
24822486
memcpy(&stitch_node->cursor, &lbr_stitch->prev_lbr_cursor[i],
24832487
sizeof(struct callchain_cursor_node));
24842488

2489+
stitch_node->cursor.ms.maps = maps__get(lbr_stitch->prev_lbr_cursor[i].ms.maps);
2490+
stitch_node->cursor.ms.map = map__get(lbr_stitch->prev_lbr_cursor[i].ms.map);
2491+
24852492
if (callee)
24862493
list_add(&stitch_node->node, &lbr_stitch->lists);
24872494
else
@@ -2505,6 +2512,8 @@ static bool alloc_lbr_stitch(struct thread *thread, unsigned int max_lbr)
25052512
if (!thread__lbr_stitch(thread)->prev_lbr_cursor)
25062513
goto free_lbr_stitch;
25072514

2515+
thread__lbr_stitch(thread)->prev_lbr_cursor_size = max_lbr + 1;
2516+
25082517
INIT_LIST_HEAD(&thread__lbr_stitch(thread)->lists);
25092518
INIT_LIST_HEAD(&thread__lbr_stitch(thread)->free_lists);
25102519

@@ -2560,8 +2569,12 @@ static int resolve_lbr_callchain_sample(struct thread *thread,
25602569
max_lbr, callee);
25612570

25622571
if (!stitched_lbr && !list_empty(&lbr_stitch->lists)) {
2563-
list_replace_init(&lbr_stitch->lists,
2564-
&lbr_stitch->free_lists);
2572+
struct stitch_list *stitch_node;
2573+
2574+
list_for_each_entry(stitch_node, &lbr_stitch->lists, node)
2575+
map_symbol__exit(&stitch_node->cursor.ms);
2576+
2577+
list_splice_init(&lbr_stitch->lists, &lbr_stitch->free_lists);
25652578
}
25662579
memcpy(&lbr_stitch->prev_sample, sample, sizeof(*sample));
25672580
}

tools/perf/util/thread.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -476,6 +476,7 @@ void thread__free_stitch_list(struct thread *thread)
476476
return;
477477

478478
list_for_each_entry_safe(pos, tmp, &lbr_stitch->lists, node) {
479+
map_symbol__exit(&pos->cursor.ms);
479480
list_del_init(&pos->node);
480481
free(pos);
481482
}
@@ -485,6 +486,9 @@ void thread__free_stitch_list(struct thread *thread)
485486
free(pos);
486487
}
487488

489+
for (unsigned int i = 0 ; i < lbr_stitch->prev_lbr_cursor_size; i++)
490+
map_symbol__exit(&lbr_stitch->prev_lbr_cursor[i].ms);
491+
488492
zfree(&lbr_stitch->prev_lbr_cursor);
489493
free(thread__lbr_stitch(thread));
490494
thread__set_lbr_stitch(thread, NULL);

tools/perf/util/thread.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ struct lbr_stitch {
2626
struct list_head free_lists;
2727
struct perf_sample prev_sample;
2828
struct callchain_cursor_node *prev_lbr_cursor;
29+
unsigned int prev_lbr_cursor_size;
2930
};
3031

3132
DECLARE_RC_STRUCT(thread) {

0 commit comments

Comments
 (0)