From: Namhyung Kim <namhyung@kernel.org>
To: haghdoost@uber.com
Cc: Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>,
Arnaldo Carvalho de Melo <acme@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Jiri Olsa <jolsa@kernel.org>, Ian Rogers <irogers@google.com>,
Adrian Hunter <adrian.hunter@intel.com>,
James Clark <james.clark@linaro.org>,
Andrii Nakryiko <andriin@fb.com>,
Alexei Starovoitov <ast@kernel.org>,
linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/4] perf script: Add --max-symbol-bytes to bound ELF symbol memory
Date: Wed, 16 Sep 2026 23:53:49 -0700 [thread overview]
Message-ID: <aquOfdBVYlr1QE8Z@z2> (raw)
In-Reply-To: <20260915-perf-symbol-memory-send-v1-2-1d3360e21f07@uber.com>
On Tue, Sep 15, 2026 at 11:42:44AM -0700, Alireza Haghdoost via B4 Relay wrote:
> From: Alireza Haghdoost <haghdoost@uber.com>
>
> perf script eagerly materializes every ELF symbol into an rb-tree kept
> until process exit. Large profiles can therefore consume substantial
> anonymous memory, causing perf script to be OOM-killed or forcing the
> kernel to reclaim memory from co-located workloads.
It's not only for perf script. Other commands like perf report and perf
annotate would have the same issue.
>
> This patch adds --max-symbol-bytes to bound struct symbol allocations.
> Once the budget is reached, the ELF loader stops loading symbols, warns
> once, and lets unresolved addresses appear as [unknown].
> This allows users to bound the memory footprint upfront and explicitly
> choose between complete symbolization and avoiding unbounded host memory
> pressure. perf record already provides a similar --max-size option to
> bound disk usage.
Please add documentation when you add a new command line option.
>
> The counter includes every symbol__new() allocation, but this patch
> enforces the limit only in the ELF loader, which is the source of the
> unbounded memory growth addressed here. In this path, reaching the limit
> can safely produce [unknown] symbols. Other loaders currently treat a
> failed symbol allocation as an error. Capping those paths would therefore
> require separate changes whose complexity may outweigh the potential
> memory savings.
What's the other loaders you meant?
I don't see it produces [unknown] symbols. Do you mean it just stops
loading symbols when it hits the limit?
>
> Sizes require a B/K/M/G suffix. Zero, the default, means unlimited.
>
> Signed-off-by: Alireza Haghdoost <haghdoost@uber.com>
> Assisted-by: Kimi:K3
> ---
> tools/perf/builtin-script.c | 31 +++++++++++++++++++++++++++++++
> tools/perf/util/symbol-elf.c | 7 +++++++
> tools/perf/util/symbol.c | 29 +++++++++++++++++++++++++++--
> tools/perf/util/symbol.h | 3 +++
> tools/perf/util/symbol_conf.h | 1 +
> 5 files changed, 69 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index ad8ca08ceb5f..50fa6ca6455a 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -68,6 +68,7 @@
> #include "util/thread.h"
> #include "util/thread_map.h"
> #include "util/time-utils.h"
> +#include "util/units.h"
> #include "util/tool.h"
> #include "util/trace-event.h"
> #include "util/unwind.h"
> @@ -4035,6 +4036,33 @@ static int parse_callret_trace(const struct option *opt __maybe_unused,
> return 0;
> }
>
> +static int parse_max_symbol_bytes(const struct option *opt,
> + const char *str, int unset)
> +{
> + unsigned long *max_bytes = (unsigned long *)opt->value;
> + static struct parse_tag size_tags[] = {
> + { .tag = 'B', .mult = 1 },
> + { .tag = 'K', .mult = 1 << 10 },
> + { .tag = 'M', .mult = 1 << 20 },
> + { .tag = 'G', .mult = 1 << 30 },
> + { .tag = 0 },
> + };
> + unsigned long bytes;
> +
> + if (unset) {
> + *max_bytes = 0;
> + return 0;
> + }
> +
> + bytes = parse_tag_value(str, size_tags);
> + if (bytes != (unsigned long)-1) {
> + *max_bytes = bytes;
> + return 0;
> + }
> +
> + return -1;
> +}
> +
> int cmd_script(int argc, const char **argv)
> {
> bool show_full_info = false;
> @@ -4135,6 +4163,9 @@ int cmd_script(int argc, const char **argv)
> "Set the maximum stack depth when parsing the callchain, "
> "anything beyond the specified depth will be ignored. "
> "Default: kernel.perf_event_max_stack or " __stringify(PERF_MAX_STACK_DEPTH)),
> + OPT_CALLBACK(0, "max-symbol-bytes", &symbol_conf.max_symbol_bytes,
> + "size", "Limit bytes for ELF struct symbol (e.g. 128M; 0=unlimited)",
> + parse_max_symbol_bytes),
> OPT_BOOLEAN(0, "reltime", &reltime, "Show time stamps relative to start"),
> OPT_BOOLEAN(0, "deltatime", &deltatime, "Show time stamps relative to previous event"),
> OPT_BOOLEAN('I', "show-info", &show_full_info,
> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> index e955c3feddcd..914e42d21f70 100644
> --- a/tools/perf/util/symbol-elf.c
> +++ b/tools/perf/util/symbol-elf.c
> @@ -1634,6 +1634,13 @@ dso__load_sym_internal(struct dso *dso, struct map *map, struct symsrc *syms_ss,
> int is_label = elf_sym__is_label(&sym);
> const char *section_name;
> bool used_opd = false;
Please keep a blank line after declaration.
> + if (symbol_conf.max_symbol_bytes &&
> + symbol__bytes_used() >= symbol_conf.max_symbol_bytes) {
> + pr_warning_once("perf: symbol memory budget exceeded (%lu bytes), "
> + "remaining symbols will be [unknown]\n",
> + symbol_conf.max_symbol_bytes);
> + break;
> + }
>
> if (!is_label && !elf_sym__filter(&sym))
> continue;
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index 3587ad243159..62a4f91c2f5d 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -310,14 +310,35 @@ void symbols__fixup_end(struct rb_root_cached *symbols, bool is_kallsyms)
> curr->end = roundup(curr->start, 4096) + 4096;
> }
>
> +static size_t symbol_bytes_used;
> +
> +size_t symbol__bytes_used(void)
> +{
> + return symbol_bytes_used;
> +}
> +
> +void symbol__account_bytes(size_t bytes)
> +{
> + symbol_bytes_used += bytes;
> +}
> +
> +void symbol__unaccount_bytes(size_t bytes)
> +{
> + symbol_bytes_used -= bytes;
> +}
> +
> struct symbol *symbol__new(u64 start, u64 len, u8 binding, u8 type, const char *name)
> {
> size_t namelen = strlen(name) + 1;
> - struct symbol *sym = calloc(1, (symbol_conf.priv_size +
> - sizeof(*sym) + namelen));
> + size_t alloc_size = symbol_conf.priv_size + sizeof(struct symbol) + namelen;
The convention is 'sizeof(*sym)' rather than 'sizeof(struct symbol)'.
So that it can easily handle type changes in the future.
> + struct symbol *sym;
> +
> + sym = calloc(1, alloc_size);
> if (sym == NULL)
> return NULL;
>
> + symbol__account_bytes(alloc_size);
> +
> if (symbol_conf.priv_size) {
> if (symbol_conf.init_annotation) {
> struct annotation *notes = (void *)sym;
> @@ -341,6 +362,9 @@ struct symbol *symbol__new(u64 start, u64 len, u8 binding, u8 type, const char *
>
> void symbol__delete(struct symbol *sym)
> {
> + size_t alloc_size = symbol_conf.priv_size + sizeof(struct symbol) +
> + sym->namelen + 1;
Ditto.
Thanks,
Namhyung
> +
> if (symbol_conf.priv_size) {
> if (symbol_conf.init_annotation) {
> struct annotation *notes = symbol__annotation(sym);
> @@ -348,6 +372,7 @@ void symbol__delete(struct symbol *sym)
> annotation__exit(notes);
> }
> }
> + symbol__unaccount_bytes(alloc_size);
> free(((void *)sym) - symbol_conf.priv_size);
> }
>
> diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
> index e5cef16b240d..0d5d3792aac1 100644
> --- a/tools/perf/util/symbol.h
> +++ b/tools/perf/util/symbol.h
> @@ -228,6 +228,9 @@ void symbol__elf_init(void);
> int symbol__annotation_init(void);
>
> struct symbol *symbol__new(u64 start, u64 len, u8 binding, u8 type, const char *name);
> +size_t symbol__bytes_used(void);
> +void symbol__account_bytes(size_t bytes);
> +void symbol__unaccount_bytes(size_t bytes);
> size_t __symbol__fprintf_symname_offs(const struct symbol *sym,
> const struct addr_location *al,
> bool unknown_as_addr,
> diff --git a/tools/perf/util/symbol_conf.h b/tools/perf/util/symbol_conf.h
> index 71f60081a85b..6a16c5badd5e 100644
> --- a/tools/perf/util/symbol_conf.h
> +++ b/tools/perf/util/symbol_conf.h
> @@ -120,6 +120,7 @@ struct symbol_conf {
> int pad_output_len_dso;
> int group_sort_idx;
> int addr_range;
> + unsigned long max_symbol_bytes;
> DECLARE_BITMAP(parallelism_filter, MAX_NR_CPUS + 1);
> };
>
>
> --
> Git-155)
>
>
next prev parent reply other threads:[~2026-09-17 6:53 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-15 18:42 [PATCH 0/4] perf script: Bounded and lazy symbol loading Alireza Haghdoost via B4 Relay
2026-09-15 18:42 ` [PATCH 1/4] perf symbols: Fix broken ELF_C_READ_MMAP fallback guard Alireza Haghdoost via B4 Relay
2026-09-15 18:42 ` [PATCH 2/4] perf script: Add --max-symbol-bytes to bound ELF symbol memory Alireza Haghdoost via B4 Relay
2026-09-17 6:53 ` Namhyung Kim [this message]
2026-09-15 18:42 ` [PATCH 3/4] perf script: Add --lazy-load-symbols for lazy symbol loading Alireza Haghdoost via B4 Relay
2026-09-17 7:24 ` Namhyung Kim
2026-09-15 18:42 ` [PATCH 4/4] perf script: Document and test --lazy-load-symbols and --max-symbol-bytes Alireza Haghdoost via B4 Relay
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aquOfdBVYlr1QE8Z@z2 \
--to=namhyung@kernel.org \
--cc=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=andriin@fb.com \
--cc=ast@kernel.org \
--cc=haghdoost@uber.com \
--cc=irogers@google.com \
--cc=james.clark@linaro.org \
--cc=jolsa@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®