mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/3] perf: Fix libdw addr2line fallback, pr_debugN overhead, and cumulative hist decay
@ 2026-10-26  6:50 Ian Rogers
  2026-10-26  6:50 ` [PATCH 1/3] perf libdw: Optimize fallback logic for missing DWARF Ian Rogers
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Ian Rogers @ 2026-10-26  6:50 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Namhyung Kim, Alireza Haghdoost, Ian Rogers
  Cc: Peter Zijlstra, Ingo Molnar, Jiri Olsa, Adrian Hunter,
	James Clark, Alessio Podda, Michael Liang, Shimin Guo,
	Tanushree Shah, linux-perf-users, linux-kernel

Three standalone fixes found while profiling `perf top` and DSO symbol/DWARF
resolution:

1. **perf libdw: Optimize fallback logic for missing DWARF**:
   `libdw__addr2line()` previously returned `-1` when `dwfl_module_getdwarf()`
   or `dwfl_module_getsrc()` returned `NULL`, while `addr2line()` in
   `srcline.c` only stopped its fallback loop on `ret > 0`. This caused
   `perf` to fall back through `llvm`, `libbfd`, and the external
   `addr2line` subprocess on every stripped DSO or unmapped address, and
   kept the `Dwfl` ELF mmap open on stripped binaries. Standardize backend
   return values so `0` denotes conclusively missing DWARF/line info
   (stopping the fallback loop and freeing the `Dwfl` session when no
   DWARF exists), `< 0` denotes backend/initialization error, and `> 0`
   denotes resolved source line info.
   An earlier version of this patch was posted at:
   https://lore.kernel.org/linux-perf-users/20260824062841.1529489-3-irogers@google.com/
   This version addresses the review feedback from that thread.

2. **perf debug: Avoid calling eprintf() in pr_debugN() when verbose < n**:
   `pr_debugN()` unconditionally evaluated its arguments and called
   `eprintf()` -> `veprintf()` before checking `verbose >= n`, accounting
   for ~15% of total CPU time in `perf top` during ELF symbol loading even
   at default `verbose == 0`. Guard `eprintf()` with `unlikely(verbose >= (n))`.

3. **perf hist: Do not delete cumulative caller entries before stat_acc decays**:
   When `symbol_conf.cumulate_callchain` is enabled (`perf top -g`),
   callchain caller entries with `he->stat.period == 0` were deleted on the
   first `hists__decay_entries()` pass even while `he->stat_acc->period > 0`.
   Require `he->stat_acc->period == 0` before deleting entries in
   cumulative mode.

Assisted-by: Antigravity:gemini-3.1-pro

Ian Rogers (3):
  perf libdw: Optimize fallback logic for missing DWARF
  perf debug: Avoid calling eprintf() in pr_debugN() when verbose < n
  perf hist: Do not delete cumulative caller entries before stat_acc
    decays

 tools/perf/tests/pmu-events.c      |  5 ++-
 tools/perf/util/addr2line.c        | 28 ++++++++++---
 tools/perf/util/debug.h            |  5 +--
 tools/perf/util/dso.h              |  2 +-
 tools/perf/util/hist.c             |  6 ++-
 tools/perf/util/libbfd.c           |  6 +--
 tools/perf/util/libdw.c            | 25 ++++++++---
 tools/perf/util/libdw.h            |  5 ++-
 tools/perf/util/llvm-c-helpers.cpp | 66 +++++++++++++++++++-----------
 tools/perf/util/llvm.c             | 12 ++++--
 tools/perf/util/srcline.c          | 21 +++++++---
 11 files changed, 125 insertions(+), 56 deletions(-)

-- 
2.56.0.307.g5fe7bd680c-goog


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/3] perf libdw: Optimize fallback logic for missing DWARF
  2026-10-26  6:50 [PATCH 0/3] perf: Fix libdw addr2line fallback, pr_debugN overhead, and cumulative hist decay Ian Rogers
@ 2026-10-26  6:50 ` Ian Rogers
  2026-10-26  6:50 ` [PATCH 2/3] perf debug: Avoid calling eprintf() in pr_debugN() when verbose < n Ian Rogers
  2026-10-26  6:50 ` [PATCH 3/3] perf hist: Do not delete cumulative caller entries before stat_acc decays Ian Rogers
  2 siblings, 0 replies; 4+ messages in thread
From: Ian Rogers @ 2026-10-26  6:50 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Namhyung Kim, Alireza Haghdoost, Ian Rogers
  Cc: Peter Zijlstra, Ingo Molnar, Jiri Olsa, Adrian Hunter,
	James Clark, Alessio Podda, Michael Liang, Shimin Guo,
	Tanushree Shah, linux-perf-users, linux-kernel

When libdw encounters a DWARF definition it cannot accurately un-inline
or an address block natively missing structural line maps,
dwfl_module_getsrc() returns NULL. Previously, libdw__addr2line()
returned -1 in both dwfl_module_getdwarf() and dwfl_module_getsrc()
failure paths while addr2line() in srcline.c only stopped its fallback
loop on ret > 0. This caused perf's addr2line_style loop to fall back to
llvm, libbfd, and the cmd__addr2line() subprocess wrapper on every
unmapped address or stripped DSO.

Update the addr2line_style pipeline return semantics consistently:
1. Backends return 0 to denote conclusively missing information
   (e.g. valid ELF but no DWARF inside, or parsed DWARF with no mapped
   lines), stopping the fallback loop.
2. Backends (and uncompiled stubs) return < 0 to denote initialization
   or backend errors, instructing the loop to iterate to the next
   configured fallback.
3. Backends return > 0 on successful source line resolution.

Fixes: bf96e296d52d ("perf libdw: Fix Dwfl discovery with split files")
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/addr2line.c        | 28 ++++++++++---
 tools/perf/util/dso.h              |  2 +-
 tools/perf/util/libbfd.c           |  6 +--
 tools/perf/util/libdw.c            | 25 ++++++++---
 tools/perf/util/libdw.h            |  5 ++-
 tools/perf/util/llvm-c-helpers.cpp | 66 +++++++++++++++++++-----------
 tools/perf/util/llvm.c             | 12 ++++--
 tools/perf/util/srcline.c          | 21 +++++++---
 8 files changed, 115 insertions(+), 50 deletions(-)

diff --git a/tools/perf/util/addr2line.c b/tools/perf/util/addr2line.c
index 6f64f6b42442..960dd48701bb 100644
--- a/tools/perf/util/addr2line.c
+++ b/tools/perf/util/addr2line.c
@@ -298,6 +298,7 @@ int cmd__addr2line(const char *dso_name, u64 addr,
 	struct io io = { .eof = false };
 	enum cmd_a2l_style cmd_a2l_style;
 	const char *current_dso_name;
+	bool a2l_synced = false;
 
 	mutex_lock(dso__lock(dso));
 	current_dso_name = dso__symsrc_filename(dso) ?: dso_name;
@@ -316,11 +317,14 @@ int cmd__addr2line(const char *dso_name, u64 addr,
 	if (a2l == NULL) {
 		if (!symbol_conf.addr2line_disable_warn)
 			pr_warning("%s %s: addr2line_subprocess_init failed\n", __func__, dso_name);
+		ret = -1;
 		goto out_unlock;
 	}
 	cmd_a2l_style = cmd_addr2line_configure(a2l, dso_name);
-	if (cmd_a2l_style == BROKEN)
+	if (cmd_a2l_style == BROKEN) {
+		ret = -1;
 		goto out_unlock;
+	}
 
 	/*
 	 * Take ownership of the a2l subprocess so we can safely perform
@@ -344,6 +348,7 @@ int cmd__addr2line(const char *dso_name, u64 addr,
 	if (written != len) {
 		if (!symbol_conf.addr2line_disable_warn)
 			pr_warning("%s %s: could not send request\n", __func__, dso_name);
+		ret = -1;
 		goto out;
 	}
 	io__init(&io, a2l->out, buf, sizeof(buf));
@@ -353,6 +358,7 @@ int cmd__addr2line(const char *dso_name, u64 addr,
 	case -1:
 		if (!symbol_conf.addr2line_disable_warn)
 			pr_warning("%s %s: could not read first record\n", __func__, dso_name);
+		ret = -1;
 		goto out;
 	case 0:
 		/*
@@ -373,6 +379,7 @@ int cmd__addr2line(const char *dso_name, u64 addr,
 			break;
 		case 0:
 			/* The sentinel as expected. */
+			a2l_synced = true;
 			break;
 		default:
 			if (!symbol_conf.addr2line_disable_warn)
@@ -388,17 +395,23 @@ int cmd__addr2line(const char *dso_name, u64 addr,
 
 	if (file) {
 		*file = strdup(record_filename);
-		ret = 1;
+		if (!*file) {
+			ret = -ENOMEM;
+			goto out;
+		}
 	}
 	if (line_nr)
 		*line_nr = record_line_nr;
+	ret = 1;
 
 	if (unwind_inlines) {
 		if (node && inline_list__append_record(dso, node, sym,
 						       record_function,
 						       record_filename,
 						       record_line_nr)) {
-			ret = 0;
+			if (file && *file)
+				zfree(file);
+			ret = -ENOMEM;
 			goto out;
 		}
 	}
@@ -421,12 +434,17 @@ int cmd__addr2line(const char *dso_name, u64 addr,
 						       record_function,
 						       record_filename,
 						       record_line_nr)) {
-				ret = 0;
+				if (file && *file)
+					zfree(file);
+				inline_node__clear_frames(node);
+				ret = -ENOMEM;
 				goto out;
 			}
 			ret = 1; /* found at least one inline frame */
 		}
 	}
+	if (record_status == 0)
+		a2l_synced = true;
 
 out:
 	mutex_lock(dso__lock(dso));
@@ -434,7 +452,7 @@ int cmd__addr2line(const char *dso_name, u64 addr,
 	free(record_filename);
 
 	current_dso_name = dso__symsrc_filename(dso) ?: dso__long_name(dso);
-	if (!io.eof && dso__a2l(dso) == NULL && current_dso_name &&
+	if (a2l_synced && !io.eof && dso__a2l(dso) == NULL && current_dso_name &&
 	    !strcmp(current_dso_name, dso_name))
 		dso__set_a2l(dso, a2l);
 	else
diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
index 3329f417810c..d5f7382d4e13 100644
--- a/tools/perf/util/dso.h
+++ b/tools/perf/util/dso.h
@@ -760,7 +760,7 @@ static inline const char *dso__symsrc_filename(const struct dso *dso)
 	return RC_CHK_ACCESS(dso)->symsrc_filename;
 }
 
-void dso__set_symsrc_filename(struct dso *dso, char *val);
+void dso__set_symsrc_filename(struct dso *dso, char *val) EXCLUSIVE_LOCKS_REQUIRED(dso__lock(dso));
 
 static inline void dso__free_symsrc_filename(struct dso *dso)
 {
diff --git a/tools/perf/util/libbfd.c b/tools/perf/util/libbfd.c
index a4dcb406c79e..992e2c2a7198 100644
--- a/tools/perf/util/libbfd.c
+++ b/tools/perf/util/libbfd.c
@@ -288,7 +288,7 @@ int libbfd__addr2line(const char *dso_name, u64 addr,
 
 		if (node && inline_list__append_dso_a2l(dso, node, sym)) {
 			inline_node__clear_frames(node);
-			ret = 0;
+			ret = -ENOMEM;
 			goto out;
 		}
 
@@ -302,7 +302,7 @@ int libbfd__addr2line(const char *dso_name, u64 addr,
 			if (node != NULL) {
 				if (inline_list__append_dso_a2l(dso, node, sym)) {
 					inline_node__clear_frames(node);
-					ret = 0;
+					ret = -ENOMEM;
 					goto out;
 				}
 			}
@@ -312,7 +312,7 @@ int libbfd__addr2line(const char *dso_name, u64 addr,
 	if (file) {
 		*file = a2l->filename ? strdup(a2l->filename) : NULL;
 		if (!*file) {
-			/* Leave ret as 0 so that another addr2line is tried. */
+			ret = a2l->filename ? -ENOMEM : 0;
 			goto out;
 		}
 	}
diff --git a/tools/perf/util/libdw.c b/tools/perf/util/libdw.c
index c4504cceb313..e660623338d5 100644
--- a/tools/perf/util/libdw.c
+++ b/tools/perf/util/libdw.c
@@ -7,6 +7,7 @@
 #include "callchain.h"
 #include <fcntl.h>
 #include <unistd.h>
+#include <string.h>
 #include <elfutils/libdwfl.h>
 
 static const Dwfl_Callbacks offline_callbacks = {
@@ -175,10 +176,15 @@ int libdw__addr2line(u64 addr, char **file, unsigned int *line_nr,
 	int lineno = 0;
 	int ret = 0;
 
+	if (!dso__has_srcline(dso))
+		return 0;
+
 	mutex_lock(dso__lock(dso));
 	dwfl = dso__libdw_dwfl(dso);
-	if (!dwfl)
+	if (!dwfl) {
+		ret = -1;
 		goto out;
+	}
 
 	mod = dwfl_addrmodule(dwfl, addr);
 	if (!mod) {
@@ -192,14 +198,23 @@ int libdw__addr2line(u64 addr, char **file, unsigned int *line_nr,
 	 * libdw.
 	 */
 	if (!dwfl_module_getdwarf(mod, &bias)) {
-		ret = -1;
+		/*
+		 * Free the Dwfl session so the mmap'd ELF binary is unmapped
+		 * immediately, and mark the DSO as lacking srcline/DWARF info
+		 * so neither libdw nor slower fallback backends (addr2line
+		 * subprocess, libbfd) are repeatedly invoked on stripped DSOs.
+		 */
+		dso__free_libdw(dso);
+		dso__set_has_srcline(dso, false);
+		ret = 0;
 		goto out;
 	}
 
 	/* Find source line information for the address. */
 	dwline = dwfl_module_getsrc(mod, addr + bias);
 	if (!dwline) {
-		ret = -1;
+		/* conclusively missing information -> abort fallbacks */
+		ret = 0;
 		goto out;
 	}
 
@@ -228,7 +243,7 @@ int libdw__addr2line(u64 addr, char **file, unsigned int *line_nr,
 				free(*file);
 				*file = NULL;
 			}
-			ret = 0;
+			ret = -ENOMEM;
 			goto out;
 		}
 
@@ -245,7 +260,7 @@ int libdw__addr2line(u64 addr, char **file, unsigned int *line_nr,
 				*file = NULL;
 			}
 			inline_node__clear_frames(node);
-			ret = 0;
+			ret = args.err;
 			goto out;
 		}
 	}
diff --git a/tools/perf/util/libdw.h b/tools/perf/util/libdw.h
index b12094737415..c8f319ce35df 100644
--- a/tools/perf/util/libdw.h
+++ b/tools/perf/util/libdw.h
@@ -23,7 +23,8 @@ struct symbol;
  * finds the source line information for the given address, and optionally
  * resolves inline function call chains.
  *
- * Returns 1 on success (found), 0 on failure (not found).
+ * Returns 1 on success (found), 0 when DWARF/line info is conclusively
+ * missing, and < 0 on initialization or uncompiled error.
  */
 int libdw__addr2line(u64 addr, char **file,
 		     unsigned int *line_nr, struct dso *dso,
@@ -47,7 +48,7 @@ static inline int libdw__addr2line(u64 addr __maybe_unused, char **file __maybe_
 				   struct inline_node *node __maybe_unused,
 				   struct symbol *sym __maybe_unused)
 {
-	return 0;
+	return -1;
 }
 
 static inline void dso__free_libdw(struct dso *dso __maybe_unused)
diff --git a/tools/perf/util/llvm-c-helpers.cpp b/tools/perf/util/llvm-c-helpers.cpp
index 004081bd12c9..d990166100f0 100644
--- a/tools/perf/util/llvm-c-helpers.cpp
+++ b/tools/perf/util/llvm-c-helpers.cpp
@@ -50,20 +50,20 @@ static LLVMSymbolizer *get_symbolizer()
 	return instance;
 }
 
-/* Returns 0 on error, 1 on success. */
+/* Returns < 0 on error, 0 when line info is missing, 1 on success. */
 static int extract_file_and_line(const DILineInfo &line_info, char **file,
 				 unsigned int *line)
 {
-	if (file) {
-		if (line_info.FileName == "<invalid>") {
-			/* Match the convention of libbfd. */
+	if (line_info.FileName == "<invalid>") {
+		if (file)
 			*file = nullptr;
-		} else {
-			/* The caller expects to get something it can free(). */
-			*file = strdup(line_info.FileName.c_str());
-			if (*file == nullptr)
-				return 0;
-		}
+		return 0;
+	}
+	if (file) {
+		/* The caller expects to get something it can free(). */
+		*file = strdup(line_info.FileName.c_str());
+		if (*file == nullptr)
+			return -ENOMEM;
 	}
 	if (line)
 		*line = line_info.Line;
@@ -86,30 +86,42 @@ int llvm_addr2line(const char *dso_name, u64 addr,
 		Expected<DIInliningInfo> res_or_err =
 			symbolizer->symbolizeInlinedCode(dso_name,
 							 sectioned_addr);
-		if (!res_or_err)
-			return 0;
+		if (!res_or_err) {
+			consumeError(res_or_err.takeError());
+			return -1;
+		}
 		unsigned num_frames = res_or_err->getNumberOfFrames();
 		if (num_frames == 0)
 			return 0;
 
-		if (extract_file_and_line(res_or_err->getFrame(0),
-					  file, line) == 0)
-			return 0;
+		int ret = extract_file_and_line(res_or_err->getFrame(0),
+						file, line);
+		if (ret <= 0)
+			return ret;
 
 		*inline_frames = (llvm_a2l_frame *)calloc(
 			num_frames, sizeof(**inline_frames));
-		if (*inline_frames == nullptr)
-			return 0;
+		if (*inline_frames == nullptr) {
+			if (file)
+				zfree(file);
+			return -ENOMEM;
+		}
 
 		for (unsigned i = 0; i < num_frames; ++i) {
 			const DILineInfo &src = res_or_err->getFrame(i);
 
 			llvm_a2l_frame &dst = (*inline_frames)[i];
-			if (src.FileName == "<invalid>")
-				/* Match the convention of libbfd. */
-				dst.filename = nullptr;
-			else
-				dst.filename = strdup(src.FileName.c_str());
+			if (src.FileName == "<invalid>") {
+				for (unsigned j = 0; j < i; ++j) {
+					zfree(&(*inline_frames)[j].filename);
+					zfree(&(*inline_frames)[j].funcname);
+				}
+				zfree(inline_frames);
+				if (file)
+					zfree(file);
+				return 0;
+			}
+			dst.filename = strdup(src.FileName.c_str());
 			dst.funcname = strdup(src.FunctionName.c_str());
 			dst.line = src.Line;
 
@@ -120,7 +132,9 @@ int llvm_addr2line(const char *dso_name, u64 addr,
 					zfree(&(*inline_frames)[j].funcname);
 				}
 				zfree(inline_frames);
-				return 0;
+				if (file)
+					zfree(file);
+				return -ENOMEM;
 			}
 		}
 
@@ -131,8 +145,10 @@ int llvm_addr2line(const char *dso_name, u64 addr,
 
 		Expected<DILineInfo> res_or_err =
 			symbolizer->symbolizeCode(dso_name, sectioned_addr);
-		if (!res_or_err)
-			return 0;
+		if (!res_or_err) {
+			consumeError(res_or_err.takeError());
+			return -1;
+		}
 		return extract_file_and_line(*res_or_err, file, line);
 	}
 }
diff --git a/tools/perf/util/llvm.c b/tools/perf/util/llvm.c
index 722e71aab100..0ecb1a53359e 100644
--- a/tools/perf/util/llvm.c
+++ b/tools/perf/util/llvm.c
@@ -44,8 +44,11 @@ int llvm__addr2line(const char *dso_name __maybe_unused, u64 addr __maybe_unused
 	int num_frames = llvm_addr2line(dso_name, addr, file, line,
 					node && unwind_inlines, &inline_frames);
 
-	if (num_frames == 0 || !inline_frames) {
-		/* Error, or we didn't want inlines. */
+	if (num_frames <= 0 || !inline_frames) {
+		/*
+		 * Error (< 0 -> iterate fallbacks), missing line info (0), or
+		 * we resolved lines without wanting inlines (1).
+		 */
 		return num_frames;
 	}
 
@@ -60,8 +63,11 @@ int llvm__addr2line(const char *dso_name __maybe_unused, u64 addr __maybe_unused
 						      inline_frames[i].line);
 		}
 		if (inline_list__append(inline_sym, srcline, node) != 0) {
+			if (file)
+				zfree(file);
+			inline_node__clear_frames(node);
 			free_llvm_inline_frames(inline_frames, num_frames);
-			return 0;
+			return -1;
 		}
 	}
 	free_llvm_inline_frames(inline_frames, num_frames);
diff --git a/tools/perf/util/srcline.c b/tools/perf/util/srcline.c
index a8d16106f619..23951c35fb66 100644
--- a/tools/perf/util/srcline.c
+++ b/tools/perf/util/srcline.c
@@ -181,9 +181,9 @@ static int addr2line(const char *dso_name, u64 addr, char **file, unsigned int *
 			break;
 		case A2L_STYLE_UNKNOWN:
 		default:
-			break;
+			return 0;
 		}
-		if (ret > 0)
+		if (ret >= 0)
 			return ret;
 	}
 
@@ -250,6 +250,10 @@ static struct inline_node *addr2inlines(const char *dso_name, u64 addr,
 
 	addr2line(dso_name, addr, /*file=*/NULL, /*line_nr=*/NULL, dso,
 		  /*unwind_inlines=*/true, node, sym);
+	if (!dso__has_srcline(dso) && list_empty(&node->val)) {
+		inline_node__delete(node);
+		return NULL;
+	}
 
 	return node;
 }
@@ -276,8 +280,8 @@ char *__get_srcline(struct dso *dso, u64 addr, struct symbol *sym,
 	if (dso_name == NULL)
 		goto out_err;
 
-	if (!addr2line(dso_name, addr, &file, &line, dso,
-		       unwind_inlines, /*node=*/NULL, sym)) {
+	if (addr2line(dso_name, addr, &file, &line, dso,
+		       unwind_inlines, /*node=*/NULL, sym) <= 0) {
 		free(dso_name);
 		goto out_err;
 	}
@@ -302,6 +306,7 @@ char *__get_srcline(struct dso *dso, u64 addr, struct symbol *sym,
 		dso__set_has_srcline(dso, false);
 		dso__free_a2l(dso);
 		dso__free_a2l_libbfd(dso);
+		dso__free_libdw(dso);
 	}
 	mutex_unlock(dso__lock(dso));
 out:
@@ -331,8 +336,8 @@ char *get_srcline_split(struct dso *dso, u64 addr, unsigned *line)
 	if (dso_name == NULL)
 		goto out_err;
 
-	if (!addr2line(dso_name, addr, &file, line, dso, /*unwind_inlines=*/true,
-			/*node=*/NULL, /*sym=*/NULL)) {
+	if (addr2line(dso_name, addr, &file, line, dso, /*unwind_inlines=*/true,
+			/*node=*/NULL, /*sym=*/NULL) <= 0) {
 		free(dso_name);
 		goto out_err;
 	}
@@ -350,6 +355,7 @@ char *get_srcline_split(struct dso *dso, u64 addr, unsigned *line)
 		dso__set_has_srcline(dso, false);
 		dso__free_a2l(dso);
 		dso__free_a2l_libbfd(dso);
+		dso__free_libdw(dso);
 	}
 	mutex_unlock(dso__lock(dso));
 
@@ -448,6 +454,9 @@ struct inline_node *dso__parse_addr_inlines(struct dso *dso, u64 addr,
 	char *dso_name;
 	struct inline_node *node;
 
+	if (!dso__has_srcline(dso))
+		return NULL;
+
 	dso_name = srcline_dso_name(dso);
 	if (dso_name == NULL)
 		return NULL;
-- 
2.56.0.307.g5fe7bd680c-goog


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 2/3] perf debug: Avoid calling eprintf() in pr_debugN() when verbose < n
  2026-10-26  6:50 [PATCH 0/3] perf: Fix libdw addr2line fallback, pr_debugN overhead, and cumulative hist decay Ian Rogers
  2026-10-26  6:50 ` [PATCH 1/3] perf libdw: Optimize fallback logic for missing DWARF Ian Rogers
@ 2026-10-26  6:50 ` Ian Rogers
  2026-10-26  6:50 ` [PATCH 3/3] perf hist: Do not delete cumulative caller entries before stat_acc decays Ian Rogers
  2 siblings, 0 replies; 4+ messages in thread
From: Ian Rogers @ 2026-10-26  6:50 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Namhyung Kim, Alireza Haghdoost, Ian Rogers
  Cc: Peter Zijlstra, Ingo Molnar, Jiri Olsa, Adrian Hunter,
	James Clark, Alessio Podda, Michael Liang, Shimin Guo,
	Tanushree Shah, linux-perf-users, linux-kernel

pr_debugN() unconditionally evaluated its arguments and called eprintf() ->
veprintf() before checking if verbose >= n. In hot paths such as
dso__load_sym_internal(), two pr_debug4() calls per symbol accounted for
~15% of total CPU time in 'perf top' even at the default verbose == 0.

Guard eprintf() in pr_debugN() with unlikely(verbose >= (n)) and define
pr_debug() in terms of pr_debugN(1, ...).

In test__parsing_callback() (tools/perf/tests/pmu-events.c), move the
test_generic_metric(mexp, 0) call out of the pr_debug() argument list so
the metric evaluation continues to run during 'perf test' when verbose == 0.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/tests/pmu-events.c | 5 ++++-
 tools/perf/util/debug.h       | 5 ++---
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/tools/perf/tests/pmu-events.c b/tools/perf/tests/pmu-events.c
index f507ce39439a..e6e4f4cf1fa2 100644
--- a/tools/perf/tests/pmu-events.c
+++ b/tools/perf/tests/pmu-events.c
@@ -906,9 +906,12 @@ static int test__parsing_callback(const struct pmu_metric *pm,
 			struct metric_expr *mexp;
 
 			list_for_each_entry (mexp, &me->head, nd) {
+				double result;
+
 				if (strcmp(mexp->metric_name, pm->metric_name))
 					continue;
-				pr_debug("Result %f\n", test_generic_metric(mexp, 0));
+				result = test_generic_metric(mexp, 0);
+				pr_debug("Result %f\n", result);
 				err = 0;
 				(*failures)--;
 				goto out_err;
diff --git a/tools/perf/util/debug.h b/tools/perf/util/debug.h
index 6b737e195ce1..7552c9171d90 100644
--- a/tools/perf/util/debug.h
+++ b/tools/perf/util/debug.h
@@ -33,10 +33,9 @@ extern int debug_type_profile;
 })
 #define pr_info(fmt, ...) \
 	eprintf(0, verbose, pr_fmt(fmt), ##__VA_ARGS__)
-#define pr_debug(fmt, ...) \
-	eprintf(1, verbose, pr_fmt(fmt), ##__VA_ARGS__)
 #define pr_debugN(n, fmt, ...) \
-	eprintf(n, verbose, pr_fmt(fmt), ##__VA_ARGS__)
+	(unlikely(verbose >= (n)) ? eprintf(n, verbose, pr_fmt(fmt), ##__VA_ARGS__) : 0)
+#define pr_debug(fmt, ...) pr_debugN(1, pr_fmt(fmt), ##__VA_ARGS__)
 #define pr_debug2(fmt, ...) pr_debugN(2, pr_fmt(fmt), ##__VA_ARGS__)
 #define pr_debug3(fmt, ...) pr_debugN(3, pr_fmt(fmt), ##__VA_ARGS__)
 #define pr_debug4(fmt, ...) pr_debugN(4, pr_fmt(fmt), ##__VA_ARGS__)
-- 
2.56.0.307.g5fe7bd680c-goog


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 3/3] perf hist: Do not delete cumulative caller entries before stat_acc decays
  2026-10-26  6:50 [PATCH 0/3] perf: Fix libdw addr2line fallback, pr_debugN overhead, and cumulative hist decay Ian Rogers
  2026-10-26  6:50 ` [PATCH 1/3] perf libdw: Optimize fallback logic for missing DWARF Ian Rogers
  2026-10-26  6:50 ` [PATCH 2/3] perf debug: Avoid calling eprintf() in pr_debugN() when verbose < n Ian Rogers
@ 2026-10-26  6:50 ` Ian Rogers
  2 siblings, 0 replies; 4+ messages in thread
From: Ian Rogers @ 2026-10-26  6:50 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Namhyung Kim, Alireza Haghdoost, Ian Rogers
  Cc: Peter Zijlstra, Ingo Molnar, Jiri Olsa, Adrian Hunter,
	James Clark, Alessio Podda, Michael Liang, Shimin Guo,
	Tanushree Shah, linux-perf-users, linux-kernel

When symbol_conf.cumulate_callchain is enabled (the default in
'perf top -g'), callchain parent/caller entries have he->stat.period == 0
and accumulate their period in he->stat_acc->period.

hists__decay_entry() decays both he->stat.period and he->stat_acc->period,
but only checks if he->stat.period == 0 as the return condition for
deleting the entry in hists__decay_entries(). As a result, every callchain
caller entry with 0 self-period is unconditionally deleted from the
histogram on the first decay pass, destroying its callchain tree while it
still has non-zero cumulative period.

Only mark an entry for deletion in hists__decay_entry() when both
he->stat.period == 0 and (when cumulate_callchain is active)
he->stat_acc->period == 0.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/hist.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 443694926f1a..e75a4fcce975 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -409,7 +409,8 @@ static bool hists__decay_entry(struct hists *hists, struct hist_entry *he)
 	u64 prev_period = he->stat.period;
 	u64 prev_latency = he->stat.latency;
 
-	if (prev_period == 0)
+	if (prev_period == 0 &&
+	    (!symbol_conf.cumulate_callchain || he->stat_acc->period == 0))
 		return true;
 
 	he_stat__decay(&he->stat);
@@ -442,7 +443,8 @@ static bool hists__decay_entry(struct hists *hists, struct hist_entry *he)
 		}
 	}
 
-	return he->stat.period == 0 && he->stat.latency == 0;
+	return he->stat.period == 0 && he->stat.latency == 0 &&
+	       (!symbol_conf.cumulate_callchain || he->stat_acc->period == 0);
 }
 
 static void hists__delete_entry(struct hists *hists, struct hist_entry *he)
-- 
2.56.0.307.g5fe7bd680c-goog


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-10-26  6:51 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-26  6:50 [PATCH 0/3] perf: Fix libdw addr2line fallback, pr_debugN overhead, and cumulative hist decay Ian Rogers
2026-10-26  6:50 ` [PATCH 1/3] perf libdw: Optimize fallback logic for missing DWARF Ian Rogers
2026-10-26  6:50 ` [PATCH 2/3] perf debug: Avoid calling eprintf() in pr_debugN() when verbose < n Ian Rogers
2026-10-26  6:50 ` [PATCH 3/3] perf hist: Do not delete cumulative caller entries before stat_acc decays Ian Rogers

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®