[PATCH dwarves v2 0/2] btf_encoder: refactor emission of BTF funcs

[PATCH dwarves v2 0/2] btf_encoder: refactor emission of BTF funcs

[Ihor Solodrai]

This series refactors a few functions that handle how BTF functions
are emitted. The first patch splits btf_encoder__add_func_proto() into
two functions by input. The second patch separates BPF kfunc handling
from generic function handling.

v1: https://lore.kernel.org/dwarves/20251029190249.3323752-2-ihor.solodrai@linux.dev/

Ihor Solodrai (2):
  btf_encoder: refactor btf_encoder__add_func_proto
  btf_encoder: factor out btf_encoder__add_bpf_kfunc()

 btf_encoder.c | 171 +++++++++++++++++++++++++++++---------------------
 1 file changed, 101 insertions(+), 70 deletions(-)

-- 
2.51.1

[PATCH dwarves v2 1/2] btf_encoder: refactor btf_encoder__add_func_proto

[Ihor Solodrai]

btf_encoder__add_func_proto() essentially implements two independent
code paths depending on input arguments: one for struct ftype and the
other for struct btf_encoder_func_state.

Split btf_encoder__add_func_proto() into two variants:
  * btf_encoder__add_func_proto_for_ftype()
  * func_state__add_func_proto()

And factor out common btf_encoder__emit_func_proto() subroutine.

No functional changes.

Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
---
 btf_encoder.c | 138 +++++++++++++++++++++++++++++---------------------
 1 file changed, 79 insertions(+), 59 deletions(-)

diff --git a/btf_encoder.c b/btf_encoder.c
index 03bc3c7..d4ab5ec 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -815,82 +815,102 @@ static inline bool is_kfunc_state(struct btf_encoder_func_state *state)
 	return state && state->elf && state->elf->kfunc;
 }
 
-static int32_t btf_encoder__add_func_proto(struct btf_encoder *encoder, struct ftype *ftype,
-					   struct btf_encoder_func_state *state)
+static int32_t btf_encoder__emit_func_proto(struct btf_encoder *encoder,
+					    uint32_t type_id,
+					    uint16_t nr_params)
 {
 	const struct btf_type *t;
-	struct btf *btf;
-	struct parameter *param;
+	uint32_t ret;
+
+	ret = btf__add_func_proto(encoder->btf, type_id);
+	if (ret > 0) {
+		t = btf__type_by_id(encoder->btf, ret);
+		btf_encoder__log_type(encoder, t, false, false,
+			"return=%u args=(%s", t->type, !nr_params ? "void)\n" : "");
+	} else {
+		btf__log_err(encoder->btf, BTF_KIND_FUNC_PROTO, NULL, true, ret,
+			     "return=%u vlen=%u Error emitting BTF type",
+			     type_id, nr_params);
+	}
+
+	return ret;
+}
+
+static int32_t btf_encoder__add_func_proto_for_ftype(struct btf_encoder *encoder,
+						     struct ftype *ftype)
+{
 	uint16_t nr_params, param_idx;
+	struct parameter *param;
 	int32_t id, type_id;
-	char tmp_name[KSYM_NAME_LEN];
 	const char *name;
 
-	assert(ftype != NULL || state != NULL);
-
-	if (is_kfunc_state(state) && encoder->tag_kfuncs && encoder->encode_attributes)
-		if (btf__add_bpf_arena_type_tags(encoder->btf, state) < 0)
-			return -1;
+	assert(ftype != NULL);
 
 	/* add btf_type for func_proto */
-	if (ftype) {
-		btf = encoder->btf;
-		nr_params = ftype->nr_parms + (ftype->unspec_parms ? 1 : 0);
-		type_id = btf_encoder__tag_type(encoder, ftype->tag.type);
-	} else if (state) {
-		encoder = state->encoder;
-		btf = state->encoder->btf;
-		nr_params = state->nr_parms;
-		type_id = state->ret_type_id;
-	} else {
-		return 0;
-	}
+	nr_params = ftype->nr_parms + (ftype->unspec_parms ? 1 : 0);
+	type_id = btf_encoder__tag_type(encoder, ftype->tag.type);
 
-	id = btf__add_func_proto(btf, type_id);
-	if (id > 0) {
-		t = btf__type_by_id(btf, id);
-		btf_encoder__log_type(encoder, t, false, false, "return=%u args=(%s", t->type, !nr_params ? "void)\n" : "");
-	} else {
-		btf__log_err(btf, BTF_KIND_FUNC_PROTO, NULL, true, id,
-			     "return=%u vlen=%u Error emitting BTF type",
-			     type_id, nr_params);
+	id = btf_encoder__emit_func_proto(encoder, type_id, nr_params);
+	if (id < 0)
 		return id;
-	}
 
 	/* add parameters */
 	param_idx = 0;
-	if (ftype) {
-		ftype__for_each_parameter(ftype, param) {
-			const char *name = parameter__name(param);
-
-			type_id = param->tag.type == 0 ? 0 : encoder->type_id_off + param->tag.type;
-			++param_idx;
-			if (btf_encoder__add_func_param(encoder, name, type_id,
-							param_idx == nr_params))
-				return -1;
-		}
 
+	ftype__for_each_parameter(ftype, param) {
+		name = parameter__name(param);
+		type_id = param->tag.type == 0 ? 0 : encoder->type_id_off + param->tag.type;
 		++param_idx;
-		if (ftype->unspec_parms)
-			if (btf_encoder__add_func_param(encoder, NULL, 0,
-							param_idx == nr_params))
-				return -1;
-	} else {
-		for (param_idx = 0; param_idx < nr_params; param_idx++) {
-			struct btf_encoder_func_parm *p = &state->parms[param_idx];
+		if (btf_encoder__add_func_param(encoder, name, type_id, param_idx == nr_params))
+			return -1;
+	}
 
-			name = btf__name_by_offset(btf, p->name_off);
+	++param_idx;
+	if (ftype->unspec_parms)
+		if (btf_encoder__add_func_param(encoder, NULL, 0, param_idx == nr_params))
+			return -1;
 
-			/* adding BTF data may result in a move of the
-			 * name string memory, so make a temporary copy.
-			 */
-			strncpy(tmp_name, name, sizeof(tmp_name) - 1);
+	return id;
+}
 
-			if (btf_encoder__add_func_param(encoder, tmp_name, p->type_id,
-							param_idx == nr_params))
-				return -1;
-		}
+static int32_t func_state__add_func_proto(struct btf_encoder_func_state *state)
+{
+	struct btf_encoder *encoder = state->encoder;
+	const struct btf *btf = encoder->btf;
+	struct btf_encoder_func_parm *p;
+	uint16_t nr_params, param_idx;
+	char tmp_name[KSYM_NAME_LEN];
+	int32_t id, type_id;
+	const char *name;
+	bool is_last;
+
+	/* Beware: btf__add_bpf_arena_type_tags may change some members of the state */
+	if (is_kfunc_state(state) && encoder->tag_kfuncs && encoder->encode_attributes)
+		if (btf__add_bpf_arena_type_tags(encoder->btf, state) < 0)
+			return -1;
+
+	type_id = state->ret_type_id;
+	nr_params = state->nr_parms;
+
+	id = btf_encoder__emit_func_proto(encoder, type_id, nr_params);
+	if (id < 0)
+		return id;
+
+	/* add parameters */
+	for (param_idx = 0; param_idx < nr_params; param_idx++) {
+		p = &state->parms[param_idx];
+		name = btf__name_by_offset(btf, p->name_off);
+		is_last = param_idx == nr_params;
+
+		/* adding BTF data may result in a move of the
+		 * name string memory, so make a temporary copy.
+		 */
+		strncpy(tmp_name, name, sizeof(tmp_name) - 1);
+
+		if (btf_encoder__add_func_param(encoder, tmp_name, p->type_id, is_last))
+			return -1;
 	}
+
 	return id;
 }
 
@@ -1349,7 +1369,7 @@ static int32_t btf_encoder__add_func(struct btf_encoder *encoder,
 	uint16_t idx;
 	int err;
 
-	btf_fnproto_id = btf_encoder__add_func_proto(encoder, NULL, state);
+	btf_fnproto_id = func_state__add_func_proto(state);
 	name = func->name;
 	if (btf_fnproto_id >= 0)
 		btf_fn_id = btf_encoder__add_ref_type(encoder, BTF_KIND_FUNC, btf_fnproto_id,
@@ -1686,7 +1706,7 @@ static int btf_encoder__encode_tag(struct btf_encoder *encoder, struct tag *tag,
 	case DW_TAG_enumeration_type:
 		return btf_encoder__add_enum_type(encoder, tag, conf_load);
 	case DW_TAG_subroutine_type:
-		return btf_encoder__add_func_proto(encoder, tag__ftype(tag), NULL);
+		return btf_encoder__add_func_proto_for_ftype(encoder, tag__ftype(tag));
         case DW_TAG_unspecified_type:
 		/* Just don't encode this for now, converting anything with this type to void (0) instead.
 		 *
-- 
2.51.1

Re: [PATCH dwarves v2 1/2] btf_encoder: refactor btf_encoder__add_func_proto

[Eduard Zingerman]

On Tue, 2025-11-04 at 15:35 -0800, Ihor Solodrai wrote:
> btf_encoder__add_func_proto() essentially implements two independent
> code paths depending on input arguments: one for struct ftype and the
> other for struct btf_encoder_func_state.
> 
> Split btf_encoder__add_func_proto() into two variants:
>   * btf_encoder__add_func_proto_for_ftype()
>   * func_state__add_func_proto()
> 
> And factor out common btf_encoder__emit_func_proto() subroutine.
> 
> No functional changes.
> 
> Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
> ---

No changes in generated BTF for my test kernel.

Acked-by: Eduard Zingerman <eddyz87@gmail.com>

[...]

[PATCH dwarves v2 2/2] btf_encoder: factor out btf_encoder__add_bpf_kfunc()

[Ihor Solodrai]

Emitting BTF for BPF kernel functions requires special
handling. Consolidate this behavior into btf_encoder__add_bpf_kfunc()
function, which uses simplified btf_encoder_add_func()

No functional changes.

Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
---
 btf_encoder.c | 43 +++++++++++++++++++++++++++----------------
 1 file changed, 27 insertions(+), 16 deletions(-)

diff --git a/btf_encoder.c b/btf_encoder.c
index d4ab5ec..7accaa0 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -774,6 +774,7 @@ static int btf__tag_bpf_arena_arg(struct btf *btf, struct btf_encoder_func_state
 	return id;
 }
 
+/* Modifies state->ret_type_id and state->parms[i].type_id for flagged kfuncs */
 static int btf__add_bpf_arena_type_tags(struct btf *btf, struct btf_encoder_func_state *state)
 {
 	uint32_t flags = state->elf->kfunc_flags;
@@ -884,11 +885,6 @@ static int32_t func_state__add_func_proto(struct btf_encoder_func_state *state)
 	const char *name;
 	bool is_last;
 
-	/* Beware: btf__add_bpf_arena_type_tags may change some members of the state */
-	if (is_kfunc_state(state) && encoder->tag_kfuncs && encoder->encode_attributes)
-		if (btf__add_bpf_arena_type_tags(encoder->btf, state) < 0)
-			return -1;
-
 	type_id = state->ret_type_id;
 	nr_params = state->nr_parms;
 
@@ -1367,7 +1363,6 @@ static int32_t btf_encoder__add_func(struct btf_encoder *encoder,
 	const char *value;
 	char tmp_value[KSYM_NAME_LEN];
 	uint16_t idx;
-	int err;
 
 	btf_fnproto_id = func_state__add_func_proto(state);
 	name = func->name;
@@ -1380,15 +1375,6 @@ static int32_t btf_encoder__add_func(struct btf_encoder *encoder,
 		return -1;
 	}
 
-	if (func->kfunc && encoder->tag_kfuncs && !encoder->skip_encoding_decl_tag) {
-		err = btf__tag_kfunc(encoder->btf, func, btf_fn_id);
-		if (err < 0)
-			return err;
-	}
-
-	if (state->nr_annots == 0)
-		return 0;
-
 	for (idx = 0; idx < state->nr_annots; idx++) {
 		struct btf_encoder_func_annot *a = &state->annots[idx];
 
@@ -1411,6 +1397,28 @@ static int32_t btf_encoder__add_func(struct btf_encoder *encoder,
 		return -1;
 	}
 
+	return btf_fn_id;
+}
+
+static int btf_encoder__add_bpf_kfunc(struct btf_encoder *encoder,
+				      struct btf_encoder_func_state *state)
+{
+	int btf_fn_id, err;
+
+	if (encoder->tag_kfuncs && encoder->encode_attributes)
+		if (btf__add_bpf_arena_type_tags(encoder->btf, state) < 0)
+			return -1;
+
+	btf_fn_id = btf_encoder__add_func(encoder, state);
+	if (btf_fn_id < 0)
+		return -1;
+
+	if (encoder->tag_kfuncs && !encoder->skip_encoding_decl_tag) {
+		err = btf__tag_kfunc(encoder->btf, state->elf, btf_fn_id);
+		if (err < 0)
+			return -1;
+	}
+
 	return 0;
 }
 
@@ -1508,7 +1516,10 @@ static int btf_encoder__add_saved_funcs(struct btf_encoder *encoder, bool skip_e
 					0, 0);
 
 		if (add_to_btf) {
-			err = btf_encoder__add_func(state->encoder, state);
+			if (is_kfunc_state(state))
+				err = btf_encoder__add_bpf_kfunc(state->encoder, state);
+			else
+				err = btf_encoder__add_func(state->encoder, state);
 			if (err < 0)
 				goto out;
 		}
-- 
2.51.1

Re: [PATCH dwarves v2 2/2] btf_encoder: factor out btf_encoder__add_bpf_kfunc()

[Eduard Zingerman]

On Tue, 2025-11-04 at 15:35 -0800, Ihor Solodrai wrote:

[...]

> @@ -1411,6 +1397,28 @@ static int32_t btf_encoder__add_func(struct btf_encoder *encoder,
>  		return -1;
>  	}
>  
> +	return btf_fn_id;
> +}
> +
> +static int btf_encoder__add_bpf_kfunc(struct btf_encoder *encoder,
> +				      struct btf_encoder_func_state *state)
> +{

As with previous iteration, 'state' has a link to 'encoder', so there
is no need to pass it as a parameter.

> +	int btf_fn_id, err;
> +
> +	if (encoder->tag_kfuncs && encoder->encode_attributes)
> +		if (btf__add_bpf_arena_type_tags(encoder->btf, state) < 0)
> +			return -1;
> +
> +	btf_fn_id = btf_encoder__add_func(encoder, state);
> +	if (btf_fn_id < 0)
> +		return -1;
> +
> +	if (encoder->tag_kfuncs && !encoder->skip_encoding_decl_tag) {
> +		err = btf__tag_kfunc(encoder->btf, state->elf, btf_fn_id);
> +		if (err < 0)
> +			return -1;
> +	}
> +
>  	return 0;
>  }

[...]

Re: [PATCH dwarves v2 2/2] btf_encoder: factor out btf_encoder__add_bpf_kfunc()

[Ihor Solodrai]

On 11/4/25 5:55 PM, Eduard Zingerman wrote:
> On Tue, 2025-11-04 at 15:35 -0800, Ihor Solodrai wrote:
> 
> [...]
> 
>> @@ -1411,6 +1397,28 @@ static int32_t btf_encoder__add_func(struct btf_encoder *encoder,
>>  		return -1;
>>  	}
>>  
>> +	return btf_fn_id;
>> +}
>> +
>> +static int btf_encoder__add_bpf_kfunc(struct btf_encoder *encoder,
>> +				      struct btf_encoder_func_state *state)
>> +{
> 
> As with previous iteration, 'state' has a link to 'encoder', so there
> is no need to pass it as a parameter.

You're right. I fixed this up in the first patch, but haven't noticed 
the same thing in the second. Will fix and send a v3.

> 
>> +	int btf_fn_id, err;
>> +
>> +	if (encoder->tag_kfuncs && encoder->encode_attributes)
>> +		if (btf__add_bpf_arena_type_tags(encoder->btf, state) < 0)
>> +			return -1;
>> +
>> +	btf_fn_id = btf_encoder__add_func(encoder, state);
>> +	if (btf_fn_id < 0)
>> +		return -1;
>> +
>> +	if (encoder->tag_kfuncs && !encoder->skip_encoding_decl_tag) {
>> +		err = btf__tag_kfunc(encoder->btf, state->elf, btf_fn_id);
>> +		if (err < 0)
>> +			return -1;
>> +	}
>> +
>>  	return 0;
>>  }
> 
> [...]