diff mbox series

[net-next,v1,2/4] net: protect net_devmem_dmabuf_bindings by new net_devmem_bindings_mutex

Message ID 20250307155725.219009-3-sdf@fomichev.me (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series net: remove rtnl_lock from the callers of queue APIs | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 8 of 8 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 73 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest fail net-next-2025-03-08--03-00 (tests: 32)

Commit Message

Stanislav Fomichev March 7, 2025, 3:57 p.m. UTC 
In the process of making queue management API rtnl_lock-less, we
need a separate lock to protect xa that keeps a global list of bindings.

Also change the ordering of 'posting' binding to
net_devmem_dmabuf_bindings: xa_alloc is done after binding is fully
initialized (so xa_load lookups fully instantiated bindings) and
xa_erase is done as a first step during unbind.

Cc: Mina Almasry <almasrymina@google.com>
Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
---
 net/core/devmem.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

Comments

Jakub Kicinski March 7, 2025, 5:49 p.m. UTC | #1 
On Fri,  7 Mar 2025 07:57:23 -0800 Stanislav Fomichev wrote:
> In the process of making queue management API rtnl_lock-less, we
> need a separate lock to protect xa that keeps a global list of bindings.
> 
> Also change the ordering of 'posting' binding to
> net_devmem_dmabuf_bindings: xa_alloc is done after binding is fully
> initialized (so xa_load lookups fully instantiated bindings) and
> xa_erase is done as a first step during unbind.

You're just wrapping the calls to xarray here, is there a plan to use
this new lock for other things? xarray has a built in spin lock, we
don't have to protect it.
Stanislav Fomichev March 7, 2025, 7:36 p.m. UTC | #2 
On 03/07, Jakub Kicinski wrote:
> On Fri,  7 Mar 2025 07:57:23 -0800 Stanislav Fomichev wrote:
> > In the process of making queue management API rtnl_lock-less, we
> > need a separate lock to protect xa that keeps a global list of bindings.
> > 
> > Also change the ordering of 'posting' binding to
> > net_devmem_dmabuf_bindings: xa_alloc is done after binding is fully
> > initialized (so xa_load lookups fully instantiated bindings) and
> > xa_erase is done as a first step during unbind.
> 
> You're just wrapping the calls to xarray here, is there a plan to use
> this new lock for other things? xarray has a built in spin lock, we
> don't have to protect it.

Oh, that is true, I completely missed that. In this case I can drop this
patch, thanks!
diff mbox series

Patch

diff --git a/net/core/devmem.c b/net/core/devmem.c
index 7c6e0b5b6acb..c16cdac46bed 100644
--- a/net/core/devmem.c
+++ b/net/core/devmem.c
@@ -25,7 +25,7 @@ 
 
 /* Device memory support */
 
-/* Protected by rtnl_lock() */
+static DEFINE_MUTEX(net_devmem_bindings_mutex);
 static DEFINE_XARRAY_FLAGS(net_devmem_dmabuf_bindings, XA_FLAGS_ALLOC1);
 
 static const struct memory_provider_ops dmabuf_devmem_ops;
@@ -119,6 +119,10 @@  void net_devmem_unbind_dmabuf(struct net_devmem_dmabuf_binding *binding)
 	unsigned long xa_idx;
 	unsigned int rxq_idx;
 
+	mutex_lock(&net_devmem_bindings_mutex);
+	xa_erase(&net_devmem_dmabuf_bindings, binding->id);
+	mutex_unlock(&net_devmem_bindings_mutex);
+
 	if (binding->list.next)
 		list_del(&binding->list);
 
@@ -133,8 +137,6 @@  void net_devmem_unbind_dmabuf(struct net_devmem_dmabuf_binding *binding)
 		WARN_ON(netdev_rx_queue_restart(binding->dev, rxq_idx));
 	}
 
-	xa_erase(&net_devmem_dmabuf_bindings, binding->id);
-
 	net_devmem_dmabuf_binding_put(binding);
 }
 
@@ -220,24 +222,15 @@  net_devmem_bind_dmabuf(struct net_device *dev, unsigned int dmabuf_fd,
 	}
 
 	binding->dev = dev;
-
-	err = xa_alloc_cyclic(&net_devmem_dmabuf_bindings, &binding->id,
-			      binding, xa_limit_32b, &id_alloc_next,
-			      GFP_KERNEL);
-	if (err < 0)
-		goto err_free_binding;
-
 	xa_init_flags(&binding->bound_rxqs, XA_FLAGS_ALLOC);
-
 	refcount_set(&binding->ref, 1);
-
 	binding->dmabuf = dmabuf;
 
 	binding->attachment = dma_buf_attach(binding->dmabuf, dev->dev.parent);
 	if (IS_ERR(binding->attachment)) {
 		err = PTR_ERR(binding->attachment);
 		NL_SET_ERR_MSG(extack, "Failed to bind dmabuf to device");
-		goto err_free_id;
+		goto err_free_binding;
 	}
 
 	binding->sgt = dma_buf_map_attachment_unlocked(binding->attachment,
@@ -305,6 +298,14 @@  net_devmem_bind_dmabuf(struct net_device *dev, unsigned int dmabuf_fd,
 		virtual += len;
 	}
 
+	mutex_lock(&net_devmem_bindings_mutex);
+	err = xa_alloc_cyclic(&net_devmem_dmabuf_bindings, &binding->id,
+			      binding, xa_limit_32b, &id_alloc_next,
+			      GFP_KERNEL);
+	mutex_unlock(&net_devmem_bindings_mutex);
+	if (err < 0)
+		goto err_free_chunks;
+
 	return binding;
 
 err_free_chunks:
@@ -316,8 +317,6 @@  net_devmem_bind_dmabuf(struct net_device *dev, unsigned int dmabuf_fd,
 					  DMA_FROM_DEVICE);
 err_detach:
 	dma_buf_detach(dmabuf, binding->attachment);
-err_free_id:
-	xa_erase(&net_devmem_dmabuf_bindings, binding->id);
 err_free_binding:
 	kfree(binding);
 err_put_dmabuf: