Re: [SPDK] spdk_set_thread() and build issues

Re: [SPDK] spdk_set_thread() and build issues

[Niu, Yawei]

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

Given that the code reconstruction for this change won't be trivial, I'd like to be cautious before taking actions, in case of things being changed in the future. ;)

Following are my questions and some thoughts:

My question is: what kind of SPDK APIs must be called through spdk_msg/poller? For example, can spdk_blob_read/write() being called directly or they must be called through spdk_msg/poller as well? Is there any document for reference?

IMHO, forcing apps which not rely on reactor to call SPDK APIs through spdk_msg/poller isn't optimal, in our use case, one SPDK thread will be associated with one pthread permanently, so we'd like to call the APIs like spdk_bdev_open(), spdk_bdev_get_io_channel() directly in that pthread context, if we call them through spdk_msg, we have to forge a spdk_msg and put it in the ring buffer, then execute it through spdk_thread_poll(), that looks redundant and inefficient to me.

I'm wondering if it's better to lose the restriction a bit and let certain apps (not rely on reactors) being able to associate SPDK thread with specified pthread (so that some SPDK APIs can be called directly)? It looks to me what we need to do is just exporting the spdk_set_thread().

Any thoughts? Did I miss anything important? Comments are appreciated.

Thanks
-Niu

On 10/07/2019, 6:33 PM, "SPDK on behalf of Niu, Yawei" <spdk-bounces(a)lists.01.org on behalf of yawei.niu(a)intel.com> wrote:

    Ok, I'll change our code as what you suggested. Thanks for the help.
    
    Thanks
    -Niu
    
    On 10/07/2019, 4:14 PM, "SPDK on behalf of Walker, Benjamin" <spdk-bounces(a)lists.01.org on behalf of benjamin.walker(a)intel.com> wrote:
    
        > -----Original Message-----
        > From: SPDK [mailto:spdk-bounces(a)lists.01.org] On Behalf Of Niu, Yawei
        > Sent: Wednesday, July 10, 2019 8:41 AM
        > To: Storage Performance Development Kit <spdk(a)lists.01.org>
        > Subject: Re: [SPDK] spdk_set_thread() and build issues
        > 
        > Hi, Seth. Thanks a lot for the detailed reply, that's very helpful for me to
        > understand the situation!
        > 
        > For the first problem of spdk_set_thread(), we (DAOS) didn't use SPDK event
        > framework, instead, the SPDK thread is used in the way similar to fio plugin,
        > which looks like (with new thread interfaces):
        > - DAOS relies on a coroutine lib called Argobots to create its own xstreams
        > (equivalent to pthreads, and many user-level-threads will be spawned for each
        > xstream) and manage the cpu bindings.
        > - For each xstream, we call spdk_thread_create() and spdk_set_thread() to
        > associate the SPDK thread to the calling xstream.
        > - spdk_bdev_initialize() is called on one per-node xstream to initialize bdevs,
        > spdk_bs_load() is called on per-device xstreams to load blobstores,
        > spdk_bs_alloc_io_channel() called on each IO xstream for IOs.
        > - For each xstream, there is a progress user-level-thread which periodically calls
        > the spdk_thread_poll() to poll the registered nvme pollers and process the
        > messages in the thread ring buffer.
        > 
        > So it looks to me that the SPDK event framework (spdk_app_start() and relating
        > interfaces) won't work well for us, we have our own threading model and use
        > only the bdev subsystem & blobstore, other subsystems like rpc isn't useful for
        > us.
        > For now, we can work around the issue by including the SPDK internal header
        > files, but I suppose the spdk_set_thread() needs be exported eventually for the
        > users like DAOS, what do you think?
        
        Instead, consider a bootstrap procedure that goes like this:
        
        1. create a thread with spdk_thread_create()
        2. send an initial message to the thread (spdk_thread_send_msg)
        	a. When this message executes, it does all of the initialization stuff (spdk_bdev_initialize(), etc.)
        3.  call spdk_thread_poll() to execute the messages queued up to the previously created thread.
        
        spdk_set_thread() is really only for unit tests. If you need to execute SPDK code, you should do it inside of an spdk_msg or an spdk_poller.
        
        
        _______________________________________________
        SPDK mailing list
        SPDK(a)lists.01.org
        https://lists.01.org/mailman/listinfo/spdk
        
    
    _______________________________________________
    SPDK mailing list
    SPDK(a)lists.01.org
    https://lists.01.org/mailman/listinfo/spdk
    

Re: [SPDK] spdk_set_thread() and build issues

[Niu, Yawei]

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

Sure, I'll submit a patch soon. Thanks a lot!

Thanks
-Niu

On 11/07/2019, 7:30 PM, "SPDK on behalf of Walker, Benjamin" <spdk-bounces(a)lists.01.org on behalf of benjamin.walker(a)intel.com> wrote:

    > -----Original Message-----
    > From: SPDK [mailto:spdk-bounces(a)lists.01.org] On Behalf Of Niu, Yawei
    > Sent: Thursday, July 11, 2019 12:27 PM
    > To: Storage Performance Development Kit <spdk(a)lists.01.org>
    > Subject: Re: [SPDK] spdk_set_thread() and build issues
    > 
    > Thanks for your reply, Ben. That really helps me understanding it better.
    > 
    > In our threading model, for each service thread, there is always a progress ULT
    > (user level thread) calls the spdk_thread_poll() periodically, and when there
    > comes a request to the service thread, a new handling ULT will be spawned to
    > handle the request, the request handling procedure could be very complex, so
    > we followed the traditional thread programing model, like: processing job A,
    > yield and wait for A's completion, then process job B... till all jobs are done.
    > SPDK I/O could be a sub job of the request, in that case, we calls
    > spdk_blob_io_read/write() then yield and wait for I/O completion, and the
    > progress ULT will keep thing moving.
    > 
    > So we are basically running in the first model you envisioned, with slight
    > difference.
    > 
    > My major concerns of calling everything by spdk_thread_send_msg() are about
    > code complexity and performance:
    > 
    > 1. As far as I know, not every SPDK API is asynchronous, for example:
    > spdk_bdev_open(), spdk_bdev_get_io_channel(), etc. If we always call these
    > synchronous APIs by spdk_thread_send_msg() to turn it into asynchronous,
    > unnecessary complexity has to be introduced to check if the APIs are executed
    > or not.
    > 
    > 2. If SPDK APIs can be called directly, it'll save us lot of code reorganizing work.
    > Actually what I don't quite see is: given that the API must be called through
    > spdk_thread_send_msg(), why don't we call the spdk_thread_send_msg()
    > internally instead of asking each caller to put a wrapper on? (probably because
    > there is another way is to register a poller and call them directly in poller?)
    > 
    > 3. I'm afraid that the overhead of calling API through spdk_thread_send_msg()
    > could be non-negligible on performance critical path. For instance, the scenario
    > of spdk_blob_io_read/write() being called in our request handling ULT, in direct
    > call, the I/O will be submitted through I/O channel directly, if we wrapping the
    > call by spdk_thread_send_msg(), a spdk_msg will be allocated and being put in
    > the thread ring buffer, and io submission will be executed until next
    > spdk_thread_poll().
    > 
    > Thoughts? Any comments are appreciated.
    
    I think I see the problem now. The issue is when you are in your application code and you want to make an SPDK call. Your application code is not already running within the context of an SPDK thread, so you'd have to pass a message to get it to work. There's two options to solve this:
    
    1) Run everything in your application inside an spdk_thread context - i.e. don't have your application code present in the top-level while loop. Instead, place it inside an spdk_poller.
    2) Expose spdk_set_thread as a public API to allow users to temporarily/efficiently flag a thread as an SPDK thread.
    
    In the spirit of not making people rewrite much code in order to use SPDK, I'm convinced that we should go ahead and do #2. That would let you write your application in the following way:
    
    void* pthread_func(void *arg)
    {
    	struct spdk_thread *thread = spdk_thread_create("event_loop_1", NULL);
    
    	while (true) {
    
    		spdk_set_thread(thread);
    		/* Do other application stuff in a cooperative way. SPDK considers all calls here to have occurred on 'event_loop_1' thread. */
    		spdk_set_thread(NULL);
    
    		spdk_thread_poll(thread);
    	}
    
    	spdk_thread_destroy(thread);
    
    	return NULL;
    }
    
    Would you be willing to submit the patch to make it a public function?
    
    Thanks,
    Ben
    
    > 
    > Thanks
    > -Niu
    > 
    > 
    > On 11/07/2019, 3:42 PM, "SPDK on behalf of Walker, Benjamin" <spdk-
    > bounces(a)lists.01.org on behalf of benjamin.walker(a)intel.com> wrote:
    > 
    >     > -----Original Message-----
    >     > From: SPDK [mailto:spdk-bounces(a)lists.01.org] On Behalf Of Niu, Yawei
    >     > Sent: Thursday, July 11, 2019 6:07 AM
    >     > To: Storage Performance Development Kit <spdk(a)lists.01.org>
    >     > Subject: Re: [SPDK] spdk_set_thread() and build issues
    >     >
    >     > Given that the code reconstruction for this change won't be trivial, I'd like to
    > be
    >     > cautious before taking actions, in case of things being changed in the future.
    > ;)
    >     >
    >     > Following are my questions and some thoughts:
    >     >
    >     > My question is: what kind of SPDK APIs must be called through
    > spdk_msg/poller?
    >     > For example, can spdk_blob_read/write() being called directly or they must
    > be
    >     > called through spdk_msg/poller as well? Is there any document for
    > reference?
    > 
    >     SPDK is built for speed, and one critical part of that is allowing for parallel
    > operations on many threads without locks or other shared data operations.
    > While the project tries very hard to not force applications to "buy-in" to its
    > framework, and rather let SPDK integrate into the application, we do have to
    > make some assumptions about the run-time threading model in order to avoid
    > locking. Specifically, SPDK assumes that the application is doing some sort of
    > cooperative multi-tasking, typically by having event loops running on system
    > threads pinned to cores. These assumptions are abstracted away by SPDK's
    > thread library (include/spdk/thread.h), which all but a few SPDK libraries depend
    > on. So to answer your question about which APIs must be called on an
    > spdk_thread - the main two exceptions are lib/nvme and lib/copy/ioat.
    > Everything else essentially must be done from an spdk_thread.
    > 
    >     >
    >     > IMHO, forcing apps which not rely on reactor to call SPDK APIs through
    >     > spdk_msg/poller isn't optimal, in our use case, one SPDK thread will be
    >     > associated with one pthread permanently, so we'd like to call the APIs like
    >     > spdk_bdev_open(), spdk_bdev_get_io_channel() directly in that pthread
    > context,
    >     > if we call them through spdk_msg, we have to forge a spdk_msg and put it in
    > the
    >     > ring buffer, then execute it through spdk_thread_poll(), that looks redundant
    >     > and inefficient to me.
    > 
    >     SPDK requires that its libraries can call spdk_thread_send_msg and
    > spdk_poller_register, and that those actually work. The way your application
    > framework makes those work is to spawn spdk_threads via spdk_thread_create,
    > and then it must poll those threads (which drives the messages and the internal
    > pollers). So you can't just set your current pthread to be associated 1:1 with an
    > spdk_thread alone - you have to be polling the spdk_thread anyway. Given that
    > your application framework must poll the thread (via spdk_thread_poll), I don't
    > see how spdk_set_thread helps you.
    > 
    >     To put it another way, SPDK is assuming any pthread you have is structured
    > like something like this (highly simplified example):
    > 
    >     void* pthread_func(void *arg)
    >     {
    >     	struct spdk_thread *thread = spdk_thread_create("event_loop_1",
    > NULL);
    > 
    >     	while (true) {
    >     		/* Do other application stuff in a cooperative way */
    > 
    >     		spdk_thread_poll(thread);
    > 
    >     		/* Do other application stuff in a cooperative way */
    >     	}
    > 
    >     	spdk_thread_destroy(thread);
    > 
    >     	return NULL;
    >     }
    > 
    >     We have to assume cooperative multi-tasking and message passing
    > infrastructure exists to avoid locking. We don't assume specifically which
    > framework you're using to do that, but we have to assume the application is
    > structured in roughly the manner above.
    > 
    >     >
    >     > I'm wondering if it's better to lose the restriction a bit and let certain apps
    > (not
    >     > rely on reactors) being able to associate SPDK thread with specified pthread
    > (so
    >     > that some SPDK APIs can be called directly)? It looks to me what we need to
    > do
    >     > is just exporting the spdk_set_thread().
    >     >
    >     > Any thoughts? Did I miss anything important? Comments are appreciated.
    > 
    >     I don't entirely understand how your application is structured at the moment,
    > but I can envision two scenarios. One is that your pthread looks like this:
    > 
    >     void* pthread_func(void *arg)
    >     {
    >     	struct spdk_thread *thread = spdk_thread_create("event_loop_1",
    > NULL);
    > 
    >     	/* DO SPDK INITIALIZATION STUFF HERE */
    > 
    >     	while (true) {
    >     		/* Do other application stuff in a cooperative way */
    > 
    >     		spdk_thread_poll(thread);
    > 
    >     		/* Do other application stuff in a cooperative way */
    >     	}
    > 
    >     	spdk_thread_destroy(thread);
    > 
    >     	return NULL;
    >     }
    > 
    >     If that's the case, then just move the /* DO SPDK INITIALIZATION STUFF HERE
    > */ code into a function and do an spdk_thread_send_msg on the local thread to
    > schedule its execution. That's an easy adjustment.
    > 
    >     The other is that your pthread looks like this:
    > 
    >     void* pthread_func(void *arg)
    >     {
    >     	struct spdk_thread *thread = spdk_thread_create("event_loop_1",
    > NULL);
    > 
    >     	/* DO SPDK INITIALIZATION STUFF HERE */
    > 
    >     	while (true) {
    >     		/* Do application stuff */
    >     	}
    > 
    >     	spdk_thread_destroy(thread);
    > 
    >     	return NULL;
    >     }
    > 
    >     And then you have explicit calls to spdk_thread_poll() scattered around your
    > application when you think you need them. That strategy is just not going to
    > work out well - you should pull the spdk_thread_poll() call up into the top level
    > event loop.
    > 
    >     Thanks,
    >     Ben
    > 
    >     >
    >     > Thanks
    >     > -Niu
    >     >
    >     > On 10/07/2019, 6:33 PM, "SPDK on behalf of Niu, Yawei" <spdk-
    >     > bounces(a)lists.01.org on behalf of yawei.niu(a)intel.com> wrote:
    >     >
    >     >     Ok, I'll change our code as what you suggested. Thanks for the help.
    >     >
    >     >     Thanks
    >     >     -Niu
    >     >
    >     >     On 10/07/2019, 4:14 PM, "SPDK on behalf of Walker, Benjamin" <spdk-
    >     > bounces(a)lists.01.org on behalf of benjamin.walker(a)intel.com> wrote:
    >     >
    >     >         > -----Original Message-----
    >     >         > From: SPDK [mailto:spdk-bounces(a)lists.01.org] On Behalf Of Niu,
    > Yawei
    >     >         > Sent: Wednesday, July 10, 2019 8:41 AM
    >     >         > To: Storage Performance Development Kit <spdk(a)lists.01.org>
    >     >         > Subject: Re: [SPDK] spdk_set_thread() and build issues
    >     >         >
    >     >         > Hi, Seth. Thanks a lot for the detailed reply, that's very helpful for me
    > to
    >     >         > understand the situation!
    >     >         >
    >     >         > For the first problem of spdk_set_thread(), we (DAOS) didn't use SPDK
    >     > event
    >     >         > framework, instead, the SPDK thread is used in the way similar to fio
    >     > plugin,
    >     >         > which looks like (with new thread interfaces):
    >     >         > - DAOS relies on a coroutine lib called Argobots to create its own
    > xstreams
    >     >         > (equivalent to pthreads, and many user-level-threads will be spawned
    > for
    >     > each
    >     >         > xstream) and manage the cpu bindings.
    >     >         > - For each xstream, we call spdk_thread_create() and
    > spdk_set_thread() to
    >     >         > associate the SPDK thread to the calling xstream.
    >     >         > - spdk_bdev_initialize() is called on one per-node xstream to initialize
    >     > bdevs,
    >     >         > spdk_bs_load() is called on per-device xstreams to load blobstores,
    >     >         > spdk_bs_alloc_io_channel() called on each IO xstream for IOs.
    >     >         > - For each xstream, there is a progress user-level-thread which
    > periodically
    >     > calls
    >     >         > the spdk_thread_poll() to poll the registered nvme pollers and process
    > the
    >     >         > messages in the thread ring buffer.
    >     >         >
    >     >         > So it looks to me that the SPDK event framework (spdk_app_start()
    > and
    >     > relating
    >     >         > interfaces) won't work well for us, we have our own threading model
    > and
    >     > use
    >     >         > only the bdev subsystem & blobstore, other subsystems like rpc isn't
    > useful
    >     > for
    >     >         > us.
    >     >         > For now, we can work around the issue by including the SPDK internal
    >     > header
    >     >         > files, but I suppose the spdk_set_thread() needs be exported
    > eventually for
    >     > the
    >     >         > users like DAOS, what do you think?
    >     >
    >     >         Instead, consider a bootstrap procedure that goes like this:
    >     >
    >     >         1. create a thread with spdk_thread_create()
    >     >         2. send an initial message to the thread (spdk_thread_send_msg)
    >     >         	a. When this message executes, it does all of the initialization
    > stuff
    >     > (spdk_bdev_initialize(), etc.)
    >     >         3.  call spdk_thread_poll() to execute the messages queued up to the
    >     > previously created thread.
    >     >
    >     >         spdk_set_thread() is really only for unit tests. If you need to execute
    > SPDK
    >     > code, you should do it inside of an spdk_msg or an spdk_poller.
    >     >
    >     >
    >     >         _______________________________________________
    >     >         SPDK mailing list
    >     >         SPDK(a)lists.01.org
    >     >         https://lists.01.org/mailman/listinfo/spdk
    >     >
    >     >
    >     >     _______________________________________________
    >     >     SPDK mailing list
    >     >     SPDK(a)lists.01.org
    >     >     https://lists.01.org/mailman/listinfo/spdk
    >     >
    >     >
    >     > _______________________________________________
    >     > SPDK mailing list
    >     > SPDK(a)lists.01.org
    >     > https://lists.01.org/mailman/listinfo/spdk
    >     _______________________________________________
    >     SPDK mailing list
    >     SPDK(a)lists.01.org
    >     https://lists.01.org/mailman/listinfo/spdk
    > 
    > 
    > _______________________________________________
    > SPDK mailing list
    > SPDK(a)lists.01.org
    > https://lists.01.org/mailman/listinfo/spdk
    _______________________________________________
    SPDK mailing list
    SPDK(a)lists.01.org
    https://lists.01.org/mailman/listinfo/spdk
    

Re: [SPDK] spdk_set_thread() and build issues

[Walker, Benjamin]

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

> -----Original Message-----
> From: SPDK [mailto:spdk-bounces(a)lists.01.org] On Behalf Of Niu, Yawei
> Sent: Thursday, July 11, 2019 12:27 PM
> To: Storage Performance Development Kit <spdk(a)lists.01.org>
> Subject: Re: [SPDK] spdk_set_thread() and build issues
> 
> Thanks for your reply, Ben. That really helps me understanding it better.
> 
> In our threading model, for each service thread, there is always a progress ULT
> (user level thread) calls the spdk_thread_poll() periodically, and when there
> comes a request to the service thread, a new handling ULT will be spawned to
> handle the request, the request handling procedure could be very complex, so
> we followed the traditional thread programing model, like: processing job A,
> yield and wait for A's completion, then process job B... till all jobs are done.
> SPDK I/O could be a sub job of the request, in that case, we calls
> spdk_blob_io_read/write() then yield and wait for I/O completion, and the
> progress ULT will keep thing moving.
> 
> So we are basically running in the first model you envisioned, with slight
> difference.
> 
> My major concerns of calling everything by spdk_thread_send_msg() are about
> code complexity and performance:
> 
> 1. As far as I know, not every SPDK API is asynchronous, for example:
> spdk_bdev_open(), spdk_bdev_get_io_channel(), etc. If we always call these
> synchronous APIs by spdk_thread_send_msg() to turn it into asynchronous,
> unnecessary complexity has to be introduced to check if the APIs are executed
> or not.
> 
> 2. If SPDK APIs can be called directly, it'll save us lot of code reorganizing work.
> Actually what I don't quite see is: given that the API must be called through
> spdk_thread_send_msg(), why don't we call the spdk_thread_send_msg()
> internally instead of asking each caller to put a wrapper on? (probably because
> there is another way is to register a poller and call them directly in poller?)
> 
> 3. I'm afraid that the overhead of calling API through spdk_thread_send_msg()
> could be non-negligible on performance critical path. For instance, the scenario
> of spdk_blob_io_read/write() being called in our request handling ULT, in direct
> call, the I/O will be submitted through I/O channel directly, if we wrapping the
> call by spdk_thread_send_msg(), a spdk_msg will be allocated and being put in
> the thread ring buffer, and io submission will be executed until next
> spdk_thread_poll().
> 
> Thoughts? Any comments are appreciated.

I think I see the problem now. The issue is when you are in your application code and you want to make an SPDK call. Your application code is not already running within the context of an SPDK thread, so you'd have to pass a message to get it to work. There's two options to solve this:

1) Run everything in your application inside an spdk_thread context - i.e. don't have your application code present in the top-level while loop. Instead, place it inside an spdk_poller.
2) Expose spdk_set_thread as a public API to allow users to temporarily/efficiently flag a thread as an SPDK thread.

In the spirit of not making people rewrite much code in order to use SPDK, I'm convinced that we should go ahead and do #2. That would let you write your application in the following way:

void* pthread_func(void *arg)
{
	struct spdk_thread *thread = spdk_thread_create("event_loop_1", NULL);

	while (true) {

		spdk_set_thread(thread);
		/* Do other application stuff in a cooperative way. SPDK considers all calls here to have occurred on 'event_loop_1' thread. */
		spdk_set_thread(NULL);

		spdk_thread_poll(thread);
	}

	spdk_thread_destroy(thread);

	return NULL;
}

Would you be willing to submit the patch to make it a public function?

Thanks,
Ben

> 
> Thanks
> -Niu
> 
> 
> On 11/07/2019, 3:42 PM, "SPDK on behalf of Walker, Benjamin" <spdk-
> bounces(a)lists.01.org on behalf of benjamin.walker(a)intel.com> wrote:
> 
>     > -----Original Message-----
>     > From: SPDK [mailto:spdk-bounces(a)lists.01.org] On Behalf Of Niu, Yawei
>     > Sent: Thursday, July 11, 2019 6:07 AM
>     > To: Storage Performance Development Kit <spdk(a)lists.01.org>
>     > Subject: Re: [SPDK] spdk_set_thread() and build issues
>     >
>     > Given that the code reconstruction for this change won't be trivial, I'd like to
> be
>     > cautious before taking actions, in case of things being changed in the future.
> ;)
>     >
>     > Following are my questions and some thoughts:
>     >
>     > My question is: what kind of SPDK APIs must be called through
> spdk_msg/poller?
>     > For example, can spdk_blob_read/write() being called directly or they must
> be
>     > called through spdk_msg/poller as well? Is there any document for
> reference?
> 
>     SPDK is built for speed, and one critical part of that is allowing for parallel
> operations on many threads without locks or other shared data operations.
> While the project tries very hard to not force applications to "buy-in" to its
> framework, and rather let SPDK integrate into the application, we do have to
> make some assumptions about the run-time threading model in order to avoid
> locking. Specifically, SPDK assumes that the application is doing some sort of
> cooperative multi-tasking, typically by having event loops running on system
> threads pinned to cores. These assumptions are abstracted away by SPDK's
> thread library (include/spdk/thread.h), which all but a few SPDK libraries depend
> on. So to answer your question about which APIs must be called on an
> spdk_thread - the main two exceptions are lib/nvme and lib/copy/ioat.
> Everything else essentially must be done from an spdk_thread.
> 
>     >
>     > IMHO, forcing apps which not rely on reactor to call SPDK APIs through
>     > spdk_msg/poller isn't optimal, in our use case, one SPDK thread will be
>     > associated with one pthread permanently, so we'd like to call the APIs like
>     > spdk_bdev_open(), spdk_bdev_get_io_channel() directly in that pthread
> context,
>     > if we call them through spdk_msg, we have to forge a spdk_msg and put it in
> the
>     > ring buffer, then execute it through spdk_thread_poll(), that looks redundant
>     > and inefficient to me.
> 
>     SPDK requires that its libraries can call spdk_thread_send_msg and
> spdk_poller_register, and that those actually work. The way your application
> framework makes those work is to spawn spdk_threads via spdk_thread_create,
> and then it must poll those threads (which drives the messages and the internal
> pollers). So you can't just set your current pthread to be associated 1:1 with an
> spdk_thread alone - you have to be polling the spdk_thread anyway. Given that
> your application framework must poll the thread (via spdk_thread_poll), I don't
> see how spdk_set_thread helps you.
> 
>     To put it another way, SPDK is assuming any pthread you have is structured
> like something like this (highly simplified example):
> 
>     void* pthread_func(void *arg)
>     {
>     	struct spdk_thread *thread = spdk_thread_create("event_loop_1",
> NULL);
> 
>     	while (true) {
>     		/* Do other application stuff in a cooperative way */
> 
>     		spdk_thread_poll(thread);
> 
>     		/* Do other application stuff in a cooperative way */
>     	}
> 
>     	spdk_thread_destroy(thread);
> 
>     	return NULL;
>     }
> 
>     We have to assume cooperative multi-tasking and message passing
> infrastructure exists to avoid locking. We don't assume specifically which
> framework you're using to do that, but we have to assume the application is
> structured in roughly the manner above.
> 
>     >
>     > I'm wondering if it's better to lose the restriction a bit and let certain apps
> (not
>     > rely on reactors) being able to associate SPDK thread with specified pthread
> (so
>     > that some SPDK APIs can be called directly)? It looks to me what we need to
> do
>     > is just exporting the spdk_set_thread().
>     >
>     > Any thoughts? Did I miss anything important? Comments are appreciated.
> 
>     I don't entirely understand how your application is structured at the moment,
> but I can envision two scenarios. One is that your pthread looks like this:
> 
>     void* pthread_func(void *arg)
>     {
>     	struct spdk_thread *thread = spdk_thread_create("event_loop_1",
> NULL);
> 
>     	/* DO SPDK INITIALIZATION STUFF HERE */
> 
>     	while (true) {
>     		/* Do other application stuff in a cooperative way */
> 
>     		spdk_thread_poll(thread);
> 
>     		/* Do other application stuff in a cooperative way */
>     	}
> 
>     	spdk_thread_destroy(thread);
> 
>     	return NULL;
>     }
> 
>     If that's the case, then just move the /* DO SPDK INITIALIZATION STUFF HERE
> */ code into a function and do an spdk_thread_send_msg on the local thread to
> schedule its execution. That's an easy adjustment.
> 
>     The other is that your pthread looks like this:
> 
>     void* pthread_func(void *arg)
>     {
>     	struct spdk_thread *thread = spdk_thread_create("event_loop_1",
> NULL);
> 
>     	/* DO SPDK INITIALIZATION STUFF HERE */
> 
>     	while (true) {
>     		/* Do application stuff */
>     	}
> 
>     	spdk_thread_destroy(thread);
> 
>     	return NULL;
>     }
> 
>     And then you have explicit calls to spdk_thread_poll() scattered around your
> application when you think you need them. That strategy is just not going to
> work out well - you should pull the spdk_thread_poll() call up into the top level
> event loop.
> 
>     Thanks,
>     Ben
> 
>     >
>     > Thanks
>     > -Niu
>     >
>     > On 10/07/2019, 6:33 PM, "SPDK on behalf of Niu, Yawei" <spdk-
>     > bounces(a)lists.01.org on behalf of yawei.niu(a)intel.com> wrote:
>     >
>     >     Ok, I'll change our code as what you suggested. Thanks for the help.
>     >
>     >     Thanks
>     >     -Niu
>     >
>     >     On 10/07/2019, 4:14 PM, "SPDK on behalf of Walker, Benjamin" <spdk-
>     > bounces(a)lists.01.org on behalf of benjamin.walker(a)intel.com> wrote:
>     >
>     >         > -----Original Message-----
>     >         > From: SPDK [mailto:spdk-bounces(a)lists.01.org] On Behalf Of Niu,
> Yawei
>     >         > Sent: Wednesday, July 10, 2019 8:41 AM
>     >         > To: Storage Performance Development Kit <spdk(a)lists.01.org>
>     >         > Subject: Re: [SPDK] spdk_set_thread() and build issues
>     >         >
>     >         > Hi, Seth. Thanks a lot for the detailed reply, that's very helpful for me
> to
>     >         > understand the situation!
>     >         >
>     >         > For the first problem of spdk_set_thread(), we (DAOS) didn't use SPDK
>     > event
>     >         > framework, instead, the SPDK thread is used in the way similar to fio
>     > plugin,
>     >         > which looks like (with new thread interfaces):
>     >         > - DAOS relies on a coroutine lib called Argobots to create its own
> xstreams
>     >         > (equivalent to pthreads, and many user-level-threads will be spawned
> for
>     > each
>     >         > xstream) and manage the cpu bindings.
>     >         > - For each xstream, we call spdk_thread_create() and
> spdk_set_thread() to
>     >         > associate the SPDK thread to the calling xstream.
>     >         > - spdk_bdev_initialize() is called on one per-node xstream to initialize
>     > bdevs,
>     >         > spdk_bs_load() is called on per-device xstreams to load blobstores,
>     >         > spdk_bs_alloc_io_channel() called on each IO xstream for IOs.
>     >         > - For each xstream, there is a progress user-level-thread which
> periodically
>     > calls
>     >         > the spdk_thread_poll() to poll the registered nvme pollers and process
> the
>     >         > messages in the thread ring buffer.
>     >         >
>     >         > So it looks to me that the SPDK event framework (spdk_app_start()
> and
>     > relating
>     >         > interfaces) won't work well for us, we have our own threading model
> and
>     > use
>     >         > only the bdev subsystem & blobstore, other subsystems like rpc isn't
> useful
>     > for
>     >         > us.
>     >         > For now, we can work around the issue by including the SPDK internal
>     > header
>     >         > files, but I suppose the spdk_set_thread() needs be exported
> eventually for
>     > the
>     >         > users like DAOS, what do you think?
>     >
>     >         Instead, consider a bootstrap procedure that goes like this:
>     >
>     >         1. create a thread with spdk_thread_create()
>     >         2. send an initial message to the thread (spdk_thread_send_msg)
>     >         	a. When this message executes, it does all of the initialization
> stuff
>     > (spdk_bdev_initialize(), etc.)
>     >         3.  call spdk_thread_poll() to execute the messages queued up to the
>     > previously created thread.
>     >
>     >         spdk_set_thread() is really only for unit tests. If you need to execute
> SPDK
>     > code, you should do it inside of an spdk_msg or an spdk_poller.
>     >
>     >
>     >         _______________________________________________
>     >         SPDK mailing list
>     >         SPDK(a)lists.01.org
>     >         https://lists.01.org/mailman/listinfo/spdk
>     >
>     >
>     >     _______________________________________________
>     >     SPDK mailing list
>     >     SPDK(a)lists.01.org
>     >     https://lists.01.org/mailman/listinfo/spdk
>     >
>     >
>     > _______________________________________________
>     > SPDK mailing list
>     > SPDK(a)lists.01.org
>     > https://lists.01.org/mailman/listinfo/spdk
>     _______________________________________________
>     SPDK mailing list
>     SPDK(a)lists.01.org
>     https://lists.01.org/mailman/listinfo/spdk
> 
> 
> _______________________________________________
> SPDK mailing list
> SPDK(a)lists.01.org
> https://lists.01.org/mailman/listinfo/spdk

Re: [SPDK] spdk_set_thread() and build issues

[Niu, Yawei]

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

Thanks for your reply, Ben. That really helps me understanding it better.

In our threading model, for each service thread, there is always a progress ULT (user level thread) calls the spdk_thread_poll() periodically, and when there comes a request to the service thread, a new handling ULT will be spawned to handle the request, the request handling procedure could be very complex, so we followed the traditional thread programing model, like: processing job A, yield and wait for A's completion, then process job B... till all jobs are done. SPDK I/O could be a sub job of the request, in that case, we calls spdk_blob_io_read/write() then yield and wait for I/O completion, and the progress ULT will keep thing moving.

So we are basically running in the first model you envisioned, with slight difference.

My major concerns of calling everything by spdk_thread_send_msg() are about code complexity and performance:

1. As far as I know, not every SPDK API is asynchronous, for example: spdk_bdev_open(), spdk_bdev_get_io_channel(), etc. If we always call these synchronous APIs by spdk_thread_send_msg() to turn it into asynchronous, unnecessary complexity has to be introduced to check if the APIs are executed or not.

2. If SPDK APIs can be called directly, it'll save us lot of code reorganizing work. Actually what I don't quite see is: given that the API must be called through spdk_thread_send_msg(), why don't we call the spdk_thread_send_msg() internally instead of asking each caller to put a wrapper on? (probably because there is another way is to register a poller and call them directly in poller?)

3. I'm afraid that the overhead of calling API through spdk_thread_send_msg() could be non-negligible on performance critical path. For instance, the scenario of spdk_blob_io_read/write() being called in our request handling ULT, in direct call, the I/O will be submitted through I/O channel directly, if we wrapping the call by spdk_thread_send_msg(), a spdk_msg will be allocated and being put in the thread ring buffer, and io submission will be executed until next spdk_thread_poll().

Thoughts? Any comments are appreciated.

Thanks
-Niu


On 11/07/2019, 3:42 PM, "SPDK on behalf of Walker, Benjamin" <spdk-bounces(a)lists.01.org on behalf of benjamin.walker(a)intel.com> wrote:

    > -----Original Message-----
    > From: SPDK [mailto:spdk-bounces(a)lists.01.org] On Behalf Of Niu, Yawei
    > Sent: Thursday, July 11, 2019 6:07 AM
    > To: Storage Performance Development Kit <spdk(a)lists.01.org>
    > Subject: Re: [SPDK] spdk_set_thread() and build issues
    > 
    > Given that the code reconstruction for this change won't be trivial, I'd like to be
    > cautious before taking actions, in case of things being changed in the future. ;)
    > 
    > Following are my questions and some thoughts:
    > 
    > My question is: what kind of SPDK APIs must be called through spdk_msg/poller?
    > For example, can spdk_blob_read/write() being called directly or they must be
    > called through spdk_msg/poller as well? Is there any document for reference?
    
    SPDK is built for speed, and one critical part of that is allowing for parallel operations on many threads without locks or other shared data operations. While the project tries very hard to not force applications to "buy-in" to its framework, and rather let SPDK integrate into the application, we do have to make some assumptions about the run-time threading model in order to avoid locking. Specifically, SPDK assumes that the application is doing some sort of cooperative multi-tasking, typically by having event loops running on system threads pinned to cores. These assumptions are abstracted away by SPDK's thread library (include/spdk/thread.h), which all but a few SPDK libraries depend on. So to answer your question about which APIs must be called on an spdk_thread - the main two exceptions are lib/nvme and lib/copy/ioat. Everything else essentially must be done from an spdk_thread.
    
    > 
    > IMHO, forcing apps which not rely on reactor to call SPDK APIs through
    > spdk_msg/poller isn't optimal, in our use case, one SPDK thread will be
    > associated with one pthread permanently, so we'd like to call the APIs like
    > spdk_bdev_open(), spdk_bdev_get_io_channel() directly in that pthread context,
    > if we call them through spdk_msg, we have to forge a spdk_msg and put it in the
    > ring buffer, then execute it through spdk_thread_poll(), that looks redundant
    > and inefficient to me.
    
    SPDK requires that its libraries can call spdk_thread_send_msg and spdk_poller_register, and that those actually work. The way your application framework makes those work is to spawn spdk_threads via spdk_thread_create, and then it must poll those threads (which drives the messages and the internal pollers). So you can't just set your current pthread to be associated 1:1 with an spdk_thread alone - you have to be polling the spdk_thread anyway. Given that your application framework must poll the thread (via spdk_thread_poll), I don't see how spdk_set_thread helps you.
    
    To put it another way, SPDK is assuming any pthread you have is structured like something like this (highly simplified example):
    
    void* pthread_func(void *arg)
    {
    	struct spdk_thread *thread = spdk_thread_create("event_loop_1", NULL);
    
    	while (true) {
    		/* Do other application stuff in a cooperative way */
    
    		spdk_thread_poll(thread);
    
    		/* Do other application stuff in a cooperative way */
    	}
    
    	spdk_thread_destroy(thread);
    
    	return NULL;
    }
    
    We have to assume cooperative multi-tasking and message passing infrastructure exists to avoid locking. We don't assume specifically which framework you're using to do that, but we have to assume the application is structured in roughly the manner above.
    
    >  
    > I'm wondering if it's better to lose the restriction a bit and let certain apps (not
    > rely on reactors) being able to associate SPDK thread with specified pthread (so
    > that some SPDK APIs can be called directly)? It looks to me what we need to do
    > is just exporting the spdk_set_thread().
    > 
    > Any thoughts? Did I miss anything important? Comments are appreciated.
    
    I don't entirely understand how your application is structured at the moment, but I can envision two scenarios. One is that your pthread looks like this:
    
    void* pthread_func(void *arg)
    {
    	struct spdk_thread *thread = spdk_thread_create("event_loop_1", NULL);
    
    	/* DO SPDK INITIALIZATION STUFF HERE */
    
    	while (true) {
    		/* Do other application stuff in a cooperative way */
    
    		spdk_thread_poll(thread);
    
    		/* Do other application stuff in a cooperative way */
    	}
    
    	spdk_thread_destroy(thread);
    
    	return NULL;
    }
    
    If that's the case, then just move the /* DO SPDK INITIALIZATION STUFF HERE */ code into a function and do an spdk_thread_send_msg on the local thread to schedule its execution. That's an easy adjustment.
    
    The other is that your pthread looks like this:
    
    void* pthread_func(void *arg)
    {
    	struct spdk_thread *thread = spdk_thread_create("event_loop_1", NULL);
    
    	/* DO SPDK INITIALIZATION STUFF HERE */
    
    	while (true) {
    		/* Do application stuff */
    	}
    
    	spdk_thread_destroy(thread);
    
    	return NULL;
    }
    
    And then you have explicit calls to spdk_thread_poll() scattered around your application when you think you need them. That strategy is just not going to work out well - you should pull the spdk_thread_poll() call up into the top level event loop.
    
    Thanks,
    Ben
    
    > 
    > Thanks
    > -Niu
    > 
    > On 10/07/2019, 6:33 PM, "SPDK on behalf of Niu, Yawei" <spdk-
    > bounces(a)lists.01.org on behalf of yawei.niu(a)intel.com> wrote:
    > 
    >     Ok, I'll change our code as what you suggested. Thanks for the help.
    > 
    >     Thanks
    >     -Niu
    > 
    >     On 10/07/2019, 4:14 PM, "SPDK on behalf of Walker, Benjamin" <spdk-
    > bounces(a)lists.01.org on behalf of benjamin.walker(a)intel.com> wrote:
    > 
    >         > -----Original Message-----
    >         > From: SPDK [mailto:spdk-bounces(a)lists.01.org] On Behalf Of Niu, Yawei
    >         > Sent: Wednesday, July 10, 2019 8:41 AM
    >         > To: Storage Performance Development Kit <spdk(a)lists.01.org>
    >         > Subject: Re: [SPDK] spdk_set_thread() and build issues
    >         >
    >         > Hi, Seth. Thanks a lot for the detailed reply, that's very helpful for me to
    >         > understand the situation!
    >         >
    >         > For the first problem of spdk_set_thread(), we (DAOS) didn't use SPDK
    > event
    >         > framework, instead, the SPDK thread is used in the way similar to fio
    > plugin,
    >         > which looks like (with new thread interfaces):
    >         > - DAOS relies on a coroutine lib called Argobots to create its own xstreams
    >         > (equivalent to pthreads, and many user-level-threads will be spawned for
    > each
    >         > xstream) and manage the cpu bindings.
    >         > - For each xstream, we call spdk_thread_create() and spdk_set_thread() to
    >         > associate the SPDK thread to the calling xstream.
    >         > - spdk_bdev_initialize() is called on one per-node xstream to initialize
    > bdevs,
    >         > spdk_bs_load() is called on per-device xstreams to load blobstores,
    >         > spdk_bs_alloc_io_channel() called on each IO xstream for IOs.
    >         > - For each xstream, there is a progress user-level-thread which periodically
    > calls
    >         > the spdk_thread_poll() to poll the registered nvme pollers and process the
    >         > messages in the thread ring buffer.
    >         >
    >         > So it looks to me that the SPDK event framework (spdk_app_start() and
    > relating
    >         > interfaces) won't work well for us, we have our own threading model and
    > use
    >         > only the bdev subsystem & blobstore, other subsystems like rpc isn't useful
    > for
    >         > us.
    >         > For now, we can work around the issue by including the SPDK internal
    > header
    >         > files, but I suppose the spdk_set_thread() needs be exported eventually for
    > the
    >         > users like DAOS, what do you think?
    > 
    >         Instead, consider a bootstrap procedure that goes like this:
    > 
    >         1. create a thread with spdk_thread_create()
    >         2. send an initial message to the thread (spdk_thread_send_msg)
    >         	a. When this message executes, it does all of the initialization stuff
    > (spdk_bdev_initialize(), etc.)
    >         3.  call spdk_thread_poll() to execute the messages queued up to the
    > previously created thread.
    > 
    >         spdk_set_thread() is really only for unit tests. If you need to execute SPDK
    > code, you should do it inside of an spdk_msg or an spdk_poller.
    > 
    > 
    >         _______________________________________________
    >         SPDK mailing list
    >         SPDK(a)lists.01.org
    >         https://lists.01.org/mailman/listinfo/spdk
    > 
    > 
    >     _______________________________________________
    >     SPDK mailing list
    >     SPDK(a)lists.01.org
    >     https://lists.01.org/mailman/listinfo/spdk
    > 
    > 
    > _______________________________________________
    > SPDK mailing list
    > SPDK(a)lists.01.org
    > https://lists.01.org/mailman/listinfo/spdk
    _______________________________________________
    SPDK mailing list
    SPDK(a)lists.01.org
    https://lists.01.org/mailman/listinfo/spdk
    

Re: [SPDK] spdk_set_thread() and build issues

[Walker, Benjamin]

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

> -----Original Message-----
> From: SPDK [mailto:spdk-bounces(a)lists.01.org] On Behalf Of Niu, Yawei
> Sent: Thursday, July 11, 2019 6:07 AM
> To: Storage Performance Development Kit <spdk(a)lists.01.org>
> Subject: Re: [SPDK] spdk_set_thread() and build issues
> 
> Given that the code reconstruction for this change won't be trivial, I'd like to be
> cautious before taking actions, in case of things being changed in the future. ;)
> 
> Following are my questions and some thoughts:
> 
> My question is: what kind of SPDK APIs must be called through spdk_msg/poller?
> For example, can spdk_blob_read/write() being called directly or they must be
> called through spdk_msg/poller as well? Is there any document for reference?

SPDK is built for speed, and one critical part of that is allowing for parallel operations on many threads without locks or other shared data operations. While the project tries very hard to not force applications to "buy-in" to its framework, and rather let SPDK integrate into the application, we do have to make some assumptions about the run-time threading model in order to avoid locking. Specifically, SPDK assumes that the application is doing some sort of cooperative multi-tasking, typically by having event loops running on system threads pinned to cores. These assumptions are abstracted away by SPDK's thread library (include/spdk/thread.h), which all but a few SPDK libraries depend on. So to answer your question about which APIs must be called on an spdk_thread - the main two exceptions are lib/nvme and lib/copy/ioat. Everything else essentially must be done from an spdk_thread.

> 
> IMHO, forcing apps which not rely on reactor to call SPDK APIs through
> spdk_msg/poller isn't optimal, in our use case, one SPDK thread will be
> associated with one pthread permanently, so we'd like to call the APIs like
> spdk_bdev_open(), spdk_bdev_get_io_channel() directly in that pthread context,
> if we call them through spdk_msg, we have to forge a spdk_msg and put it in the
> ring buffer, then execute it through spdk_thread_poll(), that looks redundant
> and inefficient to me.

SPDK requires that its libraries can call spdk_thread_send_msg and spdk_poller_register, and that those actually work. The way your application framework makes those work is to spawn spdk_threads via spdk_thread_create, and then it must poll those threads (which drives the messages and the internal pollers). So you can't just set your current pthread to be associated 1:1 with an spdk_thread alone - you have to be polling the spdk_thread anyway. Given that your application framework must poll the thread (via spdk_thread_poll), I don't see how spdk_set_thread helps you.

To put it another way, SPDK is assuming any pthread you have is structured like something like this (highly simplified example):

void* pthread_func(void *arg)
{
	struct spdk_thread *thread = spdk_thread_create("event_loop_1", NULL);

	while (true) {
		/* Do other application stuff in a cooperative way */

		spdk_thread_poll(thread);

		/* Do other application stuff in a cooperative way */
	}

	spdk_thread_destroy(thread);

	return NULL;
}

We have to assume cooperative multi-tasking and message passing infrastructure exists to avoid locking. We don't assume specifically which framework you're using to do that, but we have to assume the application is structured in roughly the manner above.

>  
> I'm wondering if it's better to lose the restriction a bit and let certain apps (not
> rely on reactors) being able to associate SPDK thread with specified pthread (so
> that some SPDK APIs can be called directly)? It looks to me what we need to do
> is just exporting the spdk_set_thread().
> 
> Any thoughts? Did I miss anything important? Comments are appreciated.

I don't entirely understand how your application is structured at the moment, but I can envision two scenarios. One is that your pthread looks like this:

void* pthread_func(void *arg)
{
	struct spdk_thread *thread = spdk_thread_create("event_loop_1", NULL);

	/* DO SPDK INITIALIZATION STUFF HERE */

	while (true) {
		/* Do other application stuff in a cooperative way */

		spdk_thread_poll(thread);

		/* Do other application stuff in a cooperative way */
	}

	spdk_thread_destroy(thread);

	return NULL;
}

If that's the case, then just move the /* DO SPDK INITIALIZATION STUFF HERE */ code into a function and do an spdk_thread_send_msg on the local thread to schedule its execution. That's an easy adjustment.

The other is that your pthread looks like this:

void* pthread_func(void *arg)
{
	struct spdk_thread *thread = spdk_thread_create("event_loop_1", NULL);

	/* DO SPDK INITIALIZATION STUFF HERE */

	while (true) {
		/* Do application stuff */
	}

	spdk_thread_destroy(thread);

	return NULL;
}

And then you have explicit calls to spdk_thread_poll() scattered around your application when you think you need them. That strategy is just not going to work out well - you should pull the spdk_thread_poll() call up into the top level event loop.

Thanks,
Ben

> 
> Thanks
> -Niu
> 
> On 10/07/2019, 6:33 PM, "SPDK on behalf of Niu, Yawei" <spdk-
> bounces(a)lists.01.org on behalf of yawei.niu(a)intel.com> wrote:
> 
>     Ok, I'll change our code as what you suggested. Thanks for the help.
> 
>     Thanks
>     -Niu
> 
>     On 10/07/2019, 4:14 PM, "SPDK on behalf of Walker, Benjamin" <spdk-
> bounces(a)lists.01.org on behalf of benjamin.walker(a)intel.com> wrote:
> 
>         > -----Original Message-----
>         > From: SPDK [mailto:spdk-bounces(a)lists.01.org] On Behalf Of Niu, Yawei
>         > Sent: Wednesday, July 10, 2019 8:41 AM
>         > To: Storage Performance Development Kit <spdk(a)lists.01.org>
>         > Subject: Re: [SPDK] spdk_set_thread() and build issues
>         >
>         > Hi, Seth. Thanks a lot for the detailed reply, that's very helpful for me to
>         > understand the situation!
>         >
>         > For the first problem of spdk_set_thread(), we (DAOS) didn't use SPDK
> event
>         > framework, instead, the SPDK thread is used in the way similar to fio
> plugin,
>         > which looks like (with new thread interfaces):
>         > - DAOS relies on a coroutine lib called Argobots to create its own xstreams
>         > (equivalent to pthreads, and many user-level-threads will be spawned for
> each
>         > xstream) and manage the cpu bindings.
>         > - For each xstream, we call spdk_thread_create() and spdk_set_thread() to
>         > associate the SPDK thread to the calling xstream.
>         > - spdk_bdev_initialize() is called on one per-node xstream to initialize
> bdevs,
>         > spdk_bs_load() is called on per-device xstreams to load blobstores,
>         > spdk_bs_alloc_io_channel() called on each IO xstream for IOs.
>         > - For each xstream, there is a progress user-level-thread which periodically
> calls
>         > the spdk_thread_poll() to poll the registered nvme pollers and process the
>         > messages in the thread ring buffer.
>         >
>         > So it looks to me that the SPDK event framework (spdk_app_start() and
> relating
>         > interfaces) won't work well for us, we have our own threading model and
> use
>         > only the bdev subsystem & blobstore, other subsystems like rpc isn't useful
> for
>         > us.
>         > For now, we can work around the issue by including the SPDK internal
> header
>         > files, but I suppose the spdk_set_thread() needs be exported eventually for
> the
>         > users like DAOS, what do you think?
> 
>         Instead, consider a bootstrap procedure that goes like this:
> 
>         1. create a thread with spdk_thread_create()
>         2. send an initial message to the thread (spdk_thread_send_msg)
>         	a. When this message executes, it does all of the initialization stuff
> (spdk_bdev_initialize(), etc.)
>         3.  call spdk_thread_poll() to execute the messages queued up to the
> previously created thread.
> 
>         spdk_set_thread() is really only for unit tests. If you need to execute SPDK
> code, you should do it inside of an spdk_msg or an spdk_poller.
> 
> 
>         _______________________________________________
>         SPDK mailing list
>         SPDK(a)lists.01.org
>         https://lists.01.org/mailman/listinfo/spdk
> 
> 
>     _______________________________________________
>     SPDK mailing list
>     SPDK(a)lists.01.org
>     https://lists.01.org/mailman/listinfo/spdk
> 
> 
> _______________________________________________
> SPDK mailing list
> SPDK(a)lists.01.org
> https://lists.01.org/mailman/listinfo/spdk

Re: [SPDK] spdk_set_thread() and build issues

[Niu, Yawei]

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

Ok, I'll change our code as what you suggested. Thanks for the help.

Thanks
-Niu

On 10/07/2019, 4:14 PM, "SPDK on behalf of Walker, Benjamin" <spdk-bounces(a)lists.01.org on behalf of benjamin.walker(a)intel.com> wrote:

    > -----Original Message-----
    > From: SPDK [mailto:spdk-bounces(a)lists.01.org] On Behalf Of Niu, Yawei
    > Sent: Wednesday, July 10, 2019 8:41 AM
    > To: Storage Performance Development Kit <spdk(a)lists.01.org>
    > Subject: Re: [SPDK] spdk_set_thread() and build issues
    > 
    > Hi, Seth. Thanks a lot for the detailed reply, that's very helpful for me to
    > understand the situation!
    > 
    > For the first problem of spdk_set_thread(), we (DAOS) didn't use SPDK event
    > framework, instead, the SPDK thread is used in the way similar to fio plugin,
    > which looks like (with new thread interfaces):
    > - DAOS relies on a coroutine lib called Argobots to create its own xstreams
    > (equivalent to pthreads, and many user-level-threads will be spawned for each
    > xstream) and manage the cpu bindings.
    > - For each xstream, we call spdk_thread_create() and spdk_set_thread() to
    > associate the SPDK thread to the calling xstream.
    > - spdk_bdev_initialize() is called on one per-node xstream to initialize bdevs,
    > spdk_bs_load() is called on per-device xstreams to load blobstores,
    > spdk_bs_alloc_io_channel() called on each IO xstream for IOs.
    > - For each xstream, there is a progress user-level-thread which periodically calls
    > the spdk_thread_poll() to poll the registered nvme pollers and process the
    > messages in the thread ring buffer.
    > 
    > So it looks to me that the SPDK event framework (spdk_app_start() and relating
    > interfaces) won't work well for us, we have our own threading model and use
    > only the bdev subsystem & blobstore, other subsystems like rpc isn't useful for
    > us.
    > For now, we can work around the issue by including the SPDK internal header
    > files, but I suppose the spdk_set_thread() needs be exported eventually for the
    > users like DAOS, what do you think?
    
    Instead, consider a bootstrap procedure that goes like this:
    
    1. create a thread with spdk_thread_create()
    2. send an initial message to the thread (spdk_thread_send_msg)
    	a. When this message executes, it does all of the initialization stuff (spdk_bdev_initialize(), etc.)
    3.  call spdk_thread_poll() to execute the messages queued up to the previously created thread.
    
    spdk_set_thread() is really only for unit tests. If you need to execute SPDK code, you should do it inside of an spdk_msg or an spdk_poller.
    
    
    _______________________________________________
    SPDK mailing list
    SPDK(a)lists.01.org
    https://lists.01.org/mailman/listinfo/spdk
    

Re: [SPDK] spdk_set_thread() and build issues

[Walker, Benjamin]

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

> -----Original Message-----
> From: SPDK [mailto:spdk-bounces(a)lists.01.org] On Behalf Of Niu, Yawei
> Sent: Wednesday, July 10, 2019 8:41 AM
> To: Storage Performance Development Kit <spdk(a)lists.01.org>
> Subject: Re: [SPDK] spdk_set_thread() and build issues
> 
> Hi, Seth. Thanks a lot for the detailed reply, that's very helpful for me to
> understand the situation!
> 
> For the first problem of spdk_set_thread(), we (DAOS) didn't use SPDK event
> framework, instead, the SPDK thread is used in the way similar to fio plugin,
> which looks like (with new thread interfaces):
> - DAOS relies on a coroutine lib called Argobots to create its own xstreams
> (equivalent to pthreads, and many user-level-threads will be spawned for each
> xstream) and manage the cpu bindings.
> - For each xstream, we call spdk_thread_create() and spdk_set_thread() to
> associate the SPDK thread to the calling xstream.
> - spdk_bdev_initialize() is called on one per-node xstream to initialize bdevs,
> spdk_bs_load() is called on per-device xstreams to load blobstores,
> spdk_bs_alloc_io_channel() called on each IO xstream for IOs.
> - For each xstream, there is a progress user-level-thread which periodically calls
> the spdk_thread_poll() to poll the registered nvme pollers and process the
> messages in the thread ring buffer.
> 
> So it looks to me that the SPDK event framework (spdk_app_start() and relating
> interfaces) won't work well for us, we have our own threading model and use
> only the bdev subsystem & blobstore, other subsystems like rpc isn't useful for
> us.
> For now, we can work around the issue by including the SPDK internal header
> files, but I suppose the spdk_set_thread() needs be exported eventually for the
> users like DAOS, what do you think?

Instead, consider a bootstrap procedure that goes like this:

1. create a thread with spdk_thread_create()
2. send an initial message to the thread (spdk_thread_send_msg)
	a. When this message executes, it does all of the initialization stuff (spdk_bdev_initialize(), etc.)
3.  call spdk_thread_poll() to execute the messages queued up to the previously created thread.

spdk_set_thread() is really only for unit tests. If you need to execute SPDK code, you should do it inside of an spdk_msg or an spdk_poller.

Re: [SPDK] spdk_set_thread() and build issues

[Niu, Yawei]

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

Hi, Seth. Thanks a lot for the detailed reply, that's very helpful for me to understand the situation!

For the first problem of spdk_set_thread(), we (DAOS) didn't use SPDK event framework, instead, the SPDK thread is used in the way similar to fio plugin, which looks like (with new thread interfaces):
- DAOS relies on a coroutine lib called Argobots to create its own xstreams (equivalent to pthreads, and many user-level-threads will be spawned for each xstream) and manage the cpu bindings.
- For each xstream, we call spdk_thread_create() and spdk_set_thread() to associate the SPDK thread to the calling xstream.
- spdk_bdev_initialize() is called on one per-node xstream to initialize bdevs, spdk_bs_load() is called on per-device xstreams to load blobstores, spdk_bs_alloc_io_channel() called on each IO xstream for IOs.
- For each xstream, there is a progress user-level-thread which periodically calls the spdk_thread_poll() to poll the registered nvme pollers and process the messages in the thread ring buffer.

So it looks to me that the SPDK event framework (spdk_app_start() and relating interfaces) won't work well for us, we have our own threading model and use only the bdev subsystem & blobstore, other subsystems like rpc isn't useful for us.
For now, we can work around the issue by including the SPDK internal header files, but I suppose the spdk_set_thread() needs be exported eventually for the users like DAOS, what do you think?

For the DPDK lib issue, we currently link all the required DPDK static libs with 'whole-archive' option like what you mentioned, but that looks not quite convenient to me, furthermore, because our build system build all dependencies including SPDK and install all the headers & libs first, then build DAOS and link the libs from installed dir, we currently need to hack our build script to copy out the DPDK libs.

You suggestion of generating a single shared DPDK lib (also install it along with SPDK shared libs) when 'shared' flag is specified sounds great to me, I think that'll make our building & packaging more easier.

Thanks
-Niu

On 10/07/2019, 10:46 AM, "SPDK on behalf of Howell, Seth" <spdk-bounces(a)lists.01.org on behalf of seth.howell(a)intel.com> wrote:

    Hi Niu,
    
    Sorry about the delayed response to your e-mail. I will try to answer your questions in order.
    
    1. The SPDK app framework has a scheduler built in, so your thread will be round robin assigned to a core. You can limit the cores to which your threads will be bound by specifying a specific core mask as the second argument to spdk_thread_create. You really only need to call spdk_thread_create when you want to spawn a new thread and spdk_thread_exit when a thread has finished its lifecycle.
    
    2. There were some changes to the SPDK shared libraries about 6 months ago. The reason DPDK is no longer rolled into the top level spdk shared library is because for packaging reasons, we wanted to make it so that the user could attempt to link to a custom built or arbitrary dpdk submodule. Obviously, there will be compatibility constraints with which dpdk versions are allowed, but the idea was it would be easier for a distribution to package it if we linked them separately. The change was intentional, but it appears to have broken your use case. I'll detail some workarounds below and ask a few questions about your use case so we can make it work well in the current shared library framework.
    
    You are correct. In order to build an application now, you will need to link against both the dpdk_env shared library and the dpdk libraries themselves. You are also correct that when you build the spdk shared library, it doesn't build the dpdk shared libraries in the submodule. 
    
    In theory, you could build an application (nvmf_tgt for example) simply by invoking gcc with a command similar to the following: 'gcc -o nvmf_tgt -L/path/to/shared/libs -lspdk -lspdk_env_dpdk -ldpdk'. This would use your system's installed version of DPDK. However, at least for my specific shared object versions, the two aren't compatible and I get a few missing symbol errors:
    rte_mem_virt2memseg
    rte_mem_event_callback_register
    rte_dev_event_callback_register
    rte_dev_event_callback_unregister
    rte_memseg_contig_walk
    
    This is because these functions are experimental and either didn't exist or were not exported as part of the shared libraries deployed by my distro (Fedora 28). It is possible that these objects are included by default in some of the newer shared objects provided by distros.
    
    When I do compile an application against the shared libraries on my local machine, I tend to link against the shared libspdk and libspdk_env_dpdk and then also link against the locally created static libraries in dpdk. That command looks roughly like this:
    
    Gcc -o nvmf_tgt -L/path/to/shared/libraries -lspdk -lspdk_env_dpdk -Wl,--start-group -Wl,--whole-archive /path/to/spdk_repo/dpdk/build/lib/sharedobject1.a /path/to/spdk_repo/dpdk/build/lib/sharedobject1.a /path/to/spdk_repo/dpdk/build/lib/sharedobject2.a . . . /path/to/spdk_repo/dpdk/build/lib/sharedobject1.a /path/to/spdk_repo/dpdk/build/lib/sharedobjectn.a -WL,--end-group -Wl,--no-whole-archive -lnuma -ldl
    
    Another option would be to force build the shared objects from the dpdk submodule and link against that dpdk. In that case, your shared object build would look similar to the first example I provided above, and very similar to your initial command.
    
    Does compiling your application and linking against the spdk shared object and the static objects from dpdk (see above) work  for you? Or if a flag existed where you could trigger the shared object build in dpdk to then link against that custom dpdk shared object, would that fit your use case?
    
    Lance Hartman from Oracle has also been doing some work on the shared object build for packaging purposes. I will reach out to him and see if he has any other suggestions/best practices.
    
    Thanks,
    
    Seth
    
    
    -----Original Message-----
    From: SPDK [mailto:spdk-bounces(a)lists.01.org] On Behalf Of Niu, Yawei
    Sent: Friday, July 5, 2019 6:22 PM
    To: Storage Performance Development Kit <spdk(a)lists.01.org>
    Subject: [SPDK] spdk_set_thread() and build issues
    
    Hi,
    
    I got some troubles when I tried to upgrade SPDK to latest master:
    
    
      1.  I see that thread interfaces are heavily changed (my current SPDK code base is quite old), seems the spdk_allocate_thread() is now replaced by spdk_thread_create(), and what I read from the new SPDK code is that, after spdk_thread_create() is being called, a spdk_set_thread() should be called to attach the SPDK thread to current pthread, is that correct? But spdk_set_thread() is an internal function supposed to be used for unit test only, so I’m wondering if I really need an explicit spdk_set_thread() call and what’s the correct way to use spdk_thread_create()?
    
    
      1.  We used to use the built-in DPDK (in the git submodule) and link our app to single spdk shared lib, now looks the spdk shared lib doesn’t cover everything anymore, some additional shared lib like spdk_env_dpdk is required too, and I got lots of undefined DPDK ‘rte_xxx’ symbols. Seems I need to link DPDK libs explicitly now? (but I didn’t see any DPDK libs were installed by ‘make install’.)
    
    
    Any comments are appreciated, thanks in advance.
    
    Thanks
    -Niu
    _______________________________________________
    SPDK mailing list
    SPDK(a)lists.01.org
    https://lists.01.org/mailman/listinfo/spdk
    _______________________________________________
    SPDK mailing list
    SPDK(a)lists.01.org
    https://lists.01.org/mailman/listinfo/spdk
    

Re: [SPDK] spdk_set_thread() and build issues

[Howell, Seth]

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

Hi Niu,

Sorry about the delayed response to your e-mail. I will try to answer your questions in order.

1. The SPDK app framework has a scheduler built in, so your thread will be round robin assigned to a core. You can limit the cores to which your threads will be bound by specifying a specific core mask as the second argument to spdk_thread_create. You really only need to call spdk_thread_create when you want to spawn a new thread and spdk_thread_exit when a thread has finished its lifecycle.

2. There were some changes to the SPDK shared libraries about 6 months ago. The reason DPDK is no longer rolled into the top level spdk shared library is because for packaging reasons, we wanted to make it so that the user could attempt to link to a custom built or arbitrary dpdk submodule. Obviously, there will be compatibility constraints with which dpdk versions are allowed, but the idea was it would be easier for a distribution to package it if we linked them separately. The change was intentional, but it appears to have broken your use case. I'll detail some workarounds below and ask a few questions about your use case so we can make it work well in the current shared library framework.

You are correct. In order to build an application now, you will need to link against both the dpdk_env shared library and the dpdk libraries themselves. You are also correct that when you build the spdk shared library, it doesn't build the dpdk shared libraries in the submodule. 

In theory, you could build an application (nvmf_tgt for example) simply by invoking gcc with a command similar to the following: 'gcc -o nvmf_tgt -L/path/to/shared/libs -lspdk -lspdk_env_dpdk -ldpdk'. This would use your system's installed version of DPDK. However, at least for my specific shared object versions, the two aren't compatible and I get a few missing symbol errors:
rte_mem_virt2memseg
rte_mem_event_callback_register
rte_dev_event_callback_register
rte_dev_event_callback_unregister
rte_memseg_contig_walk

This is because these functions are experimental and either didn't exist or were not exported as part of the shared libraries deployed by my distro (Fedora 28). It is possible that these objects are included by default in some of the newer shared objects provided by distros.

When I do compile an application against the shared libraries on my local machine, I tend to link against the shared libspdk and libspdk_env_dpdk and then also link against the locally created static libraries in dpdk. That command looks roughly like this:

Gcc -o nvmf_tgt -L/path/to/shared/libraries -lspdk -lspdk_env_dpdk -Wl,--start-group -Wl,--whole-archive /path/to/spdk_repo/dpdk/build/lib/sharedobject1.a /path/to/spdk_repo/dpdk/build/lib/sharedobject1.a /path/to/spdk_repo/dpdk/build/lib/sharedobject2.a . . . /path/to/spdk_repo/dpdk/build/lib/sharedobject1.a /path/to/spdk_repo/dpdk/build/lib/sharedobjectn.a -WL,--end-group -Wl,--no-whole-archive -lnuma -ldl

Another option would be to force build the shared objects from the dpdk submodule and link against that dpdk. In that case, your shared object build would look similar to the first example I provided above, and very similar to your initial command.

Does compiling your application and linking against the spdk shared object and the static objects from dpdk (see above) work  for you? Or if a flag existed where you could trigger the shared object build in dpdk to then link against that custom dpdk shared object, would that fit your use case?

Lance Hartman from Oracle has also been doing some work on the shared object build for packaging purposes. I will reach out to him and see if he has any other suggestions/best practices.

Thanks,

Seth


-----Original Message-----
From: SPDK [mailto:spdk-bounces(a)lists.01.org] On Behalf Of Niu, Yawei
Sent: Friday, July 5, 2019 6:22 PM
To: Storage Performance Development Kit <spdk(a)lists.01.org>
Subject: [SPDK] spdk_set_thread() and build issues

Hi,

I got some troubles when I tried to upgrade SPDK to latest master:


  1.  I see that thread interfaces are heavily changed (my current SPDK code base is quite old), seems the spdk_allocate_thread() is now replaced by spdk_thread_create(), and what I read from the new SPDK code is that, after spdk_thread_create() is being called, a spdk_set_thread() should be called to attach the SPDK thread to current pthread, is that correct? But spdk_set_thread() is an internal function supposed to be used for unit test only, so I’m wondering if I really need an explicit spdk_set_thread() call and what’s the correct way to use spdk_thread_create()?


  1.  We used to use the built-in DPDK (in the git submodule) and link our app to single spdk shared lib, now looks the spdk shared lib doesn’t cover everything anymore, some additional shared lib like spdk_env_dpdk is required too, and I got lots of undefined DPDK ‘rte_xxx’ symbols. Seems I need to link DPDK libs explicitly now? (but I didn’t see any DPDK libs were installed by ‘make install’.)


Any comments are appreciated, thanks in advance.

Thanks
-Niu
_______________________________________________
SPDK mailing list
SPDK(a)lists.01.org
https://lists.01.org/mailman/listinfo/spdk

[SPDK] spdk_set_thread() and build issues

[Niu, Yawei]

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

Hi,

I got some troubles when I tried to upgrade SPDK to latest master:


  1.  I see that thread interfaces are heavily changed (my current SPDK code base is quite old), seems the spdk_allocate_thread() is now replaced by spdk_thread_create(), and what I read from the new SPDK code is that, after spdk_thread_create() is being called, a spdk_set_thread() should be called to attach the SPDK thread to current pthread, is that correct? But spdk_set_thread() is an internal function supposed to be used for unit test only, so I’m wondering if I really need an explicit spdk_set_thread() call and what’s the correct way to use spdk_thread_create()?


  1.  We used to use the built-in DPDK (in the git submodule) and link our app to single spdk shared lib, now looks the spdk shared lib doesn’t cover everything anymore, some additional shared lib like spdk_env_dpdk is required too, and I got lots of undefined DPDK ‘rte_xxx’ symbols. Seems I need to link DPDK libs explicitly now? (but I didn’t see any DPDK libs were installed by ‘make install’.)


Any comments are appreciated, thanks in advance.

Thanks
-Niu