| Message ID | 20241016-mptcp-sched-find-rcu-v1-1-5e9af4fbce11@kernel.org (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Commit | e80dfc53fa3dcea1388de176c30d1f182373ec6e |
| Delegated to: | Matthieu Baerts |
| Headers | show |
| Series | mptcp: "fix suspicious RCU usage" warnings | expand |
| Context | Check | Description |
|---|---|---|
| matttbe/build | success | Build and static analysis OK |
| matttbe/checkpatch | warning | total: 0 errors, 1 warnings, 0 checks, 10 lines checked |
| matttbe/shellcheck | success | MPTCP selftests files have not been modified |
| matttbe/KVM_Validation__normal | success | Success! ✅ |
| matttbe/KVM_Validation__debug | success | Success! ✅ |
| matttbe/KVM_Validation__btf__only_bpftest_all_ | success | Success! ✅ |
Hi Matt, Thanks for this fix. On Wed, 2024-10-16 at 21:05 +0200, Matthieu Baerts (NGI0) wrote: > Enabling CONFIG_PROVE_RCU_LIST with its dependence CONFIG_RCU_EXPERT > creates this splat when an MPTCP socket is created: > > ============================= > WARNING: suspicious RCU usage > 6.12.0-rc2+ #11 Not tainted > ----------------------------- > net/mptcp/sched.c:44 RCU-list traversed in non-reader section!! > > other info that might help us debug this: > > rcu_scheduler_active = 2, debug_locks = 1 > no locks held by mptcp_connect/176. > > stack backtrace: > CPU: 0 UID: 0 PID: 176 Comm: mptcp_connect Not tainted 6.12.0-rc2+ > #11 > Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011 > Call Trace: > <TASK> > dump_stack_lvl (lib/dump_stack.c:123) > lockdep_rcu_suspicious (kernel/locking/lockdep.c:6822) > mptcp_sched_find (net/mptcp/sched.c:44 (discriminator 7)) > mptcp_init_sock (net/mptcp/protocol.c:2867 (discriminator 1)) > ? sock_init_data_uid (arch/x86/include/asm/atomic.h:28) > inet_create.part.0.constprop.0 (net/ipv4/af_inet.c:386) > ? __sock_create (include/linux/rcupdate.h:347 (discriminator 1)) > __sock_create (net/socket.c:1576) > __sys_socket (net/socket.c:1671) > ? __pfx___sys_socket (net/socket.c:1712) > ? do_user_addr_fault (arch/x86/mm/fault.c:1419 (discriminator 1)) > __x64_sys_socket (net/socket.c:1728) > do_syscall_64 (arch/x86/entry/common.c:52 (discriminator 1)) > entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130) > > That's because when the socket is initialised, rcu_read_lock() is not > used despite the explicit comment written above the declaration of > mptcp_sched_find() in sched.c. Adding the missing lock/unlock avoids > the > warning. > > Fixes: 1730b2b2c5a5 ("mptcp: add sched in mptcp_sock") > Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/523 > Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> Reviewed-by: Geliang Tang <geliang@kernel.org> Good catch! Some code in tcp_ca_dst_init() uses rcu_read_lock() too: rcu_read_lock(); ca = tcp_ca_find_key(ca_key); if (likely(ca && bpf_try_module_get(ca, ca->owner))) { bpf_module_put(...); icsk->icsk_ca_dst_locked = tcp_ca_dst_locked(dst); icsk->icsk_ca_ops = ca; } rcu_read_unlock(); I will also sync this part of the changes to BPF path manager code which is under review. -Geliang > --- > net/mptcp/protocol.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c > index > e420ce9bbfb6e0527ed3ce8cbe2a0990c6366d12..21bc3586c33e16471056fedf49e > e044ba27731d9 100644 > --- a/net/mptcp/protocol.c > +++ b/net/mptcp/protocol.c > @@ -2864,8 +2864,10 @@ static int mptcp_init_sock(struct sock *sk) > if (unlikely(!net->mib.mptcp_statistics) && !mptcp_mib_alloc(net)) > return -ENOMEM; > > + rcu_read_lock(); > ret = mptcp_init_sched(mptcp_sk(sk), > mptcp_sched_find(mptcp_get_scheduler(net))); > + rcu_read_unlock(); > if (ret) > return ret; > >
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c index e420ce9bbfb6e0527ed3ce8cbe2a0990c6366d12..21bc3586c33e16471056fedf49ee044ba27731d9 100644 --- a/net/mptcp/protocol.c +++ b/net/mptcp/protocol.c @@ -2864,8 +2864,10 @@ static int mptcp_init_sock(struct sock *sk) if (unlikely(!net->mib.mptcp_statistics) && !mptcp_mib_alloc(net)) return -ENOMEM; + rcu_read_lock(); ret = mptcp_init_sched(mptcp_sk(sk), mptcp_sched_find(mptcp_get_scheduler(net))); + rcu_read_unlock(); if (ret) return ret;
Enabling CONFIG_PROVE_RCU_LIST with its dependence CONFIG_RCU_EXPERT creates this splat when an MPTCP socket is created: ============================= WARNING: suspicious RCU usage 6.12.0-rc2+ #11 Not tainted ----------------------------- net/mptcp/sched.c:44 RCU-list traversed in non-reader section!! other info that might help us debug this: rcu_scheduler_active = 2, debug_locks = 1 no locks held by mptcp_connect/176. stack backtrace: CPU: 0 UID: 0 PID: 176 Comm: mptcp_connect Not tainted 6.12.0-rc2+ #11 Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011 Call Trace: <TASK> dump_stack_lvl (lib/dump_stack.c:123) lockdep_rcu_suspicious (kernel/locking/lockdep.c:6822) mptcp_sched_find (net/mptcp/sched.c:44 (discriminator 7)) mptcp_init_sock (net/mptcp/protocol.c:2867 (discriminator 1)) ? sock_init_data_uid (arch/x86/include/asm/atomic.h:28) inet_create.part.0.constprop.0 (net/ipv4/af_inet.c:386) ? __sock_create (include/linux/rcupdate.h:347 (discriminator 1)) __sock_create (net/socket.c:1576) __sys_socket (net/socket.c:1671) ? __pfx___sys_socket (net/socket.c:1712) ? do_user_addr_fault (arch/x86/mm/fault.c:1419 (discriminator 1)) __x64_sys_socket (net/socket.c:1728) do_syscall_64 (arch/x86/entry/common.c:52 (discriminator 1)) entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130) That's because when the socket is initialised, rcu_read_lock() is not used despite the explicit comment written above the declaration of mptcp_sched_find() in sched.c. Adding the missing lock/unlock avoids the warning. Fixes: 1730b2b2c5a5 ("mptcp: add sched in mptcp_sock") Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/523 Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> --- net/mptcp/protocol.c | 2 ++ 1 file changed, 2 insertions(+)