On Thu, Dec 14, 2023 at 11:15 AM John Fastabend <john.fastabend@xxxxxxxxx> wrote: > > Alexei Starovoitov wrote: > > On Wed, Dec 13, 2023 at 11:31 PM Hou Tao <houtao@xxxxxxxxxxxxxxx> wrote: > > > > > > Hi, > > > > > > On 12/14/2023 2:22 PM, John Fastabend wrote: > > > > Hou Tao 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 must still be allocated. For > > > >> bpf_fd_htab_map_update_elem(), htab_map_update_elem() only requires > > > >> rcu-read-lock to be held to avoid the WARN_ON_ONCE(), so only use > > > >> rcu_read_lock() during the invocation of htab_map_update_elem(). > > > >> > > > >> Acked-by: Yonghong Song <yonghong.song@xxxxxxxxx> > > > >> Signed-off-by: Hou Tao <houtao1@xxxxxxxxxx> > > > >> --- > > > >> kernel/bpf/hashtab.c | 6 ++++++ > > > >> kernel/bpf/syscall.c | 4 ---- > > > >> 2 files changed, 6 insertions(+), 4 deletions(-) > > > >> > > > >> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c > > > >> index 5b9146fa825f..ec3bdcc6a3cf 100644 > > > >> --- a/kernel/bpf/hashtab.c > > > >> +++ b/kernel/bpf/hashtab.c > > > >> @@ -2523,7 +2523,13 @@ int bpf_fd_htab_map_update_elem(struct bpf_map *map, struct file *map_file, > > > >> if (IS_ERR(ptr)) > > > >> return PTR_ERR(ptr); > > > >> > > > >> + /* The htab bucket lock is always held during update operations in fd > > > >> + * htab map, and the following rcu_read_lock() is only used to avoid > > > >> + * the WARN_ON_ONCE in htab_map_update_elem(). > > > >> + */ > > Ah ok but isn't this comment wrong because you do need rcu read lock to do > the walk with lookup_nulls_elem_raw where there is no lock being held? And > then the subsequent copy in place is fine because you do have a lock. Ohh. You're correct. Not sure what I was thinking. Hou, could you please send a follow up to undo my braino.
