bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 dwarves 0/8] Switch BTF loading and encoding to libbpf APIs
@ 2020-10-08 23:39 Andrii Nakryiko
  2020-10-08 23:39 ` [PATCH v2 dwarves 1/8] btf_loader: use libbpf to load BTF Andrii Nakryiko
                   ` (8 more replies)
  0 siblings, 9 replies; 15+ messages in thread
From: Andrii Nakryiko @ 2020-10-08 23:39 UTC (permalink / raw)
  To: dwarves; +Cc: bpf, kernel-team, ast, Andrii Nakryiko, Arnaldo Carvalho de Melo

This patch set switches pahole to use libbpf-provided BTF loading and encoding
APIs. This reduces pahole's own BTF encoding code, speeds up the process,
reduces amount of RAM needed for DWARF-to-BTF conversion. Also, pahole finally
gets support to generating BTF for cross-compiled ELF binaries with different
endianness (patch #8).

Additionally, patch #3 fixes previously missed problem with invalid array
index type generation.

Patches #4-7 are speeding up DWARF-to-BTF convertion/dedup pretty
significantly, saving overall about 9 seconds out of current 27 or so.

Patch #5 revamps how per-CPU BTF variables are emitted, eliminating repeated
and expensive looping over ELF symbols table. The critical detail that took
few hours of investigation is that when DW_AT_variable has
DW_AT_specification, variable address (to correlate with symbol's address) has
to be taken before specification is followed.

More details could be found in respective patches.

v1->v2:
  - rebase on latest dwarves master and fix var->spec's address problem.

Cc: Arnaldo Carvalho de Melo <acme@kernel.org>

Andrii Nakryiko (8):
  btf_loader: use libbpf to load BTF
  btf_encoder: use libbpf APIs to encode BTF type info
  btf_encoder: fix emitting __ARRAY_SIZE_TYPE__ as index range type
  btf_encoder: discard CUs after BTF encoding
  btf_encoder: revamp how per-CPU variables are encoded
  dwarf_loader: increase the size of lookup hash map
  strings: use BTF's string APIs for strings management
  btf_encoder: support cross-compiled ELF binaries with different
    endianness

 btf_encoder.c  | 370 +++++++++++++++------------
 btf_loader.c   | 244 +++++++-----------
 ctf_encoder.c  |   2 +-
 dwarf_loader.c |   2 +-
 libbtf.c       | 661 +++++++++++++++++++++----------------------------
 libbtf.h       |  41 ++-
 libctf.c       |  14 +-
 libctf.h       |   4 +-
 pahole.c       |   2 +-
 strings.c      |  91 +++----
 strings.h      |  32 +--
 11 files changed, 645 insertions(+), 818 deletions(-)

-- 
2.24.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 dwarves 1/8] btf_loader: use libbpf to load BTF
  2020-10-08 23:39 [PATCH v2 dwarves 0/8] Switch BTF loading and encoding to libbpf APIs Andrii Nakryiko
@ 2020-10-08 23:39 ` Andrii Nakryiko
  2020-10-09 14:53   ` Arnaldo Carvalho de Melo
  2020-10-08 23:39 ` [PATCH v2 dwarves 2/8] btf_encoder: use libbpf APIs to encode BTF type info Andrii Nakryiko
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 15+ messages in thread
From: Andrii Nakryiko @ 2020-10-08 23:39 UTC (permalink / raw)
  To: dwarves
  Cc: bpf, kernel-team, ast, Andrii Nakryiko, Arnaldo Carvalho de Melo,
	Andrii Nakryiko

From: Andrii Nakryiko <andriin@fb.com>

Switch BTF loading to completely use libbpf's own struct btf and related APIs.
BTF encoding is still happening with pahole's own code, so these two code
paths are not sharing anything now. String fetching is happening based on
whether btfe->strings were set to non-NULL pointer by btf_encoder.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 btf_loader.c | 244 +++++++++++++++++++--------------------------------
 libbtf.c     | 116 +++++-------------------
 libbtf.h     |  11 +--
 3 files changed, 113 insertions(+), 258 deletions(-)

diff --git a/btf_loader.c b/btf_loader.c
index 8fc6c3c15f25..9b5da3a4997a 100644
--- a/btf_loader.c
+++ b/btf_loader.c
@@ -46,21 +46,17 @@ static void *tag__alloc(const size_t size)
 }
 
 static int btf_elf__load_ftype(struct btf_elf *btfe, struct ftype *proto, uint32_t tag,
-			       uint32_t type, uint16_t vlen, struct btf_param *args, uint32_t id)
+			       const struct btf_type *tp, uint32_t id)
 {
-	int i;
+	const struct btf_param *param = btf_params(tp);
+	int i, vlen = btf_vlen(tp);
 
 	proto->tag.tag	= tag;
-	proto->tag.type = type;
+	proto->tag.type = tp->type;
 	INIT_LIST_HEAD(&proto->parms);
 
-	for (i = 0; i < vlen; ++i) {
-		struct btf_param param = {
-		       .name_off = btf_elf__get32(btfe, &args[i].name_off),
-		       .type	 = btf_elf__get32(btfe, &args[i].type),
-		};
-
-		if (param.type == 0)
+	for (i = 0; i < vlen; ++i, param++) {
+		if (param->type == 0)
 			proto->unspec_parms = 1;
 		else {
 			struct parameter *p = tag__alloc(sizeof(*p));
@@ -68,25 +64,22 @@ static int btf_elf__load_ftype(struct btf_elf *btfe, struct ftype *proto, uint32
 			if (p == NULL)
 				goto out_free_parameters;
 			p->tag.tag  = DW_TAG_formal_parameter;
-			p->tag.type = param.type;
-			p->name	    = param.name_off;
+			p->tag.type = param->type;
+			p->name	    = param->name_off;
 			ftype__add_parameter(proto, p);
 		}
 	}
 
-	vlen *= sizeof(*args);
 	cu__add_tag_with_id(btfe->priv, &proto->tag, id);
 
-	return vlen;
+	return 0;
 out_free_parameters:
 	ftype__delete(proto, btfe->priv);
 	return -ENOMEM;
 }
 
-static int create_new_function(struct btf_elf *btfe, struct btf_type *tp, uint64_t size, uint32_t id)
+static int create_new_function(struct btf_elf *btfe, const struct btf_type *tp, uint32_t id)
 {
-	strings_t name = btf_elf__get32(btfe, &tp->name_off);
-	unsigned int type_id = btf_elf__get32(btfe, &tp->type);
 	struct function *func = tag__alloc(sizeof(*func));
 
 	if (func == NULL)
@@ -96,9 +89,9 @@ static int create_new_function(struct btf_elf *btfe, struct btf_type *tp, uint64
 	// but the prototype, the return type is the one in type_id
 	func->btf = 1;
 	func->proto.tag.tag = DW_TAG_subprogram;
-	func->proto.tag.type = type_id;
+	func->proto.tag.type = tp->type;
+	func->name = tp->name_off;
 	INIT_LIST_HEAD(&func->lexblock.tags);
-	func->name = name;
 	cu__add_tag_with_id(btfe->priv, &func->proto.tag, id);
 
 	return 0;
@@ -166,26 +159,24 @@ static struct variable *variable__new(strings_t name, uint32_t linkage)
 	return var;
 }
 
-static int create_new_base_type(struct btf_elf *btfe, void *ptr, struct btf_type *tp, uint32_t id)
+static int create_new_base_type(struct btf_elf *btfe, const struct btf_type *tp, uint32_t id)
 {
-	uint32_t *enc = ptr;
-	uint32_t eval = btf_elf__get32(btfe, enc);
-	uint32_t attrs = BTF_INT_ENCODING(eval);
-	strings_t name = btf_elf__get32(btfe, &tp->name_off);
-	struct base_type *base = base_type__new(name, attrs, 0,
-						BTF_INT_BITS(eval));
+	uint32_t attrs = btf_int_encoding(tp);
+	strings_t name = tp->name_off;
+	struct base_type *base = base_type__new(name, attrs, 0, btf_int_bits(tp));
+
 	if (base == NULL)
 		return -ENOMEM;
 
 	base->tag.tag = DW_TAG_base_type;
 	cu__add_tag_with_id(btfe->priv, &base->tag, id);
 
-	return sizeof(*enc);
+	return 0;
 }
 
-static int create_new_array(struct btf_elf *btfe, void *ptr, uint32_t id)
+static int create_new_array(struct btf_elf *btfe, const struct btf_type *tp, uint32_t id)
 {
-	struct btf_array *ap = ptr;
+	struct btf_array *ap = btf_array(tp);
 	struct array_type *array = tag__alloc(sizeof(*array));
 
 	if (array == NULL)
@@ -201,81 +192,67 @@ static int create_new_array(struct btf_elf *btfe, void *ptr, uint32_t id)
 		return -ENOMEM;
 	}
 
-	array->nr_entries[0] = btf_elf__get32(btfe, &ap->nelems);
+	array->nr_entries[0] = ap->nelems;
 	array->tag.tag = DW_TAG_array_type;
-	array->tag.type = btf_elf__get32(btfe, &ap->type);
+	array->tag.type = ap->type;
 
 	cu__add_tag_with_id(btfe->priv, &array->tag, id);
 
-	return sizeof(*ap);
+	return 0;
 }
 
-static int create_members(struct btf_elf *btfe, void *ptr, int vlen, struct type *class,
-			  bool kflag)
+static int create_members(struct btf_elf *btfe, const struct btf_type *tp,
+			  struct type *class)
 {
-	struct btf_member *mp = ptr;
-	int i;
+	struct btf_member *mp = btf_members(tp);
+	int i, vlen = btf_vlen(tp);
 
 	for (i = 0; i < vlen; i++) {
 		struct class_member *member = zalloc(sizeof(*member));
-		uint32_t offset;
 
 		if (member == NULL)
 			return -ENOMEM;
 
 		member->tag.tag    = DW_TAG_member;
-		member->tag.type   = btf_elf__get32(btfe, &mp[i].type);
-		member->name	   = btf_elf__get32(btfe, &mp[i].name_off);
-		offset = btf_elf__get32(btfe, &mp[i].offset);
-		if (kflag) {
-			member->bit_offset = BTF_MEMBER_BIT_OFFSET(offset);
-			member->bitfield_size = BTF_MEMBER_BITFIELD_SIZE(offset);
-		} else {
-			member->bit_offset = offset;
-			member->bitfield_size = 0;
-		}
+		member->tag.type   = mp[i].type;
+		member->name	   = mp[i].name_off;
+		member->bit_offset = btf_member_bit_offset(tp, i);
+		member->bitfield_size = btf_member_bitfield_size(tp, i);
 		member->byte_offset = member->bit_offset / 8;
 		/* sizes and offsets will be corrected at class__fixup_btf_bitfields */
 		type__add_member(class, member);
 	}
 
-	return sizeof(*mp);
+	return 0;
 }
 
-static int create_new_class(struct btf_elf *btfe, void *ptr, int vlen,
-			    struct btf_type *tp, uint64_t size, uint32_t id,
-			    bool kflag)
+static int create_new_class(struct btf_elf *btfe, const struct btf_type *tp, uint32_t id)
 {
-	strings_t name = btf_elf__get32(btfe, &tp->name_off);
-	struct class *class = class__new(name, size);
-	int member_size = create_members(btfe, ptr, vlen, &class->type, kflag);
+	struct class *class = class__new(tp->name_off, tp->size);
+	int member_size = create_members(btfe, tp, &class->type);
 
 	if (member_size < 0)
 		goto out_free;
 
 	cu__add_tag_with_id(btfe->priv, &class->type.namespace.tag, id);
 
-	return (vlen * member_size);
+	return 0;
 out_free:
 	class__delete(class, btfe->priv);
 	return -ENOMEM;
 }
 
-static int create_new_union(struct btf_elf *btfe, void *ptr,
-			    int vlen, struct btf_type *tp,
-			    uint64_t size, uint32_t id,
-			    bool kflag)
+static int create_new_union(struct btf_elf *btfe, const struct btf_type *tp, uint32_t id)
 {
-	strings_t name = btf_elf__get32(btfe, &tp->name_off);
-	struct type *un = type__new(DW_TAG_union_type, name, size);
-	int member_size = create_members(btfe, ptr, vlen, un, kflag);
+	struct type *un = type__new(DW_TAG_union_type, tp->name_off, tp->size);
+	int member_size = create_members(btfe, tp, un);
 
 	if (member_size < 0)
 		goto out_free;
 
 	cu__add_tag_with_id(btfe->priv, &un->namespace.tag, id);
 
-	return (vlen * member_size);
+	return 0;
 out_free:
 	type__delete(un, btfe->priv);
 	return -ENOMEM;
@@ -294,22 +271,20 @@ static struct enumerator *enumerator__new(strings_t name, uint32_t value)
 	return en;
 }
 
-static int create_new_enumeration(struct btf_elf *btfe, void *ptr,
-				  int vlen, struct btf_type *tp,
-				  uint16_t size, uint32_t id)
+static int create_new_enumeration(struct btf_elf *btfe, const struct btf_type *tp, uint32_t id)
 {
-	struct btf_enum *ep = ptr;
-	uint16_t i;
+	struct btf_enum *ep = btf_enum(tp);
+	uint16_t i, vlen = btf_vlen(tp);
 	struct type *enumeration = type__new(DW_TAG_enumeration_type,
-					     btf_elf__get32(btfe, &tp->name_off),
-					     size ? size * 8 : (sizeof(int) * 8));
+					     tp->name_off,
+					     tp->size ? tp->size * 8 : (sizeof(int) * 8));
 
 	if (enumeration == NULL)
 		return -ENOMEM;
 
 	for (i = 0; i < vlen; i++) {
-		strings_t name = btf_elf__get32(btfe, &ep[i].name_off);
-		uint32_t value = btf_elf__get32(btfe, (uint32_t *)&ep[i].val);
+		strings_t name = ep[i].name_off;
+		uint32_t value = ep[i].val;
 		struct enumerator *enumerator = enumerator__new(name, value);
 
 		if (enumerator == NULL)
@@ -320,32 +295,25 @@ static int create_new_enumeration(struct btf_elf *btfe, void *ptr,
 
 	cu__add_tag_with_id(btfe->priv, &enumeration->namespace.tag, id);
 
-	return (vlen * sizeof(*ep));
+	return 0;
 out_free:
 	enumeration__delete(enumeration, btfe->priv);
 	return -ENOMEM;
 }
 
-static int create_new_subroutine_type(struct btf_elf *btfe, void *ptr,
-				      int vlen, struct btf_type *tp,
-				      uint32_t id)
+static int create_new_subroutine_type(struct btf_elf *btfe, const struct btf_type *tp, uint32_t id)
 {
-	struct btf_param *args = ptr;
-	unsigned int type = btf_elf__get32(btfe, &tp->type);
 	struct ftype *proto = tag__alloc(sizeof(*proto));
 
 	if (proto == NULL)
 		return -ENOMEM;
 
-	vlen = btf_elf__load_ftype(btfe, proto, DW_TAG_subroutine_type, type, vlen, args, id);
-	return vlen < 0 ? -ENOMEM : vlen;
+	return btf_elf__load_ftype(btfe, proto, DW_TAG_subroutine_type, tp, id);
 }
 
-static int create_new_forward_decl(struct btf_elf *btfe, struct btf_type *tp,
-				   uint64_t size, uint32_t id)
+static int create_new_forward_decl(struct btf_elf *btfe, const struct btf_type *tp, uint32_t id)
 {
-	strings_t name = btf_elf__get32(btfe, &tp->name_off);
-	struct class *fwd = class__new(name, size);
+	struct class *fwd = class__new(tp->name_off, 0);
 
 	if (fwd == NULL)
 		return -ENOMEM;
@@ -354,41 +322,33 @@ static int create_new_forward_decl(struct btf_elf *btfe, struct btf_type *tp,
 	return 0;
 }
 
-static int create_new_typedef(struct btf_elf *btfe, struct btf_type *tp, uint64_t size, uint32_t id)
+static int create_new_typedef(struct btf_elf *btfe, const struct btf_type *tp, uint32_t id)
 {
-	strings_t name = btf_elf__get32(btfe, &tp->name_off);
-	unsigned int type_id = btf_elf__get32(btfe, &tp->type);
-	struct type *type = type__new(DW_TAG_typedef, name, size);
+	struct type *type = type__new(DW_TAG_typedef, tp->name_off, 0);
 
 	if (type == NULL)
 		return -ENOMEM;
 
-	type->namespace.tag.type = type_id;
+	type->namespace.tag.type = tp->type;
 	cu__add_tag_with_id(btfe->priv, &type->namespace.tag, id);
 
 	return 0;
 }
 
-static int create_new_variable(struct btf_elf *btfe, void *ptr, struct btf_type *tp,
-			       uint64_t size, uint32_t id)
+static int create_new_variable(struct btf_elf *btfe, const struct btf_type *tp, uint32_t id)
 {
-	strings_t name = btf_elf__get32(btfe, &tp->name_off);
-	unsigned int type_id = btf_elf__get32(btfe, &tp->type);
-	struct btf_var *bvar = ptr;
-	uint32_t linkage = btf_elf__get32(btfe, &bvar->linkage);
-	struct variable *var = variable__new(name, linkage);
+	struct btf_var *bvar = btf_var(tp);
+	struct variable *var = variable__new(tp->name_off, bvar->linkage);
 
 	if (var == NULL)
 		return -ENOMEM;
 
-	var->ip.tag.type = type_id;
+	var->ip.tag.type = tp->type;
 	cu__add_tag_with_id(btfe->priv, &var->ip.tag, id);
-	return sizeof(*bvar);
+	return 0;
 }
 
-static int create_new_datasec(struct btf_elf *btfe, void *ptr, int vlen,
-			      struct btf_type *tp, uint64_t size, uint32_t id,
-			      bool kflag)
+static int create_new_datasec(struct btf_elf *btfe, const struct btf_type *tp, uint32_t id)
 {
 	//strings_t name = btf_elf__get32(btfe, &tp->name_off);
 
@@ -398,12 +358,11 @@ static int create_new_datasec(struct btf_elf *btfe, void *ptr, int vlen,
 	 * FIXME: this will not be used to reconstruct some original C code,
 	 * its about runtime placement of variables so just ignore this for now
 	 */
-	return vlen * sizeof(struct btf_var_secinfo);
+	return 0;
 }
 
-static int create_new_tag(struct btf_elf *btfe, int type, struct btf_type *tp, uint32_t id)
+static int create_new_tag(struct btf_elf *btfe, int type, const struct btf_type *tp, uint32_t id)
 {
-	unsigned int type_id = btf_elf__get32(btfe, &tp->type);
 	struct tag *tag = zalloc(sizeof(*tag));
 
 	if (tag == NULL)
@@ -420,104 +379,77 @@ static int create_new_tag(struct btf_elf *btfe, int type, struct btf_type *tp, u
 		return 0;
 	}
 
-	tag->type = type_id;
+	tag->type = tp->type;
 	cu__add_tag_with_id(btfe->priv, tag, id);
 
 	return 0;
 }
 
-void *btf_elf__get_buffer(struct btf_elf *btfe)
-{
-	return btfe->data;
-}
-
-size_t btf_elf__get_size(struct btf_elf *btfe)
-{
-	return btfe->size;
-}
-
 static int btf_elf__load_types(struct btf_elf *btfe)
 {
-	void *btf_buffer = btf_elf__get_buffer(btfe);
-	struct btf_header *hp = btf_buffer;
-	void *btf_contents = btf_buffer + sizeof(*hp),
-	     *type_section = (btf_contents + btf_elf__get32(btfe, &hp->type_off)),
-	     *strings_section = (btf_contents + btf_elf__get32(btfe, &hp->str_off));
-	struct btf_type *type_ptr = type_section,
-			*end = strings_section;
-	uint32_t type_index = 0x0001;
-
-	while (type_ptr < end) {
-		uint32_t val  = btf_elf__get32(btfe, &type_ptr->info);
-		uint32_t type = BTF_INFO_KIND(val);
-		int	 vlen = BTF_INFO_VLEN(val);
-		void	 *ptr = type_ptr;
-		uint32_t size = btf_elf__get32(btfe, &type_ptr->size);
-		bool     kflag = BTF_INFO_KFLAG(val);
-
-		ptr += sizeof(struct btf_type);
+	uint32_t type_index;
+	int err;
+
+	for (type_index = 1; type_index <= btf__get_nr_types(btfe->btf); type_index++) {
+		const struct btf_type *type_ptr = btf__type_by_id(btfe->btf, type_index);
+		uint32_t type = btf_kind(type_ptr);
 
 		switch (type) {
 		case BTF_KIND_INT:
-			vlen = create_new_base_type(btfe, ptr, type_ptr, type_index);
+			err = create_new_base_type(btfe, type_ptr, type_index);
 			break;
 		case BTF_KIND_ARRAY:
-			vlen = create_new_array(btfe, ptr, type_index);
+			err = create_new_array(btfe, type_ptr, type_index);
 			break;
 		case BTF_KIND_STRUCT:
-			vlen = create_new_class(btfe, ptr, vlen, type_ptr, size, type_index, kflag);
+			err = create_new_class(btfe, type_ptr, type_index);
 			break;
 		case BTF_KIND_UNION:
-			vlen = create_new_union(btfe, ptr, vlen, type_ptr, size, type_index, kflag);
+			err = create_new_union(btfe, type_ptr, type_index);
 			break;
 		case BTF_KIND_ENUM:
-			vlen = create_new_enumeration(btfe, ptr, vlen, type_ptr, size, type_index);
+			err = create_new_enumeration(btfe, type_ptr, type_index);
 			break;
 		case BTF_KIND_FWD:
-			vlen = create_new_forward_decl(btfe, type_ptr, size, type_index);
+			err = create_new_forward_decl(btfe, type_ptr, type_index);
 			break;
 		case BTF_KIND_TYPEDEF:
-			vlen = create_new_typedef(btfe, type_ptr, size, type_index);
+			err = create_new_typedef(btfe, type_ptr, type_index);
 			break;
 		case BTF_KIND_VAR:
-			vlen = create_new_variable(btfe, ptr, type_ptr, size, type_index);
+			err = create_new_variable(btfe, type_ptr, type_index);
 			break;
 		case BTF_KIND_DATASEC:
-			vlen = create_new_datasec(btfe, ptr, vlen, type_ptr, size, type_index, kflag);
+			err = create_new_datasec(btfe, type_ptr, type_index);
 			break;
 		case BTF_KIND_VOLATILE:
 		case BTF_KIND_PTR:
 		case BTF_KIND_CONST:
 		case BTF_KIND_RESTRICT:
-			vlen = create_new_tag(btfe, type, type_ptr, type_index);
+			err = create_new_tag(btfe, type, type_ptr, type_index);
 			break;
 		case BTF_KIND_UNKN:
 			cu__table_nullify_type_entry(btfe->priv, type_index);
-			fprintf(stderr, "BTF: idx: %d, off: %zd, Unknown kind %d\n",
-				type_index, ((void *)type_ptr) - type_section, type);
+			fprintf(stderr, "BTF: idx: %d, Unknown kind %d\n", type_index, type);
 			fflush(stderr);
-			vlen = 0;
+			err = 0;
 			break;
 		case BTF_KIND_FUNC_PROTO:
-			vlen = create_new_subroutine_type(btfe, ptr, vlen, type_ptr, type_index);
+			err = create_new_subroutine_type(btfe, type_ptr, type_index);
 			break;
 		case BTF_KIND_FUNC:
 			// BTF_KIND_FUNC corresponding to a defined subprogram.
-			vlen = create_new_function(btfe, type_ptr, size, type_index);
+			err = create_new_function(btfe, type_ptr, type_index);
 			break;
 		default:
-			fprintf(stderr, "BTF: idx: %d, off: %zd, Unknown kind %d\n",
-				type_index, ((void *)type_ptr) - type_section, type);
+			fprintf(stderr, "BTF: idx: %d, Unknown kind %d\n", type_index, type);
 			fflush(stderr);
-			vlen = 0;
+			err = 0;
 			break;
 		}
 
-		if (vlen < 0)
-			return vlen;
-
-		type_ptr = ptr + vlen;
-		type_index++;
+		if (err < 0)
+			return err;
 	}
 	return 0;
 }
diff --git a/libbtf.c b/libbtf.c
index b80c482f6ccf..20f9074ad68b 100644
--- a/libbtf.c
+++ b/libbtf.c
@@ -62,89 +62,29 @@ static int btf_var_secinfo_cmp(const void *a, const void *b)
 	return av->offset - bv->offset;
 }
 
-uint32_t btf_elf__get32(struct btf_elf *btfe, uint32_t *p)
-{
-	uint32_t val = *p;
-
-	if (btfe->swapped)
-		val = ((val >> 24) |
-		       ((val >> 8) & 0x0000ff00) |
-		       ((val << 8) & 0x00ff0000) |
-		       (val << 24));
-	return val;
-}
-
-static int btf_raw__load(struct btf_elf *btfe)
+static int libbpf_log(enum libbpf_print_level level, const char *format, va_list args)
 {
-        size_t read_cnt;
-        struct stat st;
-        void *data;
-        FILE *fp;
-
-        if (stat(btfe->filename, &st))
-                return -1;
-
-        data = malloc(st.st_size);
-        if (!data)
-                return -1;
-
-        fp = fopen(btfe->filename, "rb");
-        if (!fp)
-                goto cleanup;
-
-        read_cnt = fread(data, 1, st.st_size, fp);
-        fclose(fp);
-        if (read_cnt < st.st_size)
-                goto cleanup;
-
-	btfe->swapped	= 0;
-	btfe->data	= data;
-	btfe->size	= read_cnt;
-	return 0;
-cleanup:
-        free(data);
-        return -1;
+	return vfprintf(stderr, format, args);
 }
 
 int btf_elf__load(struct btf_elf *btfe)
 {
-	if (btfe->raw_btf)
-		return btf_raw__load(btfe);
-
-	int err = -ENOTSUP;
-	GElf_Shdr shdr;
-	Elf_Scn *sec = elf_section_by_name(btfe->elf, &btfe->ehdr, &shdr, ".BTF", NULL);
+	int err;
 
-	if (sec == NULL)
-		return -ESRCH;
-
-	Elf_Data *data = elf_getdata(sec, NULL);
-	if (data == NULL) {
-		fprintf(stderr, "%s: cannot get data of BTF section.\n", __func__);
-		return -1;
-	}
-
-	struct btf_header *hp = data->d_buf;
-	size_t orig_size = data->d_size;
-
-	if (hp->version != BTF_VERSION)
-		goto out;
+	libbpf_set_print(libbpf_log);
 
-	err = -EINVAL;
-	if (hp->magic == BTF_MAGIC)
-		btfe->swapped = 0;
+	/* free initial empty BTF */
+	btf__free(btfe->btf);
+	if (btfe->raw_btf)
+		btfe->btf = btf__parse_raw(btfe->filename);
 	else
-		goto out;
+		btfe->btf = btf__parse_elf(btfe->filename, NULL);
 
-	err = -ENOMEM;
-	btfe->data = malloc(orig_size);
-	if (btfe->data != NULL) {
-		memcpy(btfe->data, hp, orig_size);
-		btfe->size = orig_size;
-		err = 0;
-	}
-out:
-	return err;
+	err = libbpf_get_error(btfe->btf);
+	if (err)
+		return err;
+
+	return 0;
 }
 
 
@@ -252,26 +192,17 @@ void btf_elf__delete(struct btf_elf *btfe)
 
 	__gobuffer__delete(&btfe->types);
 	__gobuffer__delete(&btfe->percpu_secinfo);
+	btf__free(btfe->btf);
 	free(btfe->filename);
 	free(btfe->data);
 	free(btfe);
 }
 
-char *btf_elf__string(struct btf_elf *btfe, uint32_t ref)
+const char *btf_elf__string(struct btf_elf *btfe, uint32_t ref)
 {
-	struct btf_header *hp = btfe->hdr;
-	uint32_t off = ref;
-	char *name;
-
-	if (off >= btf_elf__get32(btfe, &hp->str_len))
-		return "(ref out-of-bounds)";
-
-	if ((off + btf_elf__get32(btfe, &hp->str_off)) >= btfe->size)
-		return "(string table truncated)";
+	const char *s = btf__str_by_offset(btfe->btf, ref);
 
-	name = ((char *)(hp + 1) + btf_elf__get32(btfe, &hp->str_off) + off);
-
-	return name[0] == '\0' ? NULL : name;
+	return s && s[0] == '\0' ? NULL : s;
 }
 
 static void *btf_elf__nohdr_data(struct btf_elf *btfe)
@@ -313,8 +244,10 @@ static const char *btf_elf__name_in_gobuf(const struct btf_elf *btfe, uint32_t o
 {
 	if (!offset)
 		return "(anon)";
-	else
+	else if (btfe->strings)
 		return &btfe->strings->entries[offset];
+	else
+		return btf__str_by_offset(btfe->btf, offset);
 }
 
 static const char * btf_elf__int_encoding_str(uint8_t encoding)
@@ -839,11 +772,6 @@ out:
 	return err;
 }
 
-static int libbpf_log(enum libbpf_print_level level, const char *format, va_list args)
-{
-	return vfprintf(stderr, format, args);
-}
-
 int btf_elf__encode(struct btf_elf *btfe, uint8_t flags)
 {
 	struct btf_header *hdr;
@@ -889,7 +817,7 @@ int btf_elf__encode(struct btf_elf *btfe, uint8_t flags)
 		return -1;
 	}
 	if (btf__dedup(btf, NULL, NULL)) {
-		fprintf(stderr, "%s: btf__dedup failed!", __func__);
+		fprintf(stderr, "%s: btf__dedup failed!\n", __func__);
 		return -1;
 	}
 
diff --git a/libbtf.h b/libbtf.h
index be06480bf854..5f29b427c4fd 100644
--- a/libbtf.h
+++ b/libbtf.h
@@ -11,6 +11,7 @@
 
 #include <stdbool.h>
 #include <stdint.h>
+#include "lib/bpf/src/btf.h"
 
 struct btf_elf {
 	union {
@@ -26,7 +27,6 @@ struct btf_elf {
 	struct gobuffer   percpu_secinfo;
 	char		  *filename;
 	size_t		  size;
-	int		  swapped;
 	int		  in_fd;
 	uint8_t		  wordsize;
 	bool		  is_big_endian;
@@ -34,6 +34,7 @@ struct btf_elf {
 	uint32_t	  type_index;
 	uint32_t	  percpu_shndx;
 	uint64_t	  percpu_base_addr;
+	struct btf	  *btf;
 };
 
 extern uint8_t btf_elf__verbose;
@@ -70,13 +71,7 @@ int32_t btf_elf__add_datasec_type(struct btf_elf *btfe, const char *section_name
 void btf_elf__set_strings(struct btf_elf *btf, struct gobuffer *strings);
 int  btf_elf__encode(struct btf_elf *btf, uint8_t flags);
 
-char *btf_elf__string(struct btf_elf *btf, uint32_t ref);
+const char *btf_elf__string(struct btf_elf *btf, uint32_t ref);
 int btf_elf__load(struct btf_elf *btf);
 
-uint32_t btf_elf__get32(struct btf_elf *btf, uint32_t *p);
-
-void *btf_elf__get_buffer(struct btf_elf *btf);
-
-size_t btf_elf__get_size(struct btf_elf *btf);
-
 #endif /* _LIBBTF_H */
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 dwarves 2/8] btf_encoder: use libbpf APIs to encode BTF type info
  2020-10-08 23:39 [PATCH v2 dwarves 0/8] Switch BTF loading and encoding to libbpf APIs Andrii Nakryiko
  2020-10-08 23:39 ` [PATCH v2 dwarves 1/8] btf_loader: use libbpf to load BTF Andrii Nakryiko
@ 2020-10-08 23:39 ` Andrii Nakryiko
  2020-10-08 23:39 ` [PATCH v2 dwarves 3/8] btf_encoder: fix emitting __ARRAY_SIZE_TYPE__ as index range type Andrii Nakryiko
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Andrii Nakryiko @ 2020-10-08 23:39 UTC (permalink / raw)
  To: dwarves
  Cc: bpf, kernel-team, ast, Andrii Nakryiko, Arnaldo Carvalho de Melo,
	Andrii Nakryiko

From: Andrii Nakryiko <andriin@fb.com>

Switch to use libbpf's BTF writing APIs to encode BTF. This reconciles
btf_elf's use of internal struct btf from libbpf for both loading and encoding
BTF type info. This change also saves a considerable amount of memory used for
DWARF to BTF conversion due to avoiding extra memory copy between gobuffers
and libbpf's struct btf. Now that pahole uses libbpf's struct btf, it's
possible to further utilize libbpf's features and APIs, e.g., for handling
endianness conversion, for dumping raw BTF type info during encoding. These
features might be implemented in the follow up patches.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 btf_encoder.c |  96 +++++-----
 libbtf.c      | 521 +++++++++++++++++++++++---------------------------
 libbtf.h      |  29 +--
 3 files changed, 295 insertions(+), 351 deletions(-)

diff --git a/btf_encoder.c b/btf_encoder.c
index e90150784282..6e6bce202438 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -67,6 +67,8 @@ static void dump_invalid_symbol(const char *msg, const char *sym, const char *cu
 	fprintf(stderr, "PAHOLE: Error: Use '-j' or '--force' to ignore such symbols and force emit the btf.\n");
 }
 
+extern struct debug_fmt_ops *dwarves__active_loader;
+
 static int tag__check_id_drift(const struct tag *tag,
 			       uint32_t core_id, uint32_t btf_type_id,
 			       uint32_t type_id_off)
@@ -82,36 +84,19 @@ static int tag__check_id_drift(const struct tag *tag,
 	return 0;
 }
 
-static int32_t structure_type__encode(struct btf_elf *btfe, struct tag *tag, uint32_t type_id_off)
+static int32_t structure_type__encode(struct btf_elf *btfe, struct cu *cu, struct tag *tag, uint32_t type_id_off)
 {
 	struct type *type = tag__type(tag);
 	struct class_member *pos;
-	bool kind_flag = false;
+	const char *name;
 	int32_t type_id;
 	uint8_t kind;
 
 	kind = (tag->tag == DW_TAG_union_type) ?
 		BTF_KIND_UNION : BTF_KIND_STRUCT;
 
-	/* Although no_bitfield_type_recode has been set true
-	 * in pahole.c if BTF encoding is requested, we still check
-	 * the value here. So if no_bitfield_type_recode is set
-	 * to false for whatever reason, we do not accidentally
-	 * set kind_flag incorrectly.
-	 */
-	if (no_bitfield_type_recode) {
-		/* kind_flag only set where there is a bitfield
-		 * in the struct.
-		 */
-		type__for_each_data_member(type, pos) {
-			if (pos->bitfield_size) {
-				kind_flag = true;
-				break;
-			}
-		}
-	}
-
-	type_id = btf_elf__add_struct(btfe, kind, type->namespace.name, kind_flag, type->size, type->nr_members);
+	name = dwarves__active_loader->strings__ptr(cu, type->namespace.name);
+	type_id = btf_elf__add_struct(btfe, kind, name, type->size);
 	if (type_id < 0)
 		return type_id;
 
@@ -121,7 +106,8 @@ static int32_t structure_type__encode(struct btf_elf *btfe, struct tag *tag, uin
 		 * scheme, which conforms to BTF requirement, so no conversion
 		 * is required.
 		 */
-		if (btf_elf__add_member(btfe, pos->name, type_id_off + pos->tag.type, kind_flag, pos->bitfield_size, pos->bit_offset))
+		name = dwarves__active_loader->strings__ptr(cu, pos->name);
+		if (btf_elf__add_member(btfe, name, type_id_off + pos->tag.type, pos->bitfield_size, pos->bit_offset))
 			return -1;
 	}
 
@@ -140,56 +126,64 @@ static uint32_t array_type__nelems(struct tag *tag)
 	return nelem;
 }
 
-static int32_t enumeration_type__encode(struct btf_elf *btfe, struct tag *tag)
+static int32_t enumeration_type__encode(struct btf_elf *btfe, struct cu *cu, struct tag *tag)
 {
 	struct type *etype = tag__type(tag);
 	struct enumerator *pos;
+	const char *name;
 	int32_t type_id;
 
-	type_id = btf_elf__add_enum(btfe, etype->namespace.name, etype->size, etype->nr_members);
+	name = dwarves__active_loader->strings__ptr(cu, etype->namespace.name);
+	type_id = btf_elf__add_enum(btfe, name, etype->size);
 	if (type_id < 0)
 		return type_id;
 
-	type__for_each_enumerator(etype, pos)
-		if (btf_elf__add_enum_val(btfe, pos->name, pos->value))
+	type__for_each_enumerator(etype, pos) {
+		name = dwarves__active_loader->strings__ptr(cu, pos->name);
+		if (btf_elf__add_enum_val(btfe, name, pos->value))
 			return -1;
+	}
 
 	return type_id;
 }
 
-static int tag__encode_btf(struct tag *tag, uint32_t core_id, struct btf_elf *btfe,
+static int tag__encode_btf(struct cu *cu, struct tag *tag, uint32_t core_id, struct btf_elf *btfe,
 			   uint32_t array_index_id, uint32_t type_id_off)
 {
 	/* single out type 0 as it represents special type "void" */
 	uint32_t ref_type_id = tag->type == 0 ? 0 : type_id_off + tag->type;
+	const char *name;
 
 	switch (tag->tag) {
 	case DW_TAG_base_type:
-		return btf_elf__add_base_type(btfe, tag__base_type(tag));
+		name = dwarves__active_loader->strings__ptr(cu, tag__base_type(tag)->name);
+		return btf_elf__add_base_type(btfe, tag__base_type(tag), name);
 	case DW_TAG_const_type:
-		return btf_elf__add_ref_type(btfe, BTF_KIND_CONST, ref_type_id, 0, false);
+		return btf_elf__add_ref_type(btfe, BTF_KIND_CONST, ref_type_id, NULL, false);
 	case DW_TAG_pointer_type:
-		return btf_elf__add_ref_type(btfe, BTF_KIND_PTR, ref_type_id, 0, false);
+		return btf_elf__add_ref_type(btfe, BTF_KIND_PTR, ref_type_id, NULL, false);
 	case DW_TAG_restrict_type:
-		return btf_elf__add_ref_type(btfe, BTF_KIND_RESTRICT, ref_type_id, 0, false);
+		return btf_elf__add_ref_type(btfe, BTF_KIND_RESTRICT, ref_type_id, NULL, false);
 	case DW_TAG_volatile_type:
-		return btf_elf__add_ref_type(btfe, BTF_KIND_VOLATILE, ref_type_id, 0, false);
+		return btf_elf__add_ref_type(btfe, BTF_KIND_VOLATILE, ref_type_id, NULL, false);
 	case DW_TAG_typedef:
-		return btf_elf__add_ref_type(btfe, BTF_KIND_TYPEDEF, ref_type_id, tag__namespace(tag)->name, false);
+		name = dwarves__active_loader->strings__ptr(cu, tag__namespace(tag)->name);
+		return btf_elf__add_ref_type(btfe, BTF_KIND_TYPEDEF, ref_type_id, name, false);
 	case DW_TAG_structure_type:
 	case DW_TAG_union_type:
 	case DW_TAG_class_type:
+		name = dwarves__active_loader->strings__ptr(cu, tag__namespace(tag)->name);
 		if (tag__type(tag)->declaration)
-			return btf_elf__add_ref_type(btfe, BTF_KIND_FWD, 0, tag__namespace(tag)->name, tag->tag == DW_TAG_union_type);
+			return btf_elf__add_ref_type(btfe, BTF_KIND_FWD, 0, name, tag->tag == DW_TAG_union_type);
 		else
-			return structure_type__encode(btfe, tag, type_id_off);
+			return structure_type__encode(btfe, cu, tag, type_id_off);
 	case DW_TAG_array_type:
 		/* TODO: Encode one dimension at a time. */
 		return btf_elf__add_array(btfe, ref_type_id, array_index_id, array_type__nelems(tag));
 	case DW_TAG_enumeration_type:
-		return enumeration_type__encode(btfe, tag);
+		return enumeration_type__encode(btfe, cu, tag);
 	case DW_TAG_subroutine_type:
-		return btf_elf__add_func_proto(btfe, tag__ftype(tag), type_id_off);
+		return btf_elf__add_func_proto(btfe, cu, tag__ftype(tag), type_id_off);
 	default:
 		fprintf(stderr, "Unsupported DW_TAG_%s(0x%x)\n",
 			dwarf_tag_name(tag->tag), tag->tag);
@@ -197,12 +191,6 @@ static int tag__encode_btf(struct tag *tag, uint32_t core_id, struct btf_elf *bt
 	}
 }
 
-/*
- * FIXME: Its in the DWARF loader, we have to find a better handoff
- * mechanizm...
- */
-extern struct strings *strings;
-
 static struct btf_elf *btfe;
 static uint32_t array_index_id;
 
@@ -265,7 +253,6 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force,
 		btfe = btf_elf__new(cu->filename, cu->elf);
 		if (!btfe)
 			return -1;
-		btf_elf__set_strings(btfe, &strings->gb);
 
 		/* cu__find_base_type_by_name() takes "type_id_t *id" */
 		type_id_t id;
@@ -280,10 +267,10 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force,
 	}
 
 	btf_elf__verbose = verbose;
-	type_id_off = btfe->type_index;
+	type_id_off = btf__get_nr_types(btfe->btf);
 
 	cu__for_each_type(cu, core_id, pos) {
-		int32_t btf_type_id = tag__encode_btf(pos, core_id, btfe, array_index_id, type_id_off);
+		int32_t btf_type_id = tag__encode_btf(cu, pos, core_id, btfe, array_index_id, type_id_off);
 
 		if (btf_type_id < 0 ||
 		    tag__check_id_drift(pos, core_id, btf_type_id, type_id_off)) {
@@ -297,17 +284,19 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force,
 
 		bt.name = 0;
 		bt.bit_size = 32;
-		btf_elf__add_base_type(btfe, &bt);
+		btf_elf__add_base_type(btfe, &bt, "__ARRAY_SIZE_TYPE__");
 	}
 
 	cu__for_each_function(cu, core_id, fn) {
 		int btf_fnproto_id, btf_fn_id;
+		const char *name;
 
 		if (fn->declaration || !fn->external)
 			continue;
 
-		btf_fnproto_id = btf_elf__add_func_proto(btfe, &fn->proto, type_id_off);
-		btf_fn_id = btf_elf__add_ref_type(btfe, BTF_KIND_FUNC, btf_fnproto_id, fn->name, false);
+		btf_fnproto_id = btf_elf__add_func_proto(btfe, cu, &fn->proto, type_id_off);
+		name = dwarves__active_loader->strings__ptr(cu, fn->name);
+		btf_fn_id = btf_elf__add_ref_type(btfe, BTF_KIND_FUNC, btf_fnproto_id, name, false);
 		if (btf_fnproto_id < 0 || btf_fn_id < 0) {
 			err = -1;
 			printf("error: failed to encode function '%s'\n", function__name(fn, cu));
@@ -349,7 +338,7 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force,
 
 	/* search within symtab for percpu variables */
 	elf_symtab__for_each_symbol(btfe->symtab, core_id, sym) {
-		uint32_t linkage, type, size, offset, name;
+		uint32_t linkage, type, size, offset;
 		int32_t btf_var_id, btf_var_secinfo_id;
 		uint64_t addr;
 		const char *sym_name;
@@ -391,7 +380,6 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force,
 			err = -1;
 			break;
 		}
-		name = strings__add(strings, sym_name);
 		if (var->ip.tag.type == 0) {
 			dump_invalid_symbol("Found symbol of void type when encoding btf",
 					    sym_name, cu->name, verbose, force);
@@ -417,7 +405,7 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force,
 
 		/* add a BTF_KIND_VAR in btfe->types */
 		linkage = var->external ? BTF_VAR_GLOBAL_ALLOCATED : BTF_VAR_STATIC;
-		btf_var_id = btf_elf__add_var_type(btfe, type, name, linkage);
+		btf_var_id = btf_elf__add_var_type(btfe, type, sym_name, linkage);
 		if (btf_var_id < 0) {
 			err = -1;
 			printf("error: failed to encode variable '%s'\n", sym_name);
@@ -440,7 +428,9 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force,
 	}
 
 out:
-	if (err)
+	if (err) {
 		btf_elf__delete(btfe);
+		btfe = NULL;
+	}
 	return err;
 }
diff --git a/libbtf.c b/libbtf.c
index 20f9074ad68b..0467f1f2a596 100644
--- a/libbtf.c
+++ b/libbtf.c
@@ -27,31 +27,6 @@
 #include "dwarves.h"
 #include "elf_symtab.h"
 
-#define BTF_INFO_ENCODE(kind, kind_flag, vlen)				\
-	((!!(kind_flag) << 31) | ((kind) << 24) | ((vlen) & BTF_MAX_VLEN))
-#define BTF_INT_ENCODE(encoding, bits_offset, nr_bits)		\
-	((encoding) << 24 | (bits_offset) << 16 | (nr_bits))
-
-struct btf_int_type {
-	struct btf_type type;
-	uint32_t 	data;
-};
-
-struct btf_enum_type {
-	struct btf_type type;
-	struct btf_enum btf_enum;
-};
-
-struct btf_array_type {
-	struct btf_type type;
-	struct btf_array array;
-};
-
-struct btf_var_type {
-	struct btf_type type;
-	struct btf_var var;
-};
-
 uint8_t btf_elf__verbose;
 
 static int btf_var_secinfo_cmp(const void *a, const void *b)
@@ -102,6 +77,12 @@ struct btf_elf *btf_elf__new(const char *filename, Elf *elf)
 	if (btfe->filename == NULL)
 		goto errout;
 
+	btfe->btf = btf__new_empty();
+	if (libbpf_get_error(btfe->btf)) {
+		fprintf(stderr, "%s: failed to create empty BTF.\n", __func__);
+		goto errout;
+	}
+
 	if (strcmp(filename, "/sys/kernel/btf/vmlinux") == 0) {
 		btfe->raw_btf  = true;
 		btfe->wordsize = sizeof(long);
@@ -189,12 +170,9 @@ void btf_elf__delete(struct btf_elf *btfe)
 	}
 
 	elf_symtab__delete(btfe->symtab);
-
-	__gobuffer__delete(&btfe->types);
 	__gobuffer__delete(&btfe->percpu_secinfo);
 	btf__free(btfe->btf);
 	free(btfe->filename);
-	free(btfe->data);
 	free(btfe);
 }
 
@@ -205,16 +183,6 @@ const char *btf_elf__string(struct btf_elf *btfe, uint32_t ref)
 	return s && s[0] == '\0' ? NULL : s;
 }
 
-static void *btf_elf__nohdr_data(struct btf_elf *btfe)
-{
-	return btfe->hdr + 1;
-}
-
-void btf_elf__set_strings(struct btf_elf *btfe, struct gobuffer *strings)
-{
-	btfe->strings = strings;
-}
-
 #define BITS_PER_BYTE 8
 #define BITS_PER_BYTE_MASK (BITS_PER_BYTE - 1)
 #define BITS_PER_BYTE_MASKED(bits) ((bits) & BITS_PER_BYTE_MASK)
@@ -240,12 +208,10 @@ static const char * const btf_kind_str[NR_BTF_KINDS] = {
 	[BTF_KIND_DATASEC]      = "DATASEC",
 };
 
-static const char *btf_elf__name_in_gobuf(const struct btf_elf *btfe, uint32_t offset)
+static const char *btf_elf__printable_name(const struct btf_elf *btfe, uint32_t offset)
 {
 	if (!offset)
 		return "(anon)";
-	else if (btfe->strings)
-		return &btfe->strings->entries[offset];
 	else
 		return btf__str_by_offset(btfe->btf, offset);
 }
@@ -264,6 +230,27 @@ static const char * btf_elf__int_encoding_str(uint8_t encoding)
 		return "UNKN";
 }
 
+
+__attribute ((format (printf, 5, 6)))
+static void btf_elf__log_err(const struct btf_elf *btfe, int kind, const char *name,
+			     bool output_cr, const char *fmt, ...)
+{
+	fprintf(stderr, "[%u] %s %s", btf__get_nr_types(btfe->btf) + 1,
+		btf_kind_str[kind], name ?: "(anon)");
+
+	if (fmt && *fmt) {
+		va_list ap;
+
+		fprintf(stderr, " ");
+		va_start(ap, fmt);
+		vfprintf(stderr, fmt, ap);
+		va_end(ap);
+	}
+
+	if (output_cr)
+		fprintf(stderr, "\n");
+}
+
 __attribute ((format (printf, 5, 6)))
 static void btf_elf__log_type(const struct btf_elf *btfe, const struct btf_type *t,
 			      bool err, bool output_cr, const char *fmt, ...)
@@ -278,8 +265,8 @@ static void btf_elf__log_type(const struct btf_elf *btfe, const struct btf_type
 	out = err ? stderr : stdout;
 
 	fprintf(out, "[%u] %s %s",
-		btfe->type_index, btf_kind_str[kind],
-		btf_elf__name_in_gobuf(btfe, t->name_off));
+		btf__get_nr_types(btfe->btf), btf_kind_str[kind],
+		btf_elf__printable_name(btfe, t->name_off));
 
 	if (fmt && *fmt) {
 		va_list ap;
@@ -296,8 +283,9 @@ static void btf_elf__log_type(const struct btf_elf *btfe, const struct btf_type
 
 __attribute ((format (printf, 5, 6)))
 static void btf_log_member(const struct btf_elf *btfe,
+			   const struct btf_type *t,
 			   const struct btf_member *member,
-			   bool kind_flag, bool err, const char *fmt, ...)
+			   bool err, const char *fmt, ...)
 {
 	FILE *out;
 
@@ -306,15 +294,15 @@ static void btf_log_member(const struct btf_elf *btfe,
 
 	out = err ? stderr : stdout;
 
-	if (kind_flag)
+	if (btf_kflag(t))
 		fprintf(out, "\t%s type_id=%u bitfield_size=%u bits_offset=%u",
-			btf_elf__name_in_gobuf(btfe, member->name_off),
+			btf_elf__printable_name(btfe, member->name_off),
 			member->type,
 			BTF_MEMBER_BITFIELD_SIZE(member->offset),
 			BTF_MEMBER_BIT_OFFSET(member->offset));
 	else
 		fprintf(out, "\t%s type_id=%u bits_offset=%u",
-			btf_elf__name_in_gobuf(btfe, member->name_off),
+			btf_elf__printable_name(btfe, member->name_off),
 			member->type,
 			member->offset);
 
@@ -332,7 +320,7 @@ static void btf_log_member(const struct btf_elf *btfe,
 
 __attribute ((format (printf, 6, 7)))
 static void btf_log_func_param(const struct btf_elf *btfe,
-			       uint32_t name_off, uint32_t type,
+			       const char *name, uint32_t type,
 			       bool err, bool is_last_param,
 			       const char *fmt, ...)
 {
@@ -346,9 +334,7 @@ static void btf_log_func_param(const struct btf_elf *btfe,
 	if (is_last_param && !type)
 		fprintf(out, "vararg)\n");
 	else
-		fprintf(out, "%u %s%s", type,
-			btf_elf__name_in_gobuf(btfe, name_off),
-			is_last_param ? ")\n" : ", ");
+		fprintf(out, "%u %s%s", type, name, is_last_param ? ")\n" : ", ");
 
 	if (fmt && *fmt) {
 		va_list ap;
@@ -360,15 +346,14 @@ static void btf_log_func_param(const struct btf_elf *btfe,
 	}
 }
 
-int32_t btf_elf__add_base_type(struct btf_elf *btfe, const struct base_type *bt)
+int32_t btf_elf__add_base_type(struct btf_elf *btfe, const struct base_type *bt,
+			       const char *name)
 {
-	struct btf_int_type int_type;
-	struct btf_type *t = &int_type.type;
+	struct btf *btf = btfe->btf;
+	const struct btf_type *t;
 	uint8_t encoding = 0;
+	int32_t id;
 
-	t->name_off = bt->name;
-	t->info = BTF_INFO_ENCODE(BTF_KIND_INT, 0, 0);
-	t->size = BITS_ROUNDUP_BYTES(bt->bit_size);
 	if (bt->is_signed) {
 		encoding = BTF_INT_SIGNED;
 	} else if (bt->is_bool) {
@@ -377,240 +362,253 @@ int32_t btf_elf__add_base_type(struct btf_elf *btfe, const struct base_type *bt)
 		fprintf(stderr, "float_type is not supported\n");
 		return -1;
 	}
-	int_type.data = BTF_INT_ENCODE(encoding, 0, bt->bit_size);
 
-	++btfe->type_index;
-	if (gobuffer__add(&btfe->types, &int_type, sizeof(int_type)) >= 0) {
-		btf_elf__log_type(btfe, t, false, true,
-			      "size=%u bit_offset=%u nr_bits=%u encoding=%s",
-			      t->size, BTF_INT_OFFSET(int_type.data),
-			      BTF_INT_BITS(int_type.data),
-			      btf_elf__int_encoding_str(BTF_INT_ENCODING(int_type.data)));
-		return btfe->type_index;
+	id = btf__add_int(btf, name, BITS_ROUNDUP_BYTES(bt->bit_size), encoding);
+	if (id < 0) {
+		btf_elf__log_err(btfe, BTF_KIND_INT, name, true, "Error emitting BTF type");
 	} else {
-		btf_elf__log_type(btfe, t, true, true,
-			      "size=%u bit_offset=%u nr_bits=%u encoding=%s Error in adding gobuffer",
-			      t->size, BTF_INT_OFFSET(int_type.data),
-			      BTF_INT_BITS(int_type.data),
-			      btf_elf__int_encoding_str(BTF_INT_ENCODING(int_type.data)));
-		return -1;
+		t = btf__type_by_id(btf, id);
+		btf_elf__log_type(btfe, t, false, true,
+				"size=%u nr_bits=%u encoding=%s%s",
+				t->size, bt->bit_size,
+				btf_elf__int_encoding_str(encoding),
+				id < 0 ? " Error in emitting BTF" : "" );
 	}
+
+	return id;
 }
 
 int32_t btf_elf__add_ref_type(struct btf_elf *btfe, uint16_t kind, uint32_t type,
-			      uint32_t name, bool kind_flag)
+			      const char *name, bool kind_flag)
 {
-	struct btf_type t;
-
-	t.name_off = name;
-	t.info = BTF_INFO_ENCODE(kind, kind_flag, 0);
-	t.type = type;
+	struct btf *btf = btfe->btf;
+	const struct btf_type *t;
+	int32_t id;
+
+	switch (kind) {
+	case BTF_KIND_PTR:
+		id = btf__add_ptr(btf, type);
+		break;
+	case BTF_KIND_VOLATILE:
+		id = btf__add_volatile(btf, type);
+		break;
+	case BTF_KIND_CONST:
+		id = btf__add_const(btf, type);
+		break;
+	case BTF_KIND_RESTRICT:
+		id = btf__add_const(btf, type);
+		break;
+	case BTF_KIND_TYPEDEF:
+		id = btf__add_typedef(btf, name, type);
+		break;
+	case BTF_KIND_FWD:
+		id = btf__add_fwd(btf, name, kind_flag);
+		break;
+	case BTF_KIND_FUNC:
+		id = btf__add_func(btf, name, BTF_FUNC_STATIC, type);
+		break;
+	default:
+		btf_elf__log_err(btfe, kind, name, true, "Unexpected kind for reference");
+		return -1;
+	}
 
-	++btfe->type_index;
-	if (gobuffer__add(&btfe->types, &t, sizeof(t)) >= 0) {
+	if (id > 0) {
+		t = btf__type_by_id(btf, id);
 		if (kind == BTF_KIND_FWD)
-			btf_elf__log_type(btfe, &t, false, true, "%s", kind_flag ? "union" : "struct");
+			btf_elf__log_type(btfe, t, false, true, "%s", kind_flag ? "union" : "struct");
 		else
-			btf_elf__log_type(btfe, &t, false, true, "type_id=%u", t.type);
-		return btfe->type_index;
+			btf_elf__log_type(btfe, t, false, true, "type_id=%u", t->type);
 	} else {
-		btf_elf__log_type(btfe, &t, true, true,
-			      "kind_flag=%d type_id=%u Error in adding gobuffer",
-			      kind_flag, t.type);
-		return -1;
+		btf_elf__log_err(btfe, kind, name, true, "Error emitting BTF type");
 	}
+	return id;
 }
 
 int32_t btf_elf__add_array(struct btf_elf *btfe, uint32_t type, uint32_t index_type, uint32_t nelems)
 {
-	struct btf_array_type array_type;
-	struct btf_type *t = &array_type.type;
-	struct btf_array *array = &array_type.array;
-
-	t->name_off = 0;
-	t->info = BTF_INFO_ENCODE(BTF_KIND_ARRAY, 0, 0);
-	t->size = 0;
-
-	array->type = type;
-	array->index_type = index_type;
-	array->nelems = nelems;
-
-	++btfe->type_index;
-	if (gobuffer__add(&btfe->types, &array_type, sizeof(array_type)) >= 0) {
+	struct btf *btf = btfe->btf;
+	const struct btf_type *t;
+	const struct btf_array *array;
+	int32_t id;
+
+	id = btf__add_array(btf, index_type, type, nelems);
+	if (id > 0) {
+		t = btf__type_by_id(btf, id);
+		array = btf_array(t);
 		btf_elf__log_type(btfe, t, false, true,
 			      "type_id=%u index_type_id=%u nr_elems=%u",
 			      array->type, array->index_type, array->nelems);
-		return btfe->type_index;
 	} else {
-		btf_elf__log_type(btfe, t, true, true,
-			      "type_id=%u index_type_id=%u nr_elems=%u Error in adding gobuffer",
-			      array->type, array->index_type, array->nelems);
-		return -1;
+		btf_elf__log_err(btfe, BTF_KIND_ARRAY, NULL, true,
+			      "type_id=%u index_type_id=%u nr_elems=%u Error emitting BTF type",
+			      type, index_type, nelems);
 	}
+	return id;
 }
 
-int btf_elf__add_member(struct btf_elf *btfe, uint32_t name, uint32_t type, bool kind_flag,
+int btf_elf__add_member(struct btf_elf *btfe, const char *name, uint32_t type,
 			uint32_t bitfield_size, uint32_t offset)
 {
-	struct btf_member member = {
-		.name_off   = name,
-		.type   = type,
-		.offset = kind_flag ? (bitfield_size << 24 | offset) : offset,
-	};
+	struct btf *btf = btfe->btf;
+	const struct btf_type *t;
+	const struct btf_member *m;
+	int err;
 
-	if (gobuffer__add(&btfe->types, &member, sizeof(member)) >= 0) {
-		btf_log_member(btfe, &member, kind_flag, false, NULL);
-		return 0;
+	err = btf__add_field(btf, name, type, offset, bitfield_size);
+	t = btf__type_by_id(btf, btf__get_nr_types(btf));
+	if (err) {
+		fprintf(stderr, "[%u] %s %s's field '%s' offset=%u bit_size=%u type=%u Error emitting field\n",
+			btf__get_nr_types(btf), btf_kind_str[btf_kind(t)],
+			btf_elf__printable_name(btfe, t->name_off),
+			name, offset, bitfield_size, type);
 	} else {
-		btf_log_member(btfe, &member, kind_flag, true, "Error in adding gobuffer");
-		return -1;
+		m = &btf_members(t)[btf_vlen(t) - 1];
+		btf_log_member(btfe, t, m, false, NULL);
 	}
+	return err;
 }
 
-int32_t btf_elf__add_struct(struct btf_elf *btfe, uint8_t kind, uint32_t name,
-			    bool kind_flag, uint32_t size, uint16_t nr_members)
+int32_t btf_elf__add_struct(struct btf_elf *btfe, uint8_t kind, const char *name, uint32_t size)
 {
-	struct btf_type t;
-
-	t.name_off = name;
-	t.info = BTF_INFO_ENCODE(kind, kind_flag, nr_members);
-	t.size = size;
+	struct btf *btf = btfe->btf;
+	const struct btf_type *t;
+	int32_t id;
+
+	switch (kind) {
+	case BTF_KIND_STRUCT:
+		id = btf__add_struct(btf, name, size);
+		break;
+	case BTF_KIND_UNION:
+		id = btf__add_union(btf, name, size);
+		break;
+	default:
+		btf_elf__log_err(btfe, kind, name, true, "Unexpected kind of struct");
+		return -1;
+	}
 
-	++btfe->type_index;
-	if (gobuffer__add(&btfe->types, &t, sizeof(t)) >= 0) {
-		btf_elf__log_type(btfe, &t, false, true, "kind_flag=%d size=%u vlen=%u",
-			      kind_flag, t.size, BTF_INFO_VLEN(t.info));
-		return btfe->type_index;
+	if (id < 0) {
+		btf_elf__log_err(btfe, kind, name, true, "Error emitting BTF type");
 	} else {
-		btf_elf__log_type(btfe, &t, true, true,
-			      "kind_flag=%d size=%u vlen=%u Error in adding gobuffer",
-			      kind_flag, t.size, BTF_INFO_VLEN(t.info));
-		return -1;
+		t = btf__type_by_id(btf, id);
+		btf_elf__log_type(btfe, t, false, true, "size=%u", t->size);
 	}
+
+	return id;
 }
 
-int32_t btf_elf__add_enum(struct btf_elf *btfe, uint32_t name, uint32_t bit_size, uint16_t nr_entries)
+int32_t btf_elf__add_enum(struct btf_elf *btfe, const char *name, uint32_t bit_size)
 {
-	struct btf_type t;
-
-	t.name_off = name;
-	t.info = BTF_INFO_ENCODE(BTF_KIND_ENUM, 0, nr_entries);
-	t.size = BITS_ROUNDUP_BYTES(bit_size);
-
-	++btfe->type_index;
-	if (gobuffer__add(&btfe->types, &t, sizeof(t)) >= 0) {
-		btf_elf__log_type(btfe, &t, false, true, "size=%u vlen=%u", t.size, BTF_INFO_VLEN(t.info));
-		return btfe->type_index;
+	struct btf *btf = btfe->btf;
+	const struct btf_type *t;
+	int32_t id, size;
+
+	size = BITS_ROUNDUP_BYTES(bit_size);
+	id = btf__add_enum(btf, name, size);
+	if (id > 0) {
+		t = btf__type_by_id(btf, id);
+		btf_elf__log_type(btfe, t, false, true, "size=%u", t->size);
 	} else {
-		btf_elf__log_type(btfe, &t, true, true,
-			      "size=%u vlen=%u Error in adding gobuffer",
-			      t.size, BTF_INFO_VLEN(t.info));
-		return -1;
+		btf_elf__log_err(btfe, BTF_KIND_ENUM, name, true,
+			      "size=%u Error emitting BTF type", size);
 	}
+	return id;
 }
 
-int btf_elf__add_enum_val(struct btf_elf *btfe, uint32_t name, int32_t value)
+int btf_elf__add_enum_val(struct btf_elf *btfe, const char *name, int32_t value)
 {
-	struct btf_enum e = {
-		.name_off = name,
-		.val  = value,
-	};
-
-	if (gobuffer__add(&btfe->types, &e, sizeof(e)) < 0) {
-		fprintf(stderr, "\t%s val=%d Error in adding gobuffer\n",
-			btf_elf__name_in_gobuf(btfe, e.name_off), e.val);
-		return -1;
-	} else if (btf_elf__verbose)
-		printf("\t%s val=%d\n", btf_elf__name_in_gobuf(btfe, e.name_off),
-		       e.val);
+	struct btf *btf = btfe->btf;
+	int err;
 
-	return 0;
+	err = btf__add_enum_value(btf, name, value);
+	if (!err) {
+		if (btf_elf__verbose)
+			printf("\t%s val=%d\n", name, value);
+	} else {
+		fprintf(stderr, "\t%s val=%d Error emitting BTF enum value\n",
+			name, value);
+	}
+	return err;
 }
 
-static int32_t btf_elf__add_func_proto_param(struct btf_elf *btfe, uint32_t name,
+static int32_t btf_elf__add_func_proto_param(struct btf_elf *btfe, const char *name,
 					     uint32_t type, bool is_last_param)
 {
-	struct btf_param param;
-
-	param.name_off = name;
-	param.type = type;
+	int err;
 
-	if (gobuffer__add(&btfe->types, &param, sizeof(param)) >= 0) {
+	err = btf__add_func_param(btfe->btf, name, type);
+	if (!err) {
 		btf_log_func_param(btfe, name, type, false, is_last_param, NULL);
 		return 0;
 	} else {
 		btf_log_func_param(btfe, name, type, true, is_last_param,
-				   "Error in adding gobuffer");
+				   "Error adding func param");
 		return -1;
 	}
 }
 
-int32_t btf_elf__add_func_proto(struct btf_elf *btfe, struct ftype *ftype, uint32_t type_id_off)
+extern struct debug_fmt_ops *dwarves__active_loader;
+
+int32_t btf_elf__add_func_proto(struct btf_elf *btfe, struct cu *cu, struct ftype *ftype, uint32_t type_id_off)
 {
-	uint16_t nr_params, param_idx;
+	struct btf *btf = btfe->btf;
+	const struct btf_type *t;
 	struct parameter *param;
-	struct btf_type t;
-	int32_t type_id;
+	uint16_t nr_params, param_idx;
+	int32_t id, type_id;
 
 	/* add btf_type for func_proto */
 	nr_params = ftype->nr_parms + (ftype->unspec_parms ? 1 : 0);
+	type_id = ftype->tag.type == 0 ? 0 : type_id_off + ftype->tag.type;
 
-	t.name_off = 0;
-	t.info = BTF_INFO_ENCODE(BTF_KIND_FUNC_PROTO, 0, nr_params);
-	t.type = ftype->tag.type == 0 ? 0 : type_id_off + ftype->tag.type;
-
-	++btfe->type_index;
-	if (gobuffer__add(&btfe->types, &t, sizeof(t)) >= 0) {
-		btf_elf__log_type(btfe, &t, false, false, "return=%u args=(%s",
-			      t.type, !nr_params ? "void)\n" : "");
-		type_id = btfe->type_index;
+	id = btf__add_func_proto(btf, type_id);
+	if (id > 0) {
+		t = btf__type_by_id(btf, id);
+		btf_elf__log_type(btfe, t, false, false, "return=%u args=(%s",
+			      t->type, !nr_params ? "void)\n" : "");
 	} else {
-		btf_elf__log_type(btfe, &t, true, true,
-			      "return=%u vlen=%u Error in adding gobuffer",
-			      t.type, BTF_INFO_VLEN(t.info));
-		return -1;
+		btf_elf__log_err(btfe, BTF_KIND_FUNC_PROTO, NULL, true,
+			      "return=%u vlen=%u Error emitting BTF type",
+			      type_id, nr_params);
+		return id;
 	}
 
 	/* add parameters */
 	param_idx = 0;
 	ftype__for_each_parameter(ftype, param) {
-		uint32_t param_type_id = param->tag.type == 0 ? 0 : type_id_off + param->tag.type;
+		const char *name = dwarves__active_loader->strings__ptr(cu, param->name);
+
+		type_id = param->tag.type == 0 ? 0 : type_id_off + param->tag.type;
 		++param_idx;
-		if (btf_elf__add_func_proto_param(btfe, param->name, param_type_id, param_idx == nr_params))
+		if (btf_elf__add_func_proto_param(btfe, name, type_id, param_idx == nr_params))
 			return -1;
 	}
 
 	++param_idx;
 	if (ftype->unspec_parms)
-		if (btf_elf__add_func_proto_param(btfe, 0, 0, param_idx == nr_params))
+		if (btf_elf__add_func_proto_param(btfe, NULL, 0, param_idx == nr_params))
 			return -1;
 
-	return type_id;
+	return id;
 }
 
-int32_t btf_elf__add_var_type(struct btf_elf *btfe, uint32_t type, uint32_t name_off,
+int32_t btf_elf__add_var_type(struct btf_elf *btfe, uint32_t type, const char *name,
 			      uint32_t linkage)
 {
-	struct btf_var_type t;
-
-	t.type.name_off = name_off;
-	t.type.info = BTF_INFO_ENCODE(BTF_KIND_VAR, 0, 0);
-	t.type.type = type;
-
-	t.var.linkage = linkage;
-
-	++btfe->type_index;
-	if (gobuffer__add(&btfe->types, &t.type, sizeof(t)) < 0) {
-		btf_elf__log_type(btfe, &t.type, true, true,
-				  "type=%u name=%s Error in adding gobuffer",
-				  t.type.type, btf_elf__name_in_gobuf(btfe, t.type.name_off));
-		return -1;
+	struct btf *btf = btfe->btf;
+	const struct btf_type *t;
+	int32_t id;
+
+	id = btf__add_var(btf, name, linkage, type);
+	if (id > 0) {
+		t = btf__type_by_id(btf, id);
+		btf_elf__log_type(btfe, t, false, true, "type=%u linkage=%u",
+				  t->type, btf_var(t)->linkage);
+	} else {
+		btf_elf__log_err(btfe, BTF_KIND_VAR, name, true,
+			      "type=%u linkage=%u Error emitting BTF type",
+			      type, linkage);
 	}
-
-	btf_elf__log_type(btfe, &t.type, false, false, "type=%u name=%s\n",
-			  t.type.type, btf_elf__name_in_gobuf(btfe, t.type.name_off));
-
-	return btfe->type_index;
+	return id;
 }
 
 int32_t btf_elf__add_var_secinfo(struct gobuffer *buf, uint32_t type,
@@ -624,52 +622,49 @@ int32_t btf_elf__add_var_secinfo(struct gobuffer *buf, uint32_t type,
 	return gobuffer__add(buf, &si, sizeof(si));
 }
 
-extern struct strings *strings;
-
 int32_t btf_elf__add_datasec_type(struct btf_elf *btfe, const char *section_name,
 				  struct gobuffer *var_secinfo_buf)
 {
-	struct btf_type type;
+	struct btf *btf = btfe->btf;
 	size_t sz = gobuffer__size(var_secinfo_buf);
 	uint16_t nr_var_secinfo = sz / sizeof(struct btf_var_secinfo);
-	uint32_t name_off;
-	struct btf_var_secinfo *last_vsi;
+	struct btf_var_secinfo *last_vsi, *vsi;
+	const struct btf_type *t;
+	uint32_t datasec_sz;
+	int32_t err, id, i;
 
 	qsort(var_secinfo_buf->entries, nr_var_secinfo,
 	      sizeof(struct btf_var_secinfo), btf_var_secinfo_cmp);
 
 	last_vsi = (struct btf_var_secinfo *)var_secinfo_buf->entries + nr_var_secinfo - 1;
+	datasec_sz = last_vsi->offset + last_vsi->size;
 
-	/*
-	 * dwarves doesn't store section names in its string table,
-	 * so we have to add it by ourselves.
-	 */
-	name_off = strings__add(strings, section_name);
-
-	type.name_off = name_off;
-	type.info = BTF_INFO_ENCODE(BTF_KIND_DATASEC, 0, nr_var_secinfo);
-	type.size = last_vsi->offset + last_vsi->size;
-
-	++btfe->type_index;
-	if (gobuffer__add(&btfe->types, &type, sizeof(type)) < 0) {
-		btf_elf__log_type(btfe, &type, true, true,
-				  "name=%s vlen=%u Error in adding datasec",
-				  btf_elf__name_in_gobuf(btfe, type.name_off),
-				  nr_var_secinfo);
-		return -1;
-	}
-	if (gobuffer__add(&btfe->types, var_secinfo_buf->entries, sz) < 0) {
-		btf_elf__log_type(btfe, &type, true, true,
-				  "name=%s vlen=%u Error in adding var_secinfo",
-				  btf_elf__name_in_gobuf(btfe, type.name_off),
-				  nr_var_secinfo);
-		return -1;
+	id = btf__add_datasec(btf, section_name, datasec_sz);
+	if (id < 0) {
+		btf_elf__log_err(btfe, BTF_KIND_DATASEC, section_name, true,
+				 "size=%u vlen=%u Error emitting BTF type",
+				 datasec_sz, nr_var_secinfo);
+	} else {
+		t = btf__type_by_id(btf, id);
+		btf_elf__log_type(btfe, t, false, true, "size=%u vlen=%u",
+				  t->size, nr_var_secinfo);
+	}
+
+	for (i = 0; i < nr_var_secinfo; i++) {
+		vsi = (struct btf_var_secinfo *)var_secinfo_buf->entries + i;
+		err = btf__add_datasec_var_info(btf, vsi->type, vsi->offset, vsi->size);
+		if (!err) {
+			if (btf_elf__verbose)
+				printf("\ttype=%u offset=%u size=%u\n",
+				       vsi->type, vsi->offset, vsi->size);
+		} else {
+			fprintf(stderr, "\ttype=%u offset=%u size=%u Error emitting BTF datasec var info\n",
+				       vsi->type, vsi->offset, vsi->size);
+			return -1;
+		}
 	}
 
-	btf_elf__log_type(btfe, &type, false, false, "type=datasec name=%s",
-			  btf_elf__name_in_gobuf(btfe, type.name_off));
-
-	return btfe->type_index;
+	return id;
 }
 
 static int btf_elf__write(const char *filename, struct btf *btf)
@@ -774,48 +769,16 @@ out:
 
 int btf_elf__encode(struct btf_elf *btfe, uint8_t flags)
 {
-	struct btf_header *hdr;
-	struct btf *btf;
-
-	/* Empty file, nothing to do, so... done! */
-	if (gobuffer__size(&btfe->types) == 0)
-		return 0;
+	struct btf *btf = btfe->btf;
 
 	if (gobuffer__size(&btfe->percpu_secinfo) != 0)
 		btf_elf__add_datasec_type(btfe, PERCPU_SECTION,
 					  &btfe->percpu_secinfo);
 
-	btfe->size = sizeof(*hdr) + (gobuffer__size(&btfe->types) + gobuffer__size(btfe->strings));
-	btfe->data = zalloc(btfe->size);
-
-	if (btfe->data == NULL) {
-		fprintf(stderr, "%s: malloc failed!\n", __func__);
-		return -1;
-	}
-
-	hdr = btfe->hdr;
-	hdr->magic = BTF_MAGIC;
-	hdr->version = 1;
-	hdr->flags = flags;
-	hdr->hdr_len = sizeof(*hdr);
-
-	hdr->type_off = 0;
-	hdr->type_len = gobuffer__size(&btfe->types);
-	hdr->str_off  = hdr->type_len;
-	hdr->str_len  = gobuffer__size(btfe->strings);
-
-	gobuffer__copy(&btfe->types, btf_elf__nohdr_data(btfe) + hdr->type_off);
-	gobuffer__copy(btfe->strings, btf_elf__nohdr_data(btfe) + hdr->str_off);
-
-	*(char *)(btf_elf__nohdr_data(btfe) + hdr->str_off) = '\0';
-
-	libbpf_set_print(libbpf_log);
+	/* Empty file, nothing to do, so... done! */
+	if (btf__get_nr_types(btf) == 0)
+		return 0;
 
-	btf = btf__new(btfe->data, btfe->size);
-	if (IS_ERR(btf)) {
-		fprintf(stderr, "%s: btf__new failed!\n", __func__);
-		return -1;
-	}
 	if (btf__dedup(btf, NULL, NULL)) {
 		fprintf(stderr, "%s: btf__dedup failed!\n", __func__);
 		return -1;
diff --git a/libbtf.h b/libbtf.h
index 5f29b427c4fd..9b3d396da31f 100644
--- a/libbtf.h
+++ b/libbtf.h
@@ -14,24 +14,16 @@
 #include "lib/bpf/src/btf.h"
 
 struct btf_elf {
-	union {
-		struct btf_header *hdr;
-		void		  *data;
-	};
 	void		  *priv;
 	Elf		  *elf;
 	GElf_Ehdr	  ehdr;
 	struct elf_symtab *symtab;
-	struct gobuffer	  types;
-	struct gobuffer   *strings;
 	struct gobuffer   percpu_secinfo;
 	char		  *filename;
-	size_t		  size;
 	int		  in_fd;
 	uint8_t		  wordsize;
 	bool		  is_big_endian;
 	bool		  raw_btf; // "/sys/kernel/btf/vmlinux"
-	uint32_t	  type_index;
 	uint32_t	  percpu_shndx;
 	uint64_t	  percpu_base_addr;
 	struct btf	  *btf;
@@ -42,33 +34,32 @@ extern uint8_t btf_elf__verbose;
 
 #define PERCPU_SECTION ".data..percpu"
 
+struct cu;
 struct base_type;
 struct ftype;
 
 struct btf_elf *btf_elf__new(const char *filename, Elf *elf);
 void btf_elf__delete(struct btf_elf *btf);
 
-int32_t btf_elf__add_base_type(struct btf_elf *btf, const struct base_type *bt);
+int32_t btf_elf__add_base_type(struct btf_elf *btf, const struct base_type *bt,
+			       const char *name);
 int32_t btf_elf__add_ref_type(struct btf_elf *btf, uint16_t kind, uint32_t type,
-			      uint32_t name, bool kind_flag);
-int btf_elf__add_member(struct btf_elf *btf, uint32_t name, uint32_t type, bool kind_flag,
+			      const char *name, bool kind_flag);
+int btf_elf__add_member(struct btf_elf *btf, const char *name, uint32_t type,
 			uint32_t bitfield_size, uint32_t bit_offset);
-int32_t btf_elf__add_struct(struct btf_elf *btf, uint8_t kind, uint32_t name,
-			    bool kind_flag, uint32_t size, uint16_t nr_members);
+int32_t btf_elf__add_struct(struct btf_elf *btf, uint8_t kind, const char *name, uint32_t size);
 int32_t btf_elf__add_array(struct btf_elf *btf, uint32_t type, uint32_t index_type,
 			   uint32_t nelems);
-int32_t btf_elf__add_enum(struct btf_elf *btf, uint32_t name, uint32_t size,
-			  uint16_t nr_entries);
-int btf_elf__add_enum_val(struct btf_elf *btf, uint32_t name, int32_t value);
-int32_t btf_elf__add_func_proto(struct btf_elf *btf, struct ftype *ftype,
+int32_t btf_elf__add_enum(struct btf_elf *btf, const char *name, uint32_t size);
+int btf_elf__add_enum_val(struct btf_elf *btf, const char *name, int32_t value);
+int32_t btf_elf__add_func_proto(struct btf_elf *btf, struct cu *cu, struct ftype *ftype,
 				uint32_t type_id_off);
-int32_t btf_elf__add_var_type(struct btf_elf *btfe, uint32_t type, uint32_t name_off,
+int32_t btf_elf__add_var_type(struct btf_elf *btfe, uint32_t type, const char *name,
 			      uint32_t linkage);
 int32_t btf_elf__add_var_secinfo(struct gobuffer *buf, uint32_t type,
 				 uint32_t offset, uint32_t size);
 int32_t btf_elf__add_datasec_type(struct btf_elf *btfe, const char *section_name,
 				  struct gobuffer *var_secinfo_buf);
-void btf_elf__set_strings(struct btf_elf *btf, struct gobuffer *strings);
 int  btf_elf__encode(struct btf_elf *btf, uint8_t flags);
 
 const char *btf_elf__string(struct btf_elf *btf, uint32_t ref);
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 dwarves 3/8] btf_encoder: fix emitting __ARRAY_SIZE_TYPE__ as index range type
  2020-10-08 23:39 [PATCH v2 dwarves 0/8] Switch BTF loading and encoding to libbpf APIs Andrii Nakryiko
  2020-10-08 23:39 ` [PATCH v2 dwarves 1/8] btf_loader: use libbpf to load BTF Andrii Nakryiko
  2020-10-08 23:39 ` [PATCH v2 dwarves 2/8] btf_encoder: use libbpf APIs to encode BTF type info Andrii Nakryiko
@ 2020-10-08 23:39 ` Andrii Nakryiko
  2020-10-08 23:39 ` [PATCH v2 dwarves 4/8] btf_encoder: discard CUs after BTF encoding Andrii Nakryiko
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Andrii Nakryiko @ 2020-10-08 23:39 UTC (permalink / raw)
  To: dwarves
  Cc: bpf, kernel-team, ast, Andrii Nakryiko, Arnaldo Carvalho de Melo,
	Andrii Nakryiko

From: Andrii Nakryiko <andriin@fb.com>

Fix the logic of determining if __ARRAY_SIZE_TYPE__ needs to be emitted.
Previously, such type could be emitted unnecessarily due to some particular CU
not having an int type in it. That would happen even if there was no array
type in that CU. Fix it by keeping track of 'int' type across CUs and only
emitting __ARRAY_SIZE_TYPE__ if a given CU has array type, but we still
haven't found 'int' type.

Testing against vmlinux shows that now there are no __ARRAY_SIZE_TYPE__
integers emitted.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 btf_encoder.c | 30 +++++++++++++++++++++---------
 1 file changed, 21 insertions(+), 9 deletions(-)

diff --git a/btf_encoder.c b/btf_encoder.c
index 6e6bce202438..2e5df03e040f 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -147,6 +147,8 @@ static int32_t enumeration_type__encode(struct btf_elf *btfe, struct cu *cu, str
 	return type_id;
 }
 
+static bool need_index_type;
+
 static int tag__encode_btf(struct cu *cu, struct tag *tag, uint32_t core_id, struct btf_elf *btfe,
 			   uint32_t array_index_id, uint32_t type_id_off)
 {
@@ -179,6 +181,7 @@ static int tag__encode_btf(struct cu *cu, struct tag *tag, uint32_t core_id, str
 			return structure_type__encode(btfe, cu, tag, type_id_off);
 	case DW_TAG_array_type:
 		/* TODO: Encode one dimension at a time. */
+		need_index_type = true;
 		return btf_elf__add_array(btfe, ref_type_id, array_index_id, array_type__nelems(tag));
 	case DW_TAG_enumeration_type:
 		return enumeration_type__encode(btfe, cu, tag);
@@ -193,6 +196,7 @@ static int tag__encode_btf(struct cu *cu, struct tag *tag, uint32_t core_id, str
 
 static struct btf_elf *btfe;
 static uint32_t array_index_id;
+static bool has_index_type;
 
 int btf_encoder__encode()
 {
@@ -228,7 +232,6 @@ static struct variable *hashaddr__find_variable(const struct hlist_head hashtabl
 int cu__encode_btf(struct cu *cu, int verbose, bool force,
 		   bool skip_encoding_vars)
 {
-	bool add_index_type = false;
 	uint32_t type_id_off;
 	uint32_t core_id;
 	struct function *fn;
@@ -254,18 +257,26 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force,
 		if (!btfe)
 			return -1;
 
-		/* cu__find_base_type_by_name() takes "type_id_t *id" */
-		type_id_t id;
-		if (!cu__find_base_type_by_name(cu, "int", &id)) {
-			add_index_type = true;
-			id = cu->types_table.nr_entries;
-		}
-		array_index_id = id;
+		has_index_type = false;
+		need_index_type = false;
+		array_index_id = 0;
 
 		if (verbose)
 			printf("File %s:\n", btfe->filename);
 	}
 
+	if (!has_index_type) {
+		/* cu__find_base_type_by_name() takes "type_id_t *id" */
+		type_id_t id;
+		if (cu__find_base_type_by_name(cu, "int", &id)) {
+			has_index_type = true;
+			array_index_id = id;
+		} else {
+			has_index_type = false;
+			array_index_id = cu->types_table.nr_entries;
+		}
+	}
+
 	btf_elf__verbose = verbose;
 	type_id_off = btf__get_nr_types(btfe->btf);
 
@@ -279,12 +290,13 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force,
 		}
 	}
 
-	if (add_index_type) {
+	if (need_index_type && !has_index_type) {
 		struct base_type bt = {};
 
 		bt.name = 0;
 		bt.bit_size = 32;
 		btf_elf__add_base_type(btfe, &bt, "__ARRAY_SIZE_TYPE__");
+		has_index_type = true;
 	}
 
 	cu__for_each_function(cu, core_id, fn) {
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 dwarves 4/8] btf_encoder: discard CUs after BTF encoding
  2020-10-08 23:39 [PATCH v2 dwarves 0/8] Switch BTF loading and encoding to libbpf APIs Andrii Nakryiko
                   ` (2 preceding siblings ...)
  2020-10-08 23:39 ` [PATCH v2 dwarves 3/8] btf_encoder: fix emitting __ARRAY_SIZE_TYPE__ as index range type Andrii Nakryiko
@ 2020-10-08 23:39 ` Andrii Nakryiko
  2020-10-09 15:47   ` Arnaldo Carvalho de Melo
  2020-10-08 23:39 ` [PATCH v2 dwarves 5/8] btf_encoder: revamp how per-CPU variables are encoded Andrii Nakryiko
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 15+ messages in thread
From: Andrii Nakryiko @ 2020-10-08 23:39 UTC (permalink / raw)
  To: dwarves
  Cc: bpf, kernel-team, ast, Andrii Nakryiko, Arnaldo Carvalho de Melo,
	Andrii Nakryiko

From: Andrii Nakryiko <andriin@fb.com>

When doing BTF encoding/deduping, DWARF CUs are never used after BTF encoding
is done, so there is no point in wasting memory and keeping them in memory. So
discard them immediately.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 pahole.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pahole.c b/pahole.c
index 61522175519e..bd9b993777ee 100644
--- a/pahole.c
+++ b/pahole.c
@@ -2384,7 +2384,7 @@ static enum load_steal_kind pahole_stealer(struct cu *cu,
 			fprintf(stderr, "Encountered error while encoding BTF.\n");
 			exit(1);
 		}
-		return LSK__KEEPIT;
+		return LSK__DELETE;
 	}
 
 	if (ctf_encode) {
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 dwarves 5/8] btf_encoder: revamp how per-CPU variables are encoded
  2020-10-08 23:39 [PATCH v2 dwarves 0/8] Switch BTF loading and encoding to libbpf APIs Andrii Nakryiko
                   ` (3 preceding siblings ...)
  2020-10-08 23:39 ` [PATCH v2 dwarves 4/8] btf_encoder: discard CUs after BTF encoding Andrii Nakryiko
@ 2020-10-08 23:39 ` Andrii Nakryiko
  2020-10-09 15:53   ` Arnaldo Carvalho de Melo
  2020-10-08 23:39 ` [PATCH v2 dwarves 6/8] dwarf_loader: increase the size of lookup hash map Andrii Nakryiko
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 15+ messages in thread
From: Andrii Nakryiko @ 2020-10-08 23:39 UTC (permalink / raw)
  To: dwarves
  Cc: bpf, kernel-team, ast, Andrii Nakryiko, Arnaldo Carvalho de Melo,
	Andrii Nakryiko

From: Andrii Nakryiko <andriin@fb.com>

Right now to encode per-CPU variables in BTF, pahole iterates complete vmlinux
symbol table for each CU. There are 2500 CUs for a typical kernel image.
Overall, to encode 287 per-CPU variables pahole spends more than 10% of its CPU
budget, this is incredibly wasteful.

This patch revamps how this is done. Now it pre-processes symbol table once
before any of per-CU processing starts. It remembers each per-CPU variable
symbol, including its address, size, and name. Then during processing each CU,
binary search is used to correlate DWARF variable with per-CPU symbols and
figure out if variable belongs to per-CPU data section. If the match is found,
BTF_KIND_VAR is emitted and var_secinfo is recorded, just like before. At the
very end, after all CUs are processed, BTF_KIND_DATASEC is emitted with sorted
variables.

This change makes per-CPU variables generation overhead pretty negligible and
returns back about 10% of CPU usage.

Performance counter stats for './pahole -J /home/andriin/linux-build/default/vmlinux':

BEFORE:
      19.160149000 seconds user
       1.304873000 seconds sys

         24,114.05 msec task-clock                #    0.999 CPUs utilized
                83      context-switches          #    0.003 K/sec
                 0      cpu-migrations            #    0.000 K/sec
           622,417      page-faults               #    0.026 M/sec
    72,897,315,125      cycles                    #    3.023 GHz                      (25.02%)
   127,807,316,959      instructions              #    1.75  insn per cycle           (25.01%)
    29,087,179,117      branches                  # 1206.234 M/sec                    (25.01%)
       464,105,921      branch-misses             #    1.60% of all branches          (25.01%)
    30,252,119,368      L1-dcache-loads           # 1254.543 M/sec                    (25.01%)
     1,156,336,207      L1-dcache-load-misses     #    3.82% of all L1-dcache hits    (25.05%)
       343,373,503      LLC-loads                 #   14.240 M/sec                    (25.02%)
        12,044,977      LLC-load-misses           #    3.51% of all LL-cache hits     (25.01%)

      24.136198321 seconds time elapsed

      22.729693000 seconds user
       1.384859000 seconds sys

AFTER:
      16.781455000 seconds user
       1.343956000 seconds sys

         23,398.77 msec task-clock                #    1.000 CPUs utilized
                86      context-switches          #    0.004 K/sec
                 0      cpu-migrations            #    0.000 K/sec
           622,420      page-faults               #    0.027 M/sec
    68,395,641,468      cycles                    #    2.923 GHz                      (25.05%)
   114,241,327,034      instructions              #    1.67  insn per cycle           (25.01%)
    26,330,711,718      branches                  # 1125.303 M/sec                    (25.01%)
       465,926,869      branch-misses             #    1.77% of all branches          (25.00%)
    24,662,984,772      L1-dcache-loads           # 1054.029 M/sec                    (25.00%)
     1,054,052,064      L1-dcache-load-misses     #    4.27% of all L1-dcache hits    (25.00%)
       340,970,622      LLC-loads                 #   14.572 M/sec                    (25.00%)
        16,032,297      LLC-load-misses           #    4.70% of all LL-cache hits     (25.03%)

      23.402259654 seconds time elapsed

      21.916437000 seconds user
       1.482826000 seconds sys

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 btf_encoder.c | 248 +++++++++++++++++++++++++++++---------------------
 libbtf.c      |   6 +-
 libbtf.h      |   1 +
 3 files changed, 148 insertions(+), 107 deletions(-)

diff --git a/btf_encoder.c b/btf_encoder.c
index 2e5df03e040f..2a6455be4c52 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -17,6 +17,7 @@
 #include "btf_encoder.h"
 
 #include <ctype.h> /* for isalpha() and isalnum() */
+#include <stdlib.h> /* for qsort() and bsearch() */
 #include <inttypes.h>
 
 /*
@@ -53,18 +54,18 @@ static bool btf_name_valid(const char *p)
 	return !*p;
 }
 
-static void dump_invalid_symbol(const char *msg, const char *sym, const char *cu,
+static void dump_invalid_symbol(const char *msg, const char *sym,
 				int verbose, bool force)
 {
 	if (force) {
 		if (verbose)
-			fprintf(stderr, "PAHOLE: Warning: %s, ignored (sym: '%s', cu: '%s').\n",
-				msg, sym, cu);
+			fprintf(stderr, "PAHOLE: Warning: %s, ignored (sym: '%s').\n",
+				msg, sym);
 		return;
 	}
 
-	fprintf(stderr, "PAHOLE: Error: %s (sym: '%s', cu: '%s').\n", msg, sym, cu);
-	fprintf(stderr, "PAHOLE: Error: Use '-j' or '--force' to ignore such symbols and force emit the btf.\n");
+	fprintf(stderr, "PAHOLE: Error: %s (sym: '%s').\n", msg, sym);
+	fprintf(stderr, "PAHOLE: Error: Use '--btf_encode_force' to ignore such symbols and force emit the btf.\n");
 }
 
 extern struct debug_fmt_ops *dwarves__active_loader;
@@ -202,6 +203,9 @@ int btf_encoder__encode()
 {
 	int err;
 
+	if (gobuffer__size(&btfe->percpu_secinfo) != 0)
+		btf_elf__add_datasec_type(btfe, PERCPU_SECTION, &btfe->percpu_secinfo);
+
 	err = btf_elf__encode(btfe, 0);
 	btf_elf__delete(btfe);
 	btfe = NULL;
@@ -209,24 +213,117 @@ int btf_encoder__encode()
 	return err;
 }
 
-#define HASHADDR__BITS 8
-#define HASHADDR__SIZE (1UL << HASHADDR__BITS)
-#define hashaddr__fn(key) hash_64(key, HASHADDR__BITS)
+#define MAX_PERCPU_VAR_CNT 4096
+
+struct var_info {
+	uint64_t addr;
+	uint32_t sz;
+	const char *name;
+};
+
+static struct var_info percpu_vars[MAX_PERCPU_VAR_CNT];
+static int percpu_var_cnt;
+
+static int percpu_var_cmp(const void *_a, const void *_b)
+{
+	const struct var_info *a = _a;
+	const struct var_info *b = _b;
+
+	if (a->addr == b->addr)
+		return 0;
+	return a->addr < b->addr ? -1 : 1;
+}
+
+static bool percpu_var_exists(uint64_t addr, uint32_t *sz, const char **name)
+{
+	const struct var_info *p;
+	struct var_info key = { .addr = addr };
+
+	p = bsearch(&key, percpu_vars, percpu_var_cnt,
+		    sizeof(percpu_vars[0]), percpu_var_cmp);
+
+	if (!p)
+		return false;
+
+	*sz = p->sz;
+	*name = p->name;
+	return true;
+}
 
-static struct variable *hashaddr__find_variable(const struct hlist_head hashtable[],
-						const uint64_t addr)
+static int find_all_percpu_vars(struct btf_elf *btfe)
 {
-	struct variable *variable;
-	struct hlist_node *pos;
-	uint16_t bucket = hashaddr__fn(addr);
-	const struct hlist_head *head = &hashtable[bucket];
-
-	hlist_for_each_entry(variable, pos, head, tool_hnode) {
-		if (variable->ip.addr == addr)
-			return variable;
+	uint32_t core_id;
+	GElf_Sym sym;
+
+	/* cache variables' addresses, preparing for searching in symtab. */
+	percpu_var_cnt = 0;
+
+	/* search within symtab for percpu variables */
+	elf_symtab__for_each_symbol(btfe->symtab, core_id, sym) {
+		const char *sym_name;
+		uint64_t addr;
+		uint32_t size;
+
+		/* compare a symbol's shndx to determine if it's a percpu variable */
+		if (elf_sym__section(&sym) != btfe->percpu_shndx)
+			continue;
+		if (elf_sym__type(&sym) != STT_OBJECT)
+			continue;
+
+		addr = elf_sym__value(&sym);
+		/*
+		 * Store only those symbols that have allocated space in the percpu section.
+		 * This excludes the following three types of symbols:
+		 *
+		 *  1. __ADDRESSABLE(sym), which are forcely emitted as symbols.
+		 *  2. __UNIQUE_ID(prefix), which are introduced to generate unique ids.
+		 *  3. __exitcall(fn), functions which are labeled as exit calls.
+		 *
+		 * In addition, the variables defined using DEFINE_PERCPU_FIRST are
+		 * also not included, which currently includes:
+		 *
+		 *  1. fixed_percpu_data
+		 */
+		if (!addr)
+			continue;
+
+		sym_name = elf_sym__name(&sym, btfe->symtab);
+		if (!btf_name_valid(sym_name)) {
+			dump_invalid_symbol("Found symbol of invalid name when encoding btf",
+					    sym_name, btf_elf__verbose, btf_elf__force);
+			if (btf_elf__force)
+				continue;
+			return -1;
+		}
+		size = elf_sym__size(&sym);
+		if (!size) {
+			dump_invalid_symbol("Found symbol of zero size when encoding btf",
+					    sym_name, btf_elf__verbose, btf_elf__force);
+			if (btf_elf__force)
+				continue;
+			return -1;
+		}
+
+		if (btf_elf__verbose)
+			printf("Found per-CPU symbol '%s' at address 0x%lx\n", sym_name, addr);
+
+		if (percpu_var_cnt == MAX_PERCPU_VAR_CNT) {
+			fprintf(stderr, "Reached the limit of per-CPU variables: %d\n",
+				MAX_PERCPU_VAR_CNT);
+			return -1;
+		}
+		percpu_vars[percpu_var_cnt].addr = addr;
+		percpu_vars[percpu_var_cnt].sz = size;
+		percpu_vars[percpu_var_cnt].name = sym_name;
+		percpu_var_cnt++;
 	}
 
-	return NULL;
+	if (percpu_var_cnt)
+		qsort(percpu_vars, percpu_var_cnt, sizeof(percpu_vars[0]), percpu_var_cmp);
+
+	if (btf_elf__verbose)
+		printf("Found %d per-CPU variables!\n", percpu_var_cnt);
+	return 0;
 }
 
 int cu__encode_btf(struct cu *cu, int verbose, bool force,
@@ -234,13 +331,10 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force,
 {
 	uint32_t type_id_off;
 	uint32_t core_id;
+	struct variable *var;
 	struct function *fn;
 	struct tag *pos;
 	int err = 0;
-	struct hlist_head hash_addr[HASHADDR__SIZE];
-	struct variable *var;
-	bool has_global_var = false;
-	GElf_Sym sym;
 
 	if (btfe && strcmp(btfe->filename, cu->filename)) {
 		err = btf_encoder__encode();
@@ -257,6 +351,9 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force,
 		if (!btfe)
 			return -1;
 
+		if (!skip_encoding_vars && find_all_percpu_vars(btfe))
+			goto out;
+
 		has_index_type = false;
 		need_index_type = false;
 		array_index_id = 0;
@@ -278,6 +375,7 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force,
 	}
 
 	btf_elf__verbose = verbose;
+	btf_elf__force = force;
 	type_id_off = btf__get_nr_types(btfe->btf);
 
 	cu__for_each_type(cu, core_id, pos) {
@@ -325,12 +423,11 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force,
 	if (verbose)
 		printf("search cu '%s' for percpu global variables.\n", cu->name);
 
-	/* cache variables' addresses, preparing for searching in symtab. */
-	for (core_id = 0; core_id < HASHADDR__SIZE; ++core_id)
-		INIT_HLIST_HEAD(&hash_addr[core_id]);
-
 	cu__for_each_variable(cu, core_id, pos) {
-		struct hlist_head *head;
+		uint32_t size, type, linkage, offset;
+		const char *name;
+		uint64_t addr;
+		int id;
 
 		var = tag__variable(pos);
 		if (var->declaration && !var->spec)
@@ -338,89 +435,37 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force,
 		/* percpu variables are allocated in global space */
 		if (variable__scope(var) != VSCOPE_GLOBAL && !var->spec)
 			continue;
-		has_global_var = true;
-		head = &hash_addr[hashaddr__fn(var->ip.addr)];
-		hlist_add_head(&var->tool_hnode, head);
-	}
-	if (!has_global_var) {
-		if (verbose)
-			printf("cu has no global variable defined, skip.\n");
-		goto out;
-	}
-
-	/* search within symtab for percpu variables */
-	elf_symtab__for_each_symbol(btfe->symtab, core_id, sym) {
-		uint32_t linkage, type, size, offset;
-		int32_t btf_var_id, btf_var_secinfo_id;
-		uint64_t addr;
-		const char *sym_name;
-
-		/* compare a symbol's shndx to determine if it's a percpu variable */
-		if (elf_sym__section(&sym) != btfe->percpu_shndx)
-			continue;
-		if (elf_sym__type(&sym) != STT_OBJECT)
-			continue;
 
-		addr = elf_sym__value(&sym);
-		/*
-		 * Store only those symbols that have allocated space in the percpu section.
-		 * This excludes the following three types of symbols:
-		 *
-		 *  1. __ADDRESSABLE(sym), which are forcely emitted as symbols.
-		 *  2. __UNIQUE_ID(prefix), which are introduced to generate unique ids.
-		 *  3. __exitcall(fn), functions which are labeled as exit calls.
-		 *
-		 * In addition, the variables defined using DEFINE_PERCPU_FIRST are
-		 * also not included, which currently includes:
-		 *
-		 *  1. fixed_percpu_data
-		 */
-		if (!addr)
-			continue;
-		var = hashaddr__find_variable(hash_addr, addr);
-		if (var == NULL)
-			continue;
+		/* addr has to be recorded before we follow spec */
+		addr = var->ip.addr;
 		if (var->spec)
 			var = var->spec;
 
-		sym_name = elf_sym__name(&sym, btfe->symtab);
-		if (!btf_name_valid(sym_name)) {
-			dump_invalid_symbol("Found symbol of invalid name when encoding btf",
-					    sym_name, cu->name, verbose, force);
-			if (force)
-				continue;
-			err = -1;
-			break;
-		}
 		if (var->ip.tag.type == 0) {
-			dump_invalid_symbol("Found symbol of void type when encoding btf",
-					    sym_name, cu->name, verbose, force);
-			if (force)
-				continue;
-			err = -1;
-			break;
-		}
-		type = type_id_off + var->ip.tag.type;
-		size = elf_sym__size(&sym);
-		if (!size) {
-			dump_invalid_symbol("Found symbol of zero size when encoding btf",
-					    sym_name, cu->name, verbose, force);
+			fprintf(stderr, "error: found variable in CU '%s' that has void type\n",
+				cu->name);
 			if (force)
 				continue;
 			err = -1;
 			break;
 		}
 
-		if (verbose)
-			printf("symbol '%s' of address 0x%lx encoded\n",
-			       sym_name, addr);
+		type = var->ip.tag.type + type_id_off;
+		linkage = var->external ? BTF_VAR_GLOBAL_ALLOCATED : BTF_VAR_STATIC;
+		if (!percpu_var_exists(addr, &size, &name))
+			continue; /* not a per-CPU variable */
+
+		if (btf_elf__verbose) {
+			printf("Variable '%s' from CU '%s' at address 0x%lx encoded\n",
+			       name, cu->name, addr);
+		}
 
 		/* add a BTF_KIND_VAR in btfe->types */
-		linkage = var->external ? BTF_VAR_GLOBAL_ALLOCATED : BTF_VAR_STATIC;
-		btf_var_id = btf_elf__add_var_type(btfe, type, sym_name, linkage);
-		if (btf_var_id < 0) {
+		id = btf_elf__add_var_type(btfe, type, name, linkage);
+		if (id < 0) {
 			err = -1;
-			printf("error: failed to encode variable '%s'\n", sym_name);
+			fprintf(stderr, "error: failed to encode variable '%s' at addr 0x%lx\n",
+			        name, addr);
 			break;
 		}
 
@@ -428,13 +473,12 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force,
 		 * add a BTF_VAR_SECINFO in btfe->percpu_secinfo, which will be added into
 		 * btfe->types later when we add BTF_VAR_DATASEC.
 		 */
-		type = btf_var_id;
 		offset = addr - btfe->percpu_base_addr;
-		btf_var_secinfo_id = btf_elf__add_var_secinfo(&btfe->percpu_secinfo,
-							      type, offset, size);
-		if (btf_var_secinfo_id < 0) {
+		id = btf_elf__add_var_secinfo(&btfe->percpu_secinfo, id, offset, size);
+		if (id < 0) {
 			err = -1;
-			printf("error: failed to encode var secinfo '%s'\n", sym_name);
+			fprintf(stderr, "error: failed to encode section info for variable '%s' at addr 0x%lx\n",
+			        name, addr);
 			break;
 		}
 	}
diff --git a/libbtf.c b/libbtf.c
index 0467f1f2a596..27aa3e5a986e 100644
--- a/libbtf.c
+++ b/libbtf.c
@@ -28,6 +28,7 @@
 #include "elf_symtab.h"
 
 uint8_t btf_elf__verbose;
+uint8_t btf_elf__force;
 
 static int btf_var_secinfo_cmp(const void *a, const void *b)
 {
@@ -62,7 +63,6 @@ int btf_elf__load(struct btf_elf *btfe)
 	return 0;
 }
 
-
 struct btf_elf *btf_elf__new(const char *filename, Elf *elf)
 {
 	struct btf_elf *btfe = zalloc(sizeof(*btfe));
@@ -771,10 +771,6 @@ int btf_elf__encode(struct btf_elf *btfe, uint8_t flags)
 {
 	struct btf *btf = btfe->btf;
 
-	if (gobuffer__size(&btfe->percpu_secinfo) != 0)
-		btf_elf__add_datasec_type(btfe, PERCPU_SECTION,
-					  &btfe->percpu_secinfo);
-
 	/* Empty file, nothing to do, so... done! */
 	if (btf__get_nr_types(btf) == 0)
 		return 0;
diff --git a/libbtf.h b/libbtf.h
index 9b3d396da31f..887b5bc55c8e 100644
--- a/libbtf.h
+++ b/libbtf.h
@@ -30,6 +30,7 @@ struct btf_elf {
 };
 
 extern uint8_t btf_elf__verbose;
+extern uint8_t btf_elf__force;
 #define btf_elf__verbose_log(fmt, ...) { if (btf_elf__verbose) printf(fmt, __VA_ARGS__); }
 
 #define PERCPU_SECTION ".data..percpu"
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 dwarves 6/8] dwarf_loader: increase the size of lookup hash map
  2020-10-08 23:39 [PATCH v2 dwarves 0/8] Switch BTF loading and encoding to libbpf APIs Andrii Nakryiko
                   ` (4 preceding siblings ...)
  2020-10-08 23:39 ` [PATCH v2 dwarves 5/8] btf_encoder: revamp how per-CPU variables are encoded Andrii Nakryiko
@ 2020-10-08 23:39 ` Andrii Nakryiko
  2020-10-08 23:39 ` [PATCH v2 dwarves 7/8] strings: use BTF's string APIs for strings management Andrii Nakryiko
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Andrii Nakryiko @ 2020-10-08 23:39 UTC (permalink / raw)
  To: dwarves
  Cc: bpf, kernel-team, ast, Andrii Nakryiko, Arnaldo Carvalho de Melo,
	Andrii Nakryiko

From: Andrii Nakryiko <andriin@fb.com>

One of the primary use cases for using pahole is BTF deduplication during
Linux kernel build. That means that DWARF contains more than 5 million types
is loaded. So using a hash map with a small number of buckets is quite
expensive due to hash collisions. This patch bumps the size of the hash map
and reduces overhead of this part of the DWARF loading process.

This shaves off about 1 second out of about 20 seconds total for Linux BTF
dedup.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 dwarf_loader.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dwarf_loader.c b/dwarf_loader.c
index d3586aa5b0dd..0e6e4f741922 100644
--- a/dwarf_loader.c
+++ b/dwarf_loader.c
@@ -93,7 +93,7 @@ static void dwarf_tag__set_spec(struct dwarf_tag *dtag, dwarf_off_ref spec)
 	*(dwarf_off_ref *)(dtag + 1) = spec;
 }
 
-#define HASHTAGS__BITS 8
+#define HASHTAGS__BITS 15
 #define HASHTAGS__SIZE (1UL << HASHTAGS__BITS)
 
 #define obstack_chunk_alloc malloc
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 dwarves 7/8] strings: use BTF's string APIs for strings management
  2020-10-08 23:39 [PATCH v2 dwarves 0/8] Switch BTF loading and encoding to libbpf APIs Andrii Nakryiko
                   ` (5 preceding siblings ...)
  2020-10-08 23:39 ` [PATCH v2 dwarves 6/8] dwarf_loader: increase the size of lookup hash map Andrii Nakryiko
@ 2020-10-08 23:39 ` Andrii Nakryiko
  2020-10-08 23:40 ` [PATCH v2 dwarves 8/8] btf_encoder: support cross-compiled ELF binaries with different endianness Andrii Nakryiko
  2020-10-09 16:22 ` [PATCH v2 dwarves 0/8] Switch BTF loading and encoding to libbpf APIs Arnaldo Carvalho de Melo
  8 siblings, 0 replies; 15+ messages in thread
From: Andrii Nakryiko @ 2020-10-08 23:39 UTC (permalink / raw)
  To: dwarves
  Cc: bpf, kernel-team, ast, Andrii Nakryiko, Arnaldo Carvalho de Melo,
	Andrii Nakryiko

From: Andrii Nakryiko <andriin@fb.com>

Switch strings container to using struct btf and its
btf__add_str()/btf__find_str() APIs, which do equivalent internal string
deduplication. This turns out to be a very significantly faster than using
tsearch functions. To satisfy CTF encoding use case, some hacky string size
fetching approach is utilized, as libbpf doesn't provide direct API to get
total string section size and to copy over just strings data section.

BEFORE:
         22,624.28 msec task-clock                #    1.000 CPUs utilized
                85      context-switches          #    0.004 K/sec
                 3      cpu-migrations            #    0.000 K/sec
           622,545      page-faults               #    0.028 M/sec
    68,177,206,387      cycles                    #    3.013 GHz                      (24.99%)
   114,370,031,619      instructions              #    1.68  insn per cycle           (25.01%)
    26,125,001,179      branches                  # 1154.733 M/sec                    (25.01%)
       458,861,243      branch-misses             #    1.76% of all branches          (25.00%)
    24,533,455,967      L1-dcache-loads           # 1084.386 M/sec                    (25.02%)
       973,500,214      L1-dcache-load-misses     #    3.97% of all L1-dcache hits    (25.05%)
       338,773,561      LLC-loads                 #   14.974 M/sec                    (25.02%)
        12,651,196      LLC-load-misses           #    3.73% of all LL-cache hits     (25.00%)

      22.628910615 seconds time elapsed

      21.341063000 seconds user
       1.283763000 seconds sys

AFTER:
         18,362.97 msec task-clock                #    1.000 CPUs utilized
                37      context-switches          #    0.002 K/sec
                 0      cpu-migrations            #    0.000 K/sec
           626,281      page-faults               #    0.034 M/sec
    52,480,619,000      cycles                    #    2.858 GHz                      (25.00%)
   104,736,434,384      instructions              #    2.00  insn per cycle           (25.01%)
    23,878,428,465      branches                  # 1300.358 M/sec                    (25.01%)
       252,669,685      branch-misses             #    1.06% of all branches          (25.03%)
    21,829,390,952      L1-dcache-loads           # 1188.772 M/sec                    (25.04%)
       638,086,339      L1-dcache-load-misses     #    2.92% of all L1-dcache hits    (25.02%)
       212,327,435      LLC-loads                 #   11.563 M/sec                    (25.00%)
        14,578,117      LLC-load-misses           #    6.87% of all LL-cache hits     (25.00%)

      18.364427347 seconds time elapsed

      16.985494000 seconds user
       1.377959000 seconds sys

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 ctf_encoder.c |  2 +-
 libctf.c      | 14 ++++----
 libctf.h      |  4 +--
 strings.c     | 91 +++++++++++++++++++--------------------------------
 strings.h     | 32 +++---------------
 5 files changed, 49 insertions(+), 94 deletions(-)

diff --git a/ctf_encoder.c b/ctf_encoder.c
index 3cb455a33098..b761287d4534 100644
--- a/ctf_encoder.c
+++ b/ctf_encoder.c
@@ -248,7 +248,7 @@ int cu__encode_ctf(struct cu *cu, int verbose)
 	if (cu__cache_symtab(cu) < 0)
 		goto out_delete;
 
-	ctf__set_strings(ctf, &strings->gb);
+	ctf__set_strings(ctf, strings);
 
 	uint32_t id;
 	struct tag *pos;
diff --git a/libctf.c b/libctf.c
index b7d237fbbd3c..95cbf1ff091e 100644
--- a/libctf.c
+++ b/libctf.c
@@ -19,6 +19,7 @@
 #include "ctf.h"
 #include "dutil.h"
 #include "gobuffer.h"
+#include "strings.h"
 
 bool ctf__ignore_symtab_function(const GElf_Sym *sym, const char *sym_name)
 {
@@ -287,7 +288,7 @@ int ctf__load_symtab(struct ctf *ctf)
 	return ctf->symtab == NULL ? -1 : 0;
 }
 
-void ctf__set_strings(struct ctf *ctf, struct gobuffer *strings)
+void ctf__set_strings(struct ctf *ctf, struct strings *strings)
 {
 	ctf->strings = strings;
 }
@@ -570,7 +571,7 @@ int ctf__encode(struct ctf *ctf, uint8_t flags)
 	size = (gobuffer__size(&ctf->types) +
 		gobuffer__size(&ctf->objects) +
 		gobuffer__size(&ctf->funcs) +
-		gobuffer__size(ctf->strings));
+		strings__size(ctf->strings));
 
 	ctf->size = sizeof(*hdr) + size;
 	ctf->buf = malloc(ctf->size);
@@ -594,13 +595,13 @@ int ctf__encode(struct ctf *ctf, uint8_t flags)
 	hdr->ctf_type_off = offset;
 	offset += gobuffer__size(&ctf->types);
 	hdr->ctf_str_off  = offset;
-	hdr->ctf_str_len  = gobuffer__size(ctf->strings);
+	hdr->ctf_str_len  = strings__size(ctf->strings);
 
 	void *payload = ctf->buf + sizeof(*hdr);
 	gobuffer__copy(&ctf->objects, payload + hdr->ctf_object_off);
 	gobuffer__copy(&ctf->funcs, payload + hdr->ctf_func_off);
 	gobuffer__copy(&ctf->types, payload + hdr->ctf_type_off);
-	gobuffer__copy(ctf->strings, payload + hdr->ctf_str_off);
+	strings__copy(ctf->strings, payload + hdr->ctf_str_off);
 
 	*(char *)(ctf->buf + sizeof(*hdr) + hdr->ctf_str_off) = '\0';
 	if (flags & CTF_FLAGS_COMPR) {
@@ -623,11 +624,10 @@ int ctf__encode(struct ctf *ctf, uint8_t flags)
 	}
 #if 0
 	printf("\n\ntypes:\n entries: %d\n size: %u"
-		 "\nstrings:\n entries: %u\n size: %u\ncompressed size: %d\n",
+		 "\nstrings:\n size: %u\ncompressed size: %d\n",
 	       ctf->type_index,
 	       gobuffer__size(&ctf->types),
-	       gobuffer__nr_entries(ctf->strings),
-	       gobuffer__size(ctf->strings), size);
+	       strings__size(ctf->strings), size);
 #endif
 	int fd = open(ctf->filename, O_RDWR);
 	if (fd < 0) {
diff --git a/libctf.h b/libctf.h
index 071616c72de3..749be8955c52 100644
--- a/libctf.h
+++ b/libctf.h
@@ -24,7 +24,7 @@ struct ctf {
 	struct gobuffer	  objects; /* data/variables */
 	struct gobuffer	  types;
 	struct gobuffer	  funcs;
-	struct gobuffer   *strings;
+	struct strings   *strings;
 	char		  *filename;
 	size_t		  size;
 	int		  swapped;
@@ -76,7 +76,7 @@ int ctf__add_function(struct ctf *ctf, uint16_t type, uint16_t nr_parms,
 
 int ctf__add_object(struct ctf *ctf, uint16_t type);
 
-void ctf__set_strings(struct ctf *ctf, struct gobuffer *strings);
+void ctf__set_strings(struct ctf *ctf, struct strings *strings);
 int  ctf__encode(struct ctf *ctf, uint8_t flags);
 
 char *ctf__string(struct ctf *ctf, uint32_t ref);
diff --git a/strings.c b/strings.c
index ddb2b1bd85b5..45f8faaeb15d 100644
--- a/strings.c
+++ b/strings.c
@@ -15,75 +15,41 @@
 #include <zlib.h>
 
 #include "dutil.h"
+#include "lib/bpf/src/libbpf.h"
 
 struct strings *strings__new(void)
 {
 	struct strings *strs = malloc(sizeof(*strs));
 
-	if (strs != NULL) {
-		strs->tree = NULL;
-		gobuffer__init(&strs->gb);
+	if (!strs)
+		return NULL;
+
+	strs->btf = btf__new_empty();
+	if (libbpf_get_error(strs->btf)) {
+		free(strs);
+		return NULL;
 	}
 
 	return strs;
-
-}
-
-static void do_nothing(void *ptr __unused)
-{
 }
 
 void strings__delete(struct strings *strs)
 {
 	if (strs == NULL)
 		return;
-	tdestroy(strs->tree, do_nothing);
-	__gobuffer__delete(&strs->gb);
+	btf__free(strs->btf);
 	free(strs);
 }
 
-static strings_t strings__insert(struct strings *strs, const char *s)
-{
-	return gobuffer__add(&strs->gb, s, strlen(s) + 1);
-}
-
-struct search_key {
-	struct strings *strs;
-	const char *str;
-};
-
-static int strings__compare(const void *a, const void *b)
-{
-	const struct search_key *key = a;
-
-	return strcmp(key->str, key->strs->gb.entries + (unsigned long)b);
-}
-
 strings_t strings__add(struct strings *strs, const char *str)
 {
-	unsigned long *s;
 	strings_t index;
-	struct search_key key = {
-		.strs = strs,
-		.str = str,
-	};
 
 	if (str == NULL)
 		return 0;
 
-	s = tsearch(&key, &strs->tree, strings__compare);
-	if (s != NULL) {
-		if (*(struct search_key **)s == (void *)&key) { /* Not found, replace with the right key */
-			index = strings__insert(strs, str);
-			if (index != 0)
-				*s = (unsigned long)index;
-			else {
-				tdelete(&key, &strs->tree, strings__compare);
-				return 0;
-			}
-		} else /* Found! */
-			index = *s;
-	} else
+	index = btf__add_str(strs->btf, str);
+	if (index < 0)
 		return 0;
 
 	return index;
@@ -91,21 +57,32 @@ strings_t strings__add(struct strings *strs, const char *str)
 
 strings_t strings__find(struct strings *strs, const char *str)
 {
-	strings_t *s;
-	struct search_key key = {
-		.strs = strs,
-		.str = str,
-	};
+	return btf__find_str(strs->btf, str);
+}
 
-	if (str == NULL)
-		return 0;
+/* a horrible and inefficient hack to get string section size out of BTF */
+strings_t strings__size(const struct strings *strs)
+{
+	const struct btf_header *p;
+	uint32_t sz;
+
+	p = btf__get_raw_data(strs->btf, &sz);
+	if (!p)
+		return -1;
 
-	s = tfind(&key, &strs->tree, strings__compare);
-	return s ? *s : 0;
+	return p->str_len;
 }
 
-int strings__cmp(const struct strings *strs, strings_t a, strings_t b)
+/* similarly horrible hack to copy out string section out of BTF */
+int strings__copy(const struct strings *strs, void *dst)
 {
-	return a == b ? 0 : strcmp(strings__ptr(strs, a),
-				   strings__ptr(strs, b));
+	const struct btf_header *p;
+	uint32_t sz;
+
+	p = btf__get_raw_data(strs->btf, &sz);
+	if (!p)
+		return -1;
+
+	memcpy(dst, (void *)p + p->str_off, p->str_len);
+	return 0;
 }
diff --git a/strings.h b/strings.h
index 01f50efd7adb..522fbf21de0d 100644
--- a/strings.h
+++ b/strings.h
@@ -6,13 +6,12 @@
   Copyright (C) 2008 Arnaldo Carvalho de Melo <acme@redhat.com>
 */
 
-#include "gobuffer.h"
+#include "lib/bpf/src/btf.h"
 
 typedef unsigned int strings_t;
 
 struct strings {
-	void		*tree;
-	struct gobuffer	gb;
+	struct btf *btf;
 };
 
 struct strings *strings__new(void);
@@ -21,33 +20,12 @@ void strings__delete(struct strings *strings);
 
 strings_t strings__add(struct strings *strings, const char *str);
 strings_t strings__find(struct strings *strings, const char *str);
-
-int strings__cmp(const struct strings *strings, strings_t a, strings_t b);
+strings_t strings__size(const struct strings *strings);
+int strings__copy(const struct strings *strings, void *dst);
 
 static inline const char *strings__ptr(const struct strings *strings, strings_t s)
 {
-	return gobuffer__ptr(&strings->gb, s);
-}
-
-static inline const char *strings__entries(const struct strings *strings)
-{
-	return gobuffer__entries(&strings->gb);
-}
-
-static inline unsigned int strings__nr_entries(const struct strings *strings)
-{
-	return gobuffer__nr_entries(&strings->gb);
-}
-
-static inline strings_t strings__size(const struct strings *strings)
-{
-	return gobuffer__size(&strings->gb);
-}
-
-static inline const char *strings__compress(struct strings *strings,
-					    unsigned int *size)
-{
-	return gobuffer__compress(&strings->gb, size);
+	return btf__str_by_offset(strings->btf, s);
 }
 
 #endif /* _STRINGS_H_ */
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 dwarves 8/8] btf_encoder: support cross-compiled ELF binaries with different endianness
  2020-10-08 23:39 [PATCH v2 dwarves 0/8] Switch BTF loading and encoding to libbpf APIs Andrii Nakryiko
                   ` (6 preceding siblings ...)
  2020-10-08 23:39 ` [PATCH v2 dwarves 7/8] strings: use BTF's string APIs for strings management Andrii Nakryiko
@ 2020-10-08 23:40 ` Andrii Nakryiko
  2020-10-09 16:22 ` [PATCH v2 dwarves 0/8] Switch BTF loading and encoding to libbpf APIs Arnaldo Carvalho de Melo
  8 siblings, 0 replies; 15+ messages in thread
From: Andrii Nakryiko @ 2020-10-08 23:40 UTC (permalink / raw)
  To: dwarves
  Cc: bpf, kernel-team, ast, Andrii Nakryiko, Arnaldo Carvalho de Melo,
	Andrii Nakryiko

From: Andrii Nakryiko <andriin@fb.com>

Ensure that output BTF endianness corresponds to target ELF's endianness. This
makes it finally possible to use pahole to generate BTF for cross-compiled
kernels with different endianness.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 libbtf.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/libbtf.c b/libbtf.c
index 27aa3e5a986e..babf4fe8cd9e 100644
--- a/libbtf.c
+++ b/libbtf.c
@@ -87,6 +87,8 @@ struct btf_elf *btf_elf__new(const char *filename, Elf *elf)
 		btfe->raw_btf  = true;
 		btfe->wordsize = sizeof(long);
 		btfe->is_big_endian = BYTE_ORDER == BIG_ENDIAN;
+		btf__set_endianness(btfe->btf,
+				    btfe->is_big_endian ? BTF_BIG_ENDIAN : BTF_LITTLE_ENDIAN);
 		return btfe;
 	}
 
@@ -118,8 +120,14 @@ struct btf_elf *btf_elf__new(const char *filename, Elf *elf)
 	}
 
 	switch (btfe->ehdr.e_ident[EI_DATA]) {
-	case ELFDATA2LSB: btfe->is_big_endian = false; break;
-	case ELFDATA2MSB: btfe->is_big_endian = true;  break;
+	case ELFDATA2LSB:
+		btfe->is_big_endian = false;
+		btf__set_endianness(btfe->btf, BTF_LITTLE_ENDIAN);
+		break;
+	case ELFDATA2MSB:
+		btfe->is_big_endian = true;
+		btf__set_endianness(btfe->btf, BTF_BIG_ENDIAN);
+		break;
 	default:
 		fprintf(stderr, "%s: unknown elf endianness.\n", __func__);
 		goto errout;
@@ -704,6 +712,18 @@ static int btf_elf__write(const char *filename, struct btf *btf)
 		goto out;
 	}
 
+	switch (ehdr_mem.e_ident[EI_DATA]) {
+	case ELFDATA2LSB:
+		btf__set_endianness(btf, BTF_LITTLE_ENDIAN);
+		break;
+	case ELFDATA2MSB:
+		btf__set_endianness(btf, BTF_BIG_ENDIAN);
+		break;
+	default:
+		fprintf(stderr, "%s: unknown elf endianness.\n", __func__);
+		goto out;
+	}
+
 	/*
 	 * First we look if there was already a .BTF section to overwrite.
 	 */
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 dwarves 1/8] btf_loader: use libbpf to load BTF
  2020-10-08 23:39 ` [PATCH v2 dwarves 1/8] btf_loader: use libbpf to load BTF Andrii Nakryiko
@ 2020-10-09 14:53   ` Arnaldo Carvalho de Melo
  2020-10-09 17:49     ` Andrii Nakryiko
  0 siblings, 1 reply; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-10-09 14:53 UTC (permalink / raw)
  To: Andrii Nakryiko; +Cc: dwarves, bpf, kernel-team, ast, Andrii Nakryiko

Em Thu, Oct 08, 2020 at 04:39:53PM -0700, Andrii Nakryiko escreveu:
> From: Andrii Nakryiko <andriin@fb.com>
> 
> Switch BTF loading to completely use libbpf's own struct btf and related APIs.
> BTF encoding is still happening with pahole's own code, so these two code
> paths are not sharing anything now. String fetching is happening based on
> whether btfe->strings were set to non-NULL pointer by btf_encoder.

While testing this one I noticed a problem, but it isn't caused by this
particular patch:

[acme@five pahole]$ cp ~/git/build/v5.9-rc6+/net/ipv4/tcp_ipv4.o .
[acme@five pahole]$ readelf -SW tcp_ipv4.o | grep BTF
[acme@five pahole]$ pahole -J tcp_ipv4.o
[acme@five pahole]$ readelf -SW tcp_ipv4.o | grep BTF
  [105] .BTF              PROGBITS        0000000000000000 0fbb3c 03f697 00      0   0  1
[acme@five pahole]$ ./btfdiff tcp_ipv4.o
--- /tmp/btfdiff.dwarf.BDAvGi	2020-10-09 11:41:45.161509391 -0300
+++ /tmp/btfdiff.btf.p81icw	2020-10-09 11:41:45.177509720 -0300
@@ -4056,7 +4056,7 @@ struct tcp_congestion_ops {
 	u32                        (*min_tso_segs)(struct sock *); /*    96     8 */
 	u32                        (*sndbuf_expand)(struct sock *); /*   104     8 */
 	void                       (*cong_control)(struct sock *, const struct rate_sample  *); /*   112     8 */
-	size_t                     (*get_info)(struct sock *, u32, int *, union tcp_cc_info *); /*   120     8 */
+	size_t                     (*get_info)(struct sock *, u32, int *, struct tcp_cc_info *); /*   120     8 */
 	/* --- cacheline 2 boundary (128 bytes) --- */
 	char                       name[16];             /*   128    16 */
 	struct module *            owner;                /*   144     8 */
[acme@five pahole]$ git log --oneline -5
ef4f971a9cf745fc (HEAD) dwarf_loader: Conditionally define DW_AT_alignment
cc3f9dce3378280f pahole: Implement --packed
08f49262f474370a man-pages: Fix 'coimbine' typo
fdc639188cb514e4 (tag: v1.18) dwarves: Prep v1.18
70c3e669709b6351 spec: Set the build type to 'Release'
[acme@five pahole]$

And looking at the source code it is a union:

include/net/tcp.h

        size_t (*get_info)(struct sock *sk, u32 ext, int *attr,
                           union tcp_cc_info *info);


So this tcp_cc_info isn't available in DWARF and thus isn't available in
the resulting BTF:

[acme@five pahole]$ pahole -F dwarf -C tcp_cc_info tcp_ipv4.o
pahole: type 'tcp_cc_info' not found
[acme@five pahole]$ pahole -F btf -C tcp_cc_info tcp_ipv4.o
pahole: type 'tcp_cc_info' not found
[acme@five pahole]$


If you look at /sys/kernel/btf/vmlinux it is there:

[acme@five pahole]$ pahole tcp_cc_info
union tcp_cc_info {
	struct tcpvegas_info       vegas;              /*     0    16 */
	struct tcp_dctcp_info      dctcp;              /*     0    16 */
	struct tcp_bbr_info        bbr;                /*     0    20 */
};
[acme@five pahole]$

I.e. when encoding vmlinux we're good and for the BTF use case so far
this is thus not a problem, will continue processing your patches and
then later try to figure this out.

For completeness when looking at tcp_congestion_ops using
/sys/kernel/btf/vmlinux, all is ok:

[acme@five pahole]$ pahole tcp_congestion_ops
struct tcp_congestion_ops {
	struct list_head           list;                 /*     0    16 */
	u32                        key;                  /*    16     4 */
	u32                        flags;                /*    20     4 */
	void                       (*init)(struct sock *); /*    24     8 */
	void                       (*release)(struct sock *); /*    32     8 */
	u32                        (*ssthresh)(struct sock *); /*    40     8 */
	void                       (*cong_avoid)(struct sock *, u32, u32); /*    48     8 */
	void                       (*set_state)(struct sock *, u8); /*    56     8 */
	/* --- cacheline 1 boundary (64 bytes) --- */
	void                       (*cwnd_event)(struct sock *, enum tcp_ca_event); /*    64     8 */
	void                       (*in_ack_event)(struct sock *, u32); /*    72     8 */
	u32                        (*undo_cwnd)(struct sock *); /*    80     8 */
	void                       (*pkts_acked)(struct sock *, const struct ack_sample  *); /*    88     8 */
	u32                        (*min_tso_segs)(struct sock *); /*    96     8 */
	u32                        (*sndbuf_expand)(struct sock *); /*   104     8 */
	void                       (*cong_control)(struct sock *, const struct rate_sample  *); /*   112     8 */
	size_t                     (*get_info)(struct sock *, u32, int *, union tcp_cc_info *); /*   120     8 */
	/* --- cacheline 2 boundary (128 bytes) --- */
	char                       name[16];             /*   128    16 */
	struct module *            owner;                /*   144     8 */

	/* size: 152, cachelines: 3, members: 18 */
	/* last cacheline: 24 bytes */
};
[acme@five pahole]$

And a btfdiff on vmlinux also shows BTF and DWARF produce the same
results:

[acme@five pahole]$ cp ~/git/build/bpf-next-v5.9.0-rc8+/vmlinux .
[acme@five pahole]$ readelf -SW vmlinux  | grep BTF
  [24] .BTF              PROGBITS        ffffffff82494ac0 1694ac0 340207 00   A  0   0  1
  [25] .BTF_ids          PROGBITS        ffffffff827d4cc8 19d4cc8 0000a4 00   A  0   0  1
[acme@five pahole]$ ./btfdiff vmlinux
[acme@five pahole]$

- Arnaldo

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 dwarves 4/8] btf_encoder: discard CUs after BTF encoding
  2020-10-08 23:39 ` [PATCH v2 dwarves 4/8] btf_encoder: discard CUs after BTF encoding Andrii Nakryiko
@ 2020-10-09 15:47   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-10-09 15:47 UTC (permalink / raw)
  To: Andrii Nakryiko; +Cc: dwarves, bpf, kernel-team, ast, Andrii Nakryiko

Em Thu, Oct 08, 2020 at 04:39:56PM -0700, Andrii Nakryiko escreveu:
> From: Andrii Nakryiko <andriin@fb.com>
> 
> When doing BTF encoding/deduping, DWARF CUs are never used after BTF encoding
> is done, so there is no point in wasting memory and keeping them in memory. So
> discard them immediately.

Right now, yes, but DW_TAG_partial_unit may require we keep them around.

I'm applying the patch since the common case, the kernel, is not yet
using that DWARF compression technique.

- Arnaldo
 
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---
>  pahole.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/pahole.c b/pahole.c
> index 61522175519e..bd9b993777ee 100644
> --- a/pahole.c
> +++ b/pahole.c
> @@ -2384,7 +2384,7 @@ static enum load_steal_kind pahole_stealer(struct cu *cu,
>  			fprintf(stderr, "Encountered error while encoding BTF.\n");
>  			exit(1);
>  		}
> -		return LSK__KEEPIT;
> +		return LSK__DELETE;
>  	}
>  
>  	if (ctf_encode) {
> -- 
> 2.24.1
> 

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 dwarves 5/8] btf_encoder: revamp how per-CPU variables are encoded
  2020-10-08 23:39 ` [PATCH v2 dwarves 5/8] btf_encoder: revamp how per-CPU variables are encoded Andrii Nakryiko
@ 2020-10-09 15:53   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-10-09 15:53 UTC (permalink / raw)
  To: Hao Luo, Andrii Nakryiko
  Cc: dwarves, bpf, kernel-team, ast, Andrii Nakryiko, Oleg Rombakh

Em Thu, Oct 08, 2020 at 04:39:57PM -0700, Andrii Nakryiko escreveu:
> From: Andrii Nakryiko <andriin@fb.com>
> 
> Right now to encode per-CPU variables in BTF, pahole iterates complete vmlinux
> symbol table for each CU. There are 2500 CUs for a typical kernel image.
> Overall, to encode 287 per-CPU variables pahole spends more than 10% of its CPU
> budget, this is incredibly wasteful.

You forgot to add Hao to the Cc list, Hao, can you take a look? I'm
tentatively applying it to my local branch, but would like to hear from
you since you wrote this code.

- Arnaldo

> 
> This patch revamps how this is done. Now it pre-processes symbol table once
> before any of per-CU processing starts. It remembers each per-CPU variable
> symbol, including its address, size, and name. Then during processing each CU,
> binary search is used to correlate DWARF variable with per-CPU symbols and
> figure out if variable belongs to per-CPU data section. If the match is found,
> BTF_KIND_VAR is emitted and var_secinfo is recorded, just like before. At the
> very end, after all CUs are processed, BTF_KIND_DATASEC is emitted with sorted
> variables.
> 
> This change makes per-CPU variables generation overhead pretty negligible and
> returns back about 10% of CPU usage.
> 
> Performance counter stats for './pahole -J /home/andriin/linux-build/default/vmlinux':
> 
> BEFORE:
>       19.160149000 seconds user
>        1.304873000 seconds sys
> 
>          24,114.05 msec task-clock                #    0.999 CPUs utilized
>                 83      context-switches          #    0.003 K/sec
>                  0      cpu-migrations            #    0.000 K/sec
>            622,417      page-faults               #    0.026 M/sec
>     72,897,315,125      cycles                    #    3.023 GHz                      (25.02%)
>    127,807,316,959      instructions              #    1.75  insn per cycle           (25.01%)
>     29,087,179,117      branches                  # 1206.234 M/sec                    (25.01%)
>        464,105,921      branch-misses             #    1.60% of all branches          (25.01%)
>     30,252,119,368      L1-dcache-loads           # 1254.543 M/sec                    (25.01%)
>      1,156,336,207      L1-dcache-load-misses     #    3.82% of all L1-dcache hits    (25.05%)
>        343,373,503      LLC-loads                 #   14.240 M/sec                    (25.02%)
>         12,044,977      LLC-load-misses           #    3.51% of all LL-cache hits     (25.01%)
> 
>       24.136198321 seconds time elapsed
> 
>       22.729693000 seconds user
>        1.384859000 seconds sys
> 
> AFTER:
>       16.781455000 seconds user
>        1.343956000 seconds sys
> 
>          23,398.77 msec task-clock                #    1.000 CPUs utilized
>                 86      context-switches          #    0.004 K/sec
>                  0      cpu-migrations            #    0.000 K/sec
>            622,420      page-faults               #    0.027 M/sec
>     68,395,641,468      cycles                    #    2.923 GHz                      (25.05%)
>    114,241,327,034      instructions              #    1.67  insn per cycle           (25.01%)
>     26,330,711,718      branches                  # 1125.303 M/sec                    (25.01%)
>        465,926,869      branch-misses             #    1.77% of all branches          (25.00%)
>     24,662,984,772      L1-dcache-loads           # 1054.029 M/sec                    (25.00%)
>      1,054,052,064      L1-dcache-load-misses     #    4.27% of all L1-dcache hits    (25.00%)
>        340,970,622      LLC-loads                 #   14.572 M/sec                    (25.00%)
>         16,032,297      LLC-load-misses           #    4.70% of all LL-cache hits     (25.03%)
> 
>       23.402259654 seconds time elapsed
> 
>       21.916437000 seconds user
>        1.482826000 seconds sys
> 
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---
>  btf_encoder.c | 248 +++++++++++++++++++++++++++++---------------------
>  libbtf.c      |   6 +-
>  libbtf.h      |   1 +
>  3 files changed, 148 insertions(+), 107 deletions(-)
> 
> diff --git a/btf_encoder.c b/btf_encoder.c
> index 2e5df03e040f..2a6455be4c52 100644
> --- a/btf_encoder.c
> +++ b/btf_encoder.c
> @@ -17,6 +17,7 @@
>  #include "btf_encoder.h"
>  
>  #include <ctype.h> /* for isalpha() and isalnum() */
> +#include <stdlib.h> /* for qsort() and bsearch() */
>  #include <inttypes.h>
>  
>  /*
> @@ -53,18 +54,18 @@ static bool btf_name_valid(const char *p)
>  	return !*p;
>  }
>  
> -static void dump_invalid_symbol(const char *msg, const char *sym, const char *cu,
> +static void dump_invalid_symbol(const char *msg, const char *sym,
>  				int verbose, bool force)
>  {
>  	if (force) {
>  		if (verbose)
> -			fprintf(stderr, "PAHOLE: Warning: %s, ignored (sym: '%s', cu: '%s').\n",
> -				msg, sym, cu);
> +			fprintf(stderr, "PAHOLE: Warning: %s, ignored (sym: '%s').\n",
> +				msg, sym);
>  		return;
>  	}
>  
> -	fprintf(stderr, "PAHOLE: Error: %s (sym: '%s', cu: '%s').\n", msg, sym, cu);
> -	fprintf(stderr, "PAHOLE: Error: Use '-j' or '--force' to ignore such symbols and force emit the btf.\n");
> +	fprintf(stderr, "PAHOLE: Error: %s (sym: '%s').\n", msg, sym);
> +	fprintf(stderr, "PAHOLE: Error: Use '--btf_encode_force' to ignore such symbols and force emit the btf.\n");
>  }
>  
>  extern struct debug_fmt_ops *dwarves__active_loader;
> @@ -202,6 +203,9 @@ int btf_encoder__encode()
>  {
>  	int err;
>  
> +	if (gobuffer__size(&btfe->percpu_secinfo) != 0)
> +		btf_elf__add_datasec_type(btfe, PERCPU_SECTION, &btfe->percpu_secinfo);
> +
>  	err = btf_elf__encode(btfe, 0);
>  	btf_elf__delete(btfe);
>  	btfe = NULL;
> @@ -209,24 +213,117 @@ int btf_encoder__encode()
>  	return err;
>  }
>  
> -#define HASHADDR__BITS 8
> -#define HASHADDR__SIZE (1UL << HASHADDR__BITS)
> -#define hashaddr__fn(key) hash_64(key, HASHADDR__BITS)
> +#define MAX_PERCPU_VAR_CNT 4096
> +
> +struct var_info {
> +	uint64_t addr;
> +	uint32_t sz;
> +	const char *name;
> +};
> +
> +static struct var_info percpu_vars[MAX_PERCPU_VAR_CNT];
> +static int percpu_var_cnt;
> +
> +static int percpu_var_cmp(const void *_a, const void *_b)
> +{
> +	const struct var_info *a = _a;
> +	const struct var_info *b = _b;
> +
> +	if (a->addr == b->addr)
> +		return 0;
> +	return a->addr < b->addr ? -1 : 1;
> +}
> +
> +static bool percpu_var_exists(uint64_t addr, uint32_t *sz, const char **name)
> +{
> +	const struct var_info *p;
> +	struct var_info key = { .addr = addr };
> +
> +	p = bsearch(&key, percpu_vars, percpu_var_cnt,
> +		    sizeof(percpu_vars[0]), percpu_var_cmp);
> +
> +	if (!p)
> +		return false;
> +
> +	*sz = p->sz;
> +	*name = p->name;
> +	return true;
> +}
>  
> -static struct variable *hashaddr__find_variable(const struct hlist_head hashtable[],
> -						const uint64_t addr)
> +static int find_all_percpu_vars(struct btf_elf *btfe)
>  {
> -	struct variable *variable;
> -	struct hlist_node *pos;
> -	uint16_t bucket = hashaddr__fn(addr);
> -	const struct hlist_head *head = &hashtable[bucket];
> -
> -	hlist_for_each_entry(variable, pos, head, tool_hnode) {
> -		if (variable->ip.addr == addr)
> -			return variable;
> +	uint32_t core_id;
> +	GElf_Sym sym;
> +
> +	/* cache variables' addresses, preparing for searching in symtab. */
> +	percpu_var_cnt = 0;
> +
> +	/* search within symtab for percpu variables */
> +	elf_symtab__for_each_symbol(btfe->symtab, core_id, sym) {
> +		const char *sym_name;
> +		uint64_t addr;
> +		uint32_t size;
> +
> +		/* compare a symbol's shndx to determine if it's a percpu variable */
> +		if (elf_sym__section(&sym) != btfe->percpu_shndx)
> +			continue;
> +		if (elf_sym__type(&sym) != STT_OBJECT)
> +			continue;
> +
> +		addr = elf_sym__value(&sym);
> +		/*
> +		 * Store only those symbols that have allocated space in the percpu section.
> +		 * This excludes the following three types of symbols:
> +		 *
> +		 *  1. __ADDRESSABLE(sym), which are forcely emitted as symbols.
> +		 *  2. __UNIQUE_ID(prefix), which are introduced to generate unique ids.
> +		 *  3. __exitcall(fn), functions which are labeled as exit calls.
> +		 *
> +		 * In addition, the variables defined using DEFINE_PERCPU_FIRST are
> +		 * also not included, which currently includes:
> +		 *
> +		 *  1. fixed_percpu_data
> +		 */
> +		if (!addr)
> +			continue;
> +
> +		sym_name = elf_sym__name(&sym, btfe->symtab);
> +		if (!btf_name_valid(sym_name)) {
> +			dump_invalid_symbol("Found symbol of invalid name when encoding btf",
> +					    sym_name, btf_elf__verbose, btf_elf__force);
> +			if (btf_elf__force)
> +				continue;
> +			return -1;
> +		}
> +		size = elf_sym__size(&sym);
> +		if (!size) {
> +			dump_invalid_symbol("Found symbol of zero size when encoding btf",
> +					    sym_name, btf_elf__verbose, btf_elf__force);
> +			if (btf_elf__force)
> +				continue;
> +			return -1;
> +		}
> +
> +		if (btf_elf__verbose)
> +			printf("Found per-CPU symbol '%s' at address 0x%lx\n", sym_name, addr);
> +
> +		if (percpu_var_cnt == MAX_PERCPU_VAR_CNT) {
> +			fprintf(stderr, "Reached the limit of per-CPU variables: %d\n",
> +				MAX_PERCPU_VAR_CNT);
> +			return -1;
> +		}
> +		percpu_vars[percpu_var_cnt].addr = addr;
> +		percpu_vars[percpu_var_cnt].sz = size;
> +		percpu_vars[percpu_var_cnt].name = sym_name;
> +		percpu_var_cnt++;
>  	}
>  
> -	return NULL;
> +	if (percpu_var_cnt)
> +		qsort(percpu_vars, percpu_var_cnt, sizeof(percpu_vars[0]), percpu_var_cmp);
> +
> +	if (btf_elf__verbose)
> +		printf("Found %d per-CPU variables!\n", percpu_var_cnt);
> +	return 0;
>  }
>  
>  int cu__encode_btf(struct cu *cu, int verbose, bool force,
> @@ -234,13 +331,10 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force,
>  {
>  	uint32_t type_id_off;
>  	uint32_t core_id;
> +	struct variable *var;
>  	struct function *fn;
>  	struct tag *pos;
>  	int err = 0;
> -	struct hlist_head hash_addr[HASHADDR__SIZE];
> -	struct variable *var;
> -	bool has_global_var = false;
> -	GElf_Sym sym;
>  
>  	if (btfe && strcmp(btfe->filename, cu->filename)) {
>  		err = btf_encoder__encode();
> @@ -257,6 +351,9 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force,
>  		if (!btfe)
>  			return -1;
>  
> +		if (!skip_encoding_vars && find_all_percpu_vars(btfe))
> +			goto out;
> +
>  		has_index_type = false;
>  		need_index_type = false;
>  		array_index_id = 0;
> @@ -278,6 +375,7 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force,
>  	}
>  
>  	btf_elf__verbose = verbose;
> +	btf_elf__force = force;
>  	type_id_off = btf__get_nr_types(btfe->btf);
>  
>  	cu__for_each_type(cu, core_id, pos) {
> @@ -325,12 +423,11 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force,
>  	if (verbose)
>  		printf("search cu '%s' for percpu global variables.\n", cu->name);
>  
> -	/* cache variables' addresses, preparing for searching in symtab. */
> -	for (core_id = 0; core_id < HASHADDR__SIZE; ++core_id)
> -		INIT_HLIST_HEAD(&hash_addr[core_id]);
> -
>  	cu__for_each_variable(cu, core_id, pos) {
> -		struct hlist_head *head;
> +		uint32_t size, type, linkage, offset;
> +		const char *name;
> +		uint64_t addr;
> +		int id;
>  
>  		var = tag__variable(pos);
>  		if (var->declaration && !var->spec)
> @@ -338,89 +435,37 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force,
>  		/* percpu variables are allocated in global space */
>  		if (variable__scope(var) != VSCOPE_GLOBAL && !var->spec)
>  			continue;
> -		has_global_var = true;
> -		head = &hash_addr[hashaddr__fn(var->ip.addr)];
> -		hlist_add_head(&var->tool_hnode, head);
> -	}
> -	if (!has_global_var) {
> -		if (verbose)
> -			printf("cu has no global variable defined, skip.\n");
> -		goto out;
> -	}
> -
> -	/* search within symtab for percpu variables */
> -	elf_symtab__for_each_symbol(btfe->symtab, core_id, sym) {
> -		uint32_t linkage, type, size, offset;
> -		int32_t btf_var_id, btf_var_secinfo_id;
> -		uint64_t addr;
> -		const char *sym_name;
> -
> -		/* compare a symbol's shndx to determine if it's a percpu variable */
> -		if (elf_sym__section(&sym) != btfe->percpu_shndx)
> -			continue;
> -		if (elf_sym__type(&sym) != STT_OBJECT)
> -			continue;
>  
> -		addr = elf_sym__value(&sym);
> -		/*
> -		 * Store only those symbols that have allocated space in the percpu section.
> -		 * This excludes the following three types of symbols:
> -		 *
> -		 *  1. __ADDRESSABLE(sym), which are forcely emitted as symbols.
> -		 *  2. __UNIQUE_ID(prefix), which are introduced to generate unique ids.
> -		 *  3. __exitcall(fn), functions which are labeled as exit calls.
> -		 *
> -		 * In addition, the variables defined using DEFINE_PERCPU_FIRST are
> -		 * also not included, which currently includes:
> -		 *
> -		 *  1. fixed_percpu_data
> -		 */
> -		if (!addr)
> -			continue;
> -		var = hashaddr__find_variable(hash_addr, addr);
> -		if (var == NULL)
> -			continue;
> +		/* addr has to be recorded before we follow spec */
> +		addr = var->ip.addr;
>  		if (var->spec)
>  			var = var->spec;
>  
> -		sym_name = elf_sym__name(&sym, btfe->symtab);
> -		if (!btf_name_valid(sym_name)) {
> -			dump_invalid_symbol("Found symbol of invalid name when encoding btf",
> -					    sym_name, cu->name, verbose, force);
> -			if (force)
> -				continue;
> -			err = -1;
> -			break;
> -		}
>  		if (var->ip.tag.type == 0) {
> -			dump_invalid_symbol("Found symbol of void type when encoding btf",
> -					    sym_name, cu->name, verbose, force);
> -			if (force)
> -				continue;
> -			err = -1;
> -			break;
> -		}
> -		type = type_id_off + var->ip.tag.type;
> -		size = elf_sym__size(&sym);
> -		if (!size) {
> -			dump_invalid_symbol("Found symbol of zero size when encoding btf",
> -					    sym_name, cu->name, verbose, force);
> +			fprintf(stderr, "error: found variable in CU '%s' that has void type\n",
> +				cu->name);
>  			if (force)
>  				continue;
>  			err = -1;
>  			break;
>  		}
>  
> -		if (verbose)
> -			printf("symbol '%s' of address 0x%lx encoded\n",
> -			       sym_name, addr);
> +		type = var->ip.tag.type + type_id_off;
> +		linkage = var->external ? BTF_VAR_GLOBAL_ALLOCATED : BTF_VAR_STATIC;
> +		if (!percpu_var_exists(addr, &size, &name))
> +			continue; /* not a per-CPU variable */
> +
> +		if (btf_elf__verbose) {
> +			printf("Variable '%s' from CU '%s' at address 0x%lx encoded\n",
> +			       name, cu->name, addr);
> +		}
>  
>  		/* add a BTF_KIND_VAR in btfe->types */
> -		linkage = var->external ? BTF_VAR_GLOBAL_ALLOCATED : BTF_VAR_STATIC;
> -		btf_var_id = btf_elf__add_var_type(btfe, type, sym_name, linkage);
> -		if (btf_var_id < 0) {
> +		id = btf_elf__add_var_type(btfe, type, name, linkage);
> +		if (id < 0) {
>  			err = -1;
> -			printf("error: failed to encode variable '%s'\n", sym_name);
> +			fprintf(stderr, "error: failed to encode variable '%s' at addr 0x%lx\n",
> +			        name, addr);
>  			break;
>  		}
>  
> @@ -428,13 +473,12 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force,
>  		 * add a BTF_VAR_SECINFO in btfe->percpu_secinfo, which will be added into
>  		 * btfe->types later when we add BTF_VAR_DATASEC.
>  		 */
> -		type = btf_var_id;
>  		offset = addr - btfe->percpu_base_addr;
> -		btf_var_secinfo_id = btf_elf__add_var_secinfo(&btfe->percpu_secinfo,
> -							      type, offset, size);
> -		if (btf_var_secinfo_id < 0) {
> +		id = btf_elf__add_var_secinfo(&btfe->percpu_secinfo, id, offset, size);
> +		if (id < 0) {
>  			err = -1;
> -			printf("error: failed to encode var secinfo '%s'\n", sym_name);
> +			fprintf(stderr, "error: failed to encode section info for variable '%s' at addr 0x%lx\n",
> +			        name, addr);
>  			break;
>  		}
>  	}
> diff --git a/libbtf.c b/libbtf.c
> index 0467f1f2a596..27aa3e5a986e 100644
> --- a/libbtf.c
> +++ b/libbtf.c
> @@ -28,6 +28,7 @@
>  #include "elf_symtab.h"
>  
>  uint8_t btf_elf__verbose;
> +uint8_t btf_elf__force;
>  
>  static int btf_var_secinfo_cmp(const void *a, const void *b)
>  {
> @@ -62,7 +63,6 @@ int btf_elf__load(struct btf_elf *btfe)
>  	return 0;
>  }
>  
> -
>  struct btf_elf *btf_elf__new(const char *filename, Elf *elf)
>  {
>  	struct btf_elf *btfe = zalloc(sizeof(*btfe));
> @@ -771,10 +771,6 @@ int btf_elf__encode(struct btf_elf *btfe, uint8_t flags)
>  {
>  	struct btf *btf = btfe->btf;
>  
> -	if (gobuffer__size(&btfe->percpu_secinfo) != 0)
> -		btf_elf__add_datasec_type(btfe, PERCPU_SECTION,
> -					  &btfe->percpu_secinfo);
> -
>  	/* Empty file, nothing to do, so... done! */
>  	if (btf__get_nr_types(btf) == 0)
>  		return 0;
> diff --git a/libbtf.h b/libbtf.h
> index 9b3d396da31f..887b5bc55c8e 100644
> --- a/libbtf.h
> +++ b/libbtf.h
> @@ -30,6 +30,7 @@ struct btf_elf {
>  };
>  
>  extern uint8_t btf_elf__verbose;
> +extern uint8_t btf_elf__force;
>  #define btf_elf__verbose_log(fmt, ...) { if (btf_elf__verbose) printf(fmt, __VA_ARGS__); }
>  
>  #define PERCPU_SECTION ".data..percpu"
> -- 
> 2.24.1
> 

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 dwarves 0/8] Switch BTF loading and encoding to libbpf APIs
  2020-10-08 23:39 [PATCH v2 dwarves 0/8] Switch BTF loading and encoding to libbpf APIs Andrii Nakryiko
                   ` (7 preceding siblings ...)
  2020-10-08 23:40 ` [PATCH v2 dwarves 8/8] btf_encoder: support cross-compiled ELF binaries with different endianness Andrii Nakryiko
@ 2020-10-09 16:22 ` Arnaldo Carvalho de Melo
  2020-10-09 17:59   ` Andrii Nakryiko
  8 siblings, 1 reply; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-10-09 16:22 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Andrii Nakryiko, Hao Luo, Oleg Rombakh, dwarves, bpf,
	kernel-team, ast, Tony Ambardar, Ilya Leoshkevich,
	David Marcinkovic, Luka Perkov, Borna Cafuk, Juraj Vijtiuk

Em Thu, Oct 08, 2020 at 04:39:52PM -0700, Andrii Nakryiko escreveu:
> This patch set switches pahole to use libbpf-provided BTF loading and encoding
> APIs. This reduces pahole's own BTF encoding code, speeds up the process,
> reduces amount of RAM needed for DWARF-to-BTF conversion. Also, pahole finally
> gets support to generating BTF for cross-compiled ELF binaries with different
> endianness (patch #8).
> 
> Additionally, patch #3 fixes previously missed problem with invalid array
> index type generation.
> 
> Patches #4-7 are speeding up DWARF-to-BTF convertion/dedup pretty
> significantly, saving overall about 9 seconds out of current 27 or so.
> 
> Patch #5 revamps how per-CPU BTF variables are emitted, eliminating repeated
> and expensive looping over ELF symbols table. The critical detail that took
> few hours of investigation is that when DW_AT_variable has
> DW_AT_specification, variable address (to correlate with symbol's address) has
> to be taken before specification is followed.
> 
> More details could be found in respective patches.
> 
> v1->v2:
>   - rebase on latest dwarves master and fix var->spec's address problem.

Thanks, I applied all of them, tested and reproduced the performance
gains, great work!

I'll do some more testing on encoding a vmlinux for some big endian arch
on my x86_64 workstation and then push things publicly.

If Hao find any issues we can fix in a follow up patch.

I also added the people involved in the discussion about cross builds
failing, please take a look, I'm pushing now to a tmp.libbtf_encoder so
that you can test it from there, ok?

- Arnaldo
 
> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> 
> Andrii Nakryiko (8):
>   btf_loader: use libbpf to load BTF
>   btf_encoder: use libbpf APIs to encode BTF type info
>   btf_encoder: fix emitting __ARRAY_SIZE_TYPE__ as index range type
>   btf_encoder: discard CUs after BTF encoding
>   btf_encoder: revamp how per-CPU variables are encoded
>   dwarf_loader: increase the size of lookup hash map
>   strings: use BTF's string APIs for strings management
>   btf_encoder: support cross-compiled ELF binaries with different
>     endianness
> 
>  btf_encoder.c  | 370 +++++++++++++++------------
>  btf_loader.c   | 244 +++++++-----------
>  ctf_encoder.c  |   2 +-
>  dwarf_loader.c |   2 +-
>  libbtf.c       | 661 +++++++++++++++++++++----------------------------
>  libbtf.h       |  41 ++-
>  libctf.c       |  14 +-
>  libctf.h       |   4 +-
>  pahole.c       |   2 +-
>  strings.c      |  91 +++----
>  strings.h      |  32 +--
>  11 files changed, 645 insertions(+), 818 deletions(-)
> 
> -- 
> 2.24.1
> 

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 dwarves 1/8] btf_loader: use libbpf to load BTF
  2020-10-09 14:53   ` Arnaldo Carvalho de Melo
@ 2020-10-09 17:49     ` Andrii Nakryiko
  0 siblings, 0 replies; 15+ messages in thread
From: Andrii Nakryiko @ 2020-10-09 17:49 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Andrii Nakryiko, dwarves, bpf, Kernel Team, Alexei Starovoitov,
	Andrii Nakryiko

On Fri, Oct 9, 2020 at 7:53 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> Em Thu, Oct 08, 2020 at 04:39:53PM -0700, Andrii Nakryiko escreveu:
> > From: Andrii Nakryiko <andriin@fb.com>
> >
> > Switch BTF loading to completely use libbpf's own struct btf and related APIs.
> > BTF encoding is still happening with pahole's own code, so these two code
> > paths are not sharing anything now. String fetching is happening based on
> > whether btfe->strings were set to non-NULL pointer by btf_encoder.
>
> While testing this one I noticed a problem, but it isn't caused by this
> particular patch:
>
> [acme@five pahole]$ cp ~/git/build/v5.9-rc6+/net/ipv4/tcp_ipv4.o .
> [acme@five pahole]$ readelf -SW tcp_ipv4.o | grep BTF
> [acme@five pahole]$ pahole -J tcp_ipv4.o
> [acme@five pahole]$ readelf -SW tcp_ipv4.o | grep BTF
>   [105] .BTF              PROGBITS        0000000000000000 0fbb3c 03f697 00      0   0  1
> [acme@five pahole]$ ./btfdiff tcp_ipv4.o
> --- /tmp/btfdiff.dwarf.BDAvGi   2020-10-09 11:41:45.161509391 -0300
> +++ /tmp/btfdiff.btf.p81icw     2020-10-09 11:41:45.177509720 -0300
> @@ -4056,7 +4056,7 @@ struct tcp_congestion_ops {
>         u32                        (*min_tso_segs)(struct sock *); /*    96     8 */
>         u32                        (*sndbuf_expand)(struct sock *); /*   104     8 */
>         void                       (*cong_control)(struct sock *, const struct rate_sample  *); /*   112     8 */
> -       size_t                     (*get_info)(struct sock *, u32, int *, union tcp_cc_info *); /*   120     8 */
> +       size_t                     (*get_info)(struct sock *, u32, int *, struct tcp_cc_info *); /*   120     8 */

It's a bug in btf_loader.c. When loading BTF_KIND_FWD, we always
assume it's struct forward reference. But I checked, generated BTF
does have the valid info (kind_flag), so it should be easy to fix,
I'll send a fix.

The reason we don't see it in vmlinux is because after dedup that
forward reference is resolved to a full type, which is clearly union.


[...]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 dwarves 0/8] Switch BTF loading and encoding to libbpf APIs
  2020-10-09 16:22 ` [PATCH v2 dwarves 0/8] Switch BTF loading and encoding to libbpf APIs Arnaldo Carvalho de Melo
@ 2020-10-09 17:59   ` Andrii Nakryiko
  0 siblings, 0 replies; 15+ messages in thread
From: Andrii Nakryiko @ 2020-10-09 17:59 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Andrii Nakryiko, Hao Luo, Oleg Rombakh, dwarves, bpf,
	Kernel Team, Alexei Starovoitov, Tony Ambardar, Ilya Leoshkevich,
	David Marcinkovic, Luka Perkov, Borna Cafuk, Juraj Vijtiuk

On Fri, Oct 9, 2020 at 9:22 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> Em Thu, Oct 08, 2020 at 04:39:52PM -0700, Andrii Nakryiko escreveu:
> > This patch set switches pahole to use libbpf-provided BTF loading and encoding
> > APIs. This reduces pahole's own BTF encoding code, speeds up the process,
> > reduces amount of RAM needed for DWARF-to-BTF conversion. Also, pahole finally
> > gets support to generating BTF for cross-compiled ELF binaries with different
> > endianness (patch #8).
> >
> > Additionally, patch #3 fixes previously missed problem with invalid array
> > index type generation.
> >
> > Patches #4-7 are speeding up DWARF-to-BTF convertion/dedup pretty
> > significantly, saving overall about 9 seconds out of current 27 or so.
> >
> > Patch #5 revamps how per-CPU BTF variables are emitted, eliminating repeated
> > and expensive looping over ELF symbols table. The critical detail that took
> > few hours of investigation is that when DW_AT_variable has
> > DW_AT_specification, variable address (to correlate with symbol's address) has
> > to be taken before specification is followed.
> >
> > More details could be found in respective patches.
> >
> > v1->v2:
> >   - rebase on latest dwarves master and fix var->spec's address problem.
>
> Thanks, I applied all of them, tested and reproduced the performance
> gains, great work!

Great, thanks a lot, Arnaldo!

Next step is adding BTF to kernel modules, where module's BTF will be
an "extension" of vmlinux's BTF, with only a minimal set of new types
used/added in the module, that are not available in vmlinux. This
should make per-module BTF really tiny.

>
> I'll do some more testing on encoding a vmlinux for some big endian arch
> on my x86_64 workstation and then push things publicly.
>
> If Hao find any issues we can fix in a follow up patch.
>
> I also added the people involved in the discussion about cross builds
> failing, please take a look, I'm pushing now to a tmp.libbtf_encoder so
> that you can test it from there, ok?
>
> - Arnaldo
>
> > Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> >
> > Andrii Nakryiko (8):
> >   btf_loader: use libbpf to load BTF
> >   btf_encoder: use libbpf APIs to encode BTF type info
> >   btf_encoder: fix emitting __ARRAY_SIZE_TYPE__ as index range type
> >   btf_encoder: discard CUs after BTF encoding
> >   btf_encoder: revamp how per-CPU variables are encoded
> >   dwarf_loader: increase the size of lookup hash map
> >   strings: use BTF's string APIs for strings management
> >   btf_encoder: support cross-compiled ELF binaries with different
> >     endianness
> >
> >  btf_encoder.c  | 370 +++++++++++++++------------
> >  btf_loader.c   | 244 +++++++-----------
> >  ctf_encoder.c  |   2 +-
> >  dwarf_loader.c |   2 +-
> >  libbtf.c       | 661 +++++++++++++++++++++----------------------------
> >  libbtf.h       |  41 ++-
> >  libctf.c       |  14 +-
> >  libctf.h       |   4 +-
> >  pahole.c       |   2 +-
> >  strings.c      |  91 +++----
> >  strings.h      |  32 +--
> >  11 files changed, 645 insertions(+), 818 deletions(-)
> >
> > --
> > 2.24.1
> >
>
> --
>
> - Arnaldo

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2020-10-09 17:59 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-08 23:39 [PATCH v2 dwarves 0/8] Switch BTF loading and encoding to libbpf APIs Andrii Nakryiko
2020-10-08 23:39 ` [PATCH v2 dwarves 1/8] btf_loader: use libbpf to load BTF Andrii Nakryiko
2020-10-09 14:53   ` Arnaldo Carvalho de Melo
2020-10-09 17:49     ` Andrii Nakryiko
2020-10-08 23:39 ` [PATCH v2 dwarves 2/8] btf_encoder: use libbpf APIs to encode BTF type info Andrii Nakryiko
2020-10-08 23:39 ` [PATCH v2 dwarves 3/8] btf_encoder: fix emitting __ARRAY_SIZE_TYPE__ as index range type Andrii Nakryiko
2020-10-08 23:39 ` [PATCH v2 dwarves 4/8] btf_encoder: discard CUs after BTF encoding Andrii Nakryiko
2020-10-09 15:47   ` Arnaldo Carvalho de Melo
2020-10-08 23:39 ` [PATCH v2 dwarves 5/8] btf_encoder: revamp how per-CPU variables are encoded Andrii Nakryiko
2020-10-09 15:53   ` Arnaldo Carvalho de Melo
2020-10-08 23:39 ` [PATCH v2 dwarves 6/8] dwarf_loader: increase the size of lookup hash map Andrii Nakryiko
2020-10-08 23:39 ` [PATCH v2 dwarves 7/8] strings: use BTF's string APIs for strings management Andrii Nakryiko
2020-10-08 23:40 ` [PATCH v2 dwarves 8/8] btf_encoder: support cross-compiled ELF binaries with different endianness Andrii Nakryiko
2020-10-09 16:22 ` [PATCH v2 dwarves 0/8] Switch BTF loading and encoding to libbpf APIs Arnaldo Carvalho de Melo
2020-10-09 17:59   ` Andrii Nakryiko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).