[PATCH 3/3] perf addr2line: Remove global variable addr2line_timeout_ms

Thomas Richter posted 3 patches 1 week, 1 day ago
There is a newer version of this series
[PATCH 3/3] perf addr2line: Remove global variable addr2line_timeout_ms
Posted by Thomas Richter 1 week, 1 day ago
Remove global variable addr2line_timeout_ms and add is as member
to symbol_conf structure. No functional change.

Fixes: 257046a36750a ("perf srcline: Fallback between addr2line implementations")
Cc: Ian Rogers <irogers@google.com>
Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
---
 tools/perf/util/addr2line.c   | 8 ++++----
 tools/perf/util/addr2line.h   | 2 --
 tools/perf/util/config.c      | 3 +--
 tools/perf/util/symbol_conf.h | 1 +
 4 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/tools/perf/util/addr2line.c b/tools/perf/util/addr2line.c
index e9f084db0802..1b7f66ece570 100644
--- a/tools/perf/util/addr2line.c
+++ b/tools/perf/util/addr2line.c
@@ -18,9 +18,6 @@
 
 #define MAX_INLINE_NEST 1024
 
-/* If addr2line doesn't return data for 5 seconds then timeout. */
-int addr2line_timeout_ms = 5 * 1000;
-
 static int filename_split(char *filename, unsigned int *line_nr)
 {
 	char *sep;
@@ -87,6 +84,9 @@ static struct child_process *addr2line_subprocess_init(const char *addr2line_pat
 		return NULL;
 	}
 
+	if (!symbol_conf.addr2line_timeout_ms)
+		symbol_conf.addr2line_timeout_ms = 5 * 1000;
+
 	return a2l;
 }
 
@@ -335,7 +335,7 @@ int cmd__addr2line(const char *dso_name, u64 addr,
 		goto out;
 	}
 	io__init(&io, a2l->out, buf, sizeof(buf));
-	io.timeout_ms = addr2line_timeout_ms;
+	io.timeout_ms = symbol_conf.addr2line_timeout_ms;
 	switch (read_addr2line_record(&io, cmd_a2l_style, dso_name, addr, /*first=*/true,
 				      &record_function, &record_filename, &record_line_nr)) {
 	case -1:
diff --git a/tools/perf/util/addr2line.h b/tools/perf/util/addr2line.h
index d35a47ba8dab..75989a92f16b 100644
--- a/tools/perf/util/addr2line.h
+++ b/tools/perf/util/addr2line.h
@@ -8,8 +8,6 @@ struct dso;
 struct inline_node;
 struct symbol;
 
-extern int addr2line_timeout_ms;
-
 int cmd__addr2line(const char *dso_name, u64 addr,
 		   char **file, unsigned int *line_nr,
 		   struct dso *dso,
diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
index 31541e03aab7..573b3a3f5999 100644
--- a/tools/perf/util/config.c
+++ b/tools/perf/util/config.c
@@ -19,7 +19,6 @@
 #include "util/hist.h"  /* perf_hist_config */
 #include "util/stat.h"  /* perf_stat__set_big_num */
 #include "util/evsel.h"  /* evsel__hw_names, evsel__use_bpf_counters */
-#include "util/addr2line.h"  /* addr2line_timeout_ms */
 #include "srcline.h"
 #include "build-id.h"
 #include "debug.h"
@@ -459,7 +458,7 @@ static int perf_default_core_config(const char *var, const char *value)
 		proc_map_timeout = strtoul(value, NULL, 10);
 
 	if (!strcmp(var, "core.addr2line-timeout"))
-		addr2line_timeout_ms = strtoul(value, NULL, 10);
+		symbol_conf.addr2line_timeout_ms = strtoul(value, NULL, 10);
 
 	if (!strcmp(var, "core.addr2line-disable-warn"))
 		symbol_conf.addr2line_disable_warn = strtoul(value, NULL, 10);
diff --git a/tools/perf/util/symbol_conf.h b/tools/perf/util/symbol_conf.h
index ff229942fbb1..c86f4c0ca46b 100644
--- a/tools/perf/util/symbol_conf.h
+++ b/tools/perf/util/symbol_conf.h
@@ -80,6 +80,7 @@ struct symbol_conf {
 			*bt_stop_list_str;
 	const char		*addr2line_path;
 	enum a2l_style	addr2line_style[MAX_A2L_STYLE];
+	int             addr2line_timeout_ms;
 	unsigned long	time_quantum;
        struct strlist	*dso_list,
 			*comm_list,
-- 
2.53.0
Re: [PATCH 3/3] perf addr2line: Remove global variable addr2line_timeout_ms
Posted by Ian Rogers 1 week, 1 day ago
On Wed, Mar 25, 2026 at 5:06 AM Thomas Richter <tmricht@linux.ibm.com> wrote:
>
> Remove global variable addr2line_timeout_ms and add is as member
> to symbol_conf structure. No functional change.
>
> Fixes: 257046a36750a ("perf srcline: Fallback between addr2line implementations")
> Cc: Ian Rogers <irogers@google.com>
> Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>

I'm a bit curious about the no functional change and the Fixes tag :-)
I think the trend for addr2line is to use libdw. The forking of both
objdump and addr2line is quite brittle and slow.

Reviewed-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> ---
>  tools/perf/util/addr2line.c   | 8 ++++----
>  tools/perf/util/addr2line.h   | 2 --
>  tools/perf/util/config.c      | 3 +--
>  tools/perf/util/symbol_conf.h | 1 +
>  4 files changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/tools/perf/util/addr2line.c b/tools/perf/util/addr2line.c
> index e9f084db0802..1b7f66ece570 100644
> --- a/tools/perf/util/addr2line.c
> +++ b/tools/perf/util/addr2line.c
> @@ -18,9 +18,6 @@
>
>  #define MAX_INLINE_NEST 1024
>
> -/* If addr2line doesn't return data for 5 seconds then timeout. */
> -int addr2line_timeout_ms = 5 * 1000;
> -
>  static int filename_split(char *filename, unsigned int *line_nr)
>  {
>         char *sep;
> @@ -87,6 +84,9 @@ static struct child_process *addr2line_subprocess_init(const char *addr2line_pat
>                 return NULL;
>         }
>
> +       if (!symbol_conf.addr2line_timeout_ms)
> +               symbol_conf.addr2line_timeout_ms = 5 * 1000;
> +
>         return a2l;
>  }
>
> @@ -335,7 +335,7 @@ int cmd__addr2line(const char *dso_name, u64 addr,
>                 goto out;
>         }
>         io__init(&io, a2l->out, buf, sizeof(buf));
> -       io.timeout_ms = addr2line_timeout_ms;
> +       io.timeout_ms = symbol_conf.addr2line_timeout_ms;
>         switch (read_addr2line_record(&io, cmd_a2l_style, dso_name, addr, /*first=*/true,
>                                       &record_function, &record_filename, &record_line_nr)) {
>         case -1:
> diff --git a/tools/perf/util/addr2line.h b/tools/perf/util/addr2line.h
> index d35a47ba8dab..75989a92f16b 100644
> --- a/tools/perf/util/addr2line.h
> +++ b/tools/perf/util/addr2line.h
> @@ -8,8 +8,6 @@ struct dso;
>  struct inline_node;
>  struct symbol;
>
> -extern int addr2line_timeout_ms;
> -
>  int cmd__addr2line(const char *dso_name, u64 addr,
>                    char **file, unsigned int *line_nr,
>                    struct dso *dso,
> diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
> index 31541e03aab7..573b3a3f5999 100644
> --- a/tools/perf/util/config.c
> +++ b/tools/perf/util/config.c
> @@ -19,7 +19,6 @@
>  #include "util/hist.h"  /* perf_hist_config */
>  #include "util/stat.h"  /* perf_stat__set_big_num */
>  #include "util/evsel.h"  /* evsel__hw_names, evsel__use_bpf_counters */
> -#include "util/addr2line.h"  /* addr2line_timeout_ms */
>  #include "srcline.h"
>  #include "build-id.h"
>  #include "debug.h"
> @@ -459,7 +458,7 @@ static int perf_default_core_config(const char *var, const char *value)
>                 proc_map_timeout = strtoul(value, NULL, 10);
>
>         if (!strcmp(var, "core.addr2line-timeout"))
> -               addr2line_timeout_ms = strtoul(value, NULL, 10);
> +               symbol_conf.addr2line_timeout_ms = strtoul(value, NULL, 10);
>
>         if (!strcmp(var, "core.addr2line-disable-warn"))
>                 symbol_conf.addr2line_disable_warn = strtoul(value, NULL, 10);
> diff --git a/tools/perf/util/symbol_conf.h b/tools/perf/util/symbol_conf.h
> index ff229942fbb1..c86f4c0ca46b 100644
> --- a/tools/perf/util/symbol_conf.h
> +++ b/tools/perf/util/symbol_conf.h
> @@ -80,6 +80,7 @@ struct symbol_conf {
>                         *bt_stop_list_str;
>         const char              *addr2line_path;
>         enum a2l_style  addr2line_style[MAX_A2L_STYLE];
> +       int             addr2line_timeout_ms;
>         unsigned long   time_quantum;
>         struct strlist  *dso_list,
>                         *comm_list,
> --
> 2.53.0
>
Re: [PATCH 3/3] perf addr2line: Remove global variable addr2line_timeout_ms
Posted by Namhyung Kim 1 day, 1 hour ago
On Wed, Mar 25, 2026 at 08:05:22AM -0700, Ian Rogers wrote:
> On Wed, Mar 25, 2026 at 5:06 AM Thomas Richter <tmricht@linux.ibm.com> wrote:
> >
> > Remove global variable addr2line_timeout_ms and add is as member
> > to symbol_conf structure. No functional change.
> >
> > Fixes: 257046a36750a ("perf srcline: Fallback between addr2line implementations")
> > Cc: Ian Rogers <irogers@google.com>
> > Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
> 
> I'm a bit curious about the no functional change and the Fixes tag :-)

Right, it should belong somewhere else.


> I think the trend for addr2line is to use libdw. The forking of both
> objdump and addr2line is quite brittle and slow.
> 
> Reviewed-by: Ian Rogers <irogers@google.com>
> 
> Thanks,
> Ian
> 
> > ---
> >  tools/perf/util/addr2line.c   | 8 ++++----
> >  tools/perf/util/addr2line.h   | 2 --
> >  tools/perf/util/config.c      | 3 +--
> >  tools/perf/util/symbol_conf.h | 1 +
> >  4 files changed, 6 insertions(+), 8 deletions(-)
> >
> > diff --git a/tools/perf/util/addr2line.c b/tools/perf/util/addr2line.c
> > index e9f084db0802..1b7f66ece570 100644
> > --- a/tools/perf/util/addr2line.c
> > +++ b/tools/perf/util/addr2line.c
> > @@ -18,9 +18,6 @@
> >
> >  #define MAX_INLINE_NEST 1024
> >
> > -/* If addr2line doesn't return data for 5 seconds then timeout. */
> > -int addr2line_timeout_ms = 5 * 1000;
> > -
> >  static int filename_split(char *filename, unsigned int *line_nr)
> >  {
> >         char *sep;
> > @@ -87,6 +84,9 @@ static struct child_process *addr2line_subprocess_init(const char *addr2line_pat
> >                 return NULL;
> >         }
> >
> > +       if (!symbol_conf.addr2line_timeout_ms)
> > +               symbol_conf.addr2line_timeout_ms = 5 * 1000;

Please put this in the util/symbol.c where the symbol_conf is defined.

Thanks,
Namhyung


> > +
> >         return a2l;
> >  }
> >
> > @@ -335,7 +335,7 @@ int cmd__addr2line(const char *dso_name, u64 addr,
> >                 goto out;
> >         }
> >         io__init(&io, a2l->out, buf, sizeof(buf));
> > -       io.timeout_ms = addr2line_timeout_ms;
> > +       io.timeout_ms = symbol_conf.addr2line_timeout_ms;
> >         switch (read_addr2line_record(&io, cmd_a2l_style, dso_name, addr, /*first=*/true,
> >                                       &record_function, &record_filename, &record_line_nr)) {
> >         case -1:
> > diff --git a/tools/perf/util/addr2line.h b/tools/perf/util/addr2line.h
> > index d35a47ba8dab..75989a92f16b 100644
> > --- a/tools/perf/util/addr2line.h
> > +++ b/tools/perf/util/addr2line.h
> > @@ -8,8 +8,6 @@ struct dso;
> >  struct inline_node;
> >  struct symbol;
> >
> > -extern int addr2line_timeout_ms;
> > -
> >  int cmd__addr2line(const char *dso_name, u64 addr,
> >                    char **file, unsigned int *line_nr,
> >                    struct dso *dso,
> > diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
> > index 31541e03aab7..573b3a3f5999 100644
> > --- a/tools/perf/util/config.c
> > +++ b/tools/perf/util/config.c
> > @@ -19,7 +19,6 @@
> >  #include "util/hist.h"  /* perf_hist_config */
> >  #include "util/stat.h"  /* perf_stat__set_big_num */
> >  #include "util/evsel.h"  /* evsel__hw_names, evsel__use_bpf_counters */
> > -#include "util/addr2line.h"  /* addr2line_timeout_ms */
> >  #include "srcline.h"
> >  #include "build-id.h"
> >  #include "debug.h"
> > @@ -459,7 +458,7 @@ static int perf_default_core_config(const char *var, const char *value)
> >                 proc_map_timeout = strtoul(value, NULL, 10);
> >
> >         if (!strcmp(var, "core.addr2line-timeout"))
> > -               addr2line_timeout_ms = strtoul(value, NULL, 10);
> > +               symbol_conf.addr2line_timeout_ms = strtoul(value, NULL, 10);
> >
> >         if (!strcmp(var, "core.addr2line-disable-warn"))
> >                 symbol_conf.addr2line_disable_warn = strtoul(value, NULL, 10);
> > diff --git a/tools/perf/util/symbol_conf.h b/tools/perf/util/symbol_conf.h
> > index ff229942fbb1..c86f4c0ca46b 100644
> > --- a/tools/perf/util/symbol_conf.h
> > +++ b/tools/perf/util/symbol_conf.h
> > @@ -80,6 +80,7 @@ struct symbol_conf {
> >                         *bt_stop_list_str;
> >         const char              *addr2line_path;
> >         enum a2l_style  addr2line_style[MAX_A2L_STYLE];
> > +       int             addr2line_timeout_ms;
> >         unsigned long   time_quantum;
> >         struct strlist  *dso_list,
> >                         *comm_list,
> > --
> > 2.53.0
> >