From 9ffa6c7512ca7aaeb30e596e2c247cb1fae7123a Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Thu, 2 Nov 2023 10:56:47 -0700 Subject: perf machine thread: Remove exited threads by default 'struct thread' values hold onto references to mmaps, DSOs, etc. When a thread exits it is necessary to clean all of this memory up by removing the thread from the machine's threads. Some tools require this doesn't happen, such as auxtrace events, 'perf report' if offcpu events exist or if a task list is being generated, so add a 'struct symbol_conf' member to make the behavior optional. When an exited thread is left in the machine's threads, mark it as exited. This change relates to commit 40826c45eb0b8856 ("perf thread: Remove notion of dead threads") . Dead threads were removed as they had a reference count of 0 and were difficult to reason about with the reference count checker. Here a thread is removed from threads when it exits, unless via symbol_conf the exited thread isn't remove and is marked as exited. Reference counting behaves as it normally does. Reviewed-by: Adrian Hunter Signed-off-by: Ian Rogers Cc: Alexander Shishkin Cc: Andi Kleen Cc: Athira Jajeev Cc: Changbin Du Cc: Colin Ian King Cc: Dmitrii Dolgov <9erthalion6@gmail.com> Cc: German Gomez Cc: Huacai Chen Cc: Ingo Molnar Cc: James Clark Cc: Jiri Olsa Cc: K Prateek Nayak Cc: Kajol Jain Cc: Kan Liang Cc: Leo Yan Cc: Li Dong Cc: Liam Howlett Cc: Mark Rutland Cc: Masami Hiramatsu Cc: Miguel Ojeda Cc: Ming Wang Cc: Namhyung Kim Cc: Nick Terrell Cc: Paolo Bonzini Cc: Peter Zijlstra Cc: Ravi Bangoria Cc: Sandipan Das Cc: Sean Christopherson Cc: Steinar H. Gunderson Cc: Vincent Whitchurch Cc: Wenyu Liu Cc: Yang Jihong Link: https://lore.kernel.org/r/20231102175735.2272696-6-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-report.c | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'tools/perf/builtin-report.c') diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c index 9cb1da2dc0c0..121a2781323c 100644 --- a/tools/perf/builtin-report.c +++ b/tools/perf/builtin-report.c @@ -1426,6 +1426,13 @@ int cmd_report(int argc, const char **argv) if (ret < 0) goto exit; + /* + * tasks_mode require access to exited threads to list those that are in + * the data file. Off-cpu events are synthesized after other events and + * reference exited threads. + */ + symbol_conf.keep_exited_threads = true; + annotation_options__init(&report.annotation_opts); ret = perf_config(report__config, &report); -- cgit From 14953f038d6b30e3dc9d1aa4d4584ac505e5a8ec Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Tue, 28 Nov 2023 09:54:35 -0800 Subject: perf report: Convert to the global annotation_options Use the global option and drop the local copy. Reviewed-by: Ian Rogers Signed-off-by: Namhyung Kim Cc: Adrian Hunter Cc: Ingo Molnar Cc: Jiri Olsa Cc: Peter Zijlstra Link: https://lore.kernel.org/r/20231128175441.721579-3-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-report.c | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) (limited to 'tools/perf/builtin-report.c') diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c index 121a2781323c..90f98953587c 100644 --- a/tools/perf/builtin-report.c +++ b/tools/perf/builtin-report.c @@ -98,7 +98,6 @@ struct report { bool skip_empty; int max_stack; struct perf_read_values show_threads_values; - struct annotation_options annotation_opts; const char *pretty_printing_style; const char *cpu_list; const char *symbol_filter_str; @@ -542,7 +541,7 @@ static int evlist__tui_block_hists_browse(struct evlist *evlist, struct report * ret = report__browse_block_hists(&rep->block_reports[i++].hist, rep->min_percent, pos, &rep->session->header.env, - &rep->annotation_opts); + &annotate_opts); if (ret != 0) return ret; } @@ -670,7 +669,7 @@ static int report__browse_hists(struct report *rep) } ret = evlist__tui_browse_hists(evlist, help, NULL, rep->min_percent, - &session->header.env, true, &rep->annotation_opts); + &session->header.env, true, &annotate_opts); /* * Usually "ret" is the last pressed key, and we only * care if the key notifies us to switch data file. @@ -745,7 +744,7 @@ static int hists__resort_cb(struct hist_entry *he, void *arg) if (rep->symbol_ipc && sym && !sym->annotate2) { struct evsel *evsel = hists_to_evsel(he->hists); - symbol__annotate2(&he->ms, evsel, &rep->annotation_opts, NULL); + symbol__annotate2(&he->ms, evsel, &annotate_opts, NULL); } return 0; @@ -1341,15 +1340,15 @@ int cmd_report(int argc, const char **argv) "list of cpus to profile"), OPT_BOOLEAN('I', "show-info", &report.show_full_info, "Display extended information about perf.data file"), - OPT_BOOLEAN(0, "source", &report.annotation_opts.annotate_src, + OPT_BOOLEAN(0, "source", &annotate_opts.annotate_src, "Interleave source code with assembly code (default)"), - OPT_BOOLEAN(0, "asm-raw", &report.annotation_opts.show_asm_raw, + OPT_BOOLEAN(0, "asm-raw", &annotate_opts.show_asm_raw, "Display raw encoding of assembly instructions (default)"), OPT_STRING('M', "disassembler-style", &disassembler_style, "disassembler style", "Specify disassembler style (e.g. -M intel for intel syntax)"), - OPT_STRING(0, "prefix", &report.annotation_opts.prefix, "prefix", + OPT_STRING(0, "prefix", &annotate_opts.prefix, "prefix", "Add prefix to source file path names in programs (with --prefix-strip)"), - OPT_STRING(0, "prefix-strip", &report.annotation_opts.prefix_strip, "N", + OPT_STRING(0, "prefix-strip", &annotate_opts.prefix_strip, "N", "Strip first N entries of source file path name in programs (with --prefix)"), OPT_BOOLEAN(0, "show-total-period", &symbol_conf.show_total_period, "Show a column with the sum of periods"), @@ -1401,7 +1400,7 @@ int cmd_report(int argc, const char **argv) "Time span of interest (start,stop)"), OPT_BOOLEAN(0, "inline", &symbol_conf.inline_name, "Show inline function"), - OPT_CALLBACK(0, "percent-type", &report.annotation_opts, "local-period", + OPT_CALLBACK(0, "percent-type", &annotate_opts, "local-period", "Set percent type local/global-period/hits", annotate_parse_percent_type), OPT_BOOLEAN(0, "ns", &symbol_conf.nanosecs, "Show times in nanosecs"), @@ -1433,7 +1432,7 @@ int cmd_report(int argc, const char **argv) */ symbol_conf.keep_exited_threads = true; - annotation_options__init(&report.annotation_opts); + annotation_options__init(&annotate_opts); ret = perf_config(report__config, &report); if (ret) @@ -1452,13 +1451,13 @@ int cmd_report(int argc, const char **argv) } if (disassembler_style) { - report.annotation_opts.disassembler_style = strdup(disassembler_style); - if (!report.annotation_opts.disassembler_style) + annotate_opts.disassembler_style = strdup(disassembler_style); + if (!annotate_opts.disassembler_style) return -ENOMEM; } if (objdump_path) { - report.annotation_opts.objdump_path = strdup(objdump_path); - if (!report.annotation_opts.objdump_path) + annotate_opts.objdump_path = strdup(objdump_path); + if (!annotate_opts.objdump_path) return -ENOMEM; } if (addr2line_path) { @@ -1467,7 +1466,7 @@ int cmd_report(int argc, const char **argv) return -ENOMEM; } - if (annotate_check_args(&report.annotation_opts) < 0) { + if (annotate_check_args(&annotate_opts) < 0) { ret = -EINVAL; goto exit; } @@ -1699,7 +1698,7 @@ repeat: */ symbol_conf.priv_size += sizeof(u32); } - annotation_config__init(&report.annotation_opts); + annotation_config__init(&annotate_opts); } if (symbol__init(&session->header.env) < 0) @@ -1753,7 +1752,7 @@ error: zstd_fini(&(session->zstd_data)); perf_session__delete(session); exit: - annotation_options__exit(&report.annotation_opts); + annotation_options__exit(&annotate_opts); free(sort_order_help); free(field_order_help); return ret; -- cgit From 41fd3cacd29f47f6b9c6474b27c5b0513786c4e9 Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Tue, 28 Nov 2023 09:54:37 -0800 Subject: perf annotate: Use global annotation_options Now it can directly use the global options and no need to pass it as an argument. Reviewed-by: Ian Rogers Signed-off-by: Namhyung Kim Cc: Adrian Hunter Cc: Ingo Molnar Cc: Jiri Olsa Cc: Peter Zijlstra Link: https://lore.kernel.org/r/20231128175441.721579-5-namhyung@kernel.org [ Fixup build with GTK2=1 ] Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-report.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tools/perf/builtin-report.c') diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c index 90f98953587c..2b86651615cd 100644 --- a/tools/perf/builtin-report.c +++ b/tools/perf/builtin-report.c @@ -744,7 +744,7 @@ static int hists__resort_cb(struct hist_entry *he, void *arg) if (rep->symbol_ipc && sym && !sym->annotate2) { struct evsel *evsel = hists_to_evsel(he->hists); - symbol__annotate2(&he->ms, evsel, &annotate_opts, NULL); + symbol__annotate2(&he->ms, evsel, NULL); } return 0; -- cgit From 22197fb296913f83c7182befd2a8b23bf042f279 Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Tue, 28 Nov 2023 09:54:38 -0800 Subject: perf ui/browser/annotate: Use global annotation_options Now it can use the global options and no need save local browser options separately. Reviewed-by: Ian Rogers Signed-off-by: Namhyung Kim Cc: Adrian Hunter Cc: Ingo Molnar Cc: Jiri Olsa Cc: Peter Zijlstra Link: https://lore.kernel.org/r/20231128175441.721579-6-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-report.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) (limited to 'tools/perf/builtin-report.c') diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c index 2b86651615cd..bc0d986c1e0c 100644 --- a/tools/perf/builtin-report.c +++ b/tools/perf/builtin-report.c @@ -540,8 +540,7 @@ static int evlist__tui_block_hists_browse(struct evlist *evlist, struct report * evlist__for_each_entry(evlist, pos) { ret = report__browse_block_hists(&rep->block_reports[i++].hist, rep->min_percent, pos, - &rep->session->header.env, - &annotate_opts); + &rep->session->header.env); if (ret != 0) return ret; } @@ -573,8 +572,7 @@ static int evlist__tty_browse_hists(struct evlist *evlist, struct report *rep, c if (rep->total_cycles_mode) { report__browse_block_hists(&rep->block_reports[i++].hist, - rep->min_percent, pos, - NULL, NULL); + rep->min_percent, pos, NULL); continue; } @@ -669,7 +667,7 @@ static int report__browse_hists(struct report *rep) } ret = evlist__tui_browse_hists(evlist, help, NULL, rep->min_percent, - &session->header.env, true, &annotate_opts); + &session->header.env, true); /* * Usually "ret" is the last pressed key, and we only * care if the key notifies us to switch data file. -- cgit From 7f929aea21fd0be5e0d9ee5827d5b809daa69f29 Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Tue, 28 Nov 2023 09:54:39 -0800 Subject: perf annotate: Ensure init/exit for global options Now it only cares about the global options so it can just handle it without the argument. Reviewed-by: Ian Rogers Signed-off-by: Namhyung Kim Cc: Adrian Hunter Cc: Ingo Molnar Cc: Jiri Olsa Cc: Peter Zijlstra Link: https://lore.kernel.org/r/20231128175441.721579-7-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-report.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'tools/perf/builtin-report.c') diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c index bc0d986c1e0c..17fb171e898b 100644 --- a/tools/perf/builtin-report.c +++ b/tools/perf/builtin-report.c @@ -1430,7 +1430,7 @@ int cmd_report(int argc, const char **argv) */ symbol_conf.keep_exited_threads = true; - annotation_options__init(&annotate_opts); + annotation_options__init(); ret = perf_config(report__config, &report); if (ret) @@ -1464,7 +1464,7 @@ int cmd_report(int argc, const char **argv) return -ENOMEM; } - if (annotate_check_args(&annotate_opts) < 0) { + if (annotate_check_args() < 0) { ret = -EINVAL; goto exit; } @@ -1696,7 +1696,7 @@ repeat: */ symbol_conf.priv_size += sizeof(u32); } - annotation_config__init(&annotate_opts); + annotation_config__init(); } if (symbol__init(&session->header.env) < 0) @@ -1750,7 +1750,7 @@ error: zstd_fini(&(session->zstd_data)); perf_session__delete(session); exit: - annotation_options__exit(&annotate_opts); + annotation_options__exit(); free(sort_order_help); free(field_order_help); return ret; -- cgit From 431be14b193ad19946aeb26a6016dc5b8eb6a248 Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Wed, 6 Dec 2023 17:16:38 -0800 Subject: perf report: Use function to add missing maps lock Switch maps__fprintf_task from loop macro maps__for_each_entry to maps__for_each_map function that takes a callback. The function holds the maps lock, which should be held during iteration. Signed-off-by: Ian Rogers Cc: Adrian Hunter Cc: Alexander Shishkin Cc: Andi Kleen Cc: Athira Rajeev Cc: Changbin Du Cc: Colin Ian King Cc: Dmitrii Dolgov <9erthalion6@gmail.com> Cc: German Gomez Cc: Guilherme Amadio Cc: Huacai Chen Cc: Ingo Molnar Cc: James Clark Cc: Jiri Olsa Cc: K Prateek Nayak Cc: Kajol Jain Cc: Kan Liang Cc: Leo Yan Cc: Li Dong Cc: Liam Howlett Cc: Mark Rutland Cc: Masami Hiramatsu Cc: Miguel Ojeda Cc: Ming Wang Cc: Namhyung Kim Cc: Nick Terrell Cc: Paolo Bonzini Cc: Peter Zijlstra Cc: Ravi Bangoria Cc: Sandipan Das Cc: Sean Christopherson Cc: Steinar H. Gunderson Cc: Vincent Whitchurch Cc: Wenyu Liu Cc: Yang Jihong Link: https://lore.kernel.org/r/20231207011722.1220634-5-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-report.c | 54 +++++++++++++++++++++++++++++++-------------- 1 file changed, 37 insertions(+), 17 deletions(-) (limited to 'tools/perf/builtin-report.c') diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c index 17fb171e898b..178fb602bc98 100644 --- a/tools/perf/builtin-report.c +++ b/tools/perf/builtin-report.c @@ -856,27 +856,47 @@ static struct task *tasks_list(struct task *task, struct machine *machine) return tasks_list(parent_task, machine); } -static size_t maps__fprintf_task(struct maps *maps, int indent, FILE *fp) +struct maps__fprintf_task_args { + int indent; + FILE *fp; + size_t printed; +}; + +static int maps__fprintf_task_cb(struct map *map, void *data) { - size_t printed = 0; - struct map_rb_node *rb_node; + struct maps__fprintf_task_args *args = data; + const struct dso *dso = map__dso(map); + u32 prot = map__prot(map); + int ret; - maps__for_each_entry(maps, rb_node) { - struct map *map = rb_node->map; - const struct dso *dso = map__dso(map); - u32 prot = map__prot(map); + ret = fprintf(args->fp, + "%*s %" PRIx64 "-%" PRIx64 " %c%c%c%c %08" PRIx64 " %" PRIu64 " %s\n", + args->indent, "", map__start(map), map__end(map), + prot & PROT_READ ? 'r' : '-', + prot & PROT_WRITE ? 'w' : '-', + prot & PROT_EXEC ? 'x' : '-', + map__flags(map) ? 's' : 'p', + map__pgoff(map), + dso->id.ino, dso->name); - printed += fprintf(fp, "%*s %" PRIx64 "-%" PRIx64 " %c%c%c%c %08" PRIx64 " %" PRIu64 " %s\n", - indent, "", map__start(map), map__end(map), - prot & PROT_READ ? 'r' : '-', - prot & PROT_WRITE ? 'w' : '-', - prot & PROT_EXEC ? 'x' : '-', - map__flags(map) ? 's' : 'p', - map__pgoff(map), - dso->id.ino, dso->name); - } + if (ret < 0) + return ret; + + args->printed += ret; + return 0; +} + +static size_t maps__fprintf_task(struct maps *maps, int indent, FILE *fp) +{ + struct maps__fprintf_task_args args = { + .indent = indent, + .fp = fp, + .printed = 0, + }; + + maps__for_each_map(maps, maps__fprintf_task_cb, &args); - return printed; + return args.printed; } static void task__print_level(struct task *task, FILE *fp, int level) -- cgit From 81e57deec32594b124d777b1d3ca8a1415410230 Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Tue, 12 Dec 2023 16:13:15 -0800 Subject: perf report: Support data type profiling Enable type annotation when the 'type' sort key is used. It shows type of variables the samples access at the moment. Users can see which types are accessed frequently. $ perf report -s dso,type --stdio ... # Overhead Shared Object Data Type # ........ ................. ......... # 35.47% [kernel.kallsyms] (unknown) 1.62% [kernel.kallsyms] struct sched_entry 1.23% [kernel.kallsyms] struct cfs_rq 0.83% [kernel.kallsyms] struct task_struct 0.34% [kernel.kallsyms] struct list_head 0.30% [kernel.kallsyms] struct mem_cgroup ... Committer testing: With the perf.data file collected in the previous cset: # perf report --stdio -s type # To display the perf.data header info, please use --header/--header-only options. # # # Total Lost Samples: 0 # # Samples: 4 of event 'cpu_atom/mem-loads,ldlat=30/P' # Event count (approx.): 7 # # Overhead Data Type # ........ ......... # 42.86% struct list_head 42.86% (unknown) 14.29% char # # (Tip: To record callchains for each sample: perf record -g) # # perf report --stdio -s dso,type # To display the perf.data header info, please use --header/--header-only options. # # # Total Lost Samples: 0 # # Samples: 4 of event 'cpu_atom/mem-loads,ldlat=30/P' # Event count (approx.): 7 # # Overhead Shared Object Data Type # ........ .................... ......... # 42.86% [kernel.kallsyms] struct list_head 28.57% libc.so.6 (unknown) 14.29% [kernel.kallsyms] char 14.29% ld-linux-x86-64.so.2 (unknown) # # (Tip: Save output of perf stat using: perf stat record ) # # Signed-off-by: Namhyung Kim Tested-by: Arnaldo Carvalho de Melo Cc: Adrian Hunter Cc: Ian Rogers Cc: Ingo Molnar Cc: Jiri Olsa Cc: Linus Torvalds Cc: Masami Hiramatsu Cc: Peter Zijlstra Cc: Stephane Eranian Cc: linux-toolchains@vger.kernel.org Cc: linux-trace-devel@vger.kernel.org Link: https://lore.kernel.org/r/20231213001323.718046-10-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-report.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) (limited to 'tools/perf/builtin-report.c') diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c index 178fb602bc98..f2ed2b7e80a3 100644 --- a/tools/perf/builtin-report.c +++ b/tools/perf/builtin-report.c @@ -96,6 +96,7 @@ struct report { bool stitch_lbr; bool disable_order; bool skip_empty; + bool data_type; int max_stack; struct perf_read_values show_threads_values; const char *pretty_printing_style; @@ -170,7 +171,7 @@ static int hist_iter__report_callback(struct hist_entry_iter *iter, struct mem_info *mi; struct branch_info *bi; - if (!ui__has_annotation() && !rep->symbol_ipc) + if (!ui__has_annotation() && !rep->symbol_ipc && !rep->data_type) return 0; if (sort__mode == SORT_MODE__BRANCH) { @@ -1639,6 +1640,16 @@ repeat: sort_order = NULL; } + if (sort_order && strstr(sort_order, "type")) { + report.data_type = true; + annotate_opts.annotate_src = false; + +#ifndef HAVE_DWARF_GETLOCATIONS_SUPPORT + pr_err("Error: Data type profiling is disabled due to missing DWARF support\n"); + goto error; +#endif + } + if (strcmp(input_name, "-") != 0) setup_browser(true); else @@ -1697,7 +1708,7 @@ repeat: * so don't allocate extra space that won't be used in the stdio * implementation. */ - if (ui__has_annotation() || report.symbol_ipc || + if (ui__has_annotation() || report.symbol_ipc || report.data_type || report.total_cycles_mode) { ret = symbol__annotation_init(); if (ret < 0) -- cgit