Messages in this thread | | | Date | Wed, 14 Feb 2018 18:58:38 +0100 | From | Michal Hocko <> | Subject | Re: [PATCH net] bpf: cpumap: use GFP_KERNEL instead of GFP_ATOMIC in __cpu_map_entry_alloc() |
| |
On Wed 14-02-18 18:34:51, Jesper Dangaard Brouer wrote: > On Wed, 14 Feb 2018 16:06:40 +0100 > Michal Hocko <mhocko@kernel.org> wrote: > > > On Wed 14-02-18 22:17:34, Jason Wang wrote: > > > There're several implications after commit 0bf7800f1799 ("ptr_ring: > > > try vmalloc() when kmalloc() fails") with the using of vmalloc() since > > > can't allow GFP_ATOMIC but mandate GFP_KERNEL. This will lead a WARN > > > since cpumap try to call with GFP_ATOMIC. Fortunately, entry > > > allocation of cpumap can only be done through syscall path which means > > > GFP_ATOMIC is not necessary, so fixing this by replacing GFP_ATOMIC > > > with GFP_KERNEL. > > > > map_update_elem does the following. Unless I am missing something and > > the callback doesn't call cpu_map_update_elem there then we are in a > > non-preemptible context there and GFP_WAIT would blow up. > > rcu_read_lock(); > > err = map->ops->map_update_elem(map, key, value, attr->flags); > > rcu_read_unlock(); > > Nope - you did miss something ;-) > > You are looking at the wrong place. Look at /kernel/bpf/syscall.c line 697. > > vim +697 kernel/bpf/syscall.c > [...] > } else if (map->map_type == BPF_MAP_TYPE_CPUMAP) { > err = map->ops->map_update_elem(map, key, value, attr->flags); > goto out; > } > > You missed that map type BPF_MAP_TYPE_CPUMAP is special cased, and > is moved outside rcu_read_{lock,unlock} (because it need to create some > kthreads). > > Further more the BPF-verifier disallow BPF programs runtime changing > the BPF_MAP_TYPE_CPUMAP. Right now, we disallow almost everything from > the bpf-side (even reading the value): > > vim +2057 kernel/bpf/verifier.c
OK, thanks for the clarification. I am not familiar with the code at all so I was merely looking at call sites and this one just hit my eyes.
-- Michal Hocko SUSE Labs
| |