search for: wlen

Appears to have a measurable benefit, see 3/3 for test results. Rich.

...--- lib/internal.h | 1 + 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/generator/states.c b/generator/states.c index e879a83..b0dab83 100644 --- a/generator/states.c +++ b/generator/states.c @@ -91,8 +91,8 @@ send_from_wbuf (struct nbd_handle *h) ssize_t r; if (h->wlen == 0) - return 0; /* move to next state */ - r = h->sock->ops->send (h, h->sock, h->wbuf, h->wlen, 0); + goto next_state; + r = h->sock->ops->send (h, h->sock, h->wbuf, h->wlen, h->wflags); if (r == -1) { if (errno == EAGAIN |...

...generator/states-issue-command.c b/generator/states-issue-command.c index 627a54f..cce43d7 100644 --- a/generator/states-issue-command.c +++ b/generator/states-issue-command.c @@ -42,6 +42,8 @@ h->request.count = htobe32 ((uint32_t) cmd->count); h->wbuf = &h->request; h->wlen = sizeof (h->request); + if (cmd->type == NBD_CMD_WRITE) + h->wflags = MSG_MORE; SET_NEXT_STATE (%SEND_REQUEST); return 0; diff --git a/generator/states-newstyle-opt-export-name.c b/generator/states-newstyle-opt-export-name.c index 774c93c..968cea8 100644 --- a/generator/states...

...tates.c | 6 ++++-- lib/internal.h | 1 + 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/generator/states.c b/generator/states.c index e879a83..36cca37 100644 --- a/generator/states.c +++ b/generator/states.c @@ -92,7 +92,7 @@ send_from_wbuf (struct nbd_handle *h) if (h->wlen == 0) return 0; /* move to next state */ - r = h->sock->ops->send (h, h->sock, h->wbuf, h->wlen, 0); + r = h->sock->ops->send (h, h->sock, h->wbuf, h->wlen, h->wflags); if (r == -1) { if (errno == EAGAIN || errno == EWOULDBLOCK...

...*/ - else - return 1; /* more data */ + if (h->rbuf) + h->rbuf += r; + h->rlen -= r; + } + + return 0; /* move to next state */ } static int @@ -90,21 +87,19 @@ send_from_wbuf (struct nbd_handle *h) { ssize_t r; - if (h->wlen == 0) - return 0; /* move to next state */ - r = h->sock->ops->send (h, h->sock, h->wbuf, h->wlen); - if (r == -1) { - if (errno == EAGAIN || errno == EWOULDBLOCK) - return 1; /* more data */ - /* sock->ops->send called set_e...

I thought this should produce a fairly dramatic performance gain. In fact I couldn't measure any performance difference at all. I think what's happening is we're actually paying an extra syscall (to discover the socket would block) and then doing the poll anyway. So I don't know if it's worth having this patch. It could be argued that it makes the code shorter (therefore

...mmand.c index e24ea34..3a5980d 100644 --- a/generator/states-issue-command.c +++ b/generator/states-issue-command.c @@ -25,6 +25,15 @@ assert (conn->cmds_to_issue != NULL); cmd = conn->cmds_to_issue; + /* Were we interrupted by reading a reply to an earlier command? */ + if (conn->wlen) { + if (conn->in_write_payload) + SET_NEXT_STATE(%SEND_WRITE_PAYLOAD); + else + SET_NEXT_STATE(%SEND_REQUEST); + return 0; + } + conn->sbuf.request.magic = htobe32 (NBD_REQUEST_MAGIC); conn->sbuf.request.flags = htobe16 (cmd->flags); conn->sbuf.request.t...

...mmand.c index e24ea34..3a5980d 100644 --- a/generator/states-issue-command.c +++ b/generator/states-issue-command.c @@ -25,6 +25,15 @@ assert (conn->cmds_to_issue != NULL); cmd = conn->cmds_to_issue; + /* Were we interrupted by reading a reply to an earlier command? */ + if (conn->wlen) { + if (conn->in_write_payload) + SET_NEXT_STATE(%SEND_WRITE_PAYLOAD); + else + SET_NEXT_STATE(%SEND_REQUEST); + return 0; + } + conn->sbuf.request.magic = htobe32 (NBD_REQUEST_MAGIC); conn->sbuf.request.flags = htobe16 (cmd->flags); conn->sbuf.request.t...

...lk_handle_output(VirtIODevice *vdev, VirtQueue *vq) +{ + VirtIOBlock *s = to_virtio_blk(vdev); + VirtQueueElement elem; + unsigned int count; + + while ((count = virtqueue_pop(vq, &elem)) != 0) { + struct virtio_blk_inhdr *in; + struct virtio_blk_outhdr *out; + unsigned int wlen; + off_t off; + int i; + + out = (void *)elem.out_sg[0].iov_base; + in = (void *)elem.in_sg[elem.in_num - 1].iov_base; + off = out->sector; + + if (out->type & VIRTIO_BLK_T_SCSI_CMD) { + wlen = sizeof(*in); + in->status = VIRTIO_BLK_S_UNSUPP; + } else...

...lk_handle_output(VirtIODevice *vdev, VirtQueue *vq) +{ + VirtIOBlock *s = to_virtio_blk(vdev); + VirtQueueElement elem; + unsigned int count; + + while ((count = virtqueue_pop(vq, &elem)) != 0) { + struct virtio_blk_inhdr *in; + struct virtio_blk_outhdr *out; + unsigned int wlen; + off_t off; + int i; + + out = (void *)elem.out_sg[0].iov_base; + in = (void *)elem.in_sg[elem.in_num - 1].iov_base; + off = out->sector; + + if (out->type & VIRTIO_BLK_T_SCSI_CMD) { + wlen = sizeof(*in); + in->status = VIRTIO_BLK_S_UNSUPP; + } else...

This isn't quite finished because not all of the tests or examples have been updated, but it demonstrates an idea: Should we forget about the concept of having multiple connections managed under a single handle? In this patch there is a single ‘struct nbd_handle *’ which manages a single state machine and connection (and therefore no nbd_connection). To connect to a multi-conn server you must

This set of patches fixes virt-rescue rather cleanly. In particular the problems with handling ^C are completely fixed. Work still to be done before this can go upstream: - Shutdown doesn't work properly if you exit the shell. At the moment to exit you must do 'reboot -f'. Future improvements: - An escape sequence and escape commands that could be handled by virt-rescue,

This fixes the main issues in virt-rescue and is usable. There are some enhancements which could be made (in follow up work): - An escape sequence and escape commands that could be handled by virt-rescue, eg. to shut down the appliance, mount or unmount filesystems. - `virt-rescue -i' could be implemented cleanly by performing the right API calls before handing control to the

I'm not sure if this is worth pursuing. On paper, it makes sense (if we know we have multiple commands batched to send over the wire, AND those commands are short in length, we might as well use MSG_MORE), but the measurement numbers with it applied might just be in the noise. Eric Blake (2): examples: Enhance access patterns of threaded-reads-and-writes states: Another use for MSG_MORE

This might not be the final solution, but it certainly seems to solve a deadlock for me that I could trigger by using 'nbdkit --filter=noparallel memory 512k' and calling nbd_aio_pread for a request larger than 256k (enough for the Linux kernel to block the server until libnbd read()s), immediately followed by nbd_aio_pwrite for a request larger than 256k (enough to block libnbd until the

...al.h | 2 +- lib/socket.c | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/generator/states.c b/generator/states.c index 145e8c1..e879a83 100644 --- a/generator/states.c +++ b/generator/states.c @@ -92,7 +92,7 @@ send_from_wbuf (struct nbd_handle *h) if (h->wlen == 0) return 0; /* move to next state */ - r = h->sock->ops->send (h, h->sock, h->wbuf, h->wlen); + r = h->sock->ops->send (h, h->sock, h->wbuf, h->wlen, 0); if (r == -1) { if (errno == EAGAIN || errno == EWOULDBLOCK) retur...

...y 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...

...sock->ops->recv (h->sock, rbuf, rlen); + r = h->sock->ops->recv (h, h->sock, rbuf, rlen); if (r == -1) { if (errno == EAGAIN || errno == EWOULDBLOCK) return 1; /* more data */ @@ -92,7 +92,7 @@ send_from_wbuf (struct nbd_handle *h) if (h->wlen == 0) return 0; /* move to next state */ - r = h->sock->ops->send (h->sock, h->wbuf, h->wlen); + r = h->sock->ops->send (h, h->sock, h->wbuf, h->wlen); if (r == -1) { if (errno == EAGAIN || errno == EWOULDBLOCK) return 1;...

On v1, we discussed whether cmds_to_issue needed to be a list, since it never had more than one element. I played with the idea of making it a list, and allowing the client to queue up new commands regardless of whether the state machine is currently in READY. I also polished up the tmp demo into a bit more full-fledged example file, worth including since it also let me discover a hard-to-hit race

On 2015-12-17 21:37, peter dalgaard wrote: > As you're dying in an else clause, a previous if () must contain the clue. Unfortunately not necessarily the matching one. > > My guess is that your TRE library is broken. The line should have matched the RE "regline" defined as > > tre_regcomp(&regline, "^[^:]+:[[:blank:]]*", REG_EXTENDED); > ...and