On Tue, Aug 1, 2023 at 12:44 AM Paolo Abeni [off-list ref] wrote:

On Mon, 2023-07-31 at 20:58 -0700, John Fastabend wrote:

quoted

Jakub Sitnicki wrote:

quoted

On Fri, Jul 28, 2023 at 08:44 AM +02, tglozar@redhat.com wrote:

quoted

From: Tomas Glozar <tglozar@redhat.com>

Disabling preemption in sock_map_sk_acquire conflicts with GFP_ATOMIC
allocation later in sk_psock_init_link on PREEMPT_RT kernels, since
GFP_ATOMIC might sleep on RT (see bpf: Make BPF and PREEMPT_RT co-exist
patchset notes for details).

This causes calling bpf_map_update_elem on BPF_MAP_TYPE_SOCKMAP maps to
BUG (sleeping function called from invalid context) on RT kernels.

preempt_disable was introduced together with lock_sk and rcu_read_lock
in commit 99ba2b5aba24e ("bpf: sockhash, disallow bpf_tcp_close and update
in parallel"), probably to match disabled migration of BPF programs, and
is no longer necessary.

Remove preempt_disable to fix BUG in sock_map_update_common on RT.

Signed-off-by: Tomas Glozar <tglozar@redhat.com>
---

We disable softirq and hold a spin lock when modifying the map/hash in
sock_{map,hash}_update_common so this LGTM:

Agree.

quoted

Reviewed-by: Jakub Sitnicki <jakub@cloudflare.com>

Reviewed-by: John Fastabend <john.fastabend@gmail.com>

quoted

You might want some extra tags:

Link: https://lore.kernel.org/all/20200224140131.461979697@linutronix.de/ (local)
Fixes: 99ba2b5aba24 ("bpf: sockhash, disallow bpf_tcp_close and update in parallel")

ENOCOFFEE here (which is never an excuse, but at least today is really
true), but I considered you may want this patch via the ebpf tree only
after applying it to net.

Hopefully should not be tragic, but please let me know if you prefer
the change reverted from net and going via the other path.

It's fine. Probably it shouldn't conflict with other sockmap fixes.