Re: [SPDK] SPDK Dynamic Threading Model

[Walker, Benjamin <benjamin.walker at intel.com>]

[-- Attachment #1: Type: text/plain, Size: 3627 bytes --]

Thank you for all of your effort on this Madhu (and everyone else involved)! I'm
just providing a little more detail on each point below. I have high hopes that
this not only solves your specific problems, but also makes it much easier to
integrate SPDK into other cooperative multitasking frameworks.

On Wed, 2018-05-30 at 17:31 +0000, Pai, Madhu wrote:
> 1. The event library is the place for customization. This is where
> applications can do their custom environment changes.

This was the intent, but it isn't clear enough in the code today in my opinion.
I think we need to make notes as we go about strategies for clarifying what is
an abstraction around the user application's framework and what is an SPDK
deliverable. We've done that a bit with the environment abstraction library, but
the abstractions around threading models are not at all clearly delineated
today. This won't affect any of the patches you make in the immediate future,
but it's something to keep in mind.

> 2. The dynamic threading idea is good. But, the implementation and high level
> approach needs work. Specifically:
> 	a. In the open source code, the DPDK thread should stay tied to a single
> reactor.
> 	b. Make the “spdk_thread” the unit of abstraction (instead of the
> reactor as done in the patch today).

To summarize, the spdk_reactor becomes the abstraction for a "native" thread on
the system, and spdk_thread becomes the abstraction for the
"user/green/virtual/lightweight" thread mapped on top of it. In the first set of
changes, I think we keep the names as they are as much as possible and minimize
changes. After the initial changes are done, I think we need to revisit some of
the names of these objects to make a few concepts a bit clearer.

> 	c. Allow the spdk_thread to be moved between reactors

The function to do the movement should be in lib/event.

> 	d. The spdk_thread data structure would need to be enhanced. It would at
> a minimum need a poller and an event ring.

It needs the active_pollers, timer_pollers, and events data members from struct
spdk_reactor.

> 	e. The DPDK thread would poll and find spdk_threads to work on, in its
> poller ring.

The spdk_reactor structure would have a separate ring data structure (one per
reactor) that holds spdk_thread objects.

> 	f. In the first iteration (in the open source code), keep a 1-1
> relationship between a spdk_thread and a reactor. That essentially means that
> the DPDK thread works on one spdk_thread. This would work as today.
> 	g. Individual implementations may create/destroy spdk_threads and move
> them as needed.

Implementations may also dynamically create spdk_reactors (i.e. new native
threads) at run time as needed. DPDK can handle this case - it just isn't coded
up in SPDK yet.

> 	h. A NVMF Poller Group would go on the polling ring in the spdk_thread.

The abstractions that spdk_thread deals with are spdk_poller, spdk_thread_fn
(which is a message), and spdk_io_channel. An NVMe-oF poll group is an unrelated
thing sitting at a much higher level of abstraction (in lib/nvmf). The NVMe-oF
poll group will not change or be used any differently as these changes are made
- it will still be mapped 1:1 to an spdk_thread. The spdk_thread will contain a
list of spdk_poller objects that it needs to process.

> 	i. Use TLS to set thread_id’s as appropriate. Goal is to NOT change any
> of the util api’s including _get_thread.

Using TLS is definitely the easiest way to make this happen, but do you have TLS
available in your environment?


Thanks,
Ben