On 2/17/2025 9:41 AM, Xiaomeng Zhang wrote:
> Remove redundant map_delete_elem callbacks with return -EOPNOTSUPP. We can
> directly return -EOPNOTSUPP when calling the unimplemented callbacks.
>
> Signed-off-by: Xiaomeng Zhang <zhangxiaomeng13@xxxxxxxxxx>
> ---
> kernel/bpf/arena.c | 6 ------
> kernel/bpf/bloom_filter.c | 6 ------
> kernel/bpf/helpers.c | 5 ++++-
> kernel/bpf/ringbuf.c | 7 -------
> kernel/bpf/syscall.c | 5 ++++-
> 5 files changed, 8 insertions(+), 21 deletions(-)
>
> diff --git a/kernel/bpf/arena.c b/kernel/bpf/arena.c
> index 2c95baa8ece2..50f07bbd33fa 100644
> --- a/kernel/bpf/arena.c
> +++ b/kernel/bpf/arena.c
> @@ -62,11 +62,6 @@ u64 bpf_arena_get_user_vm_start(struct bpf_arena *arena)
> return arena ? arena->user_vm_start : 0;
> }
>
> -static long arena_map_delete_elem(struct bpf_map *map, void *value)
> -{
> - return -EOPNOTSUPP;
> -}
> -
> static int arena_map_get_next_key(struct bpf_map *map, void *key, void *next_key)
> {
> return -EOPNOTSUPP;
> @@ -390,7 +385,6 @@ const struct bpf_map_ops arena_map_ops = {
> .map_get_next_key = arena_map_get_next_key,
> .map_lookup_elem = arena_map_lookup_elem,
> .map_update_elem = arena_map_update_elem,
> - .map_delete_elem = arena_map_delete_elem,
> .map_check_btf = arena_map_check_btf,
> .map_mem_usage = arena_map_mem_usage,
> .map_btf_id = &bpf_arena_map_btf_ids[0],
> diff --git a/kernel/bpf/bloom_filter.c b/kernel/bpf/bloom_filter.c
> index d8d4dd7b711d..f3bba8ac2532 100644
> --- a/kernel/bpf/bloom_filter.c
> +++ b/kernel/bpf/bloom_filter.c
> @@ -65,11 +65,6 @@ static long bloom_map_push_elem(struct bpf_map *map, void *value, u64 flags)
> return 0;
> }
>
> -static long bloom_map_delete_elem(struct bpf_map *map, void *value)
> -{
> - return -EOPNOTSUPP;
> -}
> -
> static int bloom_map_get_next_key(struct bpf_map *map, void *key, void *next_key)
> {
> return -EOPNOTSUPP;
> @@ -206,7 +201,6 @@ const struct bpf_map_ops bloom_filter_map_ops = {
> .map_peek_elem = bloom_map_peek_elem,
> .map_lookup_elem = bloom_map_lookup_elem,
> .map_update_elem = bloom_map_update_elem,
> - .map_delete_elem = bloom_map_delete_elem,
> .map_check_btf = bloom_map_check_btf,
> .map_mem_usage = bloom_map_mem_usage,
> .map_btf_id = &bpf_bloom_map_btf_ids[0],
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index cb61c06155c8..132c2830c758 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -74,7 +74,10 @@ BPF_CALL_2(bpf_map_delete_elem, struct bpf_map *, map, void *, key)
> {
> WARN_ON_ONCE(!rcu_read_lock_held() && !rcu_read_lock_trace_held() &&
> !rcu_read_lock_bh_held());
> - return map->ops->map_delete_elem(map, key);
> + if (map->ops->map_delete_elem)
> + return map->ops->map_delete_elem(map, key);
> + else
> + return -EOPNOTSUPP;
> }
>
It will be better to add the check for arena in
check_map_func_compatibility() statically, instead of adding an extra
branch for each invocation of bpf_map_delete_elem.
> const struct bpf_func_proto bpf_map_delete_elem_proto = {
> diff --git a/kernel/bpf/ringbuf.c b/kernel/bpf/ringbuf.c
> index 1499d8caa9a3..e2d22f10a11f 100644
> --- a/kernel/bpf/ringbuf.c
> +++ b/kernel/bpf/ringbuf.c
> @@ -247,11 +247,6 @@ static long ringbuf_map_update_elem(struct bpf_map *map, void *key, void *value,
> return -ENOTSUPP;
> }
>
> -static long ringbuf_map_delete_elem(struct bpf_map *map, void *key)
> -{
> - return -ENOTSUPP;
> -}
> -
> static int ringbuf_map_get_next_key(struct bpf_map *map, void *key,
> void *next_key)
> {
> @@ -356,7 +351,6 @@ const struct bpf_map_ops ringbuf_map_ops = {
> .map_poll = ringbuf_map_poll_kern,
> .map_lookup_elem = ringbuf_map_lookup_elem,
> .map_update_elem = ringbuf_map_update_elem,
> - .map_delete_elem = ringbuf_map_delete_elem,
> .map_get_next_key = ringbuf_map_get_next_key,
> .map_mem_usage = ringbuf_map_mem_usage,
> .map_btf_id = &ringbuf_map_btf_ids[0],
> @@ -371,7 +365,6 @@ const struct bpf_map_ops user_ringbuf_map_ops = {
> .map_poll = ringbuf_map_poll_user,
> .map_lookup_elem = ringbuf_map_lookup_elem,
> .map_update_elem = ringbuf_map_update_elem,
> - .map_delete_elem = ringbuf_map_delete_elem,
> .map_get_next_key = ringbuf_map_get_next_key,
> .map_mem_usage = ringbuf_map_mem_usage,
> .map_btf_id = &user_ringbuf_map_btf_ids[0],
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index c6f55283f4ff..36eed7016da4 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -1789,7 +1789,10 @@ static int map_delete_elem(union bpf_attr *attr, bpfptr_t uattr)
> } else if (IS_FD_PROG_ARRAY(map) ||
> map->map_type == BPF_MAP_TYPE_STRUCT_OPS) {
> /* These maps require sleepable context */
> - err = map->ops->map_delete_elem(map, key);
> + if (map->ops->map_delete_elem)
> + err = map->ops->map_delete_elem(map, key);
> + else
> + err = -EOPNOTSUPP;
> goto out;
The invocation has already checked the map type, the extra checking of
->map_delete_elem is unnecessary.
> }
>
