Re: [PATCH bpf-next v2 2/6] resolve_btfids: Process KF_ARENA_* flags in resolve_btfids

On 8/6/26 2:12 PM, Eduard Zingerman wrote:
> On Thu, 2026-08-06 at 14:02 -0700, Ihor Solodrai wrote:
> 
> ...
> 
>>>> +		if (nr_params < 1) {
>>>> +			pr_err("ERROR: resolve_btfids: kfunc %s: KF_ARENA_ARG1 but it has no argument 1\n",
>>>> +			       kfunc->name);
>>>> +			return -EINVAL;
>>>> +		}
>>>> +		id = arena_tag_ptr(btf, arg0_type_id);
>>>> +		if (id < 0) {
>>>> +			pr_err("ERROR: resolve_btfids: kfunc %s: KF_ARENA_ARG1 but argument 1 is not a pointer\n",
>>>> +			       kfunc->name);
>>>
>>> Nit: not a pointer is not the only error condition, btf__add_*()
>>>      functions might fail as well, maybe just push pr_err() down
>>>      to the arena_tag_ptr()?
>>
>> I guess the question is how much details do we want from the error
>> messages here. Since this is a part of kernel build pipeline that can
>> block it, I'd err on the side of more details.
>>
>> I'll see if I can simplify this though.
> 
> Well, we don't want the errors to lie either :)

Why not? We can create many beautiful debugging evenings for the people,
and now for AIs too!

:bilbo_why_shouldnt_I_meme:

> 
>>>
>>>> +			return id;
>>>> +		}
>>>> +		arg0_type_id = id;
>>>> +	}
>>>> +
>>>> +	if (kfunc->flags & KF_ARENA_ARG2) {
>>>> +		if (nr_params < 2) {
>>>> +			pr_err("ERROR: resolve_btfids: kfunc %s: KF_ARENA_ARG2 but it has no argument 2\n",
>>>> +			       kfunc->name);
>>>> +			return -EINVAL;
>>>> +		}
>>>> +		id = arena_tag_ptr(btf, arg1_type_id);
>>>> +		if (id < 0) {
>>>> +			pr_err("ERROR: resolve_btfids: kfunc %s: KF_ARENA_ARG2 but argument 2 is not a pointer\n",
>>>> +			       kfunc->name);
>>>> +			return id;
>>>> +		}
>>>> +		arg1_type_id = id;
>>>> +	}
>>>> +
>>>> +	new_proto_id = btf__add_func_proto(btf, ret_type_id);
>>>> +	if (new_proto_id < 0) {
>>>> +		pr_err("ERROR: resolve_btfids: kfunc %s: failed to add a func proto to BTF\n",
>>>> +		       kfunc->name);
>>>> +		return new_proto_id;
>>>> +	}
>>>> +
>>>> +	for (u32 i = 0; i < nr_params; i++) {
>>>> +		proto = btf__type_by_id(btf, proto_id);
>>>> +		params = btf_params(proto);
>>>
>>> Nit: these two do not need to be in the loop body.
>>
>> They do, because btf__add_func_param() below may move the proto
>> pointer, no?
> 
> Huh, indeed.
> 
>>>> +		name = btf__name_by_offset(btf, params[i].name_off);
>>>> +
>>>> +		switch (i) {
>>>> +		case 0:
>>>> +			param_type_id = arg0_type_id;
>>>> +			break;
>>>> +		case 1:
>>>> +			param_type_id = arg1_type_id;
>>>> +			break;
>>>> +		default:
>>>> +			param_type_id = params[i].type;
>>>> +			break;
>>>> +		}
>>>> +
>>>> +		err = btf__add_func_param(btf, name ?: "", param_type_id);
>>>> +		if (err < 0) {
>>>> +			pr_err("ERROR: resolve_btfids: kfunc %s: failed to add a proto param to BTF\n",
>>>> +			       kfunc->name);
>>>> +			return err;
>>>> +		}
>>>> +	}
>>>> +
>>>> +	pr_debug("added arena-tagged proto for kfunc %s: %d\n", kfunc->name, new_proto_id);
>>>> +
>>>> +	return new_proto_id;
>>>> +}
>>>
>>> ...