From: Eduard Zingerman <eddyz87@gmail.com>
To: Ihor Solodrai <ihor.solodrai@linux.dev>,
Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: Alexei Starovoitov <ast@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Andrii Nakryiko <andrii@kernel.org>,
Martin KaFai Lau <martin.lau@linux.dev>,
Mykyta Yatsenko <yatsenko@meta.com>, Tejun Heo <tj@kernel.org>,
Alan Maguire <alan.maguire@oracle.com>,
Benjamin Tissoires <bentiss@kernel.org>,
Jiri Kosina <jikos@kernel.org>, Amery Hung <ameryhung@gmail.com>,
bpf@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-input@vger.kernel.org, sched-ext@lists.linux.dev
Subject: Re: [PATCH bpf-next v2 05/13] resolve_btfids: Support for KF_IMPLICIT_ARGS
Date: Mon, 19 Jan 2026 16:24:30 -0800 [thread overview]
Message-ID: <c3a04c5063cc9b68f9719c27ab1acb01b199ca3b.camel@gmail.com> (raw)
In-Reply-To: <aff3f58f-aa81-44a3-ae5f-078befeceb39@linux.dev>
On Fri, 2026-01-16 at 22:36 -0800, Ihor Solodrai wrote:
[...]
> > > +static int collect_decl_tags(struct btf2btf_context *ctx)
> > > +{
> > > + const u32 type_cnt = btf__type_cnt(ctx->btf);
> > > + struct btf *btf = ctx->btf;
> > > + const struct btf_type *t;
> > > + u32 *tags, *tmp;
> > > + u32 nr_tags = 0;
> > > +
> > > + tags = malloc(type_cnt * sizeof(u32));
> >
> > waste of memory, really, see below
> >
> > > + if (!tags)
> > > + return -ENOMEM;
> > > +
> > > + for (u32 id = 1; id < type_cnt; id++) {
> > > + t = btf__type_by_id(btf, id);
> > > + if (!btf_is_decl_tag(t))
> > > + continue;
> > > + tags[nr_tags++] = id;
> > > + }
> > > +
> > > + if (nr_tags == 0) {
> > > + ctx->decl_tags = NULL;
> > > + free(tags);
> > > + return 0;
> > > + }
> > > +
> > > + tmp = realloc(tags, nr_tags * sizeof(u32));
> > > + if (!tmp) {
> > > + free(tags);
> > > + return -ENOMEM;
> > > + }
> >
> > This is an interesting realloc() usage pattern, it's quite
> > unconventional to preallocate too much memory, and then shrink (in C
> > world)
> >
> > check libbpf's libbpf_add_mem(), that's a generic "primitive" inside
> > the libbpf. Do not reuse it as is, but it should give you an idea of a
> > common pattern: you start with NULL (empty data), when you need to add
> > a new element, you calculate a new array size which normally would be
> > some minimal value (to avoid going through 1 -> 2 -> 4 -> 8, many
> > small and wasteful steps; normally we just jump straight to 16 or so)
> > or some factor of previous size (doesn't have to be 2x,
> > libbpf_add_mem() expands by 25%, for instance).
> >
> > This is a super common approach in C. Please utilize it here as well.
>
> Hi Andrii, thanks for taking a quick look.
>
> I am aware of the typical size doubling (or whatever the multiplier
> is) pattern for growing arrays. Amortized cost and all that.
>
> I don't know if this pre-alloc + shrink is common, but I did use it in
> pahole before [1], for example.
>
> The chain of thought that makes me like it is:
> * if we knew the array size beforehand, we'd simply pre-allocate it
> * here we don't, but we do know an upper limit (and it's not crazy)
> * if we pre-allocate to upper limit, we can use the array without
> worrying about the bounds checks and growing on every use
> * if we care (we might not), we can shrink to the actual size
>
> The dynamic array approach is certainly more generic, and helpers can
> be written to make it easy. But in cases like this - collect something
> once and then use - over-pre-allocating makes more sense to me.
>
> Re waste we are talking <1Mb (~100k types * 4), so it's whatever.
>
> In any case it's not super important, so I don't mind changing this if
> you insist. Being conventional has it's benefits too.
>
> [1] https://git.kernel.org/pub/scm/devel/pahole/pahole.git/tree/btf_encoder.c?h=v1.31#n2182
In my test kernel there are ~70K types and ~300 decl tags.
Allocating an array of 70K elements to store 300 seem to be quite an overkill.
I'd move to what Andrii suggests just to reduce the surprise factor for the reader.
[...]
next prev parent reply other threads:[~2026-01-20 0:24 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-01-16 20:16 [PATCH bpf-next v2 00/13] bpf: Kernel functions with KF_IMPLICIT_ARGS Ihor Solodrai
2026-01-16 20:16 ` [PATCH bpf-next v2 01/13] bpf: Refactor btf_kfunc_id_set_contains Ihor Solodrai
2026-01-16 20:16 ` [PATCH bpf-next v2 02/13] bpf: Introduce struct bpf_kfunc_meta Ihor Solodrai
2026-01-16 20:16 ` [PATCH bpf-next v2 03/13] bpf: Verifier support for KF_IMPLICIT_ARGS Ihor Solodrai
2026-01-20 0:03 ` Eduard Zingerman
2026-01-16 20:16 ` [PATCH bpf-next v2 04/13] resolve_btfids: Introduce finalize_btf() step Ihor Solodrai
2026-01-20 0:13 ` Eduard Zingerman
2026-01-20 18:11 ` Ihor Solodrai
2026-01-20 18:19 ` Eduard Zingerman
2026-01-20 18:35 ` Ihor Solodrai
2026-01-20 18:40 ` Eduard Zingerman
2026-01-16 20:16 ` [PATCH bpf-next v2 05/13] resolve_btfids: Support for KF_IMPLICIT_ARGS Ihor Solodrai
2026-01-16 20:39 ` bot+bpf-ci
2026-01-16 20:44 ` Ihor Solodrai
2026-01-17 0:06 ` Andrii Nakryiko
2026-01-17 6:36 ` Ihor Solodrai
2026-01-20 0:24 ` Eduard Zingerman [this message]
2026-01-20 0:55 ` Eduard Zingerman
2026-01-16 20:16 ` [PATCH bpf-next v2 06/13] selftests/bpf: Add tests " Ihor Solodrai
2026-01-20 1:24 ` Eduard Zingerman
2026-01-16 20:16 ` [PATCH bpf-next v2 07/13] bpf: Migrate bpf_wq_set_callback_impl() to KF_IMPLICIT_ARGS Ihor Solodrai
2026-01-20 1:50 ` Eduard Zingerman
2026-01-16 20:16 ` [PATCH bpf-next v2 08/13] HID: Use bpf_wq_set_callback kernel function Ihor Solodrai
2026-01-16 20:16 ` [PATCH bpf-next v2 09/13] bpf: Migrate bpf_task_work_schedule_* kfuncs to KF_IMPLICIT_ARGS Ihor Solodrai
2026-01-20 1:52 ` Eduard Zingerman
2026-01-16 20:16 ` [PATCH bpf-next v2 10/13] bpf: Migrate bpf_stream_vprintk() " Ihor Solodrai
2026-01-20 1:53 ` Eduard Zingerman
2026-01-16 20:16 ` [PATCH bpf-next v2 11/13] selftests/bpf: Migrate struct_ops_assoc test " Ihor Solodrai
2026-01-20 1:59 ` Eduard Zingerman
2026-01-20 18:20 ` Ihor Solodrai
2026-01-20 18:24 ` Eduard Zingerman
2026-01-16 20:16 ` [PATCH bpf-next v2 12/13] bpf: Remove __prog kfunc arg annotation Ihor Solodrai
2026-01-20 2:01 ` Eduard Zingerman
2026-01-16 20:17 ` [PATCH bpf-next v2 13/13] bpf,docs: Document KF_IMPLICIT_ARGS flag Ihor Solodrai
2026-01-20 1:49 ` [PATCH bpf-next v2 00/13] bpf: Kernel functions with KF_IMPLICIT_ARGS Eduard Zingerman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=c3a04c5063cc9b68f9719c27ab1acb01b199ca3b.camel@gmail.com \
--to=eddyz87@gmail.com \
--cc=alan.maguire@oracle.com \
--cc=ameryhung@gmail.com \
--cc=andrii.nakryiko@gmail.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bentiss@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=ihor.solodrai@linux.dev \
--cc=jikos@kernel.org \
--cc=linux-input@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=martin.lau@linux.dev \
--cc=sched-ext@lists.linux.dev \
--cc=tj@kernel.org \
--cc=yatsenko@meta.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox