On Tue, 16 Apr 2019 05:29:21 +0000 Honnappa Nagarahalli <honnappa.nagaraha...@arm.com> wrote:
> > > > > > > > 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/ C allows structure to be defined without knowing what is in it therefore. typedef struct rte_rcu_qsbr rte_rcu_qsbr_t; is preferred (or do it without typedef) struct rte_rcu_qsbr;
