> > > > > > > > > > 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;
I see that rte_hash library uses the same approach (struct rte_hash in rte_hash.h, though it is marking as internal). But the ABI Laboratory tool [1] seems to be reporting incorrect numbers for this library even though the internal structure is changed. [1] https://abi-laboratory.pro/index.php?view=compat_report&l=dpdk&v1=19.02&v2=current&obj=66794&kind=abi
