diff mbox

[v3,08/10] Use QemuEvent for POSIX AIO

Message ID b8dabbd5e26d9a9e6925331c7796e239f2d3e728.1333623555.git.jan.kiszka@siemens.com
State New
Headers show

Commit Message

Jan Kiszka April 5, 2012, 10:59 a.m. UTC 
Use QemuEvent for signaling POSIX AIO completions. If native eventfd
support is available, this avoids multiple read accesses to drain
multiple pending signals.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 posix-aio-compat.c |   62 ++++++----------------------------------------------
 1 files changed, 7 insertions(+), 55 deletions(-)

Comments

Stefan Hajnoczi April 10, 2012, 3:31 p.m. UTC | #1 
On Thu, Apr 5, 2012 at 11:59 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> Use QemuEvent for signaling POSIX AIO completions. If native eventfd
> support is available, this avoids multiple read accesses to drain
> multiple pending signals.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  posix-aio-compat.c |   62 ++++++----------------------------------------------
>  1 files changed, 7 insertions(+), 55 deletions(-)

We can save a file descriptor by scheduling a BH (if not already
scheduled) and calling qemu_notify_event() instead of using a
dedicated QemuEvent.  I may have missed something, just got back from
vacation :).

Stefan
Jan Kiszka April 10, 2012, 3:45 p.m. UTC | #2 
On 2012-04-10 17:31, Stefan Hajnoczi wrote:
> On Thu, Apr 5, 2012 at 11:59 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> Use QemuEvent for signaling POSIX AIO completions. If native eventfd
>> support is available, this avoids multiple read accesses to drain
>> multiple pending signals.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>  posix-aio-compat.c |   62 ++++++----------------------------------------------
>>  1 files changed, 7 insertions(+), 55 deletions(-)
> 
> We can save a file descriptor by scheduling a BH (if not already
> scheduled) and calling qemu_notify_event() instead of using a
> dedicated QemuEvent.  I may have missed something, just got back from
> vacation :).

Likely feasible, but not that straightforward: We would still have to
provide qemu_aio_{wait,flush,...} services for posix-aio-compat, I presume.

Jan
Stefan Hajnoczi April 10, 2012, 3:49 p.m. UTC | #3 
On Tue, Apr 10, 2012 at 4:45 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> On 2012-04-10 17:31, Stefan Hajnoczi wrote:
>> On Thu, Apr 5, 2012 at 11:59 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>> Use QemuEvent for signaling POSIX AIO completions. If native eventfd
>>> support is available, this avoids multiple read accesses to drain
>>> multiple pending signals.
>>>
>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>> ---
>>>  posix-aio-compat.c |   62 ++++++----------------------------------------------
>>>  1 files changed, 7 insertions(+), 55 deletions(-)
>>
>> We can save a file descriptor by scheduling a BH (if not already
>> scheduled) and calling qemu_notify_event() instead of using a
>> dedicated QemuEvent.  I may have missed something, just got back from
>> vacation :).
>
> Likely feasible, but not that straightforward: We would still have to
> provide qemu_aio_{wait,flush,...} services for posix-aio-compat, I presume.

True, it's much easier to use a dedicated fd.

Stefan
diff mbox

Patch

diff --git a/posix-aio-compat.c b/posix-aio-compat.c
index c9b8ebf..4581972 100644
--- a/posix-aio-compat.c
+++ b/posix-aio-compat.c
@@ -56,7 +56,7 @@  struct qemu_paiocb {
 };
 
 typedef struct PosixAioState {
-    int rfd, wfd;
+    QemuEvent event;
     struct qemu_paiocb *first_aio;
 } PosixAioState;
 
@@ -71,6 +71,7 @@  static int new_threads = 0;     /* backlog of threads we need to create */
 static int pending_threads = 0; /* threads created but not running yet */
 static QEMUBH *new_thread_bh;
 static QTAILQ_HEAD(, qemu_paiocb) request_list;
+static PosixAioState *posix_aio_state;
 
 #ifdef CONFIG_PREADV
 static int preadv_present = 1;
@@ -78,17 +79,6 @@  static int preadv_present = 1;
 static int preadv_present = 0;
 #endif
 
-static void die2(int err, const char *what)
-{
-    fprintf(stderr, "%s failed: %s\n", what, strerror(err));
-    abort();
-}
-
-static void die(const char *what)
-{
-    die2(errno, what);
-}
-
 static ssize_t handle_aiocb_ioctl(struct qemu_paiocb *aiocb)
 {
     int ret;
@@ -277,8 +267,6 @@  static ssize_t handle_aiocb_rw(struct qemu_paiocb *aiocb)
     return nbytes;
 }
 
-static void posix_aio_notify_event(void);
-
 static void *aio_thread(void *unused)
 {
     qemu_mutex_lock(&lock);
@@ -345,7 +333,7 @@  static void *aio_thread(void *unused)
         aiocb->ret = ret;
         qemu_mutex_unlock(&lock);
 
-        posix_aio_notify_event();
+        qemu_event_signal(&posix_aio_state->event);
     }
 
     cur_threads--;
@@ -479,20 +467,8 @@  static int posix_aio_process_queue(void *opaque)
 static void posix_aio_read(void *opaque)
 {
     PosixAioState *s = opaque;
-    ssize_t len;
-
-    /* read all bytes from signal pipe */
-    for (;;) {
-        char bytes[16];
-
-        len = read(s->rfd, bytes, sizeof(bytes));
-        if (len == -1 && errno == EINTR)
-            continue; /* try again */
-        if (len == sizeof(bytes))
-            continue; /* more to read */
-        break;
-    }
 
+    qemu_event_consume(&s->event);
     posix_aio_process_queue(s);
 }
 
@@ -502,18 +478,6 @@  static int posix_aio_flush(void *opaque)
     return !!s->first_aio;
 }
 
-static PosixAioState *posix_aio_state;
-
-static void posix_aio_notify_event(void)
-{
-    char byte = 0;
-    ssize_t ret;
-
-    ret = write(posix_aio_state->wfd, &byte, sizeof(byte));
-    if (ret < 0 && errno != EAGAIN)
-        die("write()");
-}
-
 static void paio_remove(struct qemu_paiocb *acb)
 {
     struct qemu_paiocb **pacb;
@@ -612,7 +576,6 @@  BlockDriverAIOCB *paio_ioctl(BlockDriverState *bs, int fd,
 int paio_init(void)
 {
     PosixAioState *s;
-    int fds[2];
 
     if (posix_aio_state)
         return 0;
@@ -623,20 +586,9 @@  int paio_init(void)
     s = g_malloc(sizeof(PosixAioState));
 
     s->first_aio = NULL;
-    if (qemu_pipe(fds) == -1) {
-        fprintf(stderr, "failed to create pipe\n");
-        g_free(s);
-        return -1;
-    }
-
-    s->rfd = fds[0];
-    s->wfd = fds[1];
-
-    fcntl(s->rfd, F_SETFL, O_NONBLOCK);
-    fcntl(s->wfd, F_SETFL, O_NONBLOCK);
-
-    qemu_aio_set_fd_handler(s->rfd, posix_aio_read, NULL, posix_aio_flush,
-        posix_aio_process_queue, s);
+    qemu_event_init(&s->event, false);
+    qemu_aio_set_fd_handler(qemu_event_get_poll_fd(&s->event), posix_aio_read,
+                            NULL, posix_aio_flush, posix_aio_process_queue, s);
 
     QTAILQ_INIT(&request_list);
     new_thread_bh = qemu_bh_new(spawn_thread_bh_fn, NULL);