Re: [Libguestfs] [PATCH libnbd v2 3/4] api: Implement concurrent writer.

Callers may now optionally set up a concurrent writer thread. The outline of this idea is explained here: https://www.redhat.com/archives/libguestfs/2019-June/msg00010.html The change is quite small, but here are some points which are true but may not be obvious: * All writes return immediately with success (unless we run out of memory), and writes never block. * When going down the READY -> ISSUE_COMMAND.START (etc) path, because writes never block, we never stop for a read notification, so we always send all commands in the issue queue before starting to read any replies. (However this only means that the raw commands are enqueued in the writer thread, which can still take its merry time writing the requests to the socket.) * Commands can be counted as "in flight" when they haven't been written to the socket yet. This is another reason for increasing the in flight limits. Does this matter? Probably not. The kernel might also queue up packets before sending them, or they might be in flight across the internet or queued at the receiver. Nothing about "in flight" ever meant that the server has received and is processing those packets. * Even with this change, full parallelism isn't quite achievable. It's still possible to be in a state such as REPLY.STRUCTURED_REPLY.RECV_* waiting for an unusual fast writer / slow reader server. If you then decide that you want to send yet

more commands then those commands will only be enqueued in the handle, not dispatched to the writer thread. To avoid this it is best to send as many commands as possible as soon as possible before entering poll, but to a certain extent this is unavoidable with having only one state machine.

--- docs/libnbd.pod | 73 +++++++++++++++++++++++++++++++++++++++++++++ generator/generator | 29 ++++++++++++++++++ lib/handle.c | 32 ++++++++++++++++++++ lib/internal.h | 7 +++++ lib/socket.c | 27 ++++++++++++++--- podwrapper.pl.in | 3 +- 6 files changed, 166 insertions(+), 5 deletions(-) diff --git a/docs/libnbd.pod b/docs/libnbd.pod index ede2539..07d259f 100644 --- a/docs/libnbd.pod +++ b/docs/libnbd.pod @@ -400,6 +400,79 @@ If you are issuing multiple in-flight requests (see above) and limiting the number, then the limit should be applied to each individual NBD connection. +=head2 Concurrent writer thread + +To achieve the maximum possible performance from libnbd and NBD +servers, as well as the above techniques you must also use a +concurrent writer thread. This feature allows requests to be issued +on the NBD socket at the same time that replies are being read from +the socket. In other words L<send(2)> and L<recv(2)> calls will be +running at the same time on the same socket from different threads. + +There is a full example using a concurrent writer available at +L<https://github.com/libguestfs/libnbd/blob/master/examples/concurrent-writer.c> + +To implement this, you change your ordinary AIO code in four ways: + +=over 4 + +=item 1. Call nbd_set_concurrent_writer + + struct writer_data { + struct nbd_handle *nbd; + /* other data here as required */ + } data; + + nbd_set_concurrent_writer (nbd, &data, writer); + +This function can be called on the handle at any time, either after +the handle is created or after the connection and handshaking has +completed. + +=item 2. Implement non-blocking writer callback + +C<writer> is a I<non-blocking> callback which enqueues the buffer into +a ring or similar FIFO structure: + + struct ring_item { + struct writer_data *data; + const void *buf; + size_t len; + }; + + void writer (void *data, const void *buf, size_t len)

+ { + struct ring_item item; + + /* add (data, buf, len) to a shared ring */ + item.data = data; + item.buf = malloc (len); + memcpy (item.buf, buf, len); + item.len = len; + ring_add (&item); + + writer_signal (); /* kick the writer thread */

+ } + +=item 3. Implement writer thread + +You must also supply another thread which picks up data off the ring +and writes it to the socket (see C<nbd_aio_get_fd>). If there is an +error when writing to the socket, call C<nbd_concurrent_writer_error> +with the C<errno>. + +You have a choice of whether to implement one thread per nbd_handle or +one thread shared between all handles. + +=item 4. Modify main loop + +Finally your main loop can unconditionally call +C<nbd_aio_notify_write> when C<nbd_aio_get_direction> returns C<WRITE> +or C<BOTH> (since the concurrent thread can always enqueue more data +and so is always "ready to write").

+ +=back + =head1 ENCRYPTION AND AUTHENTICATION The NBD protocol and libnbd supports TLS (sometimes incorrectly called diff --git a/generator/generator b/generator/generator index ff6075d..718e253 100755 --- a/generator/generator +++ b/generator/generator @@ -1094,6 +1094,35 @@ C<\"qemu:dirty-bitmap:...\"> for qemu-nbd (see qemu-nbd I<-B> option). See also C<nbd_block_status>."; }; + "set_concurrent_writer", { + default_call with + args = [ Opaque "data"; + CallbackPersist ("writer", [Opaque "data"; + BytesIn ("buf", "len")]) ]; + ret = RErr; + permitted_states = [ Created; Connecting; Connected ]; + shortdesc = "set a concurrent writer thread"; + longdesc = "\ +Provide an optional concurrent writer thread for better performance. +See L<libnbd(3)/Concurrent writer thread> for how to use this."; + }; + + "concurrent_writer_error", { + default_call with + args = [ Int "err" ]; ret = RErr; + shortdesc = "signal an error from the concurrent writer thread"; + longdesc = "\ +This can be called from the concurrent writer thread to signal +that there was an error writing to the socket. As there is no +way to recover from such errors, the connection will move to the +dead state soon after. + +The parameter is the C<errno> returned by the failed L<send(2)> call. +It must be non-zero. + +See L<libnbd(3)/Concurrent writer thread> for how to use this."; + };