On Fri 10 Aug 2018 at 21:45, Cong Wang <xiyou.wangc...@gmail.com> wrote: > On Fri, Aug 10, 2018 at 3:29 AM Vlad Buslov <vla...@mellanox.com> wrote: >> >> Approach you suggest is valid, but has its own trade-offs: >> >> - As you noted, lock granularity becomes coarse-grained due to per-netns >> scope. > > Sure, you acquire idrinfo->lock too, the only difference is how long > you take it. > > The bottleneck of your approach is the same, also you take idrinfo->lock > twice, so the contention is heavier. > > >> >> - I am not sure it is possible to call idr_replace() without obtaining >> idrinfo->lock in this particular case. Concurrent delete of action with >> same id is possible and, according to idr_replace() description, >> unlocked execution is not supported for such use-case: > > But we can hold its refcnt before releasing idrinfo->lock, so > idr_replace() can't race with concurrent delete.
Yes, for concurrent delete case I agree. Action is removed from idr only when last reference is released and, in case of existing action update, init holds a reference. What about case when multiple task race to update the same existing action? I assume idr_replace() can be used for such case, but what would be the algorithm in case init replaced some other action, and not the action it actually copied before calling idr_replace()? > > >> >> - High rate or replace request will generate a lot of unnecessary memory >> allocations and deallocations. >> > > Yes, this is literally how RCU works, always allocate and copy, > release upon error. > > Also, if this is really a problem, we have SLAB_TYPESAFE_BY_RCU > too. ;) Current action update implementation is in-place, so there is no "copy" stage, besides members of some actions that are RCU-pointers. But I guess it makes sense if your goal is to refactor all actions to be updated with RCU.
