When we are blocked waiting for POLLOUT during a request, and happen
to receive notice of POLLIN instead, we know that the work done in
response to POLLIN will be non-blocking (it returns to %.READY as soon
as it would block, which in turn jumps right back into ISSUE_COMMAND
because we have a pending request not fully sent yet). Since the
jaunt through REPLY was non-blocking, it is unlikely that the POLLOUT
situation has changed in the meantime, so if we use SET_NEXT_STATE()
to step back into SEND_REQUEST, our recv() call will likely fail with
EAGAIN, once again blocking us until our next POLLOUT. Although the
wasted syscall is not on the hot-path (after all, we can't progress
until data arrives from the server), it's slightly cleaner if we
instead declare that we are already blocked.
I tested with:
$ nbdkit -U - null 16M --run 'examples/threaded-reads-and-writes $unixsocket'
There was no real noticeable difference in timing, but I did observe
that pre-patch, the run encountered 168825 pre-emptions and 136976
send() EAGAIN failures (81%), while post-patch the run encountered
166066 pre-emptions and 552 EAGAIN failures (0.3%).
---
generator/states-issue-command.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/generator/states-issue-command.c b/generator/states-issue-command.c
index 821b28a..f746f80 100644
--- a/generator/states-issue-command.c
+++ b/generator/states-issue-command.c
@@ -25,12 +25,17 @@
assert (h->cmds_to_issue != NULL);
cmd = h->cmds_to_issue;
- /* Were we interrupted by reading a reply to an earlier command? */
+ /* Were we interrupted by reading a reply to an earlier command? If
+ * so, we can only get back here after a non-blocking jaunt through
+ * the REPLY engine, which means we are unlikely to be unblocked for
+ * writes yet; we want to advance back to the correct state but
+ * without trying a send_from_wbuf that will likely return 1.
+ */
if (h->wlen) {
if (h->in_write_payload)
- SET_NEXT_STATE(%SEND_WRITE_PAYLOAD);
+ *next_state = %SEND_WRITE_PAYLOAD;
else
- SET_NEXT_STATE(%SEND_REQUEST);
+ *next_state = %SEND_REQUEST;
return 0;
}
--
2.20.1