lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c3a04c5063cc9b68f9719c27ab1acb01b199ca3b.camel@gmail.com>
Date: Mon, 19 Jan 2026 16:24:30 -0800
From: Eduard Zingerman <eddyz87@...il.com>
To: Ihor Solodrai <ihor.solodrai@...ux.dev>, Andrii Nakryiko
	 <andrii.nakryiko@...il.com>
Cc: Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann
 <daniel@...earbox.net>,  Andrii Nakryiko <andrii@...nel.org>, Martin KaFai
 Lau <martin.lau@...ux.dev>, Mykyta Yatsenko <yatsenko@...a.com>,  Tejun Heo
 <tj@...nel.org>, Alan Maguire <alan.maguire@...cle.com>, Benjamin Tissoires
	 <bentiss@...nel.org>, Jiri Kosina <jikos@...nel.org>, Amery Hung	
 <ameryhung@...il.com>, bpf@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-input@...r.kernel.org, sched-ext@...ts.linux.dev
Subject: Re: [PATCH bpf-next v2 05/13] resolve_btfids: Support for
 KF_IMPLICIT_ARGS

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.

[...]

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ