[PATCH 2/6] perf annotate-data: Collect global variables in advance

Namhyung Kim posted 6 patches 1 year, 7 months ago
[PATCH 2/6] perf annotate-data: Collect global variables in advance
Posted by Namhyung Kim 1 year, 7 months ago
Currently it looks up global variables from the current CU using address
and name.  But it sometimes fails to find a variable as the variable can
come from a different CU - but it's still strange it failed to find a
declaration for some reason.

Anyway, it can collect all global variables from all CU once and then
lookup them later on.  This slightly improves the success rate of my
test data set.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/annotate-data.c | 57 +++++++++++++++++++++++++++++++--
 1 file changed, 55 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
index 12d5faff3b7a..4dd0911904f2 100644
--- a/tools/perf/util/annotate-data.c
+++ b/tools/perf/util/annotate-data.c
@@ -28,6 +28,8 @@
 /* register number of the stack pointer */
 #define X86_REG_SP 7
 
+static void delete_var_types(struct die_var_type *var_types);
+
 enum type_state_kind {
 	TSR_KIND_INVALID = 0,
 	TSR_KIND_TYPE,
@@ -557,8 +559,8 @@ static bool global_var__add(struct data_loc_info *dloc, u64 addr,
 	if (gvar == NULL)
 		return false;
 
-	gvar->name = strdup(name);
-	if (gvar->name == NULL) {
+	gvar->name = name ? strdup(name) : NULL;
+	if (name && gvar->name == NULL) {
 		free(gvar);
 		return false;
 	}
@@ -612,6 +614,53 @@ static bool get_global_var_info(struct data_loc_info *dloc, u64 addr,
 	return true;
 }
 
+static void global_var__collect(struct data_loc_info *dloc)
+{
+	Dwarf *dwarf = dloc->di->dbg;
+	Dwarf_Off off, next_off;
+	Dwarf_Die cu_die, type_die;
+	size_t header_size;
+
+	/* Iterate all CU and collect global variables that have no location in a register. */
+	off = 0;
+	while (dwarf_nextcu(dwarf, off, &next_off, &header_size,
+			    NULL, NULL, NULL) == 0) {
+		struct die_var_type *var_types = NULL;
+		struct die_var_type *pos;
+
+		if (dwarf_offdie(dwarf, off + header_size, &cu_die) == NULL) {
+			off = next_off;
+			continue;
+		}
+
+		die_collect_global_vars(&cu_die, &var_types);
+
+		for (pos = var_types; pos; pos = pos->next) {
+			const char *var_name = NULL;
+			int var_offset = 0;
+
+			if (pos->reg != -1)
+				continue;
+
+			if (!dwarf_offdie(dwarf, pos->die_off, &type_die))
+				continue;
+
+			if (!get_global_var_info(dloc, pos->addr, &var_name,
+						 &var_offset))
+				continue;
+
+			if (var_offset != 0)
+				continue;
+
+			global_var__add(dloc, pos->addr, var_name, &type_die);
+		}
+
+		delete_var_types(var_types);
+
+		off = next_off;
+	}
+}
+
 static bool get_global_var_type(Dwarf_Die *cu_die, struct data_loc_info *dloc,
 				u64 ip, u64 var_addr, int *var_offset,
 				Dwarf_Die *type_die)
@@ -620,8 +669,12 @@ static bool get_global_var_type(Dwarf_Die *cu_die, struct data_loc_info *dloc,
 	int offset;
 	const char *var_name = NULL;
 	struct global_var_entry *gvar;
+	struct dso *dso = map__dso(dloc->ms->map);
 	Dwarf_Die var_die;
 
+	if (RB_EMPTY_ROOT(&dso->global_vars))
+		global_var__collect(dloc);
+
 	gvar = global_var__find(dloc, var_addr);
 	if (gvar) {
 		if (!dwarf_offdie(dloc->di->dbg, gvar->die_offset, type_die))
-- 
2.45.0.rc1.225.g2a3ae87e7f-goog
Re: [PATCH 2/6] perf annotate-data: Collect global variables in advance
Posted by Arnaldo Carvalho de Melo 1 year, 7 months ago
On Wed, May 01, 2024 at 11:00:07PM -0700, Namhyung Kim wrote:
> Currently it looks up global variables from the current CU using address
> and name.  But it sometimes fails to find a variable as the variable can
> come from a different CU - but it's still strange it failed to find a
> declaration for some reason.
> 
> Anyway, it can collect all global variables from all CU once and then
> lookup them later on.  This slightly improves the success rate of my
> test data set.

It would be interesting you could provide examples from your test data
set, i.e. after this patch these extra global variables were considered
in the test results, with some tool output, etc.

This would help intersested parties to reproduce your results,
validate the end result, etc.

- Arnaldo
 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/util/annotate-data.c | 57 +++++++++++++++++++++++++++++++--
>  1 file changed, 55 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
> index 12d5faff3b7a..4dd0911904f2 100644
> --- a/tools/perf/util/annotate-data.c
> +++ b/tools/perf/util/annotate-data.c
> @@ -28,6 +28,8 @@
>  /* register number of the stack pointer */
>  #define X86_REG_SP 7
>  
> +static void delete_var_types(struct die_var_type *var_types);
> +
>  enum type_state_kind {
>  	TSR_KIND_INVALID = 0,
>  	TSR_KIND_TYPE,
> @@ -557,8 +559,8 @@ static bool global_var__add(struct data_loc_info *dloc, u64 addr,
>  	if (gvar == NULL)
>  		return false;
>  
> -	gvar->name = strdup(name);
> -	if (gvar->name == NULL) {
> +	gvar->name = name ? strdup(name) : NULL;
> +	if (name && gvar->name == NULL) {
>  		free(gvar);
>  		return false;
>  	}
> @@ -612,6 +614,53 @@ static bool get_global_var_info(struct data_loc_info *dloc, u64 addr,
>  	return true;
>  }
>  
> +static void global_var__collect(struct data_loc_info *dloc)
> +{
> +	Dwarf *dwarf = dloc->di->dbg;
> +	Dwarf_Off off, next_off;
> +	Dwarf_Die cu_die, type_die;
> +	size_t header_size;
> +
> +	/* Iterate all CU and collect global variables that have no location in a register. */
> +	off = 0;
> +	while (dwarf_nextcu(dwarf, off, &next_off, &header_size,
> +			    NULL, NULL, NULL) == 0) {
> +		struct die_var_type *var_types = NULL;
> +		struct die_var_type *pos;
> +
> +		if (dwarf_offdie(dwarf, off + header_size, &cu_die) == NULL) {
> +			off = next_off;
> +			continue;
> +		}
> +
> +		die_collect_global_vars(&cu_die, &var_types);
> +
> +		for (pos = var_types; pos; pos = pos->next) {
> +			const char *var_name = NULL;
> +			int var_offset = 0;
> +
> +			if (pos->reg != -1)
> +				continue;
> +
> +			if (!dwarf_offdie(dwarf, pos->die_off, &type_die))
> +				continue;
> +
> +			if (!get_global_var_info(dloc, pos->addr, &var_name,
> +						 &var_offset))
> +				continue;
> +
> +			if (var_offset != 0)
> +				continue;
> +
> +			global_var__add(dloc, pos->addr, var_name, &type_die);
> +		}
> +
> +		delete_var_types(var_types);
> +
> +		off = next_off;
> +	}
> +}
> +
>  static bool get_global_var_type(Dwarf_Die *cu_die, struct data_loc_info *dloc,
>  				u64 ip, u64 var_addr, int *var_offset,
>  				Dwarf_Die *type_die)
> @@ -620,8 +669,12 @@ static bool get_global_var_type(Dwarf_Die *cu_die, struct data_loc_info *dloc,
>  	int offset;
>  	const char *var_name = NULL;
>  	struct global_var_entry *gvar;
> +	struct dso *dso = map__dso(dloc->ms->map);
>  	Dwarf_Die var_die;
>  
> +	if (RB_EMPTY_ROOT(&dso->global_vars))
> +		global_var__collect(dloc);
> +
>  	gvar = global_var__find(dloc, var_addr);
>  	if (gvar) {
>  		if (!dwarf_offdie(dloc->di->dbg, gvar->die_offset, type_die))
> -- 
> 2.45.0.rc1.225.g2a3ae87e7f-goog
Re: [PATCH 2/6] perf annotate-data: Collect global variables in advance
Posted by Namhyung Kim 1 year, 7 months ago
On Thu, May 2, 2024 at 6:50 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> On Wed, May 01, 2024 at 11:00:07PM -0700, Namhyung Kim wrote:
> > Currently it looks up global variables from the current CU using address
> > and name.  But it sometimes fails to find a variable as the variable can
> > come from a different CU - but it's still strange it failed to find a
> > declaration for some reason.
> >
> > Anyway, it can collect all global variables from all CU once and then
> > lookup them later on.  This slightly improves the success rate of my
> > test data set.
>
> It would be interesting you could provide examples from your test data
> set, i.e. after this patch these extra global variables were considered
> in the test results, with some tool output, etc.
>
> This would help intersested parties to reproduce your results,
> validate the end result, etc.

Hmm.. ok.  I'll try to find public data that I can share.
But it's just a perf data file recorded with perf mem.

Thanks,
Namhyung
Re: [PATCH 2/6] perf annotate-data: Collect global variables in advance
Posted by Namhyung Kim 1 year, 7 months ago
On Thu, May 2, 2024 at 11:23 AM Namhyung Kim <namhyung@kernel.org> wrote:
>
> On Thu, May 2, 2024 at 6:50 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> >
> > On Wed, May 01, 2024 at 11:00:07PM -0700, Namhyung Kim wrote:
> > > Currently it looks up global variables from the current CU using address
> > > and name.  But it sometimes fails to find a variable as the variable can
> > > come from a different CU - but it's still strange it failed to find a
> > > declaration for some reason.
> > >
> > > Anyway, it can collect all global variables from all CU once and then
> > > lookup them later on.  This slightly improves the success rate of my
> > > test data set.
> >
> > It would be interesting you could provide examples from your test data
> > set, i.e. after this patch these extra global variables were considered
> > in the test results, with some tool output, etc.

Here's the example:

Before
-----------------------------------------------------------
find data type for 0x6014(PC) at entry_SYSCALL_64+0x7
CU for arch/x86/entry/entry_64.S (die:0x85e1d)
no variable found

After
-----------------------------------------------------------
find data type for 0x6014(PC) at entry_SYSCALL_64+0x7
CU for arch/x86/entry/entry_64.S (die:0x85e1d)
found by addr=0x6014 type_offset=0x14
 type='struct tss_struct' size=0x5000 (die:0x74dbe1)

It was asm code so it doesn't have the type info in the CU.
With this change it can see the type (from a different CU).
It seems we have a per-cpu variable called cpu_tss_rw at
address 0x6000.

  $ nm vmlinux | grep 6000 | grep tss
  0000000000006000 D cpu_tss_rw

Thanks,
Namhyung