mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Namhyung Kim <namhyung@kernel.org>
Cc: Ingo Molnar <mingo@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	James Clark <james.clark@linaro.org>,
	Jiri Olsa <jolsa@kernel.org>, Ian Rogers <irogers@google.com>,
	Adrian Hunter <adrian.hunter@intel.com>,
	Clark Williams <williams@redhat.com>,
	linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org,
	Arnaldo Carvalho de Melo <acme@redhat.com>
Subject: [PATCH 02/15] perf debuginfo: Set DEBUGINFOD_URLS from /etc/debuginfod when unset
Date: Thu, 17 Sep 2026 12:55:13 -0300	[thread overview]
Message-ID: <20260917155528.62607-3-acme@kernel.org> (raw)
In-Reply-To: <20260917155528.62607-1-acme@kernel.org>

From: Arnaldo Carvalho de Melo <acme@redhat.com>

The debuginfod client fails when DEBUGINFOD_URLS isn't set even when
what it wants is in its local cache, and the distro setup scripts that
populate it from the files in /etc/debuginfod don't reach cron jobs,
systemd services and CI, so set it from those .urls files from
symbol__init(), before any thread that can call getenv() is started:
setenv() is not thread safe and libdebuginfod reads the variable in
every debuginfod_begin().

Install it via putenv() as a string perf owns, so that it can be
emptied in place mid-session when the user disables debuginfod, and
when debuginfod is off, with --no-debuginfod, core.debuginfod=false or
by disabling the build-id cache, set the empty string, that both
libdebuginfod and libdwfl's own client read as an opt-out, instead of
exporting the variable for libdwfl to fetch behind perf's back.  Tools
that manage DEBUGINFOD_URLS themselves are left alone.

Assisted-by: LLM
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/symbol.c |   7 ++
 tools/perf/util/util.c   | 151 +++++++++++++++++++++++++++++++++++++--
 tools/perf/util/util.h   |   9 +++
 3 files changed, 161 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index b1a2684c813c5d8d..fbad770cb96f8294 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -2536,6 +2536,13 @@ int symbol__init(struct perf_env *env)
 	if (symbol_conf.initialized)
 		return 0;
 
+	/*
+	 * Set DEBUGINFOD_URLS from the distro .urls files before any thread
+	 * that getenv()s it is started: setenv() is not thread safe and
+	 * libdebuginfod reads it in every debuginfod_begin().
+	 */
+	debuginfod__setup_urls_env();
+
 	symbol_conf.priv_size = PERF_ALIGN(symbol_conf.priv_size, sizeof(u64));
 
 	symbol__elf_init();
diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c
index 8f7cd32f524dc10e..d502fd867f38196b 100644
--- a/tools/perf/util/util.c
+++ b/tools/perf/util/util.c
@@ -21,9 +21,11 @@
 #include <linux/time64.h>
 #include <linux/overflow.h>
 #include <unistd.h>
+#include "build-id.h"
 #include "cap.h"
 #include "strlist.h"
 #include "string2.h"
+#include "symbol_conf.h"
 
 /*
  * XXX We need to find a better place for these things...
@@ -431,19 +433,66 @@ char *perf_exe(char *buf, int len)
 	return strcpy(buf, "perf");
 }
 
+static bool debuginfod__urls_set_by_tool;
+
+#define DEBUGINFOD_URLS_ENV "DEBUGINFOD_URLS"
+
+/*
+ * The DEBUGINFOD_URLS string perf owns: it has to change mid-session
+ * when the user disables debuginfod, and setenv() then could race with
+ * another thread's getenv().  With the string owned by perf, disabling
+ * it is just writing a NUL over its first byte.  The string is never
+ * freed: from putenv() on it is the environment's own storage.
+ */
+static char *debuginfod_urls_env;
+
+static void debuginfod__set_urls_env(const char *urls)
+{
+	size_t len = strlen(DEBUGINFOD_URLS_ENV "=") + strlen(urls) + 1;
+	char *env = malloc(len);
+
+	if (env == NULL)
+		return;
+
+	snprintf(env, len, DEBUGINFOD_URLS_ENV "=%s", urls);
+	debuginfod_urls_env = env;
+	putenv(env);
+}
+
+/*
+ * Empty the copy of DEBUGINFOD_URLS installed by
+ * debuginfod__set_urls_env(), which both libdebuginfod and libdwfl's
+ * own client read as an opt-out.  No environ array write and no free(),
+ * so a racing getenv() sees a valid string.
+ */
+void debuginfod__disable_urls_env(void)
+{
+	if (debuginfod_urls_env != NULL)
+		debuginfod_urls_env[sizeof(DEBUGINFOD_URLS_ENV "=") - 1] = '\0';
+}
+
 void perf_debuginfod_setup(struct perf_debuginfod *di)
 {
 	/*
 	 * By default '!di->set' we clear DEBUGINFOD_URLS, so debuginfod
 	 * processing is not triggered, otherwise we set it to 'di->urls'
-	 * value. If 'di->urls' is "system" we keep DEBUGINFOD_URLS value.
+	 * value. If 'di->urls' is "system" we keep DEBUGINFOD_URLS value,
+	 * but as a copy perf owns, so that it can still be emptied
+	 * mid-session, see debuginfod__disable_urls_env().
 	 */
-	if (!di->set)
-		setenv("DEBUGINFOD_URLS", "", 1);
-	else if (di->urls && strcmp(di->urls, "system"))
-		setenv("DEBUGINFOD_URLS", di->urls, 1);
+	if (!di->set) {
+		debuginfod__set_urls_env("");
+	} else if (di->urls == NULL || !strcmp(di->urls, "system")) {
+		const char *urls = getenv(DEBUGINFOD_URLS_ENV);
+
+		if (urls != NULL)
+			debuginfod__set_urls_env(urls);
+	} else {
+		debuginfod__set_urls_env(di->urls);
+	}
 
-	pr_debug("DEBUGINFOD_URLS=%s\n", getenv("DEBUGINFOD_URLS"));
+	debuginfod__urls_set_by_tool = true;
+	pr_debug("DEBUGINFOD_URLS=%s\n", getenv(DEBUGINFOD_URLS_ENV));
 
 #ifndef HAVE_DEBUGINFOD_SUPPORT
 	if (di->set)
@@ -451,6 +500,96 @@ void perf_debuginfod_setup(struct perf_debuginfod *di)
 #endif
 }
 
+#ifdef HAVE_DEBUGINFOD_SUPPORT
+/*
+ * The debuginfod client checks its local cache only as part of the
+ * server query flow, so with no servers configured it fails even when
+ * the file is cached; the distro setup scripts that export
+ * DEBUGINFOD_URLS from /etc/debuginfod don't reach cron jobs, systemd
+ * services and CI, so do it here when the variable isn't set.  An
+ * explicitly empty DEBUGINFOD_URLS is an opt-out and is left alone.
+ *
+ * Done from symbol__init(), on the single-threaded setup: setenv() is
+ * not thread safe and libdebuginfod reads it in every
+ * debuginfod_begin().  When debuginfod is off, set the empty opt-out
+ * instead of exporting it, so that libdwfl's own client doesn't fetch.
+ * The value is a copy owned by perf, see debuginfod__set_urls_env().
+ */
+void debuginfod__setup_urls_env(void)
+{
+	const char *env_urls;
+	char *urls = NULL;
+	DIR *dir;
+	struct dirent *dent;
+
+	/*
+	 * Tools that set DEBUGINFOD_URLS themselves already made their choice,
+	 * leave it alone.
+	 */
+	if (debuginfod__urls_set_by_tool)
+		return;
+
+	if (!symbol_conf.debuginfod || !strcmp(buildid_dir, "/dev/null")) {
+		debuginfod__set_urls_env("");
+		pr_debug("DEBUGINFOD_URLS cleared, debuginfod is disabled\n");
+		return;
+	}
+
+	env_urls = getenv(DEBUGINFOD_URLS_ENV);
+	if (env_urls != NULL) {
+		/*
+		 * Take ownership of the value, so that it can be emptied mid-session
+		 * with the 'd' key.
+		 */
+		debuginfod__set_urls_env(env_urls);
+		return;
+	}
+
+	dir = opendir("/etc/debuginfod");
+	if (dir == NULL)
+		return;
+
+	while ((dent = readdir(dir)) != NULL) {
+		char *content = NULL;
+		char *new_urls;
+		char path[PATH_MAX];
+		size_t len = strlen(dent->d_name), i, size;
+		int n;
+
+		if (len < 5 || strcmp(dent->d_name + len - 5, ".urls"))
+			continue;
+
+		snprintf(path, sizeof(path), "/etc/debuginfod/%s", dent->d_name);
+		if (filename__read_str(path, &content, &size) < 0)
+			continue;
+
+		for (i = 0; i < size; i++)
+			if (content[i] == '\n' || content[i] == '\r')
+				content[i] = ' ';
+
+		if (urls == NULL) {
+			urls = strdup(content);
+		} else {
+			n = asprintf(&new_urls, "%s %s", urls, content);
+			if (n < 0) {
+				free(content);
+				continue;
+			}
+			free(urls);
+			urls = new_urls;
+		}
+		free(content);
+	}
+	closedir(dir);
+
+	if (urls != NULL) {
+		debuginfod__set_urls_env(urls);
+		pr_debug("Set DEBUGINFOD_URLS from /etc/debuginfod: %s\n", urls);
+	}
+	free(urls);
+}
+#endif /* HAVE_DEBUGINFOD_SUPPORT */
+
 /*
  * Return a new filename prepended with task's root directory if it's in
  * a chroot.  Callers should free the returned string.
diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index f8f1ff603c728086..42feccf18f0d163f 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -81,6 +81,15 @@ struct perf_debuginfod {
 	bool		 set;
 };
 void perf_debuginfod_setup(struct perf_debuginfod *di);
+void debuginfod__disable_urls_env(void);
+
+#ifdef HAVE_DEBUGINFOD_SUPPORT
+void debuginfod__setup_urls_env(void);
+#else
+static inline void debuginfod__setup_urls_env(void)
+{
+}
+#endif
 
 const char *perf_basename(const char *path);
 
-- 
2.55.0


  parent reply	other threads:[~2026-09-17 15:55 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-17 15:55 [PATCH v7 0/15] perf tools: Annotate fixes, stdio progress indication, debuginfo-client in more places Arnaldo Carvalho de Melo
2026-09-17 15:55 ` [PATCH 01/15] perf debuginfo: Fetch debuginfo keyed by build ID using debuginfod Arnaldo Carvalho de Melo
2026-09-17 17:58   ` Ian Rogers
2026-09-17 20:36     ` Arnaldo Carvalho de Melo
2026-09-17 15:55 ` Arnaldo Carvalho de Melo [this message]
2026-09-17 15:55 ` [PATCH 03/15] perf config: Move perf_config__set_variable() to util/config.c Arnaldo Carvalho de Melo
2026-09-17 15:55 ` [PATCH 04/15] perf debuginfo: Let the user skip and disable debuginfod fetches Arnaldo Carvalho de Melo
2026-09-17 15:55 ` [PATCH 05/15] perf debuginfo: Show the debuginfod fetch progress and keys in the TUI Arnaldo Carvalho de Melo
2026-09-17 15:55 ` [PATCH 06/15] perf symbol: Fall back to fetching the vmlinux by build ID Arnaldo Carvalho de Melo
2026-09-17 15:55 ` [PATCH 07/15] perf annotate-data: Show the sample count in the data-type browser Arnaldo Carvalho de Melo
2026-09-17 15:55 ` [PATCH 08/15] perf report: Add --progress option Arnaldo Carvalho de Melo
2026-09-17 15:55 ` [PATCH 09/15] perf scripts: Add perf-stuck, to tell where a running perf is stuck Arnaldo Carvalho de Melo
2026-09-17 15:55 ` [PATCH 10/15] perf dwarf-aux: Bound the type chases for broken debug info Arnaldo Carvalho de Melo
2026-09-17 15:55 ` [PATCH 11/15] perf dwarf-aux: Add die_same_file() and die_get_type_die() Arnaldo Carvalho de Melo
2026-09-17 15:55 ` [PATCH 12/15] perf annotate-data: Resolve type DIEs in the debug file they came from Arnaldo Carvalho de Melo
2026-09-17 15:55 ` [PATCH 13/15] perf annotate-data: Bound the member nesting recursion Arnaldo Carvalho de Melo
2026-09-17 15:55 ` [PATCH 14/15] perf mem record: Request PERF_SAMPLE_CPU by default Arnaldo Carvalho de Melo
2026-09-17 15:55 ` [PATCH 15/15] perf mem record: Use the IBS swfilt filter when available Arnaldo Carvalho de Melo

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=20260917155528.62607-3-acme@kernel.org \
    --to=acme@kernel.org \
    --cc=acme@redhat.com \
    --cc=adrian.hunter@intel.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=mingo@kernel.org \
    --cc=namhyung@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=williams@redhat.com \
    /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®