Re: [PATCH net-next v2] net: convert netdev_nl_sock bindings list to xarray

On Fri, Aug 14, 2026 at 6:50 AM Jakub Kicinski <[email protected]> wrote:
>
> On Fri, 14 Aug 2026 12:04:59 +0530 Shivaji Kant wrote:
> > I understand, my thinking for this was removing priv->lock is more
> > than just simplification, it can be a concurrency win. Sorry for not
> > making it more explicit in the patch description.
> > As you pointed out, priv->lock current serializes the entire doit
> > block (including netdev_get_by_index_lock() and the entire
> > net_devmem_bind_dmabuf_to_queue() loop). Furthermore,
> > net_devmem_unbind_dmabuf() blocks on synchronize_net(). With the
> > current mutex implementation, if one thread is destroying a socket or
> > unbinding a dmabuf, it holds priv->lock across synchronize_net(),
> > completely blocking any concurrent netlink commands on the same socket
> > (e.g., binding to a different queue/interface).
> > Converting to an xarray allows us to cleanly dissolve this wide
> > serialization scope. xa_for_each() handles the RCU grace periods
> > internally without holding a manual lock across synchronize_net(). I
> > agree the changelog must be updated to clearly state that reducing
> > this serialization scope is the explicit goal.
>
> If it's an optimization you have to show prod data (as in not a micro
> benchmark) that the problem actually exists.
>

Sorry for the late reply. My reasoning for this change is code
simplification, not optimization.

The history of priv->lock is that it was added to protect concurrent
access to priv->bindings and only locked around list access to
priv->bindings. But then Stan and Taehee ran into deadlocks in
relation with locking order with the netdev_lock and the binding->lock
so we ended up defining intricate (undocumented) locking order between
the 3 locks and expanding the code serialized by priv->lock and the
code now feels confusing. Removing 1 of these 3 locks to simplify the
mental model seemed like a clear win for me. We do not need priv->lock
if we use a data structure that manages its own concurrency like
xarray.

But this is not a huge issue worth burning your review cycles on. If
you don't want us to resolve the clashiko issues and follow up on
this, we will drop it indeed :-). Sorry about that!

-- 
Thanks,
Mina