Skip to content

Commit

Permalink
perf annotate-data: Add dso->data_types tree
Browse files Browse the repository at this point in the history
To aggregate accesses to the same data type, add 'data_types' tree in
DSO to maintain data types and find it by name and size.

It might have different data types that happen to have the same name,
so it also compares the size of the type.

Even if it doesn't 100% guarantee, it reduces the possibility of
mis-handling of such conflicts.

And I don't think it's common to have different types with the same
name.

Committer notes:

Very few cases on the Linux kernel, but there are some different types
with the same name, unsure if there is a debug mode in libbpf dedup that
warns about such cases, but there are provisions in pahole for that,
see:

  "emit: Notice type shadowing, i.e. multiple types with the same name (enum, struct, union, etc)"
    https://git.kernel.org/pub/scm/devel/pahole/pahole.git/commit/?id=4f332dbfd02072e4f410db7bdcda8d6e3422974b

  $ pahole --compile > vmlinux.h
  $ rm -f a ; make a
  cc     a.c   -o a
  $ grep __[0-9] vmlinux.h
  union irte__1 {
  struct map_info__1;
  struct map_info__1 {
  	struct map_info__1 *       next;                 /*     0     8 */
  $

  drivers/iommu/amd/amd_iommu_types.h 'union irte'
  include/linux/dmar.h                'struct irte'

  include/linux/device-mapper.h:

    union map_info {
            void *ptr;
    };

  include/linux/mtd/map.h:

    struct map_info {
        const char *name;
        unsigned long size;
        resource_size_t phys;
   <SNIP>

  kernel/events/uprobes.c:

   struct map_info {
        struct map_info *next;
        struct mm_struct *mm;
        unsigned long vaddr;
  };

Signed-off-by: Namhyung Kim <[email protected]>
Cc: Adrian Hunter <[email protected]>
Cc: Ian Rogers <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Masami Hiramatsu <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Cc: [email protected]
Cc: [email protected]
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
  • Loading branch information
namhyung authored and acmel committed Dec 24, 2023
1 parent b9c87f5 commit fc044c5
Show file tree
Hide file tree
Showing 4 changed files with 100 additions and 10 deletions.
95 changes: 85 additions & 10 deletions tools/perf/util/annotate-data.c
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,76 @@
#include "strbuf.h"
#include "symbol.h"

/*
* Compare type name and size to maintain them in a tree.
* I'm not sure if DWARF would have information of a single type in many
* different places (compilation units). If not, it could compare the
* offset of the type entry in the .debug_info section.
*/
static int data_type_cmp(const void *_key, const struct rb_node *node)
{
const struct annotated_data_type *key = _key;
struct annotated_data_type *type;

type = rb_entry(node, struct annotated_data_type, node);

if (key->type_size != type->type_size)
return key->type_size - type->type_size;
return strcmp(key->type_name, type->type_name);
}

static bool data_type_less(struct rb_node *node_a, const struct rb_node *node_b)
{
struct annotated_data_type *a, *b;

a = rb_entry(node_a, struct annotated_data_type, node);
b = rb_entry(node_b, struct annotated_data_type, node);

if (a->type_size != b->type_size)
return a->type_size < b->type_size;
return strcmp(a->type_name, b->type_name) < 0;
}

static struct annotated_data_type *dso__findnew_data_type(struct dso *dso,
Dwarf_Die *type_die)
{
struct annotated_data_type *result = NULL;
struct annotated_data_type key;
struct rb_node *node;
struct strbuf sb;
char *type_name;
Dwarf_Word size;

strbuf_init(&sb, 32);
if (die_get_typename_from_type(type_die, &sb) < 0)
strbuf_add(&sb, "(unknown type)", 14);
type_name = strbuf_detach(&sb, NULL);
dwarf_aggregate_size(type_die, &size);

/* Check existing nodes in dso->data_types tree */
key.type_name = type_name;
key.type_size = size;
node = rb_find(&key, &dso->data_types, data_type_cmp);
if (node) {
result = rb_entry(node, struct annotated_data_type, node);
free(type_name);
return result;
}

/* If not, add a new one */
result = zalloc(sizeof(*result));
if (result == NULL) {
free(type_name);
return NULL;
}

result->type_name = type_name;
result->type_size = size;

rb_add(&result->node, &dso->data_types, data_type_less);
return result;
}

static bool find_cu_die(struct debuginfo *di, u64 pc, Dwarf_Die *cu_die)
{
Dwarf_Off off, next_off;
Expand Down Expand Up @@ -130,7 +200,6 @@ struct annotated_data_type *find_data_type(struct map_symbol *ms, u64 ip,
struct dso *dso = map__dso(ms->map);
struct debuginfo *di;
Dwarf_Die type_die;
struct strbuf sb;
u64 pc;

di = debuginfo__new(dso->long_name);
Expand All @@ -148,17 +217,23 @@ struct annotated_data_type *find_data_type(struct map_symbol *ms, u64 ip,
if (find_data_type_die(di, pc, reg, offset, &type_die) < 0)
goto out;

result = zalloc(sizeof(*result));
if (result == NULL)
goto out;

strbuf_init(&sb, 32);
if (die_get_typename_from_type(&type_die, &sb) < 0)
strbuf_add(&sb, "(unknown type)", 14);

result->type_name = strbuf_detach(&sb, NULL);
result = dso__findnew_data_type(dso, &type_die);

out:
debuginfo__delete(di);
return result;
}

void annotated_data_type__tree_delete(struct rb_root *root)
{
struct annotated_data_type *pos;

while (!RB_EMPTY_ROOT(root)) {
struct rb_node *node = rb_first(root);

rb_erase(node, root);
pos = rb_entry(node, struct annotated_data_type, node);
free(pos->type_name);
free(pos);
}
}
9 changes: 9 additions & 0 deletions tools/perf/util/annotate-data.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include <errno.h>
#include <linux/compiler.h>
#include <linux/rbtree.h>
#include <linux/types.h>

struct map_symbol;
Expand All @@ -16,6 +17,7 @@ struct map_symbol;
* This represents a data type accessed by samples in the profile data.
*/
struct annotated_data_type {
struct rb_node node;
char *type_name;
int type_size;
};
Expand All @@ -26,6 +28,9 @@ struct annotated_data_type {
struct annotated_data_type *find_data_type(struct map_symbol *ms, u64 ip,
int reg, int offset);

/* Release all data type information in the tree */
void annotated_data_type__tree_delete(struct rb_root *root);

#else /* HAVE_DWARF_SUPPORT */

static inline struct annotated_data_type *
Expand All @@ -35,6 +40,10 @@ find_data_type(struct map_symbol *ms __maybe_unused, u64 ip __maybe_unused,
return NULL;
}

static inline void annotated_data_type__tree_delete(struct rb_root *root __maybe_unused)
{
}

#endif /* HAVE_DWARF_SUPPORT */

#endif /* _PERF_ANNOTATE_DATA_H */
4 changes: 4 additions & 0 deletions tools/perf/util/dso.c
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#include "debug.h"
#include "string2.h"
#include "vdso.h"
#include "annotate-data.h"

static const char * const debuglink_paths[] = {
"%.0s%s",
Expand Down Expand Up @@ -1327,6 +1328,7 @@ struct dso *dso__new_id(const char *name, struct dso_id *id)
dso->data.cache = RB_ROOT;
dso->inlined_nodes = RB_ROOT_CACHED;
dso->srclines = RB_ROOT_CACHED;
dso->data_types = RB_ROOT;
dso->data.fd = -1;
dso->data.status = DSO_DATA_STATUS_UNKNOWN;
dso->symtab_type = DSO_BINARY_TYPE__NOT_FOUND;
Expand Down Expand Up @@ -1370,6 +1372,8 @@ void dso__delete(struct dso *dso)
symbols__delete(&dso->symbols);
dso->symbol_names_len = 0;
zfree(&dso->symbol_names);
annotated_data_type__tree_delete(&dso->data_types);

if (dso->short_name_allocated) {
zfree((char **)&dso->short_name);
dso->short_name_allocated = false;
Expand Down
2 changes: 2 additions & 0 deletions tools/perf/util/dso.h
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,8 @@ struct dso {
size_t symbol_names_len;
struct rb_root_cached inlined_nodes;
struct rb_root_cached srclines;
struct rb_root data_types;

struct {
u64 addr;
struct symbol *symbol;
Expand Down

0 comments on commit fc044c5

Please sign in to comment.