Re: [PATCH bpf-next v3 3/6] resolve_btfids: Introduce enum btf_id_kind

Eduard Zingerman
2025-12-12T07:09:29Z

On Fri, 2025-12-05 at 14:30 -0800, Ihor Solodrai wrote:
> Instead of using multiple flags, make struct btf_id tagged with an
> enum value indicating its kind in the context of resolve_btfids.
>
> Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
> ---

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

(But see a question below).

> @@ -213,14 +218,19 @@ btf_id__add(struct rb_root *root, char *name, bool unique)
> p = &(*p)->rb_left;
> else if (cmp > 0)
> p = &(*p)->rb_right;
> - else
> - return unique ? NULL : id;
> + else if (kind == BTF_ID_KIND_SYM && id->kind == BTF_ID_KIND_SYM)

Nit: I'd keep the 'unique' parameter alongside 'kind' and resolve this
condition on the function callsite.

> + return id;
> + else {
> + pr_err("Unexpected duplicate symbol %s of kind %d\n", name, id->kind);
> + return NULL;
> + }

[...]

> @@ -491,28 +515,24 @@ static int symbols_collect(struct object *obj)
> id = add_symbol(&obj->funcs, prefix, sizeof(BTF_FUNC) - 1);
> /* set8 */
> } else if (!strncmp(prefix, BTF_SET8, sizeof(BTF_SET8) - 1)) {
> - id = add_set(obj, prefix, true);
> + id = add_set(obj, prefix, BTF_ID_KIND_SET8);
> /*
> * SET8 objects store list's count, which is encoded
> * in symbol's size, together with 'cnt' field hence
> * that - 1.
> */
> - if (id) {
> + if (id)
> id->cnt = sym.st_size / sizeof(uint64_t) - 1;
> - id->is_set8 = true;
> - }
> /* set */
> } else if (!strncmp(prefix, BTF_SET, sizeof(BTF_SET) - 1)) {
> - id = add_set(obj, prefix, false);
> + id = add_set(obj, prefix, BTF_ID_KIND_SET);
> /*
> * SET objects store list's count, which is encoded
> * in symbol's size, together with 'cnt' field hence
> * that - 1.
> */
> - if (id) {
> + if (id)

Current patch is not a culprit, but shouldn't resolve_btfids fail if
`id` cannot be added? (here and in a hunk above).

> id->cnt = sym.st_size / sizeof(int) - 1;
> - id->is_set = true;
> - }
> } else {
> pr_err("FAILED unsupported prefix %s\n", prefix);
> return -1; …