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

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 :)

> > 
> > > +			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;
> > > +}
> > 
> > ...