[Qemu-devel] [PATCH v2 0/2] sheepdog: don't sleep in coroutine context

[Qemu-devel] [PATCH v2 0/2] sheepdog: don't sleep in coroutine context

[MORITA Kazutaka]

This patch prevents the sheepdog driver from sleeping in coroutine
context long time.

The first patch makes the driver use a non-blocking socket and the
second one fixes a bug that yielded coroutines aren't entered.

Changes from v2:
 - add a patch to use non-blocking fd
 - add explanation why it is safe to io_flush to NULL


MORITA Kazutaka (2):
  sheepdog: use non-blocking fd in coroutine context
  sheepdog: set io_flush handler in do_co_req

 block/sheepdog.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

-- 
1.8.1.3.566.gaa39828

[Qemu-devel] [PATCH v2 1/2] sheepdog: use non-blocking fd in coroutine context

[MORITA Kazutaka]

Using a blocking socket in the coroutine context reduces the chance of
switching to other work.  This patch makes the sheepdog driver use a
non-blocking fd always.

Signed-off-by: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
---
 block/sheepdog.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index c711c28..27abef2 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -468,6 +468,8 @@ static int connect_to_sdog(BDRVSheepdogState *s)
     if (err != NULL) {
         qerror_report_err(err);
         error_free(err);
+    } else {
+        socket_set_nonblock(fd);
     }
 
     return fd;
@@ -523,7 +525,6 @@ static coroutine_fn void do_co_req(void *opaque)
     co = qemu_coroutine_self();
     qemu_aio_set_fd_handler(sockfd, NULL, restart_co_req, NULL, co);
 
-    socket_set_block(sockfd);
     ret = send_co_req(sockfd, hdr, data, wlen);
     if (ret < 0) {
         goto out;
@@ -553,7 +554,6 @@ static coroutine_fn void do_co_req(void *opaque)
     ret = 0;
 out:
     qemu_aio_set_fd_handler(sockfd, NULL, NULL, NULL, NULL);
-    socket_set_nonblock(sockfd);
 
     srco->ret = ret;
     srco->finished = true;
@@ -776,8 +776,6 @@ static int get_sheep_fd(BDRVSheepdogState *s)
         return fd;
     }
 
-    socket_set_nonblock(fd);
-
     qemu_aio_set_fd_handler(fd, co_read_response, NULL, aio_flush_request, s);
     return fd;
 }
-- 
1.8.1.3.566.gaa39828

[Qemu-devel] [PATCH v2 2/2] sheepdog: set io_flush handler in do_co_req

[MORITA Kazutaka]

If an io_flush handler is not set, qemu_aio_wait doesn't invoke
callbacks.

Signed-off-by: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
---
 block/sheepdog.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index 27abef2..4245328 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -501,6 +501,13 @@ static void restart_co_req(void *opaque)
     qemu_coroutine_enter(co, NULL);
 }
 
+static int have_co_req(void *opaque)
+{
+    /* this handler is set only when there is a pending request, so
+     * always returns 1. */
+    return 1;
+}
+
 typedef struct SheepdogReqCo {
     int sockfd;
     SheepdogReq *hdr;
@@ -523,14 +530,14 @@ static coroutine_fn void do_co_req(void *opaque)
     unsigned int *rlen = srco->rlen;
 
     co = qemu_coroutine_self();
-    qemu_aio_set_fd_handler(sockfd, NULL, restart_co_req, NULL, co);
+    qemu_aio_set_fd_handler(sockfd, NULL, restart_co_req, have_co_req, co);
 
     ret = send_co_req(sockfd, hdr, data, wlen);
     if (ret < 0) {
         goto out;
     }
 
-    qemu_aio_set_fd_handler(sockfd, restart_co_req, NULL, NULL, co);
+    qemu_aio_set_fd_handler(sockfd, restart_co_req, NULL, have_co_req, co);
 
     ret = qemu_co_recv(sockfd, hdr, sizeof(*hdr));
     if (ret < sizeof(*hdr)) {
@@ -553,6 +560,8 @@ static coroutine_fn void do_co_req(void *opaque)
     }
     ret = 0;
 out:
+    /* there is at most one request for this sockfd, so it is safe to
+     * set each handler to NULL. */
     qemu_aio_set_fd_handler(sockfd, NULL, NULL, NULL, NULL);
 
     srco->ret = ret;
-- 
1.8.1.3.566.gaa39828

Re: [Qemu-devel] [PATCH v2 0/2] sheepdog: don't sleep in coroutine context

[Stefan Hajnoczi]

On Tue, Mar 12, 2013 at 04:05:41PM +0900, MORITA Kazutaka wrote:
> This patch prevents the sheepdog driver from sleeping in coroutine
> context long time.
> 
> The first patch makes the driver use a non-blocking socket and the
> second one fixes a bug that yielded coroutines aren't entered.
> 
> Changes from v2:
>  - add a patch to use non-blocking fd
>  - add explanation why it is safe to io_flush to NULL
> 
> 
> MORITA Kazutaka (2):
>   sheepdog: use non-blocking fd in coroutine context
>   sheepdog: set io_flush handler in do_co_req
> 
>  block/sheepdog.c | 19 +++++++++++++------
>  1 file changed, 13 insertions(+), 6 deletions(-)
> 
> -- 
> 1.8.1.3.566.gaa39828
> 
> 

Thanks, applied to my block-next tree:
https://github.com/stefanha/qemu/commits/block-next

Stefan