[SPDK] Using spdk_thread_poll with custom scheduler

[SPDK] Using spdk_thread_poll with custom scheduler

[Gupta, Sumit]

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

Hi

I am looking for using spdk_threads with a custom scheduler as presented by Ben in the recent summit. I understand that I can do that by calling spdk_thread_poll from within my scheduler's poller. But I see that there are thread local variables used by various modules (such as bdev) which wont work if I move my poller to a different core. One example is spdk_bdev_open which gets spdk_thread from the tls variable. Thoughts/ideas ?

Thanks
Sumit

Re: [SPDK] Using spdk_thread_poll with custom scheduler

[Walker, Benjamin]

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

On Tue, 2019-04-30 at 21:50 +0000, Gupta, Sumit wrote:
> Hi
> 
> I am looking for using spdk_threads with a custom scheduler as presented by
> Ben in the recent summit. I understand that I can do that by calling
> spdk_thread_poll from within my scheduler's poller. But I see that there are
> thread local variables used by various modules (such as bdev) which wont work
> if I move my poller to a different core. One example is spdk_bdev_open which
> gets spdk_thread from the tls variable. Thoughts/ideas ?

I just did a search for all occurrences of __thread in the code. I see several,
but if I filter out the ones in tests or example applications (where we can
deduce they're safe due to the structure of the example or test) then only a few
remain.

The most important one is the one in lib/thread which stores the current thread.
This thread local variable is set when the user calls spdk_thread_poll() and is
unset when that function returns. This allows us to implement an efficient
version of spdk_get_thread() for use within pollers or events. All code within
SPDK is driven by calls to spdk_thread_poll(), so all calls to spdk_get_thread()
are necessarily inside of an spdk_thread_poll() call while the thread local
variable is set. Between calls to spdk_thread_poll(), you are free to move the
spdk_thread to a different system thread and everything continues to function as
expected.

There's two additional ones that require some consideration.

1. nvme_pcie.c: There is a thread local variable used when reading/writing the
PCI BAR. This is part of hot remove handling. If the device is hot-removed,
writes to the PCI BAR generate a SIGBUS. We install a SIGBUS handler to deal
with this (by swapping in a memory region mapped with all 0xF). That SIGBUS
handler needs some information to function correctly, which we store in a thread
local variable. The SIGBUS handler is guaranteed to run on the same system
thread as where the signal was generated and the system thread resumes execution
afterward. Because all of these PCI BAR writes are done within the context of an
spdk_thread_poll(), and the lifetime of the thread local variable does not
extend across calls to spdk_thread_poll(), this one is safe.

2. strerror_tls.cs: We have a utility function for converting error codes to
strings that uses a thread local variable as the pre-allocated string output.
The typical use here is that you are going to convert the errnum to a string and
print it out immediately. This whole process also occurs entirely within the
context of one spdk_thread_poll() call, so the system thread never switches
during the lifetime of the thread local variable unless you were to hang on to
that string output for an extended period of time. We deal with this by simply
saying to not do that - make a copy or print it out and be done with it.

So looking it over, I think all uses of thread local variables are safe. You can
move spdk_threads around between calls to spdk_thread_poll() and everything will
work as expected.

Thanks,
Ben


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

Re: [SPDK] Using spdk_thread_poll with custom scheduler

[Michael Haeuptle]

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

Piggybacking on this question, I wonder how we can run the reactor in our
custom scheduler. I know I've asked this before but it is still unclear to
me how a customer scheduler can be integrated and make use of the reactor
without modifying SPDK code.
While there are great improvements in making spdk_thread/poll more
reusable, there are still dependencies to the reactor from various
components. For example, spdk_event_allocate (implemented in reactor.c) is
used by lib/event/subsystems/nvmf, lib/scsi, lib/rocksdb, and lib/vhost.
This pretty much prevents me from creating our own reactor.

Are there any plans to make changes to these modules?

Any good ideas what to do in the meantime? In essence, I need to make
_spdk_reactor_run a single function call without the while loop.

Thanks.

-- Michael


On Wed, May 1, 2019 at 11:41 AM Walker, Benjamin <benjamin.walker(a)intel.com>
wrote:

> On Tue, 2019-04-30 at 21:50 +0000, Gupta, Sumit wrote:
> > Hi
> >
> > I am looking for using spdk_threads with a custom scheduler as presented
> by
> > Ben in the recent summit. I understand that I can do that by calling
> > spdk_thread_poll from within my scheduler's poller. But I see that there
> are
> > thread local variables used by various modules (such as bdev) which wont
> work
> > if I move my poller to a different core. One example is spdk_bdev_open
> which
> > gets spdk_thread from the tls variable. Thoughts/ideas ?
>
> I just did a search for all occurrences of __thread in the code. I see
> several,
> but if I filter out the ones in tests or example applications (where we can
> deduce they're safe due to the structure of the example or test) then only
> a few
> remain.
>
> The most important one is the one in lib/thread which stores the current
> thread.
> This thread local variable is set when the user calls spdk_thread_poll()
> and is
> unset when that function returns. This allows us to implement an efficient
> version of spdk_get_thread() for use within pollers or events. All code
> within
> SPDK is driven by calls to spdk_thread_poll(), so all calls to
> spdk_get_thread()
> are necessarily inside of an spdk_thread_poll() call while the thread local
> variable is set. Between calls to spdk_thread_poll(), you are free to move
> the
> spdk_thread to a different system thread and everything continues to
> function as
> expected.
>
> There's two additional ones that require some consideration.
>
> 1. nvme_pcie.c: There is a thread local variable used when reading/writing
> the
> PCI BAR. This is part of hot remove handling. If the device is hot-removed,
> writes to the PCI BAR generate a SIGBUS. We install a SIGBUS handler to
> deal
> with this (by swapping in a memory region mapped with all 0xF). That SIGBUS
> handler needs some information to function correctly, which we store in a
> thread
> local variable. The SIGBUS handler is guaranteed to run on the same system
> thread as where the signal was generated and the system thread resumes
> execution
> afterward. Because all of these PCI BAR writes are done within the context
> of an
> spdk_thread_poll(), and the lifetime of the thread local variable does not
> extend across calls to spdk_thread_poll(), this one is safe.
>
> 2. strerror_tls.cs: We have a utility function for converting error codes
> to
> strings that uses a thread local variable as the pre-allocated string
> output.
> The typical use here is that you are going to convert the errnum to a
> string and
> print it out immediately. This whole process also occurs entirely within
> the
> context of one spdk_thread_poll() call, so the system thread never switches
> during the lifetime of the thread local variable unless you were to hang
> on to
> that string output for an extended period of time. We deal with this by
> simply
> saying to not do that - make a copy or print it out and be done with it.
>
> So looking it over, I think all uses of thread local variables are safe.
> You can
> move spdk_threads around between calls to spdk_thread_poll() and
> everything will
> work as expected.
>
> Thanks,
> Ben
>
>
> >
> > Thanks
> > Sumit
> > _______________________________________________
> > 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] Using spdk_thread_poll with custom scheduler

[Walker, Benjamin]

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

On Fri, 2019-05-03 at 08:27 -0600, Michael Haeuptle wrote:
> Piggybacking on this question, I wonder how we can run the reactor in our
> custom scheduler. I know I've asked this before but it is still unclear to
> me how a customer scheduler can be integrated and make use of the reactor
> without modifying SPDK code.
> While there are great improvements in making spdk_thread/poll more
> reusable, there are still dependencies to the reactor from various
> components. For example, spdk_event_allocate (implemented in reactor.c) is
> used by lib/event/subsystems/nvmf, lib/scsi, lib/rocksdb, and lib/vhost.
> This pretty much prevents me from creating our own reactor.
> 
> Are there any plans to make changes to these modules?

This is exactly the plan and the work is underway right now. For example, see
this NVMe-oF series:

https://review.gerrithub.io/c/spdk/spdk/+/451760

and this vhost series:

https://review.gerrithub.io/c/spdk/spdk/+/452206

and this iSCSI series:

https://review.gerrithub.io/c/spdk/spdk/+/452780

These are all in-flight at the moment, but the plan for the next release is to
have removed all uses of spdk_event_* functions from the SPDK libraries.
Everything will use the abstractions in include/spdk/thread.h instead.

> 
> Any good ideas what to do in the meantime? In essence, I need to make
> _spdk_reactor_run a single function call without the while loop.

At this point we're getting close to having the full conversion done, so I'm not
sure it would be worthwhile to do anything temporary. I think that NVMe-oF patch
series above is the last one required for NVMe-oF, for example. So I'd recommend
trying to incorporate NVMe-oF into your framework using that patch series and
then report back any issues you encounter so we can get this properly documented
and make sure any required fixes get merged as quickly as possible.

I can start walking you through how to do that if you want to go that direction
- but the best example is example/bdev/fio_plugin/fio_plugin.c, which is already
converted entirely and doesn't link to the SPDK event framework at all.

Thanks,
Ben

> 
> Thanks.
> 
> -- Michael
> 
> 
> On Wed, May 1, 2019 at 11:41 AM Walker, Benjamin <benjamin.walker(a)intel.com>
> wrote:
> 
> > On Tue, 2019-04-30 at 21:50 +0000, Gupta, Sumit wrote:
> > > Hi
> > > 
> > > I am looking for using spdk_threads with a custom scheduler as presented
> > by
> > > Ben in the recent summit. I understand that I can do that by calling
> > > spdk_thread_poll from within my scheduler's poller. But I see that there
> > are
> > > thread local variables used by various modules (such as bdev) which wont
> > work
> > > if I move my poller to a different core. One example is spdk_bdev_open
> > which
> > > gets spdk_thread from the tls variable. Thoughts/ideas ?
> > 
> > I just did a search for all occurrences of __thread in the code. I see
> > several,
> > but if I filter out the ones in tests or example applications (where we can
> > deduce they're safe due to the structure of the example or test) then only
> > a few
> > remain.
> > 
> > The most important one is the one in lib/thread which stores the current
> > thread.
> > This thread local variable is set when the user calls spdk_thread_poll()
> > and is
> > unset when that function returns. This allows us to implement an efficient
> > version of spdk_get_thread() for use within pollers or events. All code
> > within
> > SPDK is driven by calls to spdk_thread_poll(), so all calls to
> > spdk_get_thread()
> > are necessarily inside of an spdk_thread_poll() call while the thread local
> > variable is set. Between calls to spdk_thread_poll(), you are free to move
> > the
> > spdk_thread to a different system thread and everything continues to
> > function as
> > expected.
> > 
> > There's two additional ones that require some consideration.
> > 
> > 1. nvme_pcie.c: There is a thread local variable used when reading/writing
> > the
> > PCI BAR. This is part of hot remove handling. If the device is hot-removed,
> > writes to the PCI BAR generate a SIGBUS. We install a SIGBUS handler to
> > deal
> > with this (by swapping in a memory region mapped with all 0xF). That SIGBUS
> > handler needs some information to function correctly, which we store in a
> > thread
> > local variable. The SIGBUS handler is guaranteed to run on the same system
> > thread as where the signal was generated and the system thread resumes
> > execution
> > afterward. Because all of these PCI BAR writes are done within the context
> > of an
> > spdk_thread_poll(), and the lifetime of the thread local variable does not
> > extend across calls to spdk_thread_poll(), this one is safe.
> > 
> > 2. strerror_tls.cs: We have a utility function for converting error codes
> > to
> > strings that uses a thread local variable as the pre-allocated string
> > output.
> > The typical use here is that you are going to convert the errnum to a
> > string and
> > print it out immediately. This whole process also occurs entirely within
> > the
> > context of one spdk_thread_poll() call, so the system thread never switches
> > during the lifetime of the thread local variable unless you were to hang
> > on to
> > that string output for an extended period of time. We deal with this by
> > simply
> > saying to not do that - make a copy or print it out and be done with it.
> > 
> > So looking it over, I think all uses of thread local variables are safe.
> > You can
> > move spdk_threads around between calls to spdk_thread_poll() and
> > everything will
> > work as expected.
> > 
> > Thanks,
> > Ben
> > 
> > 
> > > Thanks
> > > Sumit
> > > _______________________________________________
> > > 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] Using spdk_thread_poll with custom scheduler

[Michael Haeuptle]

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

Thanks for the info, Ben.

I added your nvmf patches and I'm currently working on the subsystem and
RPC initialization without using any of the lib/event code.
There is one issue I've encountered so far: the subsystems like iscsi and
nvmf have their RPC config processing under lib/event/subsystems (e.g.
nvmf/nvmf_rpc.c). Is there a reason for this? All of the bdevs have the
RPCs with their implementation.
Are there plans to move this outside of the event library or should I be
using the configuration APIs in nvmf?

Thanks.

-- Michael


On Fri, May 3, 2019 at 10:45 AM Walker, Benjamin <benjamin.walker(a)intel.com>
wrote:

> On Fri, 2019-05-03 at 08:27 -0600, Michael Haeuptle wrote:
> > Piggybacking on this question, I wonder how we can run the reactor in our
> > custom scheduler. I know I've asked this before but it is still unclear
> to
> > me how a customer scheduler can be integrated and make use of the reactor
> > without modifying SPDK code.
> > While there are great improvements in making spdk_thread/poll more
> > reusable, there are still dependencies to the reactor from various
> > components. For example, spdk_event_allocate (implemented in reactor.c)
> is
> > used by lib/event/subsystems/nvmf, lib/scsi, lib/rocksdb, and lib/vhost.
> > This pretty much prevents me from creating our own reactor.
> >
> > Are there any plans to make changes to these modules?
>
> This is exactly the plan and the work is underway right now. For example,
> see
> this NVMe-oF series:
>
> https://review.gerrithub.io/c/spdk/spdk/+/451760
>
> and this vhost series:
>
> https://review.gerrithub.io/c/spdk/spdk/+/452206
>
> and this iSCSI series:
>
> https://review.gerrithub.io/c/spdk/spdk/+/452780
>
> These are all in-flight at the moment, but the plan for the next release
> is to
> have removed all uses of spdk_event_* functions from the SPDK libraries.
> Everything will use the abstractions in include/spdk/thread.h instead.
>
> >
> > Any good ideas what to do in the meantime? In essence, I need to make
> > _spdk_reactor_run a single function call without the while loop.
>
> At this point we're getting close to having the full conversion done, so
> I'm not
> sure it would be worthwhile to do anything temporary. I think that NVMe-oF
> patch
> series above is the last one required for NVMe-oF, for example. So I'd
> recommend
> trying to incorporate NVMe-oF into your framework using that patch series
> and
> then report back any issues you encounter so we can get this properly
> documented
> and make sure any required fixes get merged as quickly as possible.
>
> I can start walking you through how to do that if you want to go that
> direction
> - but the best example is example/bdev/fio_plugin/fio_plugin.c, which is
> already
> converted entirely and doesn't link to the SPDK event framework at all.
>
> Thanks,
> Ben
>
> >
> > Thanks.
> >
> > -- Michael
> >
> >
> > On Wed, May 1, 2019 at 11:41 AM Walker, Benjamin <
> benjamin.walker(a)intel.com>
> > wrote:
> >
> > > On Tue, 2019-04-30 at 21:50 +0000, Gupta, Sumit wrote:
> > > > Hi
> > > >
> > > > I am looking for using spdk_threads with a custom scheduler as
> presented
> > > by
> > > > Ben in the recent summit. I understand that I can do that by calling
> > > > spdk_thread_poll from within my scheduler's poller. But I see that
> there
> > > are
> > > > thread local variables used by various modules (such as bdev) which
> wont
> > > work
> > > > if I move my poller to a different core. One example is
> spdk_bdev_open
> > > which
> > > > gets spdk_thread from the tls variable. Thoughts/ideas ?
> > >
> > > I just did a search for all occurrences of __thread in the code. I see
> > > several,
> > > but if I filter out the ones in tests or example applications (where
> we can
> > > deduce they're safe due to the structure of the example or test) then
> only
> > > a few
> > > remain.
> > >
> > > The most important one is the one in lib/thread which stores the
> current
> > > thread.
> > > This thread local variable is set when the user calls
> spdk_thread_poll()
> > > and is
> > > unset when that function returns. This allows us to implement an
> efficient
> > > version of spdk_get_thread() for use within pollers or events. All code
> > > within
> > > SPDK is driven by calls to spdk_thread_poll(), so all calls to
> > > spdk_get_thread()
> > > are necessarily inside of an spdk_thread_poll() call while the thread
> local
> > > variable is set. Between calls to spdk_thread_poll(), you are free to
> move
> > > the
> > > spdk_thread to a different system thread and everything continues to
> > > function as
> > > expected.
> > >
> > > There's two additional ones that require some consideration.
> > >
> > > 1. nvme_pcie.c: There is a thread local variable used when
> reading/writing
> > > the
> > > PCI BAR. This is part of hot remove handling. If the device is
> hot-removed,
> > > writes to the PCI BAR generate a SIGBUS. We install a SIGBUS handler to
> > > deal
> > > with this (by swapping in a memory region mapped with all 0xF). That
> SIGBUS
> > > handler needs some information to function correctly, which we store
> in a
> > > thread
> > > local variable. The SIGBUS handler is guaranteed to run on the same
> system
> > > thread as where the signal was generated and the system thread resumes
> > > execution
> > > afterward. Because all of these PCI BAR writes are done within the
> context
> > > of an
> > > spdk_thread_poll(), and the lifetime of the thread local variable does
> not
> > > extend across calls to spdk_thread_poll(), this one is safe.
> > >
> > > 2. strerror_tls.cs: We have a utility function for converting error
> codes
> > > to
> > > strings that uses a thread local variable as the pre-allocated string
> > > output.
> > > The typical use here is that you are going to convert the errnum to a
> > > string and
> > > print it out immediately. This whole process also occurs entirely
> within
> > > the
> > > context of one spdk_thread_poll() call, so the system thread never
> switches
> > > during the lifetime of the thread local variable unless you were to
> hang
> > > on to
> > > that string output for an extended period of time. We deal with this by
> > > simply
> > > saying to not do that - make a copy or print it out and be done with
> it.
> > >
> > > So looking it over, I think all uses of thread local variables are
> safe.
> > > You can
> > > move spdk_threads around between calls to spdk_thread_poll() and
> > > everything will
> > > work as expected.
> > >
> > > Thanks,
> > > Ben
> > >
> > >
> > > > Thanks
> > > > Sumit
> > > > _______________________________________________
> > > > 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
>