[dpdk-dev] [RFC 0/3] tqs: add thread quiescent state library

[Luca Boccassi]

On Fri, 2018-11-30 at 02:13 +0000, Honnappa Nagarahalli wrote:
> > 
> > > > > Mixed feelings about this one.
> > > > > 
> > > > > Love to see RCU used for more things since it is much better
> > > > > than
> > > > > reader/writer locks for many applications. But hate to see
> > > > > DPDK
> > > > > reinventing every other library and not reusing code.
> > > > > Userspace
> > > > > RCU https://liburcu.org/ is widely supported by distro's,
> > > > > more
> > > > > throughly tested and documented, and more flexiple.
> > > > > 
> > > > > The issue with many of these reinventions is a tradeoff of
> > > > > DPDK
> > > > > growing another dependency on external library versus using
> > > > > common
> > 
> > code.
> > > > > 
> > > 
> > > Agree with the dependency issues. Sometimes flexibility also
> > > causes confusion
> > 
> > and features that are not necessarily required for a targeted use
> > case. I have
> > seen that much of the functionality that can be left to the
> > application is
> > implemented as part of the library.
> > > I think having it in DPDK will give us control over the amount of
> > > capability this
> > 
> > library will have and freedom over changes we would like to make to
> > such a
> > library. I also view DPDK as one package where all things required
> > for data
> > plane development are available.
> > > 
> > > > > For RCU, the big issue for me is the testing and
> > > > > documentation of
> > > > > how to do RCU safely. Many people get it wrong!
> > > 
> > > Hopefully, we all will do a better job collectively :)
> > > 
> > > > 
> > 
> > Reinventing RCU is not helping anyone.
> 
> IMO, this depends on what the rte_tqs has to offer and what the
> requirements are. Before starting this patch, I looked at the liburcu
> APIs. I have to say, fairly quickly (no offense) I concluded that
> this does not address DPDK's needs. I took a deeper look at the
> APIs/code in the past day and I still concluded the same. My partial
> analysis (analysis of more APIs can be done, I do not have cycles at
> this point) is as follows:
> 
> The reader threads' information is maintained in a linked list[1].
> This linked list is protected by a mutex lock[2]. Any
> additions/deletions/traversals of this list are blocking and cannot
> happen in parallel.
> 
> The API, 'synchronize_rcu' [3] (similar functionality to
> rte_tqs_check call) is a blocking call. There is no option provided
> to make it non-blocking. The writer spins cycles while waiting for
> the grace period to get over.
> 
> 'synchronize_rcu' also has grace period lock [4]. If I have multiple
> writers running on data plane threads, I cannot call this API to
> reclaim the memory in the worker threads as it will block other
> worker threads. This means, there is an extra thread required (on the
> control plane?) which does garbage collection and a method to push
> the pointers from worker threads to the garbage collection thread.
> This also means the time duration from delete to free increases
> putting pressure on amount of memory held up.
> Since this API cannot be called concurrently by multiple writers,
> each writer has to wait for other writer's grace period to get over
> (i.e. multiple writer threads cannot overlap their grace periods).
> 
> This API also has to traverse the linked list which is not very well
> suited for calling on data plane.
> 
> I have not gone too much into rcu_thread_offline[5] API. This again
> needs to be used in worker cores and does not look to be very
> optimal.
> 
> I have glanced at rcu_quiescent_state [6], it wakes up the thread
> calling 'synchronize_rcu' which seems good amount of code for the
> data plane.
> 
> [1] https://github.com/urcu/userspace-rcu/blob/master/include/urcu/st
> atic/urcu-qsbr.h#L85
> [2] https://github.com/urcu/userspace-rcu/blob/master/src/urcu-qsbr.c
> #L68
> [3] https://github.com/urcu/userspace-rcu/blob/master/src/urcu-qsbr.c
> #L344
> [4] https://github.com/urcu/userspace-rcu/blob/master/src/urcu-qsbr.c
> #L58
> [5] https://github.com/urcu/userspace-rcu/blob/master/include/urcu/st
> atic/urcu-qsbr.h#L211
> [6] https://github.com/urcu/userspace-rcu/blob/master/include/urcu/st
> atic/urcu-qsbr.h#L193
> 
> Coming to what is provided in rte_tqs:
> 
> The synchronize_rcu functionality is split in to 2 APIs:
> rte_tqs_start and rte_tqs_check. The reader data is maintained as an
> array.
> 
> Both the APIs are lock-free, allowing them to be called from multiple
> threads concurrently. This allows multiple writers to wait for their
> grace periods concurrently as well as overlap their grace periods.
> rte_tqs_start API returns a token which provides the ability to
> separate the quiescent state waiting of different writers. Hence, no
> writer waits for other writer's grace period to get over. 
> Since these 2 APIs are lock-free, they can be called from writers
> running on worker cores as well without the need for a separate
> thread to do garbage collection.
> 
> The separation into 2 APIs provides the ability for writers to not
> spin cycles waiting for the grace period to get over. This enables
> different ways of doing garbage collection. For ex: a data structure
> delete API could remove the entry from the data structure, call
> rte_tqs_start and return back to the caller. On the invocation of
> next API call of the library, the API can call rte_tqs_check (which
> will mostly indicate that the grace period is complete) and free the
> previously deleted entry.
> 
> rte_tqs_update (mapping to rcu_quiescent_state) is pretty small and
> simple.
> 
> rte_tqs_register and rte_tqs_unregister APIs are lock free. Hence
> additional APIs like rcu_thread_online and rcu_thread_offline are not
> required. The rte_tqs_unregister API (when compared to
> rcu_thread_offline) is much simple and conducive to be used in worker
> threads.

liburcu has many flavours already, qsbr being one of them. If none of
those are optimal for this use case, why not work with upstream to
either improve the existing flavours, or add a new one?

You have the specific knowledge and expertise about the requirements
needs for the implementation for this use case, and they have the long-
time and extensive experience maintaining the library on a wide range
of systems and use cases. Why not combine both?
I might be wrong, but to me, nothing described above seems to be
particularly or uniquely tied to implementing a software dataplane or
to DPDK. IMHO we should pool and share wherever possible, rather than
build an ecosystem closed onto itself.

Just my 2c of course!

> > DPDK needs to fix its dependency model, and just admit that it is
> > ok to build off
> > of more than glibc.
> > 
> > Having used liburcu, it can be done in a small manner and really
> > isn't that
> > confusing.
> > 
> > Is your real issue the LGPL license of liburcu for your skittish
> > customers?
> 
> I have not had any discussions on this. Customers are mainly focused
> on having a solution on which they have meaningful control. They want
> to be able to submit a patch and change things if required. For ex:
> barriers for Arm [7] are not optimal. How easy is it to change this
> and get it into distros (there are both internal and external factors
> here)?

It's just as easy (or as hard) as it is with DPDK. So it's either wait
for the distros to update, or rebuild locally.

-- 
Kind regards,
Luca Boccassi