Re: [PATCH bpf-next v5 2/2] bpf: htab: Reduce elem_size by 8 bytes for small key sizes

[T.J. Mercier]

On Sun, Sep 20, 2026 at 5:57 PM Alexei Starovoitov
<alexei.starovoitov@xxxxxxxxx> wrote:
>
> On Sun, Sep 20, 2026 at 05:22 PM T.J. Mercier <tjmercier@xxxxxxxxxx> wrote:
> > +     l = __lookup_elem_raw(htab, head, hash, key, key_size, &n);
> > +     if (l) {
> > +             /*
> > +              * Pairs with smp_wmb() in alloc_htab_elem() to ensure
> > +              * value/pptr reads happen after key/hash match on
> > +              * recycled elements.
> > +              */
> > +             smp_rmb();
> > +             return l;
> > +     }
>
> Is this AI suggestion to add barriers?

This is the issue that Sashiko found with lockless readers when I ran
it locally. Before this change a lookup with a pointer to a recycled
element had to match both hash and key before accessing the element.
The ordering of those assignments meant that they were mismatched
during initialization of the recycled element until the hash
assignment at the very bottom of alloc_htab_elem(), so
lookup_nulls_elem_raw() would reject any partially initialized
elements (unless there happened to be a hash collision with the old
and new keys).
Now without hash, only key can be checked. Keeping the key assignment
early in alloc_htab_elem() would mean that a lookup might match the
key and look at pptr or value which haven't been reassigned yet. So
for hashless elements I moved the key assignment to the bottom of
alloc_htab_elem() like hash.

That leaves the old key in the element while the new value is
assigned, so a lookup racing with deletion and immediate reallocation
can match the old key and observe the new value. But that is already
the situation today. lookup_nulls_elem_raw() can pass the key
comparison just before the element is deleted and recycled, and then
its caller can read the new value later after the htab_elem pointer is
returned from lookup_nulls_elem_raw().

So the relocation of the key assignment depends on it actually
happening last, but on arm64 we have to worry about weak ordering...
thus the smp_wmb() and this read barrier to ensure the element is
fully initialized before accessing value / ppr.

Today hash doesn't have the memory barrier protection so it's not a
new issue. I haven't observed issues in my testing but most of that
has been on x86 not arm64.

> I think it should be on the program side.
> If users really care then they will add such barriers.
> Doing it unconditionally will cost performance for everyone.
> Especially on arm64.

This is fair, a program doing concurrent, lockless updates and deletes
should know what it's doing and should be able to provide its own
barriers. So I'll drop these new barriers.

> If you disagree, please provide ./bench bpf-hashmap-lookup numbers on arm64
> to demonstrate that perf is the same.
>
> pw-bot: cr