| Message ID | 20250324224537.248800-2-kuba@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | ba6f418fbf64bb8c0e98dc1b548c151beeedd16c |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: skip taking rtnl_lock for queue GET | expand |
On Mon, Mar 24, 2025 at 3:46 PM Jakub Kicinski <kuba@kernel.org> wrote: > > A recent commit added taking the netdev instance lock > in netdev_nl_bind_rx_doit(), but didn't remove it in > net_devmem_unbind_dmabuf() which it calls from an error path. > Always expect the callers of net_devmem_unbind_dmabuf() to > hold the lock. This is consistent with net_devmem_bind_dmabuf(). > > (Not so) coincidentally this also protects mp_param with the instance > lock, which the rest of this series needs. > > Fixes: 1d22d3060b9b ("net: drop rtnl_lock for queue_mgmt operations") > Signed-off-by: Jakub Kicinski <kuba@kernel.org> Sorry for not noticing this earlier. Reviewed-by: Mina Almasry <almasrymina@google.com> -- Thanks, Mina
diff --git a/net/core/devmem.c b/net/core/devmem.c index 6802e82a4d03..ee145a2aa41c 100644 --- a/net/core/devmem.c +++ b/net/core/devmem.c @@ -128,12 +128,10 @@ void net_devmem_unbind_dmabuf(struct net_devmem_dmabuf_binding *binding) rxq->mp_params.mp_priv = NULL; rxq->mp_params.mp_ops = NULL; - netdev_lock(binding->dev); rxq_idx = get_netdev_rx_queue_index(rxq); err = netdev_rx_queue_restart(binding->dev, rxq_idx); WARN_ON(err && err != -ENETDOWN); - netdev_unlock(binding->dev); } xa_erase(&net_devmem_dmabuf_bindings, binding->id); diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c index a186fea63c09..9e4882a22407 100644 --- a/net/core/netdev-genl.c +++ b/net/core/netdev-genl.c @@ -947,7 +947,9 @@ void netdev_nl_sock_priv_destroy(struct netdev_nl_sock *priv) mutex_lock(&priv->lock); list_for_each_entry_safe(binding, temp, &priv->bindings, list) { + netdev_lock(binding->dev); net_devmem_unbind_dmabuf(binding); + netdev_unlock(binding->dev); } mutex_unlock(&priv->lock); }
A recent commit added taking the netdev instance lock in netdev_nl_bind_rx_doit(), but didn't remove it in net_devmem_unbind_dmabuf() which it calls from an error path. Always expect the callers of net_devmem_unbind_dmabuf() to hold the lock. This is consistent with net_devmem_bind_dmabuf(). (Not so) coincidentally this also protects mp_param with the instance lock, which the rest of this series needs. Fixes: 1d22d3060b9b ("net: drop rtnl_lock for queue_mgmt operations") Signed-off-by: Jakub Kicinski <kuba@kernel.org> --- v2: - rebase on a70f891e0fa0 ("net: devmem: do not WARN conditionally after netdev_rx_queue_restart()") v1: https://lore.kernel.org/20250312223507.805719-2-kuba@kernel.org --- net/core/devmem.c | 2 -- net/core/netdev-genl.c | 2 ++ 2 files changed, 2 insertions(+), 2 deletions(-)