[Libguestfs] [PATCH libnbd v2 1/4] examples, tests: Remove want_to_send / ready logic, increase limit on cmds in flight.
Richard W.M. Jones
rjones at redhat.com
Tue Jun 4 09:59:14 UTC 2019
Since Eric's improvements to the command queue in commit 6af72b8 (and
following) there's now a queue of commands waiting to be issued stored
in the handle, and there's no need to issue commands only from the
ready state. We can therefore remove the want_to_send logic, queue as
many commands as possible, and don't need to test if the socket is
ready for POLLOUT.
This commit also removes some misleading comments, improves the
documentation, and increases the limit on commands in flight (since
this limit is effectively in place to stop memory exhaustion, not
because of server limits).
---
docs/libnbd.pod | 9 ++--
examples/threaded-reads-and-writes.c | 65 +++++++++++-----------------
tests/aio-parallel-load.c | 56 ++++++++++--------------
tests/aio-parallel-tls.sh | 2 +-
tests/aio-parallel.c | 58 +++++++++++--------------
tests/aio-parallel.sh | 2 +-
6 files changed, 83 insertions(+), 109 deletions(-)
diff --git a/docs/libnbd.pod b/docs/libnbd.pod
index a5dfb99..ede2539 100644
--- a/docs/libnbd.pod
+++ b/docs/libnbd.pod
@@ -362,9 +362,12 @@ Replies may arrive out of order.
Although in theory you can have an indefinite number of requests in
flight at the same time, in practice it's a good idea to limit them to
-some number. It is suggested to start with a limit of 16 requests in
-flight (per NBD connection), and measure how adjusting the limit up
-and down affects performance for your local configuration.
+some number. Libnbd will queue commands in the handle even if it
+cannot write them to the server, so this limit is largely to prevent a
+backlog of commands from consuming too much memory. It is suggested
+to start with a limit of 64 requests in flight (per NBD connection),
+and measure how adjusting the limit up and down affects performance
+for your local configuration.
There is a full example using multiple in-flight requests available at
L<https://github.com/libguestfs/libnbd/blob/master/examples/threaded-reads-and-writes.c>
diff --git a/examples/threaded-reads-and-writes.c b/examples/threaded-reads-and-writes.c
index 5d0d2bd..bb82641 100644
--- a/examples/threaded-reads-and-writes.c
+++ b/examples/threaded-reads-and-writes.c
@@ -41,20 +41,16 @@ static int64_t exportsize;
/* Number of commands that can be "in flight" at the same time on each
* connection. (Therefore the total number of requests in flight may
- * be up to NR_MULTI_CONN * MAX_IN_FLIGHT). qemu's NBD client can
- * have up to 16 requests in flight.
- *
- * Some servers do not support multiple requests in flight and may
- * deadlock or even crash if this is larger than 1, but common NBD
- * servers should be OK.
+ * be up to NR_MULTI_CONN * MAX_IN_FLIGHT). See libnbd(3) section
+ * "Issuing multiple in-flight requests".
*/
-#define MAX_IN_FLIGHT 16
+#define MAX_IN_FLIGHT 64
/* The size of reads and writes. */
#define BUFFER_SIZE (1024*1024)
/* Number of commands we issue (per thread). */
-#define NR_CYCLES 100000
+#define NR_CYCLES 1000000
struct thread_status {
size_t i; /* Thread index, 0 .. NR_MULTI_CONN-1 */
@@ -192,7 +188,6 @@ start_thread (void *arg)
uint64_t handles[MAX_IN_FLIGHT];
size_t in_flight; /* counts number of requests in flight */
int dir, r, cmd;
- bool want_to_send;
buf = malloc (BUFFER_SIZE);
if (buf == NULL) {
@@ -238,15 +233,31 @@ start_thread (void *arg)
goto error;
}
- /* Do we want to send another request and there's room to issue it
- * and the connection is in the READY state so it can be used to
- * issue a request.
+ /* If we want to issue another request, do so. Note that we reuse
+ * the same buffer for multiple in-flight requests. It doesn't
+ * matter here because we're just trying to write random stuff,
+ * but that would be Very Bad in a real application.
*/
- want_to_send =
- i > 0 && in_flight < MAX_IN_FLIGHT && nbd_aio_is_ready (nbd);
+ while (i > 0 && in_flight < MAX_IN_FLIGHT) {
+ offset = rand () % (exportsize - sizeof buf);
+ cmd = rand () & 1;
+ if (cmd == 0)
+ handle = nbd_aio_pwrite (nbd, buf, sizeof buf, offset, 0);
+ else
+ handle = nbd_aio_pread (nbd, buf, sizeof buf, offset, 0);
+ if (handle == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ goto error;
+ }
+ handles[in_flight] = handle;
+ i--;
+ in_flight++;
+ if (in_flight > status->most_in_flight)
+ status->most_in_flight = in_flight;
+ }
fds[0].fd = nbd_aio_get_fd (nbd);
- fds[0].events = want_to_send ? POLLOUT : 0;
+ fds[0].events = 0;
fds[0].revents = 0;
dir = nbd_aio_get_direction (nbd);
if ((dir & LIBNBD_AIO_DIRECTION_READ) != 0)
@@ -266,30 +277,6 @@ start_thread (void *arg)
(fds[0].revents & POLLOUT) != 0)
nbd_aio_notify_write (nbd);
- /* If we can issue another request, do so. Note that we reuse the
- * same buffer for multiple in-flight requests. It doesn't matter
- * here because we're just trying to write random stuff, but that
- * would be Very Bad in a real application.
- */
- if (want_to_send && (fds[0].revents & POLLOUT) != 0 &&
- nbd_aio_is_ready (nbd)) {
- offset = rand () % (exportsize - sizeof buf);
- cmd = rand () & 1;
- if (cmd == 0)
- handle = nbd_aio_pwrite (nbd, buf, sizeof buf, offset, 0);
- else
- handle = nbd_aio_pread (nbd, buf, sizeof buf, offset, 0);
- if (handle == -1) {
- fprintf (stderr, "%s\n", nbd_get_error ());
- goto error;
- }
- handles[in_flight] = handle;
- i--;
- in_flight++;
- if (in_flight > status->most_in_flight)
- status->most_in_flight = in_flight;
- }
-
/* If a command is ready to retire, retire it. */
for (j = 0; j < in_flight; ++j) {
r = nbd_aio_command_completed (nbd, handles[j]);
diff --git a/tests/aio-parallel-load.c b/tests/aio-parallel-load.c
index 20fa358..398312f 100644
--- a/tests/aio-parallel-load.c
+++ b/tests/aio-parallel-load.c
@@ -44,7 +44,7 @@
#define NR_MULTI_CONN 8
/* Number of commands in flight per connection. */
-#define MAX_IN_FLIGHT 16
+#define MAX_IN_FLIGHT 64
/* Unix socket. */
static const char *unixsocket;
@@ -169,7 +169,7 @@ start_thread (void *arg)
size_t in_flight; /* counts number of requests in flight */
int dir, r, cmd;
time_t t;
- bool expired = false, want_to_send;
+ bool expired = false;
nbd = nbd_create ();
if (nbd == NULL) {
@@ -219,15 +219,30 @@ start_thread (void *arg)
break;
}
- /* Do we want to send another request and there's room to issue it
- * and the connection is in the READY state so it can be used to
- * issue a request.
- */
- want_to_send =
- !expired && in_flight < MAX_IN_FLIGHT && nbd_aio_is_ready (nbd);
+ /* If we can issue another request, do so. */
+ while (!expired && in_flight < MAX_IN_FLIGHT) {
+ offset = rand () % (EXPORTSIZE - sizeof buf);
+ cmd = rand () & 1;
+ if (cmd == 0) {
+ handle = nbd_aio_pwrite (nbd, buf, sizeof buf, offset, 0);
+ status->bytes_sent += sizeof buf;
+ }
+ else {
+ handle = nbd_aio_pread (nbd, buf, sizeof buf, offset, 0);
+ status->bytes_received += sizeof buf;
+ }
+ if (handle == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ goto error;
+ }
+ handles[in_flight] = handle;
+ in_flight++;
+ if (in_flight > status->most_in_flight)
+ status->most_in_flight = in_flight;
+ }
fds[0].fd = nbd_aio_get_fd (nbd);
- fds[0].events = want_to_send ? POLLOUT : 0;
+ fds[0].events = 0;
fds[0].revents = 0;
dir = nbd_aio_get_direction (nbd);
if ((dir & LIBNBD_AIO_DIRECTION_READ) != 0)
@@ -247,29 +262,6 @@ start_thread (void *arg)
(fds[0].revents & POLLOUT) != 0)
nbd_aio_notify_write (nbd);
- /* If we can issue another request, do so. */
- if (want_to_send && (fds[0].revents & POLLOUT) != 0 &&
- nbd_aio_is_ready (nbd)) {
- offset = rand () % (EXPORTSIZE - sizeof buf);
- cmd = rand () & 1;
- if (cmd == 0) {
- handle = nbd_aio_pwrite (nbd, buf, sizeof buf, offset, 0);
- status->bytes_sent += sizeof buf;
- }
- else {
- handle = nbd_aio_pread (nbd, buf, sizeof buf, offset, 0);
- status->bytes_received += sizeof buf;
- }
- if (handle == -1) {
- fprintf (stderr, "%s\n", nbd_get_error ());
- goto error;
- }
- handles[in_flight] = handle;
- in_flight++;
- if (in_flight > status->most_in_flight)
- status->most_in_flight = in_flight;
- }
-
/* If a command is ready to retire, retire it. */
for (i = 0; i < in_flight; ++i) {
r = nbd_aio_command_completed (nbd, handles[i]);
diff --git a/tests/aio-parallel-tls.sh b/tests/aio-parallel-tls.sh
index d60d3fa..c0e5e91 100755
--- a/tests/aio-parallel-tls.sh
+++ b/tests/aio-parallel-tls.sh
@@ -20,5 +20,5 @@
nbdkit -U - --tls=require --tls-verify-peer --tls-psk=keys.psk \
--filter=cow \
- pattern size=8M \
+ pattern size=64M \
--run '$VG ./aio-parallel-tls $unixsocket'
diff --git a/tests/aio-parallel.c b/tests/aio-parallel.c
index d3217a8..a9b0fd9 100644
--- a/tests/aio-parallel.c
+++ b/tests/aio-parallel.c
@@ -41,7 +41,7 @@ static char *ramdisk;
#define BUFFERSIZE 16384
/* This is also defined in aio-parallel.sh and checked here. */
-#define EXPORTSIZE (8*1024*1024)
+#define EXPORTSIZE (64*1024*1024)
/* How long (seconds) that the test will run for. */
#define RUN_TIME 10
@@ -50,7 +50,7 @@ static char *ramdisk;
#define NR_MULTI_CONN 8
/* Number of commands in flight per connection. */
-#define MAX_IN_FLIGHT 16
+#define MAX_IN_FLIGHT 64
#if BUFFERSIZE >= EXPORTSIZE / NR_MULTI_CONN / MAX_IN_FLIGHT
#error "EXPORTSIZE too small"
@@ -201,7 +201,7 @@ start_thread (void *arg)
size_t in_flight; /* counts number of requests in flight */
int dir, r, cmd;
time_t t;
- bool expired = false, want_to_send;
+ bool expired = false;
for (i = 0; i < MAX_IN_FLIGHT; ++i)
commands[status->i][i].offset = -1;
@@ -254,37 +254,8 @@ start_thread (void *arg)
break;
}
- /* Do we want to send another request and there's room to issue it
- * and the connection is in the READY state so it can be used to
- * issue a request.
- */
- want_to_send =
- !expired && in_flight < MAX_IN_FLIGHT && nbd_aio_is_ready (nbd);
-
- fds[0].fd = nbd_aio_get_fd (nbd);
- fds[0].events = want_to_send ? POLLOUT : 0;
- fds[0].revents = 0;
- dir = nbd_aio_get_direction (nbd);
- if ((dir & LIBNBD_AIO_DIRECTION_READ) != 0)
- fds[0].events |= POLLIN;
- if ((dir & LIBNBD_AIO_DIRECTION_WRITE) != 0)
- fds[0].events |= POLLOUT;
-
- if (poll (fds, 1, -1) == -1) {
- perror ("poll");
- goto error;
- }
-
- if ((dir & LIBNBD_AIO_DIRECTION_READ) != 0 &&
- (fds[0].revents & POLLIN) != 0)
- nbd_aio_notify_read (nbd);
- else if ((dir & LIBNBD_AIO_DIRECTION_WRITE) != 0 &&
- (fds[0].revents & POLLOUT) != 0)
- nbd_aio_notify_write (nbd);
-
/* If we can issue another request, do so. */
- if (want_to_send && (fds[0].revents & POLLOUT) != 0 &&
- nbd_aio_is_ready (nbd)) {
+ while (!expired && in_flight < MAX_IN_FLIGHT) {
/* Find a free command slot. */
for (i = 0; i < MAX_IN_FLIGHT; ++i)
if (commands[status->i][i].offset == -1)
@@ -316,6 +287,27 @@ start_thread (void *arg)
status->most_in_flight = in_flight;
}
+ fds[0].fd = nbd_aio_get_fd (nbd);
+ fds[0].events = 0;
+ fds[0].revents = 0;
+ dir = nbd_aio_get_direction (nbd);
+ if ((dir & LIBNBD_AIO_DIRECTION_READ) != 0)
+ fds[0].events |= POLLIN;
+ if ((dir & LIBNBD_AIO_DIRECTION_WRITE) != 0)
+ fds[0].events |= POLLOUT;
+
+ if (poll (fds, 1, -1) == -1) {
+ perror ("poll");
+ goto error;
+ }
+
+ if ((dir & LIBNBD_AIO_DIRECTION_READ) != 0 &&
+ (fds[0].revents & POLLIN) != 0)
+ nbd_aio_notify_read (nbd);
+ else if ((dir & LIBNBD_AIO_DIRECTION_WRITE) != 0 &&
+ (fds[0].revents & POLLOUT) != 0)
+ nbd_aio_notify_write (nbd);
+
/* If a command is ready to retire, retire it. */
for (i = 0; i < MAX_IN_FLIGHT; ++i) {
if (commands[status->i][i].offset >= 0) {
diff --git a/tests/aio-parallel.sh b/tests/aio-parallel.sh
index 40cf794..fd0c5ce 100755
--- a/tests/aio-parallel.sh
+++ b/tests/aio-parallel.sh
@@ -20,5 +20,5 @@
nbdkit -U - \
--filter=cow \
- pattern size=8M \
+ pattern size=64M \
--run '$VG ./aio-parallel $unixsocket'
--
2.21.0
More information about the Libguestfs
mailing list