search for: nbd_aio_notify_write

...descriptor using nbd_aio_get_fd, which is what you poll on. You also have to call nbd_aio_get_direction which returns READ, WRITE or BOTH (== READ|WRITE). You then set up some events mechanism (eg. poll, epoll, etc.), poll until the file descriptor is ready, and call one of nbd_aio_notify_read or nbd_aio_notify_write. The direction can change any time the handle state changes, which includes whenever you issue a command (eg. nbd_aio_pread), or whenever you call nbd_aio_notify_*. You therefore have to call nbd_aio_get_direction frequently. A typical loop using poll might look like: fd = nbd_aio_get_fd (nbd...

...ll spuriously wake up, check the directions, and see that it still only needs POLLIN for the next server response) - if the request is large, the first half of the request is written from the nbdkit during nbd_aio_pwrite, and the second half of the request is written from the reader thread during nbd_aio_notify_write (here, our write to the pipe-to-self is essential, because we HAD to break the reader loop out of its POLLIN poll() in order to learn that it now needs to poll for POLLIN|POLLOUT) - if the state machine is not ready, the libnbd fd is either in the middle of processing a reply (POLLIN) or a request...

Rich recently patched things to generate one man page per function rather than libnbd-api.3 (nice), but in doing so got stumped by a problem with a fresh git clone (automake fails for any 'include' directive that does not already exist). I've figured out how to hack around it, but the hack requires GNU make. We already use GNU make constructs elsewhere (such as $(wildcard)), but

...bd); - 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 (...

...> -such, it is unsafe for the callback to call any C<nbd_*> APIs on the > > -same nbd object, as it would cause deadlock. > > +The callbacks are invoked at a point where the libnbd lock is held, > > +typically during a call to C<nbd_aio_notify_read>, > > +C<nbd_aio_notify_write>, C<nbd_aio_poll>, or other call that can > > +advance libnbd's state machine. Depending on system load, it is even > > +possible for a callback to reached before completion of the > > > Shouldn't it be "to be reached" instead of "to reached&q...

...test failed: " + "nbd_connect_command did not reject repeat attempt\n", + argv[0]); + exit (EXIT_FAILURE); + } + check (EINVAL, "nbd_connect_command: "); + + /* Try to notify that writes are ready when we aren't blocked on POLLOUT */ + if (nbd_aio_notify_write (nbd) != -1) { + fprintf (stderr, "%s: test failed: " + "nbd_aio_notify_write in wrong state did not fail\n", + argv[0]); + exit (EXIT_FAILURE); + } + check (EINVAL, "nbd_aio_notify_write: "); + + /* Check for status of a bogus handle *...

...ctivity on a +pipe-to-self, or using L<pthread_kill(3)> to send a signal that is +masked except during L<ppoll(2)>. + =item C<LIBNBD_AIO_DIRECTION_WRITE> = 2 We are expected next to write to the server. If using L<poll(2)> @@ -1727,13 +1739,12 @@ you would then call C<nbd_aio_notify_write>. =item C<LIBNBD_AIO_DIRECTION_BOTH> = 3 -We are expected next to either read or write to the server. -If using L<poll(2)> you would set C<events = POLLIN|POLLOUT>. -If one of C<POLLIN> or C<POLLOUT> is returned, then see above. -However note that you shouldn'...

v1: https://www.redhat.com/archives/libguestfs/2019-June/msg00014.html I pushed a few bits which are uncontroversial. The main changes since v1 are: An extra patch removes the want_to_send / check for nbd_aio_is_ready in examples/threaded-reads-and-writes.c. This logic was wrong since commit 6af72b87 as was pointed out by Eric in his review. Comments and structure of

..._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"). Will we ever actually reach a state that is blocked on a write completion for aio_get_direction to eve...

...bd lock is held; as > -such, it is unsafe for the callback to call any C<nbd_*> APIs on the > -same nbd object, as it would cause deadlock. > +The callbacks are invoked at a point where the libnbd lock is held, > +typically during a call to C<nbd_aio_notify_read>, > +C<nbd_aio_notify_write>, C<nbd_aio_poll>, or other call that can > +advance libnbd's state machine. Depending on system load, it is even > +possible for a callback to reached before completion of the Shouldn't it be "to be reached" instead of "to reached"? Best regards, Ta...

...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_WRITE) != 0) { + /* The concurrent writer is always writable, we don't have to + * test the socket in poll. Since calling nbd_aio_notify_write + * can change the state, after doing it we must restart the + * loop. + */ + nbd_aio_notify_write (nbd); + continue; + } + + if ((dir & LIBNBD_AIO_DIRECTION_READ) != 0) + fds[0].events |= POLLIN; + + if (poll (fds, 1, -1) == -1) { + perror ("...

...bd); + fds[0].events = want_to_send ? POLLOUT : 0; + fds[0].revents = 0; + + dir = nbd_aio_get_direction (nbd); + if ((dir & LIBNBD_AIO_DIRECTION_WRITE) != 0) { + /* The concurrent writer is always writable, we don't have to + * test the socket in poll. Since calling nbd_aio_notify_write + * can change the state, after doing it we must restart the + * loop. + */ + nbd_aio_notify_write (nbd); + continue; + } + + if ((dir & LIBNBD_AIO_DIRECTION_READ) != 0) + fds[0].events |= POLLIN; + + if (poll (fds, 1, -1) == -1) { + perror ("...

This works, but there's no time saving and I'm still investigating whether it does what I think it does. Nevertheless I thought I would post it because it (probably) implements the idea I had last night outlined in: https://www.redhat.com/archives/libguestfs/2019-June/msg00010.html The meat of the change is patch 4. Patch 5 is an example which I would probably fold into patch 4 for

...at a point where the libnbd lock is held; as -such, it is unsafe for the callback to call any C<nbd_*> APIs on the -same nbd object, as it would cause deadlock. +The callbacks are invoked at a point where the libnbd lock is held, +typically during a call to C<nbd_aio_notify_read>, +C<nbd_aio_notify_write>, C<nbd_aio_poll>, or other call that can +advance libnbd's state machine. Depending on system load, it is even +possible for a callback to reached before completion of the +C<nbd_aio_*> call that specified the callback. As such, it is unsafe +for the callback to call any C<...

...ll.c +++ b/lib/poll.c @@ -57,21 +57,28 @@ nbd_unlocked_poll (struct nbd_handle *h, int timeout) set_error (errno, "poll"); return -1; } + if (r == 0) + return 0; /* POLLIN and POLLOUT might both be set. However we shouldn't call * both nbd_aio_notify_read and nbd_aio_notify_write at this time * since the first might change the handle state, making the second * notification invalid. Nothing bad happens by ignoring one of the * notifications since if it's still valid it will be picked up by a - * subsequent poll. + * subsequent poll. Prefer notifying on...

...io_connect_tcp \ - nbd_aio_connect_command \ - nbd_aio_pread \ - nbd_aio_pread_structured \ - nbd_aio_pwrite \ - nbd_aio_disconnect \ - nbd_aio_flush \ - nbd_aio_trim \ - nbd_aio_cache \ - nbd_aio_zero \ - nbd_aio_block_status \ - nbd_aio_get_fd \ - nbd_aio_get_direction \ - nbd_aio_notify_read \ - nbd_aio_notify_write \ - nbd_aio_is_created \ - nbd_aio_is_connecting \ - nbd_aio_is_ready \ - nbd_aio_is_processing \ - nbd_aio_is_dead \ - nbd_aio_is_closed \ - nbd_aio_command_completed \ - nbd_aio_peek_command_completed \ - nbd_aio_in_flight \ - nbd_connection_state \ - nbd_get_package_name \ - nbd_get_version \ -...

This is working now, and incorporates all of the changes in Eric's review, *except* that it still doesn't retire commands (although this seems to make no obvious difference, except possibly a performance and memory impact). Rich.

...io_connect_tcp \ + nbd_aio_connect_command \ + nbd_aio_pread \ + nbd_aio_pread_structured \ + nbd_aio_pwrite \ + nbd_aio_disconnect \ + nbd_aio_flush \ + nbd_aio_trim \ + nbd_aio_cache \ + nbd_aio_zero \ + nbd_aio_block_status \ + nbd_aio_get_fd \ + nbd_aio_get_direction \ + nbd_aio_notify_read \ + nbd_aio_notify_write \ + nbd_aio_is_created \ + nbd_aio_is_connecting \ + nbd_aio_is_ready \ + nbd_aio_is_processing \ + nbd_aio_is_dead \ + nbd_aio_is_closed \ + nbd_aio_command_completed \ + nbd_aio_peek_command_completed \ + nbd_aio_in_flight \ + nbd_connection_state \ + nbd_get_package_name \ + nbd_get_version \ +...

...; + goto error; + } + + if ((dir & LIBNBD_AIO_DIRECTION_READ) != 0 && + (fds[0].revents & POLLIN) != 0) + nbd_aio_notify_read (conn); + else if ((dir & LIBNBD_AIO_DIRECTION_WRITE) != 0 && + (fds[0].revents & POLLOUT) != 0) + nbd_aio_notify_write (conn); + + /* If a command is ready to retire, retire it. */ + for (j = 0; j < in_flight; ++j) { + r = nbd_aio_command_completed (conn, handles[j]); + if (r == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + goto error; + } + if (r) { +...

...; + goto error; + } + + if ((dir & LIBNBD_AIO_DIRECTION_READ) != 0 && + (fds[0].revents & POLLIN) != 0) + nbd_aio_notify_read (conn); + else if ((dir & LIBNBD_AIO_DIRECTION_WRITE) != 0 && + (fds[0].revents & POLLOUT) != 0) + nbd_aio_notify_write (conn); + + /* If a command is ready to retire, retire it. */ + while ((done = nbd_aio_peek_command_completed (conn)) >= 0) { + for (i = 0; i < in_flight; ++i) { + if (handles[i] == done) { + r = nbd_aio_command_completed (conn, handles[i]); + if (r == -1)...