Re: [PATCH bpf-next v7 1/2] bpf: htab: Split htab_elem_lru and htab_elem_pcpu off of htab_elem

[T.J. Mercier]

On Mon, Sep 28, 2026 at 8:31 PM Mykyta Yatsenko
<mykyta.yatsenko5@xxxxxxxxx> wrote:
>
>
>
> On 9/28/26 4:46 PM, bot+bpf-ci@xxxxxxxxxx wrote:
> >> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
> >> index 53c99fe4f176..5db11a21ce98 100644
> >> --- a/kernel/bpf/hashtab.c
> >> +++ b/kernel/bpf/hashtab.c
> >
> > [ ... ]
> >
> >> @@ -828,8 +861,12 @@ static __always_inline void *__htab_lru_map_lookup_elem(struct bpf_map *map,
> >>      struct htab_elem *l = __htab_map_lookup_elem(map, key);
> >>
> >>      if (l) {
> >> -            if (mark)
> >> -                    bpf_lru_node_set_ref(&l->lru_node);
> >> +            if (mark) {
> >> +                    struct htab_elem_lru *l_lru =
> >> +                            container_of(l, struct htab_elem_lru, elem);
> >> +
> >> +                    bpf_lru_node_set_ref(&l_lru->lru_node);
> >> +            }
> >>              return htab_elem_value(l, map->key_size);
> >>      }
> >>
> >
> > [ ... ]
> >
> >> @@ -1310,16 +1349,19 @@ static long htab_map_update_elem(struct bpf_map *map, void *key, void *value,
> >>
> >>  static void htab_lru_push_free(struct bpf_htab *htab, struct htab_elem *elem)
> >>  {
> >> +    struct htab_elem_lru *l = container_of(elem, struct htab_elem_lru, elem);
> >> +
> >>      check_and_cancel_fields(htab, elem);
> >>      bpf_map_dec_elem_count(&htab->map);
> >> -    bpf_lru_push_free(&htab->lru, &elem->lru_node);
> >> +    bpf_lru_push_free(&htab->lru, &l->lru_node);
> >>  }
> >>
> >
> > [ ... ]
> >
> >> @@ -1500,15 +1543,18 @@ static long __htab_lru_percpu_map_update_elem(struct bpf_map *map, void *key,
> >>              goto err;
> >>
> >>      if (l_old) {
> >> -            bpf_lru_node_set_ref(&l_old->lru_node);
> >> +            struct htab_elem_lru *l_old_lru =
> >> +                    container_of(l_old, struct htab_elem_lru, elem);
> >> +
> >> +            bpf_lru_node_set_ref(&l_old_lru->lru_node);
> >>
> >>              /* per-cpu hash map can update value in-place */
> >>              pcpu_copy_value(htab, htab_elem_get_ptr(l_old, key_size),
> >
> > [ ... ]
> >
> >> @@ -2505,7 +2551,10 @@ static void *htab_lru_percpu_map_lookup_elem(struct bpf_map *map, void *key)
> >>      struct htab_elem *l = __htab_map_lookup_elem(map, key);
> >>
> >>      if (l) {
> >> -            bpf_lru_node_set_ref(&l->lru_node);
> >> +            struct htab_elem_lru *l_lru =
> >> +                    container_of(l, struct htab_elem_lru, elem);
> >> +
> >> +            bpf_lru_node_set_ref(&l_lru->lru_node);
> >>              return this_cpu_ptr(htab_elem_get_ptr(l, map->key_size));
> >>      }
> >>
> >> @@ -2521,7 +2570,10 @@ static void *htab_lru_percpu_map_lookup_percpu_elem(struct bpf_map *map, void *k
> >>
> >>      l = __htab_map_lookup_elem(map, key);
> >>      if (l) {
> >> -            bpf_lru_node_set_ref(&l->lru_node);
> >> +            struct htab_elem_lru *l_lru =
> >> +                    container_of(l, struct htab_elem_lru, elem);
> >> +
> >> +            bpf_lru_node_set_ref(&l_lru->lru_node);
> >>              return per_cpu_ptr(htab_elem_get_ptr(l, map->key_size), cpu);
> >>      }
> >>
> >
> > This isn't a bug, but would a small helper keep these call sites to one
> > line, matching the pcpu accessors this patch adds? Each site now repeats
> > the same container_of(l, struct htab_elem_lru, elem) boilerplate followed
> > by bpf_lru_node_set_ref(). The patch adds htab_elem_get_ptr_to_pptr() and
> > htab_elem_set_ptr_to_pptr() for the pcpu variant, but open-codes the
> > container_of for the LRU variant at every site.
> >
>
> this sounds like a good refactoring:
>
> static inline void bpf_lru_node_set_ref(struct htab_elem *l)
> {
>         struct htab_elem_lru *l_lru =
>                 container_of(l, struct htab_elem_lru, elem);
>
>         if (!READ_ONCE(l_lru.lru_node->ref))
>                 WRITE_ONCE(l_lru.lru_node, 1);
> }

The htab_elem type is internal to hashtab.c, so I've added a helper
htab_elem_lru_node() which makes all the callsites Sashiko mentioned
one liners.