> > > > > > > On Fri, 12 Apr 2019 15:20:37 -0500 Honnappa Nagarahalli > > > > > > > <honnappa.nagaraha...@arm.com> wrote: > > > > > > > > > > > > > > > Add RCU library supporting quiescent state based memory > > > > > > > > reclamation > > > > > > > method. > > > > > > > > This library helps identify the quiescent state of the > > > > > > > > reader threads so that the writers can free the memory > > > > > > > > associated with the lock less data structures. > > > > > > > > > > > > > > > > Signed-off-by: Honnappa Nagarahalli > > > > > > > > <honnappa.nagaraha...@arm.com> > > > > > > > > Reviewed-by: Steve Capper <steve.cap...@arm.com> > > > > > > > > Reviewed-by: Gavin Hu <gavin...@arm.com> > > > > > > > > Reviewed-by: Ola Liljedahl <ola.liljed...@arm.com> > > > > > > > > Acked-by: Konstantin Ananyev > > > > > > > > <konstantin.anan...@intel.com> > > > > > > > > > > > > > > After evaluating long term API/ABI issues, I think you need > > > > > > > to get rid of almost all use of inline and visible > > > > > > > structures. Yes it might be marginally slower, but you thank me > the first time you have to fix something. > > > > > > > > > > > > > Agree, I was planning on another version to address this (I am yet > to take a look at your patch addressing the ABI). > > > > > > The structure visibility definitely needs to be addressed. > > > > > > For the inline functions, is the plan to convert all the > > > > > > inline functions in DPDK? If yes, I think we need to consider > > > > > > the performance > > > > > difference. May be consider L3-fwd application, change all the > inline functions in its path and run a test? > > > > > > > > > > Every function that is not in the direct datapath should not be > inline. > > > > > Exceptions or things like rx/tx burst, ring enqueue/dequeue, and > > > > > packet alloc/free I do not understand how DPDK can claim ABI compatibility if we have inline functions (unless we freeze any development in these inline functions forever).
> > > > > > > > Plus synchronization routines: spin/rwlock/barrier, etc. > > > > I think rcu should be one of such exceptions - it is just another > > > > synchronization mechanism after all (just a bit more sophisticated). > > > > Konstantin > > > > > > If you look at the other userspace RCU, you wil see that the only > > > inlines are the rcu_read_lock,rcu_read_unlock and > rcu_reference/rcu_assign_pointer. > > > > > > The synchronization logic is all real functions. > > > > In fact, I think urcu provides both flavors: > > https://github.com/urcu/userspace- > rcu/blob/master/include/urcu/static/ > > urcu-qsbr.h I still don't understand why we have to treat it > > differently then let say spin-lock/ticket-lock or rwlock. > > If we gone all the way to create our own version of rcu, we probably > > want it to be as fast as possible (I know that main speedup should > > come from the fact that readers don't have to wait for writer to > > finish, but still...) > > > > Konstantin > > > > Having locking functions inline is already a problem in current releases. > The implementation can not be improved without breaking ABI (or doing > special workarounds like lock v2) I think ABI and inline function discussion needs to be taken up in a different thread. Currently, I am looking to hide the structure visibility. I looked at your patch [1], it is a different case than what I have in this patch. It is a pretty generic use case as well (similar situation exists in other libraries). I think a generic solution should be agreed upon. If we have to hide the structure content, the handle to QS variable returned to the application needs to be opaque. I suggest using 'void *' behind which any structure can be used. typedef void * rte_rcu_qsbr_t; typedef void * rte_hash_t; But it requires typecasting. [1] http://patchwork.dpdk.org/cover/52609/
