qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] Use qemu_eventfd for POSIX AIO


From: Anthony Liguori
Subject: Re: [Qemu-devel] [PATCH] Use qemu_eventfd for POSIX AIO
Date: Tue, 27 Sep 2011 09:07:07 -0500
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.17) Gecko/20110516 Lightning/1.0b2 Thunderbird/3.1.10

On 09/27/2011 08:56 AM, Jan Kiszka wrote:
Move qemu_eventfd unmodified to oslib-posix and use it for signaling
POSIX AIO completions. If native eventfd suport is available, this
avoids multiple read accesses to drain multiple pending signals. As
before we use a pipe if eventfd is not supported.

Signed-off-by: Jan Kiszka<address@hidden>
---
  os-posix.c         |   32 --------------------------------
  oslib-posix.c      |   32 +++++++++++++++++++++++++++++++-
  posix-aio-compat.c |   12 ++++++++----
  3 files changed, 39 insertions(+), 37 deletions(-)

diff --git a/os-posix.c b/os-posix.c
index dbf3b24..a918895 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -45,10 +45,6 @@
  #include<sys/syscall.h>
  #endif

-#ifdef CONFIG_EVENTFD
-#include<sys/eventfd.h>
-#endif
-
  static struct passwd *user_pwd;
  static const char *chroot_dir;
  static int daemonize;
@@ -333,34 +329,6 @@ void os_set_line_buffering(void)
      setvbuf(stdout, NULL, _IOLBF, 0);
  }

-/*
- * Creates an eventfd that looks like a pipe and has EFD_CLOEXEC set.
- */
-int qemu_eventfd(int fds[2])
-{
-#ifdef CONFIG_EVENTFD
-    int ret;
-
-    ret = eventfd(0, 0);
-    if (ret>= 0) {
-        fds[0] = ret;
-        qemu_set_cloexec(ret);
-        if ((fds[1] = dup(ret)) == -1) {
-            close(ret);
-            return -1;
-        }
-        qemu_set_cloexec(fds[1]);
-        return 0;
-    }
-
-    if (errno != ENOSYS) {
-        return -1;
-    }
-#endif
-
-    return qemu_pipe(fds);
-}
-
  int qemu_create_pidfile(const char *filename)
  {
      char buffer[128];
diff --git a/oslib-posix.c b/oslib-posix.c
index a304fb0..8ef7bd7 100644
--- a/oslib-posix.c
+++ b/oslib-posix.c
@@ -47,7 +47,9 @@ extern int daemon(int, int);
  #include "trace.h"
  #include "qemu_socket.h"

-
+#ifdef CONFIG_EVENTFD
+#include<sys/eventfd.h>
+#endif

  int qemu_daemon(int nochdir, int noclose)
  {
@@ -139,6 +141,34 @@ int qemu_pipe(int pipefd[2])
      return ret;
  }

+/*
+ * Creates an eventfd that looks like a pipe and has EFD_CLOEXEC set.
+ */
+int qemu_eventfd(int fds[2])
+{
+#ifdef CONFIG_EVENTFD
+    int ret;
+
+    ret = eventfd(0, 0);
+    if (ret>= 0) {
+        fds[0] = ret;
+        qemu_set_cloexec(ret);
+        if ((fds[1] = dup(ret)) == -1) {
+            close(ret);
+            return -1;
+        }
+        qemu_set_cloexec(fds[1]);
+        return 0;
+    }
+
+    if (errno != ENOSYS) {
+        return -1;
+    }
+#endif
+
+    return qemu_pipe(fds);
+}
+

I think it's a bit dangerous to implement eventfd() in terms of pipe().

You don't expect to handle EAGAIN with eventfd() whereas you have to handle it with pipe().

Moreover, the eventfd() counter is not lossy (practically speaking) whereas if you use pipe() as a counter, it will be lossy in practice.

This is why posix aio uses pipe() and not eventfd().

Regards,

Anthony Liguori

  int qemu_utimensat(int dirfd, const char *path, const struct timespec *times,
                     int flags)
  {
diff --git a/posix-aio-compat.c b/posix-aio-compat.c
index d3c1174..2aa5ba3 100644
--- a/posix-aio-compat.c
+++ b/posix-aio-compat.c
@@ -521,7 +521,7 @@ static void posix_aio_read(void *opaque)
      PosixAioState *s = opaque;
      ssize_t len;

-    /* read all bytes from signal pipe */
+    /* read all bytes from eventfd or signal pipe */
      for (;;) {
          char bytes[16];

@@ -546,10 +546,14 @@ static PosixAioState *posix_aio_state;

  static void posix_aio_notify_event(void)
  {
-    char byte = 0;
+    /* Write 8 bytes to be compatible with eventfd.  */
+    static const uint64_t val = 1;
      ssize_t ret;

-    ret = write(posix_aio_state->wfd,&byte, sizeof(byte));
+    do {
+        ret = write(posix_aio_state->wfd,&val, sizeof(val));
+    } while (ret<  0&&  errno == EINTR);
+
      if (ret<  0&&  errno != EAGAIN)
          die("write()");
  }
@@ -665,7 +669,7 @@ int paio_init(void)
      s = g_malloc(sizeof(PosixAioState));

      s->first_aio = NULL;
-    if (qemu_pipe(fds) == -1) {
+    if (qemu_eventfd(fds) == -1) {
          fprintf(stderr, "failed to create pipe\n");
          return -1;
      }




reply via email to

[Prev in Thread] Current Thread [Next in Thread]