On Wed, Jun 19, 2019 at 09:11:52PM -0500, Eric Blake wrote:
> 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;
It would be nice to do this without fiddling with essentially an
internal detail of the generated code.
Could we add another macro, something like "SET_NEXT_STATE_AND_BLOCK"?
On the other hand if it's not on the hot path, maybe we shouldn't
do this at all?
Rich.
> return 0;
> }
>
> --
> 2.20.1
>
> _______________________________________________
> Libguestfs mailing list
> Libguestfs@redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines. Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top
_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs
