All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH dwarves v6 0/3] pahole: Inject kfunc decl tags into BTF
@ 2024-03-25 17:53 Daniel Xu
  2024-03-25 17:53 ` [PATCH dwarves v6 1/3] gobuffer: Add gobuffer__sort() helper Daniel Xu
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Daniel Xu @ 2024-03-25 17:53 UTC (permalink / raw
  To: acme, jolsa, quentin, alan.maguire, eddyz87
  Cc: andrii.nakryiko, ast, daniel, bpf

This patchset teaches pahole to parse symbols in .BTF_ids section in
vmlinux and discover exported kfuncs. Pahole then takes the list of
kfuncs and injects a BTF_KIND_DECL_TAG for each kfunc.

Example of encoding:

        $ bpftool btf dump file .tmp_vmlinux.btf | rg "DECL_TAG 'bpf_kfunc'" | wc -l
        121

        $ bpftool btf dump file .tmp_vmlinux.btf | rg 56337
        [56337] FUNC 'bpf_ct_change_timeout' type_id=56336 linkage=static
        [127861] DECL_TAG 'bpf_kfunc' type_id=56337 component_idx=-1

This enables downstream users and tools to dynamically discover which
kfuncs are available on a system by parsing vmlinux or module BTF, both
available in /sys/kernel/btf.

This feature is enabled with --btf_features=decl_tag,decl_tag_kfuncs.

=== Changelog ===

Changes from v5:
* Add gobuffer__sort() helper
* Use strstarts() instead of strncmp()
* Use uint64_t instead of size_t
* Add clarifying comments for get_func_name()

Changes from v4:
* Update man page with decl_tag_kfuncs feature
* Fix release mode build warnings
* Add elf_getshrstrndx() error checking
* Disable tagging if decl_tag feature is off
* Fix malformed func name handling

Changes from v3:
* Guard kfunc tagging behind feature flag
* Use struct btf_id_set8 definition
* Remove unnecessary member from btf_encoder
* Fix code styling

Changes from v2:
* More reliably detect kfunc membership in set8 by tracking set addr ranges
* Rename some variables/functions to be more clear about kfunc vs func

Changes from v1:
* Fix resource leaks
* Fix callee -> caller typo
* Rename btf_decl_tag from kfunc -> bpf_kfunc
* Only grab btf_id_set funcs tagged kfunc
* Presort btf func list

Daniel Xu (3):
  gobuffer: Add gobuffer__sort() helper
  pahole: Add --btf_feature=decl_tag_kfuncs feature
  pahole: Inject kfunc decl tags into BTF

 btf_encoder.c      | 374 +++++++++++++++++++++++++++++++++++++++++++++
 dwarves.h          |   1 +
 gobuffer.c         |   5 +
 gobuffer.h         |   2 +
 man-pages/pahole.1 |   1 +
 pahole.c           |   1 +
 6 files changed, 384 insertions(+)

-- 
2.44.0


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

* [PATCH dwarves v6 1/3] gobuffer: Add gobuffer__sort() helper
  2024-03-25 17:53 [PATCH dwarves v6 0/3] pahole: Inject kfunc decl tags into BTF Daniel Xu
@ 2024-03-25 17:53 ` Daniel Xu
  2024-04-17  9:20   ` Alan Maguire
  2024-03-25 17:53 ` [PATCH dwarves v6 2/3] pahole: Add --btf_feature=decl_tag_kfuncs feature Daniel Xu
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Daniel Xu @ 2024-03-25 17:53 UTC (permalink / raw
  To: acme, jolsa, quentin, alan.maguire, eddyz87
  Cc: andrii.nakryiko, ast, daniel, bpf

Add a helper to sort the gobuffer. Trivial wrapper around qsort().

Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>
---
 gobuffer.c | 5 +++++
 gobuffer.h | 2 ++
 2 files changed, 7 insertions(+)

diff --git a/gobuffer.c b/gobuffer.c
index 02b2084..4655339 100644
--- a/gobuffer.c
+++ b/gobuffer.c
@@ -102,6 +102,11 @@ void gobuffer__copy(const struct gobuffer *gb, void *dest)
 	}
 }
 
+void gobuffer__sort(struct gobuffer *gb, unsigned int size, int (*compar)(const void *, const void *))
+{
+	qsort((void *)gb->entries, gb->nr_entries, size, compar);
+}
+
 const void *gobuffer__compress(struct gobuffer *gb, unsigned int *size)
 {
 	z_stream z = {
diff --git a/gobuffer.h b/gobuffer.h
index a12c5c8..cd218b6 100644
--- a/gobuffer.h
+++ b/gobuffer.h
@@ -21,6 +21,8 @@ void __gobuffer__delete(struct gobuffer *gb);
 
 void gobuffer__copy(const struct gobuffer *gb, void *dest);
 
+void gobuffer__sort(struct gobuffer *gb, unsigned int size, int (*compar)(const void *, const void *));
+
 int gobuffer__add(struct gobuffer *gb, const void *s, unsigned int len);
 int gobuffer__allocate(struct gobuffer *gb, unsigned int len);
 
-- 
2.44.0


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

* [PATCH dwarves v6 2/3] pahole: Add --btf_feature=decl_tag_kfuncs feature
  2024-03-25 17:53 [PATCH dwarves v6 0/3] pahole: Inject kfunc decl tags into BTF Daniel Xu
  2024-03-25 17:53 ` [PATCH dwarves v6 1/3] gobuffer: Add gobuffer__sort() helper Daniel Xu
@ 2024-03-25 17:53 ` Daniel Xu
  2024-03-25 17:53 ` [PATCH dwarves v6 3/3] pahole: Inject kfunc decl tags into BTF Daniel Xu
  2024-04-02 20:24 ` [PATCH dwarves v6 0/3] " Arnaldo Carvalho de Melo
  3 siblings, 0 replies; 9+ messages in thread
From: Daniel Xu @ 2024-03-25 17:53 UTC (permalink / raw
  To: acme, jolsa, quentin, alan.maguire, eddyz87
  Cc: andrii.nakryiko, ast, daniel, bpf

Add a feature flag to guard tagging of kfuncs. The next commit will
implement the actual tagging.

Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>
---
 btf_encoder.c      | 2 ++
 dwarves.h          | 1 +
 man-pages/pahole.1 | 1 +
 pahole.c           | 1 +
 4 files changed, 5 insertions(+)

diff --git a/btf_encoder.c b/btf_encoder.c
index e1e3529..850e36f 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -75,6 +75,7 @@ struct btf_encoder {
 			  verbose,
 			  force,
 			  gen_floats,
+			  tag_kfuncs,
 			  is_rel;
 	uint32_t	  array_index_id;
 	struct {
@@ -1659,6 +1660,7 @@ struct btf_encoder *btf_encoder__new(struct cu *cu, const char *detached_filenam
 		encoder->force		 = conf_load->btf_encode_force;
 		encoder->gen_floats	 = conf_load->btf_gen_floats;
 		encoder->skip_encoding_vars = conf_load->skip_encoding_btf_vars;
+		encoder->tag_kfuncs	 = conf_load->btf_decl_tag_kfuncs;
 		encoder->verbose	 = verbose;
 		encoder->has_index_type  = false;
 		encoder->need_index_type = false;
diff --git a/dwarves.h b/dwarves.h
index 2393a6c..2a679bb 100644
--- a/dwarves.h
+++ b/dwarves.h
@@ -87,6 +87,7 @@ struct conf_load {
 	bool			skip_encoding_btf_vars;
 	bool			btf_gen_floats;
 	bool			btf_encode_force;
+	bool			btf_decl_tag_kfuncs;
 	uint8_t			hashtable_bits;
 	uint8_t			max_hashtable_bits;
 	uint16_t		kabi_prefix_len;
diff --git a/man-pages/pahole.1 b/man-pages/pahole.1
index 2be165d..5ea41ff 100644
--- a/man-pages/pahole.1
+++ b/man-pages/pahole.1
@@ -308,6 +308,7 @@ Encode BTF using the specified feature list, or specify 'all' for all features s
 	                   in some CUs and not others, or when the same
 	                   function name has inconsistent BTF descriptions
 	                   in different CUs.
+	decl_tag_kfuncs    Inject a BTF_KIND_DECL_TAG for each discovered kfunc.
 .fi
 
 So for example, specifying \-\-btf_encode=var,enum64 will result in a BTF encoding that (as well as encoding basic BTF information) will contain variables and enum64 values.
diff --git a/pahole.c b/pahole.c
index 0b9c2de..353a080 100644
--- a/pahole.c
+++ b/pahole.c
@@ -1281,6 +1281,7 @@ struct btf_feature {
 	BTF_FEATURE(enum64, skip_encoding_btf_enum64, true),
 	BTF_FEATURE(optimized_func, btf_gen_optimized, false),
 	BTF_FEATURE(consistent_func, skip_encoding_btf_inconsistent_proto, false),
+	BTF_FEATURE(decl_tag_kfuncs, btf_decl_tag_kfuncs, false),
 };
 
 #define BTF_MAX_FEATURE_STR	1024
-- 
2.44.0


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

* [PATCH dwarves v6 3/3] pahole: Inject kfunc decl tags into BTF
  2024-03-25 17:53 [PATCH dwarves v6 0/3] pahole: Inject kfunc decl tags into BTF Daniel Xu
  2024-03-25 17:53 ` [PATCH dwarves v6 1/3] gobuffer: Add gobuffer__sort() helper Daniel Xu
  2024-03-25 17:53 ` [PATCH dwarves v6 2/3] pahole: Add --btf_feature=decl_tag_kfuncs feature Daniel Xu
@ 2024-03-25 17:53 ` Daniel Xu
  2024-04-17  9:32   ` Alan Maguire
  2024-04-02 20:24 ` [PATCH dwarves v6 0/3] " Arnaldo Carvalho de Melo
  3 siblings, 1 reply; 9+ messages in thread
From: Daniel Xu @ 2024-03-25 17:53 UTC (permalink / raw
  To: acme, jolsa, quentin, alan.maguire, eddyz87
  Cc: andrii.nakryiko, ast, daniel, bpf

This commit teaches pahole to parse symbols in .BTF_ids section in
vmlinux and discover exported kfuncs. Pahole then takes the list of
kfuncs and injects a BTF_KIND_DECL_TAG for each kfunc.

Example of encoding:

        $ bpftool btf dump file .tmp_vmlinux.btf | rg "DECL_TAG 'bpf_kfunc'" | wc -l
        121

        $ bpftool btf dump file .tmp_vmlinux.btf | rg 56337
        [56337] FUNC 'bpf_ct_change_timeout' type_id=56336 linkage=static
        [127861] DECL_TAG 'bpf_kfunc' type_id=56337 component_idx=-1

This enables downstream users and tools to dynamically discover which
kfuncs are available on a system by parsing vmlinux or module BTF, both
available in /sys/kernel/btf.

This feature is enabled with --btf_features=decl_tag,decl_tag_kfuncs.

Acked-by: Jiri Olsa <jolsa@kernel.org>
Tested-by: Jiri Olsa <jolsa@kernel.org>
Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
Tested-by: Alan Maguire <alan.maguire@oracle.com>
Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>
---
 btf_encoder.c | 372 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 372 insertions(+)

diff --git a/btf_encoder.c b/btf_encoder.c
index 850e36f..d326404 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -34,6 +34,21 @@
 #include <pthread.h>
 
 #define BTF_ENCODER_MAX_PROTO	512
+#define BTF_IDS_SECTION		".BTF_ids"
+#define BTF_ID_FUNC_PFX		"__BTF_ID__func__"
+#define BTF_ID_SET8_PFX		"__BTF_ID__set8__"
+#define BTF_SET8_KFUNCS		(1 << 0)
+#define BTF_KFUNC_TYPE_TAG	"bpf_kfunc"
+
+/* Adapted from include/linux/btf_ids.h */
+struct btf_id_set8 {
+        uint32_t cnt;
+        uint32_t flags;
+        struct {
+                uint32_t id;
+                uint32_t flags;
+        } pairs[];
+};
 
 /* state used to do later encoding of saved functions */
 struct btf_encoder_state {
@@ -75,6 +90,7 @@ struct btf_encoder {
 			  verbose,
 			  force,
 			  gen_floats,
+			  skip_encoding_decl_tag,
 			  tag_kfuncs,
 			  is_rel;
 	uint32_t	  array_index_id;
@@ -94,6 +110,17 @@ struct btf_encoder {
 	} functions;
 };
 
+struct btf_func {
+	const char *name;
+	int	    type_id;
+};
+
+/* Half open interval representing range of addresses containing kfuncs */
+struct btf_kfunc_set_range {
+	uint64_t start;
+	uint64_t end;
+};
+
 static LIST_HEAD(encoders);
 static pthread_mutex_t encoders__lock = PTHREAD_MUTEX_INITIALIZER;
 
@@ -1363,8 +1390,343 @@ out:
 	return err;
 }
 
+/* Returns if `sym` points to a kfunc set */
+static int is_sym_kfunc_set(GElf_Sym *sym, const char *name, Elf_Data *idlist, size_t idlist_addr)
+{
+	void *ptr = idlist->d_buf;
+	struct btf_id_set8 *set;
+	int off;
+
+	/* kfuncs are only found in BTF_SET8's */
+	if (!strstarts(name, BTF_ID_SET8_PFX))
+		return false;
+
+	off = sym->st_value - idlist_addr;
+	if (off >= idlist->d_size) {
+		fprintf(stderr, "%s: symbol '%s' out of bounds\n", __func__, name);
+		return false;
+	}
+
+	/* Check the set8 flags to see if it was marked as kfunc */
+	set = ptr + off;
+	return set->flags & BTF_SET8_KFUNCS;
+}
+
+/*
+ * Parse BTF_ID symbol and return the func name.
+ *
+ * Returns:
+ *	Caller-owned string containing func name if successful.
+ *	NULL if !func or on error.
+ */
+static char *get_func_name(const char *sym)
+{
+	char *func, *end;
+
+	/* Example input: __BTF_ID__func__vfs_close__1
+	 *
+	 * The goal is to strip the prefix and suffix such that we only
+	 * return vfs_close.
+	 */
+
+	if (!strstarts(sym, BTF_ID_FUNC_PFX))
+		return NULL;
+
+	/* Strip prefix and handle malformed input such as  __BTF_ID__func___ */
+	func = strdup(sym + sizeof(BTF_ID_FUNC_PFX) - 1);
+	if (strlen(func) < 2) {
+                free(func);
+                return NULL;
+        }
+
+	/* Strip suffix */
+	end = strrchr(func, '_');
+	if (!end || *(end - 1) != '_') {
+		free(func);
+		return NULL;
+	}
+	*(end - 1) = '\0';
+
+	return func;
+}
+
+static int btf_func_cmp(const void *_a, const void *_b)
+{
+	const struct btf_func *a = _a;
+	const struct btf_func *b = _b;
+
+	return strcmp(a->name, b->name);
+}
+
+/*
+ * Collects all functions described in BTF.
+ * Returns non-zero on error.
+ */
+static int btf_encoder__collect_btf_funcs(struct btf_encoder *encoder, struct gobuffer *funcs)
+{
+	struct btf *btf = encoder->btf;
+	int nr_types, type_id;
+	int err = -1;
+
+	/* First collect all the func entries into an array */
+	nr_types = btf__type_cnt(btf);
+	for (type_id = 1; type_id < nr_types; type_id++) {
+		const struct btf_type *type;
+		struct btf_func func = {};
+		const char *name;
+
+		type = btf__type_by_id(btf, type_id);
+		if (!type) {
+			fprintf(stderr, "%s: malformed BTF, can't resolve type for ID %d\n",
+				__func__, type_id);
+			err = -EINVAL;
+			goto out;
+		}
+
+		if (!btf_is_func(type))
+			continue;
+
+		name = btf__name_by_offset(btf, type->name_off);
+		if (!name) {
+			fprintf(stderr, "%s: malformed BTF, can't resolve name for ID %d\n",
+				__func__, type_id);
+			err = -EINVAL;
+			goto out;
+		}
+
+		func.name = name;
+		func.type_id = type_id;
+		err = gobuffer__add(funcs, &func, sizeof(func));
+		if (err < 0)
+			goto out;
+	}
+
+	/* Now that we've collected funcs, sort them by name */
+	gobuffer__sort(funcs, sizeof(struct btf_func), btf_func_cmp);
+
+	err = 0;
+out:
+	return err;
+}
+
+static int btf_encoder__tag_kfunc(struct btf_encoder *encoder, struct gobuffer *funcs, const char *kfunc)
+{
+	struct btf_func key = { .name = kfunc };
+	struct btf *btf = encoder->btf;
+	struct btf_func *target;
+	const void *base;
+	unsigned int cnt;
+	int err = -1;
+
+	base = gobuffer__entries(funcs);
+	cnt = gobuffer__nr_entries(funcs);
+	target = bsearch(&key, base, cnt, sizeof(key), btf_func_cmp);
+	if (!target) {
+		fprintf(stderr, "%s: failed to find kfunc '%s' in BTF\n", __func__, kfunc);
+		goto out;
+	}
+
+	/* Note we are unconditionally adding the btf_decl_tag even
+	 * though vmlinux may already contain btf_decl_tags for kfuncs.
+	 * We are ok to do this b/c we will later btf__dedup() to remove
+	 * any duplicates.
+	 */
+	err = btf__add_decl_tag(btf, BTF_KFUNC_TYPE_TAG, target->type_id, -1);
+	if (err < 0) {
+		fprintf(stderr, "%s: failed to insert kfunc decl tag for '%s': %d\n",
+			__func__, kfunc, err);
+		goto out;
+	}
+
+	err = 0;
+out:
+	return err;
+}
+
+static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder)
+{
+	const char *filename = encoder->filename;
+	struct gobuffer btf_kfunc_ranges = {};
+	struct gobuffer btf_funcs = {};
+	Elf_Data *symbols = NULL;
+	Elf_Data *idlist = NULL;
+	Elf_Scn *symscn = NULL;
+	int symbols_shndx = -1;
+	size_t idlist_addr = 0;
+	int fd = -1, err = -1;
+	int idlist_shndx = -1;
+	size_t strtabidx = 0;
+	Elf_Scn *scn = NULL;
+	Elf *elf = NULL;
+	GElf_Shdr shdr;
+	size_t strndx;
+	char *secname;
+	int nr_syms;
+	int i = 0;
+
+	fd = open(filename, O_RDONLY);
+	if (fd < 0) {
+		fprintf(stderr, "Cannot open %s\n", filename);
+		goto out;
+	}
+
+	if (elf_version(EV_CURRENT) == EV_NONE) {
+		elf_error("Cannot set libelf version");
+		goto out;
+	}
+
+	elf = elf_begin(fd, ELF_C_READ, NULL);
+	if (elf == NULL) {
+		elf_error("Cannot update ELF file");
+		goto out;
+	}
+
+	/* Locate symbol table and .BTF_ids sections */
+	if (elf_getshdrstrndx(elf, &strndx) < 0)
+		goto out;
+
+	while ((scn = elf_nextscn(elf, scn)) != NULL) {
+		Elf_Data *data;
+
+		i++;
+		if (!gelf_getshdr(scn, &shdr)) {
+			elf_error("Failed to get ELF section(%d) hdr", i);
+			goto out;
+		}
+
+		secname = elf_strptr(elf, strndx, shdr.sh_name);
+		if (!secname) {
+			elf_error("Failed to get ELF section(%d) hdr name", i);
+			goto out;
+		}
+
+		data = elf_getdata(scn, 0);
+		if (!data) {
+			elf_error("Failed to get ELF section(%d) data", i);
+			goto out;
+		}
+
+		if (shdr.sh_type == SHT_SYMTAB) {
+			symbols_shndx = i;
+			symscn = scn;
+			symbols = data;
+			strtabidx = shdr.sh_link;
+		} else if (!strcmp(secname, BTF_IDS_SECTION)) {
+			idlist_shndx = i;
+			idlist_addr = shdr.sh_addr;
+			idlist = data;
+		}
+	}
+
+	/* Cannot resolve symbol or .BTF_ids sections. Nothing to do. */
+	if (symbols_shndx == -1 || idlist_shndx == -1) {
+		err = 0;
+		goto out;
+	}
+
+	if (!gelf_getshdr(symscn, &shdr)) {
+		elf_error("Failed to get ELF symbol table header");
+		goto out;
+	}
+	nr_syms = shdr.sh_size / shdr.sh_entsize;
+
+	err = btf_encoder__collect_btf_funcs(encoder, &btf_funcs);
+	if (err) {
+		fprintf(stderr, "%s: failed to collect BTF funcs\n", __func__);
+		goto out;
+	}
+
+	/* First collect all kfunc set ranges.
+	 *
+	 * Note we choose not to sort these ranges and accept a linear
+	 * search when doing lookups. Reasoning is that the number of
+	 * sets is ~O(100) and not worth the additional code to optimize.
+	 */
+	for (i = 0; i < nr_syms; i++) {
+		struct btf_kfunc_set_range range = {};
+		const char *name;
+		GElf_Sym sym;
+
+		if (!gelf_getsym(symbols, i, &sym)) {
+			elf_error("Failed to get ELF symbol(%d)", i);
+			goto out;
+		}
+
+		if (sym.st_shndx != idlist_shndx)
+			continue;
+
+		name = elf_strptr(elf, strtabidx, sym.st_name);
+		if (!is_sym_kfunc_set(&sym, name, idlist, idlist_addr))
+			continue;
+
+		range.start = sym.st_value;
+		range.end = sym.st_value + sym.st_size;
+		gobuffer__add(&btf_kfunc_ranges, &range, sizeof(range));
+	}
+
+	/* Now inject BTF with kfunc decl tag for detected kfuncs */
+	for (i = 0; i < nr_syms; i++) {
+		const struct btf_kfunc_set_range *ranges;
+		unsigned int ranges_cnt;
+		char *func, *name;
+		GElf_Sym sym;
+		bool found;
+		int err;
+		int j;
+
+		if (!gelf_getsym(symbols, i, &sym)) {
+			elf_error("Failed to get ELF symbol(%d)", i);
+			goto out;
+		}
+
+		if (sym.st_shndx != idlist_shndx)
+			continue;
+
+		name = elf_strptr(elf, strtabidx, sym.st_name);
+		func = get_func_name(name);
+		if (!func)
+			continue;
+
+		/* Check if function belongs to a kfunc set */
+		ranges = gobuffer__entries(&btf_kfunc_ranges);
+		ranges_cnt = gobuffer__nr_entries(&btf_kfunc_ranges);
+		found = false;
+		for (j = 0; j < ranges_cnt; j++) {
+			size_t addr = sym.st_value;
+
+			if (ranges[j].start <= addr && addr < ranges[j].end) {
+				found = true;
+				break;
+			}
+		}
+		if (!found) {
+			free(func);
+			continue;
+		}
+
+		err = btf_encoder__tag_kfunc(encoder, &btf_funcs, func);
+		if (err) {
+			fprintf(stderr, "%s: failed to tag kfunc '%s'\n", __func__, func);
+			free(func);
+			goto out;
+		}
+		free(func);
+	}
+
+	err = 0;
+out:
+	__gobuffer__delete(&btf_funcs);
+	__gobuffer__delete(&btf_kfunc_ranges);
+	if (elf)
+		elf_end(elf);
+	if (fd != -1)
+		close(fd);
+	return err;
+}
+
 int btf_encoder__encode(struct btf_encoder *encoder)
 {
+	bool should_tag_kfuncs;
 	int err;
 
 	/* for single-threaded case, saved funcs are added here */
@@ -1377,6 +1739,15 @@ int btf_encoder__encode(struct btf_encoder *encoder)
 	if (btf__type_cnt(encoder->btf) == 1)
 		return 0;
 
+	/* Note vmlinux may already contain btf_decl_tag's for kfuncs. So
+	 * take care to call this before btf_dedup().
+	 */
+	should_tag_kfuncs = encoder->tag_kfuncs && !encoder->skip_encoding_decl_tag;
+	if (should_tag_kfuncs && btf_encoder__tag_kfuncs(encoder)) {
+		fprintf(stderr, "%s: failed to tag kfuncs!\n", __func__);
+		return -1;
+	}
+
 	if (btf__dedup(encoder->btf, NULL)) {
 		fprintf(stderr, "%s: btf__dedup failed!\n", __func__);
 		return -1;
@@ -1660,6 +2031,7 @@ struct btf_encoder *btf_encoder__new(struct cu *cu, const char *detached_filenam
 		encoder->force		 = conf_load->btf_encode_force;
 		encoder->gen_floats	 = conf_load->btf_gen_floats;
 		encoder->skip_encoding_vars = conf_load->skip_encoding_btf_vars;
+		encoder->skip_encoding_decl_tag	 = conf_load->skip_encoding_btf_decl_tag;
 		encoder->tag_kfuncs	 = conf_load->btf_decl_tag_kfuncs;
 		encoder->verbose	 = verbose;
 		encoder->has_index_type  = false;
-- 
2.44.0


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

* Re: [PATCH dwarves v6 0/3] pahole: Inject kfunc decl tags into BTF
  2024-03-25 17:53 [PATCH dwarves v6 0/3] pahole: Inject kfunc decl tags into BTF Daniel Xu
                   ` (2 preceding siblings ...)
  2024-03-25 17:53 ` [PATCH dwarves v6 3/3] pahole: Inject kfunc decl tags into BTF Daniel Xu
@ 2024-04-02 20:24 ` Arnaldo Carvalho de Melo
  3 siblings, 0 replies; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2024-04-02 20:24 UTC (permalink / raw
  To: Daniel Xu
  Cc: jolsa, quentin, alan.maguire, eddyz87, andrii.nakryiko, ast,
	daniel, bpf

On Mon, Mar 25, 2024 at 11:53:36AM -0600, Daniel Xu wrote:
> This patchset teaches pahole to parse symbols in .BTF_ids section in
> vmlinux and discover exported kfuncs. Pahole then takes the list of
> kfuncs and injects a BTF_KIND_DECL_TAG for each kfunc.
> 
> Example of encoding:
> 
>         $ bpftool btf dump file .tmp_vmlinux.btf | rg "DECL_TAG 'bpf_kfunc'" | wc -l
>         121
> 
>         $ bpftool btf dump file .tmp_vmlinux.btf | rg 56337
>         [56337] FUNC 'bpf_ct_change_timeout' type_id=56336 linkage=static
>         [127861] DECL_TAG 'bpf_kfunc' type_id=56337 component_idx=-1
> 
> This enables downstream users and tools to dynamically discover which
> kfuncs are available on a system by parsing vmlinux or module BTF, both
> available in /sys/kernel/btf.
> 
> This feature is enabled with --btf_features=decl_tag,decl_tag_kfuncs.
> 
> === Changelog ===

I'm a bit lagged on my backlog, will try and process these soon,

- Arnaldo
 
> Changes from v5:
> * Add gobuffer__sort() helper
> * Use strstarts() instead of strncmp()
> * Use uint64_t instead of size_t
> * Add clarifying comments for get_func_name()
> 
> Changes from v4:
> * Update man page with decl_tag_kfuncs feature
> * Fix release mode build warnings
> * Add elf_getshrstrndx() error checking
> * Disable tagging if decl_tag feature is off
> * Fix malformed func name handling
> 
> Changes from v3:
> * Guard kfunc tagging behind feature flag
> * Use struct btf_id_set8 definition
> * Remove unnecessary member from btf_encoder
> * Fix code styling
> 
> Changes from v2:
> * More reliably detect kfunc membership in set8 by tracking set addr ranges
> * Rename some variables/functions to be more clear about kfunc vs func
> 
> Changes from v1:
> * Fix resource leaks
> * Fix callee -> caller typo
> * Rename btf_decl_tag from kfunc -> bpf_kfunc
> * Only grab btf_id_set funcs tagged kfunc
> * Presort btf func list
> 
> Daniel Xu (3):
>   gobuffer: Add gobuffer__sort() helper
>   pahole: Add --btf_feature=decl_tag_kfuncs feature
>   pahole: Inject kfunc decl tags into BTF
> 
>  btf_encoder.c      | 374 +++++++++++++++++++++++++++++++++++++++++++++
>  dwarves.h          |   1 +
>  gobuffer.c         |   5 +
>  gobuffer.h         |   2 +
>  man-pages/pahole.1 |   1 +
>  pahole.c           |   1 +
>  6 files changed, 384 insertions(+)
> 
> -- 
> 2.44.0

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

* Re: [PATCH dwarves v6 1/3] gobuffer: Add gobuffer__sort() helper
  2024-03-25 17:53 ` [PATCH dwarves v6 1/3] gobuffer: Add gobuffer__sort() helper Daniel Xu
@ 2024-04-17  9:20   ` Alan Maguire
  2024-04-17 13:40     ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 9+ messages in thread
From: Alan Maguire @ 2024-04-17  9:20 UTC (permalink / raw
  To: Daniel Xu, acme, jolsa, quentin, eddyz87
  Cc: andrii.nakryiko, ast, daniel, bpf

On 25/03/2024 17:53, Daniel Xu wrote:
> Add a helper to sort the gobuffer. Trivial wrapper around qsort().
> 
> Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>
Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
> ---
>  gobuffer.c | 5 +++++
>  gobuffer.h | 2 ++
>  2 files changed, 7 insertions(+)
> 
> diff --git a/gobuffer.c b/gobuffer.c
> index 02b2084..4655339 100644
> --- a/gobuffer.c
> +++ b/gobuffer.c
> @@ -102,6 +102,11 @@ void gobuffer__copy(const struct gobuffer *gb, void *dest)
>  	}
>  }
>  
> +void gobuffer__sort(struct gobuffer *gb, unsigned int size, int (*compar)(const void *, const void *))
> +{
> +	qsort((void *)gb->entries, gb->nr_entries, size, compar);

nit shouldn't need to cast char * gb->entries to void * ; not worth
respinning the series for though unless there are other issues

> +}
> +
>  const void *gobuffer__compress(struct gobuffer *gb, unsigned int *size)
>  {
>  	z_stream z = {
> diff --git a/gobuffer.h b/gobuffer.h
> index a12c5c8..cd218b6 100644
> --- a/gobuffer.h
> +++ b/gobuffer.h
> @@ -21,6 +21,8 @@ void __gobuffer__delete(struct gobuffer *gb);
>  
>  void gobuffer__copy(const struct gobuffer *gb, void *dest);
>  
> +void gobuffer__sort(struct gobuffer *gb, unsigned int size, int (*compar)(const void *, const void *));
> +
>  int gobuffer__add(struct gobuffer *gb, const void *s, unsigned int len);
>  int gobuffer__allocate(struct gobuffer *gb, unsigned int len);
>  

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

* Re: [PATCH dwarves v6 3/3] pahole: Inject kfunc decl tags into BTF
  2024-03-25 17:53 ` [PATCH dwarves v6 3/3] pahole: Inject kfunc decl tags into BTF Daniel Xu
@ 2024-04-17  9:32   ` Alan Maguire
  0 siblings, 0 replies; 9+ messages in thread
From: Alan Maguire @ 2024-04-17  9:32 UTC (permalink / raw
  To: Daniel Xu, acme, jolsa, quentin, eddyz87
  Cc: andrii.nakryiko, ast, daniel, bpf

On 25/03/2024 17:53, Daniel Xu wrote:
> This commit teaches pahole to parse symbols in .BTF_ids section in
> vmlinux and discover exported kfuncs. Pahole then takes the list of
> kfuncs and injects a BTF_KIND_DECL_TAG for each kfunc.
> 
> Example of encoding:
> 
>         $ bpftool btf dump file .tmp_vmlinux.btf | rg "DECL_TAG 'bpf_kfunc'" | wc -l
>         121
> 
>         $ bpftool btf dump file .tmp_vmlinux.btf | rg 56337
>         [56337] FUNC 'bpf_ct_change_timeout' type_id=56336 linkage=static
>         [127861] DECL_TAG 'bpf_kfunc' type_id=56337 component_idx=-1
> 
> This enables downstream users and tools to dynamically discover which
> kfuncs are available on a system by parsing vmlinux or module BTF, both
> available in /sys/kernel/btf.
> 
> This feature is enabled with --btf_features=decl_tag,decl_tag_kfuncs.
> 
> Acked-by: Jiri Olsa <jolsa@kernel.org>
> Tested-by: Jiri Olsa <jolsa@kernel.org>
> Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
> Tested-by: Alan Maguire <alan.maguire@oracle.com>

I re-tested for both vmlinux and a module with a kfunc (nf_nat); both
generated decl_tags pointing at the kfunc functions so all working
great. Nice work!

> Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>
> ---
>  btf_encoder.c | 372 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 372 insertions(+)
> 
> diff --git a/btf_encoder.c b/btf_encoder.c
> index 850e36f..d326404 100644
> --- a/btf_encoder.c
> +++ b/btf_encoder.c
> @@ -34,6 +34,21 @@
>  #include <pthread.h>
>  
>  #define BTF_ENCODER_MAX_PROTO	512
> +#define BTF_IDS_SECTION		".BTF_ids"
> +#define BTF_ID_FUNC_PFX		"__BTF_ID__func__"
> +#define BTF_ID_SET8_PFX		"__BTF_ID__set8__"
> +#define BTF_SET8_KFUNCS		(1 << 0)
> +#define BTF_KFUNC_TYPE_TAG	"bpf_kfunc"
> +
> +/* Adapted from include/linux/btf_ids.h */
> +struct btf_id_set8 {
> +        uint32_t cnt;
> +        uint32_t flags;
> +        struct {
> +                uint32_t id;
> +                uint32_t flags;
> +        } pairs[];
> +};
>  
>  /* state used to do later encoding of saved functions */
>  struct btf_encoder_state {
> @@ -75,6 +90,7 @@ struct btf_encoder {
>  			  verbose,
>  			  force,
>  			  gen_floats,
> +			  skip_encoding_decl_tag,
>  			  tag_kfuncs,
>  			  is_rel;
>  	uint32_t	  array_index_id;
> @@ -94,6 +110,17 @@ struct btf_encoder {
>  	} functions;
>  };
>  
> +struct btf_func {
> +	const char *name;
> +	int	    type_id;
> +};
> +
> +/* Half open interval representing range of addresses containing kfuncs */
> +struct btf_kfunc_set_range {
> +	uint64_t start;
> +	uint64_t end;
> +};
> +
>  static LIST_HEAD(encoders);
>  static pthread_mutex_t encoders__lock = PTHREAD_MUTEX_INITIALIZER;
>  
> @@ -1363,8 +1390,343 @@ out:
>  	return err;
>  }
>  
> +/* Returns if `sym` points to a kfunc set */
> +static int is_sym_kfunc_set(GElf_Sym *sym, const char *name, Elf_Data *idlist, size_t idlist_addr)
> +{
> +	void *ptr = idlist->d_buf;
> +	struct btf_id_set8 *set;
> +	int off;
> +
> +	/* kfuncs are only found in BTF_SET8's */
> +	if (!strstarts(name, BTF_ID_SET8_PFX))
> +		return false;
> +
> +	off = sym->st_value - idlist_addr;
> +	if (off >= idlist->d_size) {
> +		fprintf(stderr, "%s: symbol '%s' out of bounds\n", __func__, name);
> +		return false;
> +	}
> +
> +	/* Check the set8 flags to see if it was marked as kfunc */
> +	set = ptr + off;
> +	return set->flags & BTF_SET8_KFUNCS;
> +}
> +
> +/*
> + * Parse BTF_ID symbol and return the func name.
> + *
> + * Returns:
> + *	Caller-owned string containing func name if successful.
> + *	NULL if !func or on error.
> + */
> +static char *get_func_name(const char *sym)
> +{
> +	char *func, *end;
> +
> +	/* Example input: __BTF_ID__func__vfs_close__1
> +	 *
> +	 * The goal is to strip the prefix and suffix such that we only
> +	 * return vfs_close.
> +	 */
> +
> +	if (!strstarts(sym, BTF_ID_FUNC_PFX))
> +		return NULL;
> +
> +	/* Strip prefix and handle malformed input such as  __BTF_ID__func___ */
> +	func = strdup(sym + sizeof(BTF_ID_FUNC_PFX) - 1);
> +	if (strlen(func) < 2) {
> +                free(func);
> +                return NULL;
> +        }
> +
> +	/* Strip suffix */
> +	end = strrchr(func, '_');
> +	if (!end || *(end - 1) != '_') {
> +		free(func);
> +		return NULL;
> +	}
> +	*(end - 1) = '\0';
> +
> +	return func;
> +}
> +
> +static int btf_func_cmp(const void *_a, const void *_b)
> +{
> +	const struct btf_func *a = _a;
> +	const struct btf_func *b = _b;
> +
> +	return strcmp(a->name, b->name);
> +}
> +
> +/*
> + * Collects all functions described in BTF.
> + * Returns non-zero on error.
> + */
> +static int btf_encoder__collect_btf_funcs(struct btf_encoder *encoder, struct gobuffer *funcs)
> +{
> +	struct btf *btf = encoder->btf;
> +	int nr_types, type_id;
> +	int err = -1;
> +
> +	/* First collect all the func entries into an array */
> +	nr_types = btf__type_cnt(btf);
> +	for (type_id = 1; type_id < nr_types; type_id++) {
> +		const struct btf_type *type;
> +		struct btf_func func = {};
> +		const char *name;
> +
> +		type = btf__type_by_id(btf, type_id);
> +		if (!type) {
> +			fprintf(stderr, "%s: malformed BTF, can't resolve type for ID %d\n",
> +				__func__, type_id);
> +			err = -EINVAL;
> +			goto out;
> +		}
> +
> +		if (!btf_is_func(type))
> +			continue;
> +
> +		name = btf__name_by_offset(btf, type->name_off);
> +		if (!name) {
> +			fprintf(stderr, "%s: malformed BTF, can't resolve name for ID %d\n",
> +				__func__, type_id);
> +			err = -EINVAL;
> +			goto out;
> +		}
> +
> +		func.name = name;
> +		func.type_id = type_id;
> +		err = gobuffer__add(funcs, &func, sizeof(func));
> +		if (err < 0)
> +			goto out;
> +	}
> +
> +	/* Now that we've collected funcs, sort them by name */
> +	gobuffer__sort(funcs, sizeof(struct btf_func), btf_func_cmp);
> +
> +	err = 0;
> +out:
> +	return err;
> +}
> +
> +static int btf_encoder__tag_kfunc(struct btf_encoder *encoder, struct gobuffer *funcs, const char *kfunc)
> +{
> +	struct btf_func key = { .name = kfunc };
> +	struct btf *btf = encoder->btf;
> +	struct btf_func *target;
> +	const void *base;
> +	unsigned int cnt;
> +	int err = -1;
> +
> +	base = gobuffer__entries(funcs);
> +	cnt = gobuffer__nr_entries(funcs);
> +	target = bsearch(&key, base, cnt, sizeof(key), btf_func_cmp);
> +	if (!target) {
> +		fprintf(stderr, "%s: failed to find kfunc '%s' in BTF\n", __func__, kfunc);
> +		goto out;
> +	}
> +
> +	/* Note we are unconditionally adding the btf_decl_tag even
> +	 * though vmlinux may already contain btf_decl_tags for kfuncs.
> +	 * We are ok to do this b/c we will later btf__dedup() to remove
> +	 * any duplicates.
> +	 */
> +	err = btf__add_decl_tag(btf, BTF_KFUNC_TYPE_TAG, target->type_id, -1);
> +	if (err < 0) {
> +		fprintf(stderr, "%s: failed to insert kfunc decl tag for '%s': %d\n",
> +			__func__, kfunc, err);
> +		goto out;
> +	}
> +
> +	err = 0;
> +out:
> +	return err;
> +}
> +
> +static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder)
> +{
> +	const char *filename = encoder->filename;
> +	struct gobuffer btf_kfunc_ranges = {};
> +	struct gobuffer btf_funcs = {};
> +	Elf_Data *symbols = NULL;
> +	Elf_Data *idlist = NULL;
> +	Elf_Scn *symscn = NULL;
> +	int symbols_shndx = -1;
> +	size_t idlist_addr = 0;
> +	int fd = -1, err = -1;
> +	int idlist_shndx = -1;
> +	size_t strtabidx = 0;
> +	Elf_Scn *scn = NULL;
> +	Elf *elf = NULL;
> +	GElf_Shdr shdr;
> +	size_t strndx;
> +	char *secname;
> +	int nr_syms;
> +	int i = 0;
> +
> +	fd = open(filename, O_RDONLY);
> +	if (fd < 0) {
> +		fprintf(stderr, "Cannot open %s\n", filename);
> +		goto out;
> +	}
> +
> +	if (elf_version(EV_CURRENT) == EV_NONE) {
> +		elf_error("Cannot set libelf version");
> +		goto out;
> +	}
> +
> +	elf = elf_begin(fd, ELF_C_READ, NULL);
> +	if (elf == NULL) {
> +		elf_error("Cannot update ELF file");
> +		goto out;
> +	}
> +
> +	/* Locate symbol table and .BTF_ids sections */
> +	if (elf_getshdrstrndx(elf, &strndx) < 0)
> +		goto out;
> +
> +	while ((scn = elf_nextscn(elf, scn)) != NULL) {
> +		Elf_Data *data;
> +
> +		i++;
> +		if (!gelf_getshdr(scn, &shdr)) {
> +			elf_error("Failed to get ELF section(%d) hdr", i);
> +			goto out;
> +		}
> +
> +		secname = elf_strptr(elf, strndx, shdr.sh_name);
> +		if (!secname) {
> +			elf_error("Failed to get ELF section(%d) hdr name", i);
> +			goto out;
> +		}
> +
> +		data = elf_getdata(scn, 0);
> +		if (!data) {
> +			elf_error("Failed to get ELF section(%d) data", i);
> +			goto out;
> +		}
> +
> +		if (shdr.sh_type == SHT_SYMTAB) {
> +			symbols_shndx = i;
> +			symscn = scn;
> +			symbols = data;
> +			strtabidx = shdr.sh_link;
> +		} else if (!strcmp(secname, BTF_IDS_SECTION)) {
> +			idlist_shndx = i;
> +			idlist_addr = shdr.sh_addr;
> +			idlist = data;
> +		}
> +	}
> +
> +	/* Cannot resolve symbol or .BTF_ids sections. Nothing to do. */
> +	if (symbols_shndx == -1 || idlist_shndx == -1) {
> +		err = 0;
> +		goto out;
> +	}
> +
> +	if (!gelf_getshdr(symscn, &shdr)) {
> +		elf_error("Failed to get ELF symbol table header");
> +		goto out;
> +	}
> +	nr_syms = shdr.sh_size / shdr.sh_entsize;
> +
> +	err = btf_encoder__collect_btf_funcs(encoder, &btf_funcs);
> +	if (err) {
> +		fprintf(stderr, "%s: failed to collect BTF funcs\n", __func__);
> +		goto out;
> +	}
> +
> +	/* First collect all kfunc set ranges.
> +	 *
> +	 * Note we choose not to sort these ranges and accept a linear
> +	 * search when doing lookups. Reasoning is that the number of
> +	 * sets is ~O(100) and not worth the additional code to optimize.
> +	 */
> +	for (i = 0; i < nr_syms; i++) {
> +		struct btf_kfunc_set_range range = {};
> +		const char *name;
> +		GElf_Sym sym;
> +
> +		if (!gelf_getsym(symbols, i, &sym)) {
> +			elf_error("Failed to get ELF symbol(%d)", i);
> +			goto out;
> +		}
> +
> +		if (sym.st_shndx != idlist_shndx)
> +			continue;
> +
> +		name = elf_strptr(elf, strtabidx, sym.st_name);
> +		if (!is_sym_kfunc_set(&sym, name, idlist, idlist_addr))
> +			continue;
> +
> +		range.start = sym.st_value;
> +		range.end = sym.st_value + sym.st_size;
> +		gobuffer__add(&btf_kfunc_ranges, &range, sizeof(range));
> +	}
> +
> +	/* Now inject BTF with kfunc decl tag for detected kfuncs */
> +	for (i = 0; i < nr_syms; i++) {
> +		const struct btf_kfunc_set_range *ranges;
> +		unsigned int ranges_cnt;
> +		char *func, *name;
> +		GElf_Sym sym;
> +		bool found;
> +		int err;
> +		int j;
> +
> +		if (!gelf_getsym(symbols, i, &sym)) {
> +			elf_error("Failed to get ELF symbol(%d)", i);
> +			goto out;
> +		}
> +
> +		if (sym.st_shndx != idlist_shndx)
> +			continue;
> +
> +		name = elf_strptr(elf, strtabidx, sym.st_name);
> +		func = get_func_name(name);
> +		if (!func)
> +			continue;
> +
> +		/* Check if function belongs to a kfunc set */
> +		ranges = gobuffer__entries(&btf_kfunc_ranges);
> +		ranges_cnt = gobuffer__nr_entries(&btf_kfunc_ranges);
> +		found = false;
> +		for (j = 0; j < ranges_cnt; j++) {
> +			size_t addr = sym.st_value;
> +
> +			if (ranges[j].start <= addr && addr < ranges[j].end) {
> +				found = true;
> +				break;
> +			}
> +		}
> +		if (!found) {
> +			free(func);
> +			continue;
> +		}
> +
> +		err = btf_encoder__tag_kfunc(encoder, &btf_funcs, func);
> +		if (err) {
> +			fprintf(stderr, "%s: failed to tag kfunc '%s'\n", __func__, func);
> +			free(func);
> +			goto out;
> +		}
> +		free(func);
> +	}
> +
> +	err = 0;
> +out:
> +	__gobuffer__delete(&btf_funcs);
> +	__gobuffer__delete(&btf_kfunc_ranges);
> +	if (elf)
> +		elf_end(elf);
> +	if (fd != -1)
> +		close(fd);
> +	return err;
> +}
> +
>  int btf_encoder__encode(struct btf_encoder *encoder)
>  {
> +	bool should_tag_kfuncs;
>  	int err;
>  
>  	/* for single-threaded case, saved funcs are added here */
> @@ -1377,6 +1739,15 @@ int btf_encoder__encode(struct btf_encoder *encoder)
>  	if (btf__type_cnt(encoder->btf) == 1)
>  		return 0;
>  
> +	/* Note vmlinux may already contain btf_decl_tag's for kfuncs. So
> +	 * take care to call this before btf_dedup().
> +	 */
> +	should_tag_kfuncs = encoder->tag_kfuncs && !encoder->skip_encoding_decl_tag;
> +	if (should_tag_kfuncs && btf_encoder__tag_kfuncs(encoder)) {
> +		fprintf(stderr, "%s: failed to tag kfuncs!\n", __func__);
> +		return -1;
> +	}
> +
>  	if (btf__dedup(encoder->btf, NULL)) {
>  		fprintf(stderr, "%s: btf__dedup failed!\n", __func__);
>  		return -1;
> @@ -1660,6 +2031,7 @@ struct btf_encoder *btf_encoder__new(struct cu *cu, const char *detached_filenam
>  		encoder->force		 = conf_load->btf_encode_force;
>  		encoder->gen_floats	 = conf_load->btf_gen_floats;
>  		encoder->skip_encoding_vars = conf_load->skip_encoding_btf_vars;
> +		encoder->skip_encoding_decl_tag	 = conf_load->skip_encoding_btf_decl_tag;
>  		encoder->tag_kfuncs	 = conf_load->btf_decl_tag_kfuncs;
>  		encoder->verbose	 = verbose;
>  		encoder->has_index_type  = false;

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

* Re: [PATCH dwarves v6 1/3] gobuffer: Add gobuffer__sort() helper
  2024-04-17  9:20   ` Alan Maguire
@ 2024-04-17 13:40     ` Arnaldo Carvalho de Melo
  2024-04-18  9:31       ` Alan Maguire
  0 siblings, 1 reply; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2024-04-17 13:40 UTC (permalink / raw
  To: Alan Maguire
  Cc: Daniel Xu, jolsa, quentin, eddyz87, andrii.nakryiko, ast, daniel,
	bpf

On Wed, Apr 17, 2024 at 10:20:41AM +0100, Alan Maguire wrote:
> On 25/03/2024 17:53, Daniel Xu wrote:
> > Add a helper to sort the gobuffer. Trivial wrapper around qsort().
> > 
> > Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>
> Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
> > ---
> >  gobuffer.c | 5 +++++
> >  gobuffer.h | 2 ++
> >  2 files changed, 7 insertions(+)
> > 
> > diff --git a/gobuffer.c b/gobuffer.c
> > index 02b2084..4655339 100644
> > --- a/gobuffer.c
> > +++ b/gobuffer.c
> > @@ -102,6 +102,11 @@ void gobuffer__copy(const struct gobuffer *gb, void *dest)
> >  	}
> >  }
> >  
> > +void gobuffer__sort(struct gobuffer *gb, unsigned int size, int (*compar)(const void *, const void *))
> > +{
> > +	qsort((void *)gb->entries, gb->nr_entries, size, compar);
> 
> nit shouldn't need to cast char * gb->entries to void * ; not worth
> respinning the series for though unless there are other issues

I can remove that while applying the series, which I'm doing now.

- Arnaldo
 
> > +}
> > +
> >  const void *gobuffer__compress(struct gobuffer *gb, unsigned int *size)
> >  {
> >  	z_stream z = {
> > diff --git a/gobuffer.h b/gobuffer.h
> > index a12c5c8..cd218b6 100644
> > --- a/gobuffer.h
> > +++ b/gobuffer.h
> > @@ -21,6 +21,8 @@ void __gobuffer__delete(struct gobuffer *gb);
> >  
> >  void gobuffer__copy(const struct gobuffer *gb, void *dest);
> >  
> > +void gobuffer__sort(struct gobuffer *gb, unsigned int size, int (*compar)(const void *, const void *));
> > +
> >  int gobuffer__add(struct gobuffer *gb, const void *s, unsigned int len);
> >  int gobuffer__allocate(struct gobuffer *gb, unsigned int len);
> >  

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

* Re: [PATCH dwarves v6 1/3] gobuffer: Add gobuffer__sort() helper
  2024-04-17 13:40     ` Arnaldo Carvalho de Melo
@ 2024-04-18  9:31       ` Alan Maguire
  0 siblings, 0 replies; 9+ messages in thread
From: Alan Maguire @ 2024-04-18  9:31 UTC (permalink / raw
  To: Arnaldo Carvalho de Melo
  Cc: Daniel Xu, jolsa, quentin, eddyz87, andrii.nakryiko, ast, daniel,
	bpf

On 17/04/2024 14:40, Arnaldo Carvalho de Melo wrote:
> On Wed, Apr 17, 2024 at 10:20:41AM +0100, Alan Maguire wrote:
>> On 25/03/2024 17:53, Daniel Xu wrote:
>>> Add a helper to sort the gobuffer. Trivial wrapper around qsort().
>>>
>>> Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>
>> Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
>>> ---
>>>  gobuffer.c | 5 +++++
>>>  gobuffer.h | 2 ++
>>>  2 files changed, 7 insertions(+)
>>>
>>> diff --git a/gobuffer.c b/gobuffer.c
>>> index 02b2084..4655339 100644
>>> --- a/gobuffer.c
>>> +++ b/gobuffer.c
>>> @@ -102,6 +102,11 @@ void gobuffer__copy(const struct gobuffer *gb, void *dest)
>>>  	}
>>>  }
>>>  
>>> +void gobuffer__sort(struct gobuffer *gb, unsigned int size, int (*compar)(const void *, const void *))
>>> +{
>>> +	qsort((void *)gb->entries, gb->nr_entries, size, compar);
>>
>> nit shouldn't need to cast char * gb->entries to void * ; not worth
>> respinning the series for though unless there are other issues
> 
> I can remove that while applying the series, which I'm doing now.
>

great, thanks! BTW patch 2 will probably require a small tweak for
merging into next:

-	BTF_FEATURE(decl_tag_kfuncs, btf_decl_tag_kfuncs, false),
+	BTF_FEATURE(decl_tag_kfuncs, btf_decl_tag_kfuncs, false, true),

this will ensure it is considered a standard feature and is enabled for
"all"; for readability it might make sense to position it wth the other
features prior to

        BTF_FEATURE(reproducible_build, reproducible_build, false, false),

...since then we'd have the standard features grouped before the
non-standard ones, making it a bit more readable; not a big deal tho.
thanks!

> - Arnaldo
>  
>>> +}
>>> +
>>>  const void *gobuffer__compress(struct gobuffer *gb, unsigned int *size)
>>>  {
>>>  	z_stream z = {
>>> diff --git a/gobuffer.h b/gobuffer.h
>>> index a12c5c8..cd218b6 100644
>>> --- a/gobuffer.h
>>> +++ b/gobuffer.h
>>> @@ -21,6 +21,8 @@ void __gobuffer__delete(struct gobuffer *gb);
>>>  
>>>  void gobuffer__copy(const struct gobuffer *gb, void *dest);
>>>  
>>> +void gobuffer__sort(struct gobuffer *gb, unsigned int size, int (*compar)(const void *, const void *));
>>> +
>>>  int gobuffer__add(struct gobuffer *gb, const void *s, unsigned int len);
>>>  int gobuffer__allocate(struct gobuffer *gb, unsigned int len);
>>>  

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

end of thread, other threads:[~2024-04-18  9:32 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-25 17:53 [PATCH dwarves v6 0/3] pahole: Inject kfunc decl tags into BTF Daniel Xu
2024-03-25 17:53 ` [PATCH dwarves v6 1/3] gobuffer: Add gobuffer__sort() helper Daniel Xu
2024-04-17  9:20   ` Alan Maguire
2024-04-17 13:40     ` Arnaldo Carvalho de Melo
2024-04-18  9:31       ` Alan Maguire
2024-03-25 17:53 ` [PATCH dwarves v6 2/3] pahole: Add --btf_feature=decl_tag_kfuncs feature Daniel Xu
2024-03-25 17:53 ` [PATCH dwarves v6 3/3] pahole: Inject kfunc decl tags into BTF Daniel Xu
2024-04-17  9:32   ` Alan Maguire
2024-04-02 20:24 ` [PATCH dwarves v6 0/3] " Arnaldo Carvalho de Melo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.