| Message ID | 20240823142257.692667-1-jchapman@katalix.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 73d33bd063c4cfef3db17f9bec3d202928ed8631 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next,v2] l2tp: avoid using drain_workqueue in l2tp_pre_exit_net | expand |
Hello: This patch was applied to netdev/net-next.git (main) by Jakub Kicinski <kuba@kernel.org>: On Fri, 23 Aug 2024 15:22:57 +0100 you wrote: > Recent commit fc7ec7f554d7 ("l2tp: delete sessions using work queue") > incorrectly uses drain_workqueue. The use of drain_workqueue in > l2tp_pre_exit_net is flawed because the workqueue is shared by all > nets and it is therefore possible for new work items to be queued > for other nets while drain_workqueue runs. > > Instead of using drain_workqueue, use __flush_workqueue twice. The > first one will run all tunnel delete work items and any work already > queued. When tunnel delete work items are run, they may queue > new session delete work items, which the second __flush_workqueue will > run. > > [...] Here is the summary with links: - [net-next,v2] l2tp: avoid using drain_workqueue in l2tp_pre_exit_net https://git.kernel.org/netdev/net-next/c/73d33bd063c4 You are awesome, thank you!
diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index af87c781d6a6..e5e492284997 100644 --- a/net/l2tp/l2tp_core.c +++ b/net/l2tp/l2tp_core.c @@ -1856,14 +1856,14 @@ static __net_exit void l2tp_pre_exit_net(struct net *net) rcu_read_unlock_bh(); if (l2tp_wq) { - /* ensure that all TUNNEL_DELETE work items are run before - * draining the work queue since TUNNEL_DELETE requests may - * queue SESSION_DELETE work items for each session in the - * tunnel. drain_workqueue may otherwise warn if SESSION_DELETE - * requests are queued while the work queue is being drained. + /* Run all TUNNEL_DELETE work items just queued. */ + __flush_workqueue(l2tp_wq); + + /* Each TUNNEL_DELETE work item will queue a SESSION_DELETE + * work item for each session in the tunnel. Flush the + * workqueue again to process these. */ __flush_workqueue(l2tp_wq); - drain_workqueue(l2tp_wq); } } @@ -1871,8 +1871,11 @@ static __net_exit void l2tp_exit_net(struct net *net) { struct l2tp_net *pn = l2tp_pernet(net); + WARN_ON_ONCE(!idr_is_empty(&pn->l2tp_v2_session_idr)); idr_destroy(&pn->l2tp_v2_session_idr); + WARN_ON_ONCE(!idr_is_empty(&pn->l2tp_v3_session_idr)); idr_destroy(&pn->l2tp_v3_session_idr); + WARN_ON_ONCE(!idr_is_empty(&pn->l2tp_tunnel_idr)); idr_destroy(&pn->l2tp_tunnel_idr); }
Recent commit fc7ec7f554d7 ("l2tp: delete sessions using work queue") incorrectly uses drain_workqueue. The use of drain_workqueue in l2tp_pre_exit_net is flawed because the workqueue is shared by all nets and it is therefore possible for new work items to be queued for other nets while drain_workqueue runs. Instead of using drain_workqueue, use __flush_workqueue twice. The first one will run all tunnel delete work items and any work already queued. When tunnel delete work items are run, they may queue new session delete work items, which the second __flush_workqueue will run. In l2tp_exit_net, warn if any of the net's idr lists are not empty. Fixes: fc7ec7f554d7 ("l2tp: delete sessions using work queue") Signed-off-by: James Chapman <jchapman@katalix.com> --- v2: - remove unneeded per-net net_closing flag (paolo) - remove loop waiting for idr lists to be empty (paolo) - add correct fixes tag (paolo) - in l2tp_exit_net warn if idr lists not empty v1: https://lore.kernel.org/netdev/20240819145208.3209296-1-jchapman@katalix.com/ --- net/l2tp/l2tp_core.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-)