perf record: Disable debuginfod by default

Fedora 35 sets DEBUGINFOD_URLS by default, which might lead to
unexpected stalls in perf record exit path, when we try to cache
profiled binaries.

  # DEBUGINFOD_PROGRESS=1 ./perf record -a
  ^C[ perf record: Woken up 1 times to write data ]
  Downloading from https://debuginfod.fedoraproject.org/ 447069
  Downloading from https://debuginfod.fedoraproject.org/ 1502175
  Downloading \^Z

Disabling DEBUGINFOD_URLS by default in perf record and adding
debuginfod option and .perfconfig variable support to enable id.

  Default without debuginfo processing:
  # perf record -a

  Using system debuginfod setup:
  # perf record -a --debuginfod

  Using custom debuginfd url:
  # perf record -a --debuginfod='https://evenbetterdebuginfodserver.krava'

Adding single perf_debuginfod_setup function and using
it also in perf buildid-cache command.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Frank Ch. Eigler <fche@redhat.com>
Cc: Ian Rogers <irogers@google.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Michael Petlan <mpetlan@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lore.kernel.org/lkml/20211209200425.303561-1-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

diff --git a/tools/perf/Documentation/perf-buildid-cache.txt b/tools/perf/Documentation/perf-buildid-cache.txt
index cd8ce6e..7e44b41 100644 (file)
--- a/tools/perf/Documentation/perf-buildid-cache.txt
+++ b/tools/perf/Documentation/perf-buildid-cache.txt
@@ -74,12 +74,15 @@ OPTIONS
        used when creating a uprobe for a process that resides in a
        different mount namespace from the perf(1) utility.
 
---debuginfod=URLs::
+--debuginfod[=URLs]::
        Specify debuginfod URL to be used when retrieving perf.data binaries,
        it follows the same syntax as the DEBUGINFOD_URLS variable, like:
 
          buildid-cache.debuginfod=http://192.168.122.174:8002
 
+       If the URLs is not specified, the value of DEBUGINFOD_URLS
+       system environment variable is used.
+
 SEE ALSO
 --------
 linkperf:perf-record[1], linkperf:perf-report[1], linkperf:perf-buildid-list[1]

diff --git a/tools/perf/Documentation/perf-config.txt b/tools/perf/Documentation/perf-config.txt
index 3bb75c1..0420e71 100644 (file)
--- a/tools/perf/Documentation/perf-config.txt
+++ b/tools/perf/Documentation/perf-config.txt
@@ -587,6 +587,15 @@ record.*::
                Use 'n' control blocks in asynchronous (Posix AIO) trace writing
                mode ('n' default: 1, max: 4).
 
+       record.debuginfod::
+               Specify debuginfod URL to be used when cacheing perf.data binaries,
+               it follows the same syntax as the DEBUGINFOD_URLS variable, like:
+
+                 http://192.168.122.174:8002
+
+               If the URLs is 'system', the value of DEBUGINFOD_URLS system environment
+               variable is used.
+
 diff.*::
        diff.order::
                This option sets the number of columns to sort the result.

diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index 55df7b0..9ccc759 100644 (file)
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -715,6 +715,15 @@ measurements:
 
 include::intel-hybrid.txt[]
 
+--debuginfod[=URLs]::
+       Specify debuginfod URL to be used when cacheing perf.data binaries,
+       it follows the same syntax as the DEBUGINFOD_URLS variable, like:
+
+         http://192.168.122.174:8002
+
+       If the URLs is not specified, the value of DEBUGINFOD_URLS
+       system environment variable is used.
+
 SEE ALSO
 --------
 linkperf:perf-stat[1], linkperf:perf-list[1], linkperf:perf-intel-pt[1]

diff --git a/tools/perf/builtin-buildid-cache.c b/tools/perf/builtin-buildid-cache.c
index 0db3cfc..cd38169 100644 (file)
--- a/tools/perf/builtin-buildid-cache.c
+++ b/tools/perf/builtin-buildid-cache.c
@@ -351,10 +351,14 @@ static int build_id_cache__show_all(void)
 
 static int perf_buildid_cache_config(const char *var, const char *value, void *cb)
 {
-       const char **debuginfod = cb;
+       struct perf_debuginfod *di = cb;
 
-       if (!strcmp(var, "buildid-cache.debuginfod"))
-               *debuginfod = strdup(value);
+       if (!strcmp(var, "buildid-cache.debuginfod")) {
+               di->urls = strdup(value);
+               if (!di->urls)
+                       return -ENOMEM;
+               di->set = true;
+       }
 
        return 0;
 }
@@ -373,8 +377,8 @@ int cmd_buildid_cache(int argc, const char **argv)
                   *purge_name_list_str = NULL,
                   *missing_filename = NULL,
                   *update_name_list_str = NULL,
-                  *kcore_filename = NULL,
-                  *debuginfod = NULL;
+                  *kcore_filename = NULL;
+       struct perf_debuginfod debuginfod = { };
        char sbuf[STRERR_BUFSIZE];
 
        struct perf_data data = {
@@ -399,8 +403,10 @@ int cmd_buildid_cache(int argc, const char **argv)
        OPT_BOOLEAN('f', "force", &force, "don't complain, do it"),
        OPT_STRING('u', "update", &update_name_list_str, "file list",
                    "file(s) to update"),
-       OPT_STRING(0, "debuginfod", &debuginfod, "debuginfod url",
-                   "set debuginfod url"),
+       OPT_STRING_OPTARG_SET(0, "debuginfod", &debuginfod.urls,
+                       &debuginfod.set, "debuginfod urls",
+                       "Enable debuginfod data retrieval from DEBUGINFOD_URLS or specified urls",
+                       "system"),
        OPT_INCR('v', "verbose", &verbose, "be more verbose"),
        OPT_INTEGER(0, "target-ns", &ns_id, "target pid for namespace context"),
        OPT_END()
@@ -425,10 +431,7 @@ int cmd_buildid_cache(int argc, const char **argv)
        if (argc || !(list_files || opts_flag))
                usage_with_options(buildid_cache_usage, buildid_cache_options);
 
-       if (debuginfod) {
-               pr_debug("DEBUGINFOD_URLS=%s\n", debuginfod);
-               setenv("DEBUGINFOD_URLS", debuginfod, 1);
-       }
+       perf_debuginfod_setup(&debuginfod);
 
        /* -l is exclusive. It can not be used with other options. */
        if (list_files && opts_flag) {

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 0a63295..bb716c9 100644 (file)
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -111,6 +111,7 @@ struct record {
        unsigned long long      samples;
        struct mmap_cpu_mask    affinity_mask;
        unsigned long           output_max_size;        /* = 0: unlimited */
+       struct perf_debuginfod  debuginfod;
 };
 
 static volatile int done;
@@ -2177,6 +2178,12 @@ static int perf_record_config(const char *var, const char *value, void *cb)
                        rec->opts.nr_cblocks = nr_cblocks_default;
        }
 #endif
+       if (!strcmp(var, "record.debuginfod")) {
+               rec->debuginfod.urls = strdup(value);
+               if (!rec->debuginfod.urls)
+                       return -ENOMEM;
+               rec->debuginfod.set = true;
+       }
 
        return 0;
 }
@@ -2667,6 +2674,10 @@ static struct option __record_options[] = {
                      parse_control_option),
        OPT_CALLBACK(0, "synth", &record.opts, "no|all|task|mmap|cgroup",
                     "Fine-tune event synthesis: default=all", parse_record_synth_option),
+       OPT_STRING_OPTARG_SET(0, "debuginfod", &record.debuginfod.urls,
+                         &record.debuginfod.set, "debuginfod urls",
+                         "Enable debuginfod data retrieval from DEBUGINFOD_URLS or specified urls",
+                         "system"),
        OPT_END()
 };
 
@@ -2720,6 +2731,8 @@ int cmd_record(int argc, const char **argv)
        if (err)
                return err;
 
+       perf_debuginfod_setup(&record.debuginfod);
+
        /* Make system wide (-a) the default target. */
        if (!argc && target__none(&rec->opts.target))
                rec->opts.target.system_wide = true;

diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c
index df3c467..fb4f661 100644 (file)
--- a/tools/perf/util/util.c
+++ b/tools/perf/util/util.c
@@ -416,3 +416,18 @@ char *perf_exe(char *buf, int len)
        }
        return strcpy(buf, "perf");
 }
+
+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.
+        */
+       if (!di->set)
+               setenv("DEBUGINFOD_URLS", "", 1);
+       else if (di->urls && strcmp(di->urls, "system"))
+               setenv("DEBUGINFOD_URLS", di->urls, 1);
+
+       pr_debug("DEBUGINFOD_URLS=%s\n", getenv("DEBUGINFOD_URLS"));
+}

diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index 9443c29..7b625cb 100644 (file)
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -71,4 +71,10 @@ void test_attr__init(void);
 struct perf_event_attr;
 void test_attr__open(struct perf_event_attr *attr, pid_t pid, struct perf_cpu cpu,
                     int fd, int group_fd, unsigned long flags);
+
+struct perf_debuginfod {
+       const char      *urls;
+       bool             set;
+};
+void perf_debuginfod_setup(struct perf_debuginfod *di);
 #endif /* GIT_COMPAT_UTIL_H */