On Fri, Dec 8, 2023 at 2:32 AM Hou Tao <houtao@xxxxxxxxxxxxxxx> wrote:
>
> From: Hou Tao <houtao1@xxxxxxxxxx>
>
> There is no rcu-read-lock requirement for ops->map_fd_get_ptr() or
> ops->map_fd_put_ptr(), so doesn't use rcu-read-lock for these two
> callbacks.
>
> For bpf_fd_array_map_update_elem(), accessing array->ptrs doesn't need
> rcu-read-lock because array->ptrs will not be freed until the map-in-map
> is released. For bpf_fd_htab_map_update_elem(), htab_map_update_elem()
> requires rcu-read-lock to be held, so only use rcu_read_lock() during
> the invocation of htab_map_update_elem().
>
> Signed-off-by: Hou Tao <houtao1@xxxxxxxxxx>
> ---
> kernel/bpf/hashtab.c | 2 ++
> kernel/bpf/syscall.c | 4 ----
> 2 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
> index b777bd8d4f8d..50b539c11b29 100644
> --- a/kernel/bpf/hashtab.c
> +++ b/kernel/bpf/hashtab.c
> @@ -2525,7 +2525,9 @@ int bpf_fd_htab_map_update_elem(struct bpf_map *map, struct file *map_file,
> if (IS_ERR(ptr))
> return PTR_ERR(ptr);
>
> + rcu_read_lock();
> ret = htab_map_update_elem(map, key, &ptr, map_flags);
> + rcu_read_unlock();
> if (ret)
> map->ops->map_fd_put_ptr(map, ptr, false);
>
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 6b9d7990d95f..fd9b73e02c7a 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -190,15 +190,11 @@ static int bpf_map_update_value(struct bpf_map *map, struct file *map_file,
> err = bpf_percpu_cgroup_storage_update(map, key, value,
> flags);
> } else if (IS_FD_ARRAY(map)) {
> - rcu_read_lock();
> err = bpf_fd_array_map_update_elem(map, map_file, key, value,
> flags);
> - rcu_read_unlock();
> } else if (map->map_type == BPF_MAP_TYPE_HASH_OF_MAPS) {
> - rcu_read_lock();
> err = bpf_fd_htab_map_update_elem(map, map_file, key, value,
> flags);
> - rcu_read_unlock();
I feel it's inconsistent to treat an array of FDs differently than
hashmap of FDs.
The patch is correct, but the users shouldn't be exposed
to array vs hashtab implementation details.
