[dpdk-dev] [PATCH v5 1/3] rcu: add RCU library supporting QSBR mechanism

[Ananyev, Konstantin]

> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen at networkplumber.org]
> Sent: Monday, April 15, 2019 4:39 PM
> To: Ananyev, Konstantin <konstantin.ananyev at intel.com>
> Cc: Honnappa Nagarahalli <Honnappa.Nagarahalli at arm.com>; paulmck at linux.ibm.com; Kovacevic, Marko
> <marko.kovacevic at intel.com>; dev at dpdk.org; Gavin Hu (Arm Technology China) <Gavin.Hu at arm.com>; Dharmik Thakkar
> <Dharmik.Thakkar at arm.com>; Malvika Gupta <Malvika.Gupta at arm.com>; nd <nd at arm.com>
> Subject: Re: [PATCH v5 1/3] rcu: add RCU library supporting QSBR mechanism
> 
> On Mon, 15 Apr 2019 12:24:47 +0000
> "Ananyev, Konstantin" <konstantin.ananyev at intel.com> wrote:
> 
> > > -----Original Message-----
> > > From: Stephen Hemminger [mailto:stephen at networkplumber.org]
> > > Sent: Saturday, April 13, 2019 12:06 AM
> > > To: Honnappa Nagarahalli <Honnappa.Nagarahalli at arm.com>
> > > Cc: Ananyev, Konstantin <konstantin.ananyev at intel.com>; paulmck at linux.ibm.com; Kovacevic, Marko <marko.kovacevic at intel.com>;
> > > dev at dpdk.org; Gavin Hu (Arm Technology China) <Gavin.Hu at arm.com>; Dharmik Thakkar <Dharmik.Thakkar at arm.com>; Malvika
> Gupta
> > > <Malvika.Gupta at arm.com>; nd <nd at arm.com>
> > > Subject: Re: [PATCH v5 1/3] rcu: add RCU library supporting QSBR mechanism
> > >
> > > On Fri, 12 Apr 2019 22:24:45 +0000
> > > Honnappa Nagarahalli <Honnappa.Nagarahalli at arm.com> wrote:
> > >
> > > > >
> > > > > On Fri, 12 Apr 2019 15:20:37 -0500
> > > > > Honnappa Nagarahalli <honnappa.nagarahalli at 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.nagarahalli at arm.com>
> > > > > > Reviewed-by: Steve Capper <steve.capper at arm.com>
> > > > > > Reviewed-by: Gavin Hu <gavin.hu at arm.com>
> > > > > > Reviewed-by: Ola Liljedahl <ola.liljedahl at arm.com>
> > > > > > Acked-by: Konstantin Ananyev <konstantin.ananyev at 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
> >
> > 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