* [PATCH v1 1/4] perf tools record: Modernize -W man page
@ 2026-08-13 13:29 Andi Kleen
2026-08-13 13:29 ` [PATCH v1 2/4] perf tools top: Add --weight option Andi Kleen
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Andi Kleen @ 2026-08-13 13:29 UTC (permalink / raw)
To: namhyung; +Cc: acme, linux-perf-users, Andi Kleen
Modernize the -W / --weight description in the manpage to cover more
cases that are supported now.
Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
tools/perf/Documentation/perf-record.txt | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index 178f483140ed..faf6a4419f23 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -480,9 +480,11 @@ Note that this feature may not be available on all processors.
-W::
--weight::
-Enable weightened sampling. An additional weight is recorded per sample and can be
-displayed with the weight and local_weight sort keys. This currently works for TSX
-abort events and some memory events in precise mode on modern Intel CPUs.
+Enable weightened sampling. One or more additional weights can be recorded per sample
+and can be displayed with the weight, weight2, weight3 and local_weight sort keys.
+Possible uses for weights are memory or instruction retirement latencies.
+Support depends on the PMU and the event, as well as its configuration. Typically
+it requires use of 'p'.
--namespaces::
Record events of type PERF_RECORD_NAMESPACES. This enables 'cgroup_id' sort key.
--
2.55.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v1 2/4] perf tools top: Add --weight option 2026-08-13 13:29 [PATCH v1 1/4] perf tools record: Modernize -W man page Andi Kleen @ 2026-08-13 13:29 ` Andi Kleen 2026-08-13 13:53 ` sashiko-bot 2026-08-13 13:29 ` [PATCH v1 3/4] perf tools: Add support for displaying weights in annotate Andi Kleen ` (2 subsequent siblings) 3 siblings, 1 reply; 8+ messages in thread From: Andi Kleen @ 2026-08-13 13:29 UTC (permalink / raw) To: namhyung; +Cc: acme, linux-perf-users, Andi Kleen Add a -W/--weight option to perf top to collect weights too. Useful with follow on patches. Assisted-by: omp:GPT-5.6-Luna Signed-off-by: Andi Kleen <ak@linux.intel.com> --- tools/perf/Documentation/perf-top.txt | 6 ++++++ tools/perf/builtin-top.c | 2 ++ 2 files changed, 8 insertions(+) diff --git a/tools/perf/Documentation/perf-top.txt b/tools/perf/Documentation/perf-top.txt index af3e4230c72f..2da2a16bbf26 100644 --- a/tools/perf/Documentation/perf-top.txt +++ b/tools/perf/Documentation/perf-top.txt @@ -25,6 +25,12 @@ OPTIONS --count=<count>:: Event period to sample. +-W:: +--weight:: +Enable weighted sampling. One or more additional weights can be recorded per sample +and can be displayed with the weight, weight2, weight3 and local_weight sort keys. +Possible uses for weights are memory or instruction retirement latencies. + -C <cpu-list>:: --cpu=<cpu>:: Monitor only on the list of CPUs provided. Multiple CPUs can be provided as a diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c index 570410599f1b..2888ac3b7b92 100644 --- a/tools/perf/builtin-top.c +++ b/tools/perf/builtin-top.c @@ -1493,6 +1493,8 @@ int cmd_top(int argc, const char **argv) OPT_CALLBACK(0, "filter", &top.evlist, "filter", "event filter", parse_filter), OPT_U64('c', "count", &opts->user_interval, "event period to sample"), + OPT_BOOLEAN('W', "weight", &opts->sample_weight, + "sample by weight (on special events only)"), OPT_STRING('p', "pid", &target->pid, "pid", "profile events on existing process id"), OPT_STRING('t', "tid", &target->tid, "tid", -- 2.55.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v1 2/4] perf tools top: Add --weight option 2026-08-13 13:29 ` [PATCH v1 2/4] perf tools top: Add --weight option Andi Kleen @ 2026-08-13 13:53 ` sashiko-bot 0 siblings, 0 replies; 8+ messages in thread From: sashiko-bot @ 2026-08-13 13:53 UTC (permalink / raw) To: Andi Kleen; +Cc: linux-perf-users > Add a -W/--weight option to perf top to collect weights too. Useful with > follow on patches. > > Assisted-by: omp:GPT-5.6-Luna > Signed-off-by: Andi Kleen <ak@linux.intel.com> Sashiko has reviewed this patch and found no issues. It looks great! -- Sashiko AI review · https://sashiko.dev/#/patchset/20260813132907.50738-1-ak@linux.intel.com?part=2 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v1 3/4] perf tools: Add support for displaying weights in annotate 2026-08-13 13:29 [PATCH v1 1/4] perf tools record: Modernize -W man page Andi Kleen 2026-08-13 13:29 ` [PATCH v1 2/4] perf tools top: Add --weight option Andi Kleen @ 2026-08-13 13:29 ` Andi Kleen 2026-08-13 14:04 ` sashiko-bot 2026-08-13 13:29 ` [PATCH v1 4/4] perf tools: Add test for weight annotation Andi Kleen 2026-08-13 13:47 ` [PATCH v1 1/4] perf tools record: Modernize -W man page sashiko-bot 3 siblings, 1 reply; 8+ messages in thread From: Andi Kleen @ 2026-08-13 13:29 UTC (permalink / raw) To: namhyung; +Cc: acme, linux-perf-users, Andi Kleen Add support for showing all the three possible per IP weights in annotate. The weights are shown by defaults if any are non zero. This is useful, especially with the new insn lat statistics, but also for all the existing weights. Add a hotkey to the interactive browser to turn them off (w), as well as a perf annotate command line option. The weights are stored unconditionally in the sym_hist_entry, which will increase memory consumption somewhat. Assisted-by: omp:GPT-5.6-Luna Signed-off-by: Andi Kleen <ak@linux.intel.com> --- tools/perf/Documentation/perf-annotate.txt | 3 + tools/perf/builtin-annotate.c | 4 + tools/perf/builtin-report.c | 2 + tools/perf/builtin-top.c | 1 + tools/perf/ui/browsers/annotate.c | 13 ++- tools/perf/util/annotate.c | 94 +++++++++++++++++++--- tools/perf/util/annotate.h | 6 +- tools/perf/util/symbol.c | 11 +++ tools/perf/util/symbol_conf.h | 18 ++++- 9 files changed, 140 insertions(+), 12 deletions(-) diff --git a/tools/perf/Documentation/perf-annotate.txt b/tools/perf/Documentation/perf-annotate.txt index a688738809c4..50c6ea680d0d 100644 --- a/tools/perf/Documentation/perf-annotate.txt +++ b/tools/perf/Documentation/perf-annotate.txt @@ -79,6 +79,9 @@ include::itrace.txt[] --stdio:: Use the stdio interface. +--no-weights:: Hide weight columns in annotation output. By default, weight columns + are shown when samples contain non-zero weights. + --stdio2:: Use the stdio2 interface, non-interactive, uses the TUI formatting. --stdio-color=<mode>:: diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c index 69cb72b2082a..a78decad3f40 100644 --- a/tools/perf/builtin-annotate.c +++ b/tools/perf/builtin-annotate.c @@ -259,6 +259,8 @@ static int add_sample(struct perf_sample *sample, return 0; } + symbol_conf__set_weight_mode(&symbol_conf, sample); + /* * XXX filtered samples can still have branch entries pointing into our * symbol and are missed. @@ -719,6 +721,8 @@ int cmd_annotate(int argc, const char **argv) OPT_BOOLEAN(0, "tui", &annotate.use_tui, "Use the TUI interface"), #endif OPT_BOOLEAN(0, "stdio", &annotate.use_stdio, "Use the stdio interface"), + OPT_BOOLEAN(0, "no-weights", &symbol_conf.hide_weight, + "Hide weight columns in annotation"), OPT_BOOLEAN(0, "stdio2", &annotate.use_stdio2, "Use the stdio interface"), OPT_BOOLEAN(0, "ignore-vmlinux", &symbol_conf.ignore_vmlinux, "don't load vmlinux even if found"), diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c index 60d1f166629e..5ee7bc62b6b6 100644 --- a/tools/perf/builtin-report.c +++ b/tools/perf/builtin-report.c @@ -329,6 +329,8 @@ static int process_sample_event(const struct perf_tool *tool, &rep->total_cycles); } + symbol_conf__set_weight_mode(&symbol_conf, sample); + rep->total_samples++; if (al.parallelism == 1) rep->singlethreaded_samples++; diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c index 2888ac3b7b92..32bd6e23f6fd 100644 --- a/tools/perf/builtin-top.c +++ b/tools/perf/builtin-top.c @@ -827,6 +827,7 @@ static void perf_event__process_sample(const struct perf_tool *tool, top->vmlinux_warned = true; } } + symbol_conf__set_weight_mode(&symbol_conf, sample); if (al.sym == NULL || !symbol__is_idle(al.sym, al.map ? map__dso(al.map) : NULL, machine->env)) { diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c index e47a46775089..dada5fa6d29f 100644 --- a/tools/perf/ui/browsers/annotate.c +++ b/tools/perf/ui/browsers/annotate.c @@ -972,6 +972,7 @@ static int annotate_browser__run(struct annotate_browser *browser, "O Bump offset level (jump targets -> +call -> all -> cycle thru)\n" "s Toggle source code view\n" "t Circulate percent, total period, samples view\n" + "w Toggle weight columns\n" "c Show min/max cycle\n" "/ Search string\n" "k Toggle line numbers\n" @@ -1091,6 +1092,14 @@ static int annotate_browser__run(struct annotate_browser *browser, symbol_conf.show_total_period = true; annotation__update_column_widths(notes); continue; + case 'w': + symbol_conf.hide_weight = !symbol_conf.hide_weight; + symbol_conf.weight_mask = symbol_conf.hide_weight ? 0 : + annotation__histogram(notes, evsel)->weight_mask; + browser->b.width = notes->src->widths.max_line_len + + annotation__pcnt_width(notes); + ui_browser__refresh_dimensions(&browser->b); + continue; case 'c': if (annotate_opts.show_minmax_cycle) annotate_opts.show_minmax_cycle = false; @@ -1225,10 +1234,12 @@ int __hist_entry__tui_annotate(struct hist_entry *he, struct map_symbol *ms, browser.type_hash = hashmap__new(type_hash, type_equal, /*ctx=*/NULL); } + symbol_conf.weight_mask = symbol_conf.hide_weight ? 0 : + annotation__histogram(notes, evsel)->weight_mask; browser.b.width = notes->src->widths.max_line_len; browser.b.nr_entries = notes->src->nr_entries; browser.b.entries = ¬es->src->source; - browser.b.width += 18; /* Percentage */ + browser.b.width += annotation__pcnt_width(notes); if (annotate_opts.hide_src_code) ui_browser__init_asm_mode(&browser.b); diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c index df70e95a8470..6be6ec47f298 100644 --- a/tools/perf/util/annotate.c +++ b/tools/perf/util/annotate.c @@ -222,6 +222,7 @@ static int __symbol__inc_addr_samples(struct map_symbol *ms, u64 offset; struct sym_hist *h; struct sym_hist_entry *entry; + u64 weight = sample->weight ?: sample->ins_lat ?: sample->weight3; pr_debug3("%s: addr=%#" PRIx64 "\n", __func__, map__unmap_ip(ms->map, addr)); @@ -256,10 +257,28 @@ static int __symbol__inc_addr_samples(struct map_symbol *ms, entry->nr_samples++; entry->period += sample->period; + if (sample->evsel->core.attr.sample_type & PERF_SAMPLE_WEIGHT_TYPE) { + entry->weight_sum[WEIGHT_WEIGHT] += sample->weight; + entry->weight_num[WEIGHT_WEIGHT]++; + if (sample->weight) + h->weight_mask |= BIT(WEIGHT_WEIGHT); + } + if (sample->evsel->core.attr.sample_type & PERF_SAMPLE_WEIGHT_STRUCT) { + entry->weight_sum[WEIGHT_INSNLAT] += sample->ins_lat; + entry->weight_num[WEIGHT_INSNLAT]++; + entry->weight_sum[WEIGHT_WEIGHT3] += sample->weight3; + entry->weight_num[WEIGHT_WEIGHT3]++; + if (sample->ins_lat) + h->weight_mask |= BIT(WEIGHT_INSNLAT); + if (sample->weight3) + h->weight_mask |= BIT(WEIGHT_WEIGHT3); + } + pr_debug3("%#" PRIx64 " %s: period++ [addr: %#" PRIx64 ", %#" PRIx64 - ", evidx=%d] => nr_samples: %" PRIu64 ", period: %" PRIu64 "\n", + ", evidx=%d] => nr_samples: %" PRIu64 ", period: %" PRIu64 + " weight %" PRIu64 "\n", sym->start, sym->name, addr, addr - sym->start, evsel->core.idx, - entry->nr_samples, entry->period); + entry->nr_samples, entry->period, weight); return 0; } @@ -778,6 +797,34 @@ static bool needs_type_info(struct annotated_data_type *data_type) return (data_type != &stackop_type) && (data_type != &canary_type); } +static const char *annotation__weight_mode_str(enum symbol__weight_mode mode) +{ + switch (mode) { + case WEIGHT_NONE: + return ""; + case WEIGHT_WEIGHT: + return "Weight"; + case WEIGHT_INSNLAT: + return "InsnLat"; + case WEIGHT_WEIGHT3: + return "Weight3"; + default: + return ""; + } +} + +static void annotation__column_title(char *buf, size_t size) +{ + const char *base = symbol_conf.show_total_period ? "Period" : + symbol_conf.show_nr_samples ? "Samples" : "Percent"; + int i; + scnprintf(buf, size, "%s", base); + for_each_weight(i) + if (symbol_conf.weight_mask & BIT(i)) + scnprintf(buf + strlen(buf), size - strlen(buf), " %s", + annotation__weight_mode_str(i)); +} + static int annotation_line__print(struct annotation_line *al, struct annotation_print_data *apd, struct annotation_options *opts, int printed, @@ -845,6 +892,12 @@ annotation_line__print(struct annotation_line *al, struct annotation_print_data data->he.nr_samples); else color_fprintf(stdout, color, " %7.2f", percent); + for_each_weight(i) { + if (symbol_conf.weight_mask & BIT(i)) + color_fprintf(stdout, color, " %7" PRIu64, + data->he.weight_num[i] ? + data->he.weight_sum[i] / data->he.weight_num[i] : 0); + } } printf(" : "); @@ -915,6 +968,9 @@ static void calc_percent(struct annotation *notes, struct sym_hist *sym_hist = annotation__histogram(notes, evsel); unsigned int hits = 0; u64 period = 0; + int i; + u64 weight_sum[WEIGHT_WEIGHT3 + 1] = { 0 }; + u64 weight_num[WEIGHT_WEIGHT3 + 1] = { 0 }; while (offset < end) { struct sym_hist_entry *entry; @@ -923,6 +979,10 @@ static void calc_percent(struct annotation *notes, if (entry) { hits += entry->nr_samples; period += entry->period; + for_each_weight(i) { + weight_sum[i] += entry->weight_sum[i]; + weight_num[i] += entry->weight_num[i]; + } } ++offset; } @@ -930,6 +990,10 @@ static void calc_percent(struct annotation *notes, if (sym_hist->nr_samples) { data->he.period = period; data->he.nr_samples = hits; + for_each_weight(i) { + data->he.weight_sum[i] = weight_sum[i]; + data->he.weight_num[i] = weight_num[i]; + } data->percent[PERCENT_HITS_LOCAL] = 100.0 * hits / sym_hist->nr_samples; } @@ -1252,9 +1316,12 @@ int hist_entry__annotate_printf(struct hist_entry *he, struct evsel *evsel) int printed = 2, queue_len = 0; int more = 0; bool context = opts->context; - int width = annotation__pcnt_width(notes); + int width; int graph_dotted_len; + char title[64]; char buf[512]; + symbol_conf.weight_mask = symbol_conf.hide_weight ? 0 : h->weight_mask; + width = annotation__pcnt_width(notes); filename = strdup(dso__long_name(dso)); if (!filename) @@ -1275,10 +1342,10 @@ int hist_entry__annotate_printf(struct hist_entry *he, struct evsel *evsel) return ENOTSUP; } - graph_dotted_len = printf(" %-*.*s| Source code & Disassembly of %s for %s (%" PRIu64 " samples, " + annotation__column_title(title, sizeof(title)); + graph_dotted_len = printf(" %-*.*s|\tSource code & Disassembly of %s for %s (%" PRIu64 " samples, " "percent: %s)\n", - width, width, symbol_conf.show_total_period ? "Period" : - symbol_conf.show_nr_samples ? "Samples" : "Percent", + width, width, title, d_filename, evsel_name, h->nr_samples, percent_type_str(opts->percent_type)); @@ -2062,6 +2129,7 @@ void annotation_line__write(struct annotation_line *al, struct annotation *notes for (i = 0; i < al->data_nr; i++) { double percent; + int weight; percent = annotation_data__percent(&al->data[i], annotate_opts.percent_type); @@ -2075,6 +2143,14 @@ void annotation_line__write(struct annotation_line *al, struct annotation *notes } else { obj__printf(obj, "%7.2f ", percent); } + + for_each_weight(weight) { + if (symbol_conf.weight_mask & BIT(weight)) + obj__printf(obj, "%7" PRIu64 " ", + al->data[i].he.weight_num[weight] ? + al->data[i].he.weight_sum[weight] / + al->data[i].he.weight_num[weight] : 0); + } } } else { obj__set_percent_color(obj, 0, current_entry); @@ -2082,9 +2158,9 @@ void annotation_line__write(struct annotation_line *al, struct annotation *notes if (!show_title) obj__printf(obj, "%-*s", pcnt_width, " "); else { - obj__printf(obj, "%-*s", pcnt_width, - symbol_conf.show_total_period ? "Period" : - symbol_conf.show_nr_samples ? "Samples" : "Percent"); + char buf[64]; + annotation__column_title(buf, sizeof(buf)); + obj__printf(obj, "%-*s", pcnt_width, buf); } } width -= pcnt_width; diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h index fa08d09b80f7..0880605b4e1d 100644 --- a/tools/perf/util/annotate.h +++ b/tools/perf/util/annotate.h @@ -86,6 +86,8 @@ struct annotation; struct sym_hist_entry { u64 nr_samples; u64 period; + u64 weight_sum[WEIGHT_WEIGHT3 + 1]; + u64 weight_num[WEIGHT_WEIGHT3 + 1]; }; enum { @@ -231,6 +233,7 @@ void symbol__calc_percent(struct symbol *sym, struct evsel *evsel); struct sym_hist { u64 nr_samples; u64 period; + u8 weight_mask; }; /** @@ -378,7 +381,8 @@ static inline int annotation__cycles_width(struct annotation *notes) static inline int annotation__pcnt_width(struct annotation *notes) { - return (symbol_conf.show_total_period ? 12 : 8) * notes->src->nr_events; + int extra = __builtin_popcount(symbol_conf.weight_mask) * 8; + return ((symbol_conf.show_total_period ? 12 : 8) + extra) * notes->src->nr_events; } static inline bool annotation_line__filter(struct annotation_line *al) diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c index 94f9c8faedda..4b04320e35ec 100644 --- a/tools/perf/util/symbol.c +++ b/tools/perf/util/symbol.c @@ -2763,3 +2763,14 @@ char *dso__demangle_sym(struct dso *dso, int kmodule, const char *elf_name) return java_demangle_sym(elf_name, JAVA_DEMANGLE_NORET); } + +void symbol_conf__set_weight_mode(struct symbol_conf *c, + struct perf_sample *sample) +{ + if (sample->weight) + c->weight_mask |= BIT(WEIGHT_WEIGHT); + if (sample->ins_lat) + c->weight_mask |= BIT(WEIGHT_INSNLAT); + if (sample->weight3) + c->weight_mask |= BIT(WEIGHT_WEIGHT3); +} diff --git a/tools/perf/util/symbol_conf.h b/tools/perf/util/symbol_conf.h index 0dee5aa6a534..36328edb635b 100644 --- a/tools/perf/util/symbol_conf.h +++ b/tools/perf/util/symbol_conf.h @@ -27,6 +27,16 @@ enum a2l_style { }; #define MAX_A2L_STYLE (A2L_STYLE_CMD + 1) +enum symbol__weight_mode { + WEIGHT_NONE = 0, + WEIGHT_WEIGHT, + WEIGHT_INSNLAT, + WEIGHT_WEIGHT3, +}; + +#define for_each_weight(_weight) \ + for ((_weight) = WEIGHT_WEIGHT; (_weight) <= WEIGHT_WEIGHT3; (_weight)++) + struct symbol_conf { bool nanosecs; unsigned short priv_size; @@ -69,7 +79,9 @@ struct symbol_conf { annotate_data_sample, skip_empty, enable_latency, - prefer_latency; + prefer_latency, + hide_weight; + u8 weight_mask; const char *vmlinux_name, *kallsyms_name, *source_prefix, @@ -114,4 +126,8 @@ struct symbol_conf { extern struct symbol_conf symbol_conf; +struct perf_sample; +void symbol_conf__set_weight_mode(struct symbol_conf *c, + struct perf_sample *sample); + #endif // __PERF_SYMBOL_CONF -- 2.55.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v1 3/4] perf tools: Add support for displaying weights in annotate 2026-08-13 13:29 ` [PATCH v1 3/4] perf tools: Add support for displaying weights in annotate Andi Kleen @ 2026-08-13 14:04 ` sashiko-bot 0 siblings, 0 replies; 8+ messages in thread From: sashiko-bot @ 2026-08-13 14:04 UTC (permalink / raw) To: Andi Kleen; +Cc: linux-perf-users Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] Variable shadowing by `for_each_weight` macro in `annotation_line__print` causes either premature loop termination or an infinite loop. - [Medium] Asynchronous global modification of `symbol_conf.weight_mask` by the background thread in `perf top` causes data races and defeats the user's hotkey toggle. -- --- Patch [3]: [PATCH v1 3/4] perf tools: Add support for displaying weights in annotate --- commit 7947c9d694355b6721ccd7944dc41891a9968db4 Author: Andi Kleen <ak@linux.intel.com> Date: Thu Aug 13 13:29:06 2026 +0000 perf tools: Add support for displaying weights in annotate Add support for showing all the three possible per IP weights in annotate. The weights are shown by defaults if any are non zero. This is useful, especially with the new insn lat statistics, but also for all the existing weights. > diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c > index 2888ac3b7b926..32bd6e23f6fdd 100644 > --- a/tools/perf/builtin-top.c > +++ b/tools/perf/builtin-top.c > @@ -827,6 +827,7 @@ static void perf_event__process_sample(const struct perf_tool *tool, > top->vmlinux_warned = true; > } > } > + symbol_conf__set_weight_mode(&symbol_conf, sample); > > if (al.sym == NULL || > !symbol__is_idle(al.sym, al.map ? map__dso(al.map) : NULL, machine->env)) { [ ... ] > diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c > index e47a467750890..dada5fa6d29f2 100644 > --- a/tools/perf/ui/browsers/annotate.c > +++ b/tools/perf/ui/browsers/annotate.c [ ... ] > @@ -1091,6 +1092,14 @@ static int annotate_browser__run(struct annotate_browser *browser, > symbol_conf.show_total_period = true; > annotation__update_column_widths(notes); > continue; > + case 'w': > + symbol_conf.hide_weight = !symbol_conf.hide_weight; > + symbol_conf.weight_mask = symbol_conf.hide_weight ? 0 : > + annotation__histogram(notes, evsel)->weight_mask; > + browser->b.width = notes->src->widths.max_line_len + > + annotation__pcnt_width(notes); > + ui_browser__refresh_dimensions(&browser->b); > + continue; [ ... ] > diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c > index 94f9c8faeddab..4b04320e35ecb 100644 > --- a/tools/perf/util/symbol.c > +++ b/tools/perf/util/symbol.c [ ... ] > @@ -2763,3 +2763,14 @@ char *dso__demangle_sym(struct dso *dso, int kmodule, const char *elf_name) > > return java_demangle_sym(elf_name, JAVA_DEMANGLE_NORET); > } > + > +void symbol_conf__set_weight_mode(struct symbol_conf *c, > + struct perf_sample *sample) > +{ > + if (sample->weight) > + c->weight_mask |= BIT(WEIGHT_WEIGHT); > + if (sample->ins_lat) > + c->weight_mask |= BIT(WEIGHT_INSNLAT); > + if (sample->weight3) > + c->weight_mask |= BIT(WEIGHT_WEIGHT3); > +} [Severity: Medium] In perf top, event processing runs in a background thread while the UI runs in the foreground. When a user presses the 'w' hotkey, it sets symbol_conf.weight_mask to 0. However, the background thread constantly calls symbol_conf__set_weight_mode() for new samples, which unconditionally modifies symbol_conf.weight_mask without checking if symbol_conf.hide_weight is set. Could this data race cause the TUI to tear or miscalculate column widths, effectively defeating the user's attempt to hide the weight columns? [ ... ] > diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c > index df70e95a84704..6be6ec47f298b 100644 > --- a/tools/perf/util/annotate.c > +++ b/tools/perf/util/annotate.c [ ... ] > @@ -845,6 +892,12 @@ annotation_line__print(struct annotation_line *al, struct annotation_print_data > data->he.nr_samples); > else > color_fprintf(stdout, color, " %7.2f", percent); > + for_each_weight(i) { > + if (symbol_conf.weight_mask & BIT(i)) > + color_fprintf(stdout, color, " %7" PRIu64, > + data->he.weight_num[i] ? > + data->he.weight_sum[i] / data->he.weight_num[i] : 0); > + } > } [Severity: Medium] Will this for_each_weight(i) macro silently shadow and corrupt the outer loop's iterator variable 'i'? The outer loop in annotation_line__print() uses 'i' to iterate up to nr_percent: for (i = 0; i < nr_percent; i++) { struct annotation_data *data = &al->data[i]; ... for_each_weight(i) { ... } } Since for_each_weight() expands to a for loop that reuses and resets 'i' from WEIGHT_WEIGHT to WEIGHT_WEIGHT3, won't this cause either premature loop termination (if nr_percent <= 5) or an infinite loop (if nr_percent > 5) when processing event groups? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260813132907.50738-1-ak@linux.intel.com?part=3 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v1 4/4] perf tools: Add test for weight annotation 2026-08-13 13:29 [PATCH v1 1/4] perf tools record: Modernize -W man page Andi Kleen 2026-08-13 13:29 ` [PATCH v1 2/4] perf tools top: Add --weight option Andi Kleen 2026-08-13 13:29 ` [PATCH v1 3/4] perf tools: Add support for displaying weights in annotate Andi Kleen @ 2026-08-13 13:29 ` Andi Kleen 2026-08-13 14:18 ` sashiko-bot 2026-08-13 13:47 ` [PATCH v1 1/4] perf tools record: Modernize -W man page sashiko-bot 3 siblings, 1 reply; 8+ messages in thread From: Andi Kleen @ 2026-08-13 13:29 UTC (permalink / raw) To: namhyung; +Cc: acme, linux-perf-users, Andi Kleen Add a test case to check if --stdio weight annotation works Assisted-by: omp:GPT-5.6-Luna Signed-off-by: Andi Kleen <ak@linux.intel.com> --- tools/perf/tests/shell/annotate_weight.sh | 63 +++++++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100755 tools/perf/tests/shell/annotate_weight.sh diff --git a/tools/perf/tests/shell/annotate_weight.sh b/tools/perf/tests/shell/annotate_weight.sh new file mode 100755 index 000000000000..6b8c105c048b --- /dev/null +++ b/tools/perf/tests/shell/annotate_weight.sh @@ -0,0 +1,63 @@ +#!/bin/bash +# perf annotate weight regression test +# SPDX-License-Identifier: GPL-2.0 + +set -e + +shelldir=$(dirname "$0") +# shellcheck source=tools/perf/tests/shell/lib/perf_has_symbol.sh +. "${shelldir}"/lib/perf_has_symbol.sh + +testsym="test_loop" +skip_test_missing_symbol "${testsym}" + +perfdata=$(mktemp /tmp/__perf_test.annotate_weight.XXXXX) +record_log=$(mktemp /tmp/__perf_test.annotate_weight.XXXXX.log) +report_out=$(mktemp /tmp/__perf_test.annotate_weight.XXXXX.report) +annotate_out=$(mktemp /tmp/__perf_test.annotate_weight.XXXXX.annotate) + +cleanup() { + rm -f "${perfdata}" "${record_log}" "${report_out}" "${annotate_out}" + trap - EXIT TERM INT +} + +trap 'cleanup; exit 1' TERM INT +trap cleanup EXIT + +# mem-loads:pu requests a precise user PEBS event whose sample weight should +# be populated by -W. Unsupported PEBS/weight PMUs are skipped below. +if ! perf record -W -e mem-loads:pu -o "${perfdata}" -- perf test -w thloop \ + > /dev/null 2> "${record_log}"; then + echo "[SKIP] precise PEBS weight sampling is unavailable" + exit 2 +fi + +# Confirm the PMU actually produced nonzero weights. A successful record alone +# is insufficient: some PMUs accept the event but provide no weight payload. +if ! perf report --stdio -i "${perfdata}" --fields=weight1,weight2,weight3,symbol --percent-limit 0 \ + > "${report_out}" 2> "${record_log}"; then + echo "[SKIP] weighted samples cannot be decoded" + exit 2 +fi + +if ! awk '$1 ~ /^[0-9]/ && ($1 + 0) > 0 { found = 1 } END { exit !found }' \ + "${report_out}"; then + echo "[SKIP] PEBS weight sampling produced no nonzero weights" + exit 2 +fi + +perf annotate --stdio -i "${perfdata}" --symbol "${testsym}" \ + > "${annotate_out}" 2> "${record_log}" + +grep -q 'Percent Weight' "${annotate_out}" + +# The second numeric column is the rendered average weight. This assertion +# fails on the original regression because calc_percent() left it at zero. +if ! awk '$1 ~ /^[0-9]/ && $2 ~ /^[0-9]/ && ($2 + 0) > 0 { found = 1 } END { exit !found }' \ + "${annotate_out}"; then + echo "Annotation output contained no nonzero weight" + cat "${annotate_out}" + exit 1 +fi + +echo "PEBS annotation weights: PASS" -- 2.55.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v1 4/4] perf tools: Add test for weight annotation 2026-08-13 13:29 ` [PATCH v1 4/4] perf tools: Add test for weight annotation Andi Kleen @ 2026-08-13 14:18 ` sashiko-bot 0 siblings, 0 replies; 8+ messages in thread From: sashiko-bot @ 2026-08-13 14:18 UTC (permalink / raw) To: Andi Kleen; +Cc: linux-perf-users Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] Background thread in perf top continually overwrites the TUI user preference for displaying weight columns due to an unprotected data race. -- --- Patch [4]: [PATCH v1 4/4] perf tools: Add test for weight annotation --- commit 7947c9d694355b6721ccd7944dc41891a9968db4 Author: Andi Kleen <ak@linux.intel.com> perf tools: Add support for displaying weights in annotate Add support for showing all the three possible per IP weights in annotate. The weights are shown by defaults if any are non zero. This is useful, especially with the new insn lat statistics, but also for all the existing weights. > diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c > index 2888ac3b7b926..32bd6e23f6fdd 100644 > --- a/tools/perf/builtin-top.c > +++ b/tools/perf/builtin-top.c > @@ -827,6 +827,7 @@ static void perf_event__process_sample(const struct perf_tool *tool, > top->vmlinux_warned = true; > } > } > + symbol_conf__set_weight_mode(&symbol_conf, sample); [Severity: Medium] Will this background sample processing thread concurrently overwrite the TUI user preference for displaying weight columns? The TUI thread modifies the global symbol_conf structure when the user attempts to hide the weight columns. If the background thread is continually calling symbol_conf__set_weight_mode() on every sample without locking, it appears the user's preference could get immediately overridden, causing the TUI to flicker or misalign columns. [ ... ] > diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c > index e47a467750890..dada5fa6d29f2 100644 > --- a/tools/perf/ui/browsers/annotate.c > +++ b/tools/perf/ui/browsers/annotate.c [ ... ] > @@ -1091,6 +1092,14 @@ static int annotate_browser__run(struct annotate_browser *browser, > symbol_conf.show_total_period = true; > annotation__update_column_widths(notes); > continue; > + case 'w': > + symbol_conf.hide_weight = !symbol_conf.hide_weight; > + symbol_conf.weight_mask = symbol_conf.hide_weight ? 0 : > + annotation__histogram(notes, evsel)->weight_mask; [Severity: Medium] Could this unprotected update to the global symbol_conf.weight_mask race with the background thread's read-modify-write operations? [ ... ] > diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c > index 94f9c8faeddab..4b04320e35ecb 100644 > --- a/tools/perf/util/symbol.c > +++ b/tools/perf/util/symbol.c [ ... ] > +void symbol_conf__set_weight_mode(struct symbol_conf *c, > + struct perf_sample *sample) > +{ > + if (sample->weight) > + c->weight_mask |= BIT(WEIGHT_WEIGHT); [Severity: Medium] Does this unprotected read-modify-write on the global symbol_conf structure corrupt the state when called concurrently with the TUI thread? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260813132907.50738-1-ak@linux.intel.com?part=4 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1 1/4] perf tools record: Modernize -W man page 2026-08-13 13:29 [PATCH v1 1/4] perf tools record: Modernize -W man page Andi Kleen ` (2 preceding siblings ...) 2026-08-13 13:29 ` [PATCH v1 4/4] perf tools: Add test for weight annotation Andi Kleen @ 2026-08-13 13:47 ` sashiko-bot 3 siblings, 0 replies; 8+ messages in thread From: sashiko-bot @ 2026-08-13 13:47 UTC (permalink / raw) To: Andi Kleen; +Cc: linux-perf-users Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] The patch introduces a double space in the updated text for the `--weight` option ('weight3 and local_weight'). Additionally, it propagates the pre-existing typo 'weightened' (which should likely be 'weighted') in the same modified block. -- --- Patch [1]: [PATCH v1 1/4] perf tools record: Modernize -W man page --- commit 3b23df9a5d2c30f9e6db8d77004cb5c3a03f441d Author: Andi Kleen <ak@linux.intel.com> perf tools record: Modernize -W man page This commit updates the -W / --weight description in the perf-record man page to cover newly supported cases and configurations. > diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt > --- a/tools/perf/Documentation/perf-record.txt > +++ b/tools/perf/Documentation/perf-record.txt > @@ -480,9 +480,11 @@ Note that this feature may not be available on all processors. > > -W:: > --weight:: > -Enable weightened sampling. An additional weight is recorded per sample and can be > -displayed with the weight and local_weight sort keys. This currently works for TSX > -abort events and some memory events in precise mode on modern Intel CPUs. > +Enable weightened sampling. One or more additional weights can be recorded per sample > +and can be displayed with the weight, weight2, weight3 and local_weight sort keys. [Severity: Low] This isn't a bug, but it looks like a double space was introduced between weight3 and the word and. Additionally, this is a pre-existing issue, but the phrase Enable weightened sampling should likely be Enable weighted sampling. Since this block of text is being modified, would it make sense to correct the typo here? > +Possible uses for weights are memory or instruction retirement latencies. > +Support depends on the PMU and the event, as well as its configuration. Typically > +it requires use of 'p'. -- Sashiko AI review · https://sashiko.dev/#/patchset/20260813132907.50738-1-ak@linux.intel.com?part=1 ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-08-13 14:18 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-08-13 13:29 [PATCH v1 1/4] perf tools record: Modernize -W man page Andi Kleen 2026-08-13 13:29 ` [PATCH v1 2/4] perf tools top: Add --weight option Andi Kleen 2026-08-13 13:53 ` sashiko-bot 2026-08-13 13:29 ` [PATCH v1 3/4] perf tools: Add support for displaying weights in annotate Andi Kleen 2026-08-13 14:04 ` sashiko-bot 2026-08-13 13:29 ` [PATCH v1 4/4] perf tools: Add test for weight annotation Andi Kleen 2026-08-13 14:18 ` sashiko-bot 2026-08-13 13:47 ` [PATCH v1 1/4] perf tools record: Modernize -W man page sashiko-bot
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.