[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <9c9ac17e916162d8921e4829153b350080923339.camel@gmail.com>
Date: Mon, 15 Dec 2025 18:54:42 -0800
From: Eduard Zingerman <eddyz87@...il.com>
To: Ihor Solodrai <ihor.solodrai@...ux.dev>, Alexei Starovoitov
<ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>, Andrii Nakryiko
<andrii@...nel.org>, Martin KaFai Lau <martin.lau@...ux.dev>, Song Liu
<song@...nel.org>, Yonghong Song <yonghong.song@...ux.dev>, John Fastabend
<john.fastabend@...il.com>, KP Singh <kpsingh@...nel.org>, Stanislav
Fomichev <sdf@...ichev.me>, Hao Luo <haoluo@...gle.com>, Jiri Olsa
<jolsa@...nel.org>, Andrew Morton <akpm@...ux-foundation.org>, Nathan
Chancellor <nathan@...nel.org>, Nicolas Schier <nsc@...nel.org>, Tejun Heo
<tj@...nel.org>, David Vernet <void@...ifault.com>, Andrea Righi
<arighi@...dia.com>, Changwoo Min <changwoo@...lia.com>, Shuah Khan
<shuah@...nel.org>, Nick Desaulniers <nick.desaulniers+lkml@...il.com>,
Bill Wendling <morbo@...gle.com>, Justin Stitt <justinstitt@...gle.com>,
Alan Maguire <alan.maguire@...cle.com>, Donglin Peng
<dolinux.peng@...il.com>
Cc: bpf@...r.kernel.org, dwarves@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-kbuild@...r.kernel.org
Subject: Re: [PATCH bpf-next v3 3/6] resolve_btfids: Introduce enum
btf_id_kind
On Mon, 2025-12-15 at 18:52 -0800, Ihor Solodrai wrote:
> On 12/15/25 6:38 PM, Eduard Zingerman wrote:
> > On Mon, 2025-12-15 at 18:31 -0800, Ihor Solodrai wrote:
> > > On 12/11/25 11:09 PM, Eduard Zingerman wrote:
> > > > 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@...ux.dev>
> > > > > ---
> > > >
> > > > Acked-by: Eduard Zingerman <eddyz87@...il.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.
> > >
> > > I don't like the boolean args, they're always opaque on the callsite.
> > >
> > > We want to allow duplicates for _KIND_SYM and forbid for other kinds.
> > > Since we are passing the kind from outside, I think it makes sense to
> > > check for this inside the function. It makes the usage simpler.
> >
> > On the contrary, the callsite knows exactly what it wants:
> > unique or non-unique entries. Here you need additional logic
> > to figure out the intent.
> >
> > Arguably the uniqueness is associated not with entry type,
> > but with a particular tree the entry is added to.
> > And that is a property of the callsite.
>
> You're right that the uniqueness is associated with a tree.
> This means we could even check the kind of the root...
>
> I'm thinking maybe it's cleaner to have btf_id__add() and
> btf_id__add_unique(). It can even be a wrapper around btf_id__add()
> with a boolean. wdyt?
Well, sure, that would be a bit cleaner on the callsite.
Up to you, given the number of the callsites I wouldn't bother.
Powered by blists - more mailing lists