Re: [PATCH bpf-next v3 01/10] bpf: Implement resizable hashmap basic functions

[Mykyta Yatsenko <mykyta.yatsenko5@gmail.com>]

On 4/24/26 9:45 PM, bot+bpf-ci@kernel.org wrote:
>> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
>> index 3dd9b4924ae4..a37bd2a7b30f 100644
>> --- a/kernel/bpf/hashtab.c
>> +++ b/kernel/bpf/hashtab.c
> 
> [ ... ]
> 
>> +static int rhtab_map_check_btf(struct bpf_map *map, const struct btf *btf,
>> +			       const struct btf_type *key_type,
>> +			       const struct btf_type *value_type)
>> +{
>> +	struct bpf_rhtab *rhtab = container_of(map, struct bpf_rhtab, map);
>> +
>> +	return bpf_ma_set_dtor(map, &rhtab->ma, rhtab_mem_dtor);
>> +}
> 
> Does bpf_ma_set_dtor() exist at this commit?
> 
> A git-wide search shows no definition or declaration of bpf_ma_set_dtor().
> The kernel provides bpf_mem_alloc_set_dtor() with a 4-argument signature
> (ma, dtor, dtor_ctx_free, ctx) in include/linux/bpf_mem_alloc.h:
> 
>     int bpf_mem_alloc_set_dtor(struct bpf_mem_alloc *ma, dtor_fn dtor,
>                                dtor_ctx_fn dtor_ctx_free, void *ctx);
> 
> The existing htab_set_dtor() in kernel/bpf/hashtab.c builds a struct
> htab_btf_record context and calls bpf_mem_alloc_set_dtor() with all four
> arguments.
> 
> The new rhtab_map_check_btf() attempts to call a not-yet-existing helper
> with a 3-argument signature. This breaks compilation with 'implicit
> declaration of function "bpf_ma_set_dtor"' (fatal with kernel's
> -Werror=implicit-function-declaration) or link failure with 'undefined
> reference to bpf_ma_set_dtor'.
> 
> Looking at the later commit b3eef0fdafe1 ("bpf: Allow timers, workqueues
> and task_work in resizable hashtab"), it renames htab_set_dtor() to
> bpf_ma_set_dtor() and changes its signature to (map, ma, dtor), providing
> the missing symbol. That commit also wires up .map_check_btf =
> rhtab_map_check_btf in rhtab_map_ops.
> 
> Can this commit be reordered or squashed to avoid the forward reference
> and maintain bisectability?

this is right, this function should be added later, at the point of this
commit special fields are not supported, so this is not needed anyway.

> 
>> +static void rhtab_free_elem(void *ptr, void *arg)
>> +{
>> +	struct bpf_rhtab *rhtab = arg;
>> +	struct rhtab_elem *elem = ptr;
>> +
>> +	bpf_map_free_internal_structs(&rhtab->map, rhtab_elem_value(elem, rhtab->map.key_size));
>> +	bpf_mem_cache_free_rcu(&rhtab->ma, elem);
>> +}
> 
> [ ... ]
> 
>> +static int rhtab_delete_elem(struct bpf_rhtab *rhtab, struct rhtab_elem *elem)
>> +{
>> +	int err;
>> +
>> +	err = rhashtable_remove_fast(&rhtab->ht, &elem->node, rhtab->params);
>> +	if (err)
>> +		return err;
>> +
>> +	bpf_mem_cache_free_rcu(&rhtab->ma, elem);
>> +	return 0;
>> +}
> 
> Does rhtab_delete_elem() handle special fields properly?
> 
> Looking at the v2 review thread
> (https://lore.kernel.org/bpf/DHSBDLRKCAQO.1IPJTSEMPESV7@etsalapatis.com/),
> Emil Tsalapatis and sashiko raised concerns that special fields are not
> handled properly during in-place updates. The v3 changelog claims this was
> addressed, but rhtab_delete_elem() does not call
> bpf_map_free_internal_structs() before freeing the element.
> 
> This is inconsistent with rhtab_free_elem() which properly calls
> bpf_map_free_internal_structs(). Could this lead to resource leaks for
> maps with timers, workqueues, or other internal structures when elements
> are deleted via rhtab_map_delete_elem()?
> 
At this point special fields are not supported, the corresponding changes
are introduced in commit 5.

> 
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
> 
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/24909591959