Patchwork Revert "aio: clean up now-unused functions"

login
register
mail settings
Submitter malc
Date Nov. 9, 2012, 8:43 p.m.
Message ID <alpine.LNX.2.00.1211100040001.9952@linmac>
Download mbox | patch
Permalink /patch/198150/
State New
Headers show

Comments

malc - Nov. 9, 2012, 8:43 p.m.
Breaks (at least) SB16. Apparently doesn't do what commit message says
it does.

This reverts commit 22bfa75eafc21522afbb265091faa9cc0649e9fb.
---
 async.c       |   23 ++++++++++++++++-------
 oslib-posix.c |   31 +++++++++++++++++++++++++++++++
 qemu-aio.h    |    1 +
 qemu-common.h |    1 +
 4 files changed, 49 insertions(+), 7 deletions(-)
Paolo Bonzini - Nov. 9, 2012, 11:28 p.m.
Il 09/11/2012 21:43, malc ha scritto:
> 
> Breaks (at least) SB16. Apparently doesn't do what commit message says
> it does.
> 
> This reverts commit 22bfa75eafc21522afbb265091faa9cc0649e9fb.

NACK in this shape, but reverting just the async.c parts (and making
aio_bh_update_timeout static otherwise you get a warning) is fine.

Paolo

> ---
>  async.c       |   23 ++++++++++++++++-------
>  oslib-posix.c |   31 +++++++++++++++++++++++++++++++
>  qemu-aio.h    |    1 +
>  qemu-common.h |    1 +
>  4 files changed, 49 insertions(+), 7 deletions(-)
> 
> diff --git a/async.c b/async.c
> index 04f9dcb..564526f 100644
> --- a/async.c
> +++ b/async.c
> @@ -117,20 +117,16 @@ void qemu_bh_delete(QEMUBH *bh)
>      bh->deleted = 1;
>  }
>  
> -static gboolean
> -aio_ctx_prepare(GSource *source, gint    *timeout)
> +void aio_bh_update_timeout(AioContext *ctx, uint32_t *timeout)
>  {
> -    AioContext *ctx = (AioContext *) source;
>      QEMUBH *bh;
> -    bool scheduled = false;
>  
>      for (bh = ctx->first_bh; bh; bh = bh->next) {
>          if (!bh->deleted && bh->scheduled) {
> -            scheduled = true;
>              if (bh->idle) {
>                  /* idle bottom halves will be polled at least
>                   * every 10ms */
> -                *timeout = 10;
> +                *timeout = MIN(10, *timeout);
>              } else {
>                  /* non-idle bottom halves will be executed
>                   * immediately */
> @@ -139,8 +135,21 @@ aio_ctx_prepare(GSource *source, gint    *timeout)
>              }
>          }
>      }
> +}
> +
> +static gboolean
> +aio_ctx_prepare(GSource *source, gint    *timeout)
> +{
> +    AioContext *ctx = (AioContext *) source;
> +    uint32_t wait = -1;
> +    aio_bh_update_timeout(ctx, &wait);
> +
> +    if (wait != -1) {
> +        *timeout = MIN(*timeout, wait);


> +        return wait == 0;
> +    }
>  
> -    return scheduled;
> +    return false;
>  }
>  
>  static gboolean
> diff --git a/oslib-posix.c b/oslib-posix.c
> index 9db9c3d..dbeb627 100644
> --- a/oslib-posix.c
> +++ b/oslib-posix.c
> @@ -61,6 +61,9 @@ static int running_on_valgrind = -1;
>  #ifdef CONFIG_LINUX
>  #include <sys/syscall.h>
>  #endif
> +#ifdef CONFIG_EVENTFD
> +#include <sys/eventfd.h>
> +#endif
>  
>  int qemu_get_thread_id(void)
>  {
> @@ -180,6 +183,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;
> +        fds[1] = dup(ret);
> +        if (fds[1] == -1) {
> +            close(ret);
> +            return -1;
> +        }
> +        qemu_set_cloexec(ret);
> +        qemu_set_cloexec(fds[1]);
> +        return 0;
> +    }
> +    if (errno != ENOSYS) {
> +        return -1;
> +    }
> +#endif
> +
> +    return qemu_pipe(fds);
> +}
> +
>  int qemu_utimens(const char *path, const struct timespec *times)
>  {
>      struct timeval tv[2], tv_now;
> diff --git a/qemu-aio.h b/qemu-aio.h
> index 1b7eb6e..2354617 100644
> --- a/qemu-aio.h
> +++ b/qemu-aio.h
> @@ -125,6 +125,7 @@ void aio_notify(AioContext *ctx);
>   * These are internal functions used by the QEMU main loop.
>   */
>  int aio_bh_poll(AioContext *ctx);
> +void aio_bh_update_timeout(AioContext *ctx, uint32_t *timeout);
>  
>  /**
>   * qemu_bh_schedule: Schedule a bottom half.
> diff --git a/qemu-common.h b/qemu-common.h
> index ac9985c..5059a97 100644
> --- a/qemu-common.h
> +++ b/qemu-common.h
> @@ -218,6 +218,7 @@ ssize_t qemu_recv_full(int fd, void *buf, size_t count, int flags)
>      QEMU_WARN_UNUSED_RESULT;
>  
>  #ifndef _WIN32
> +int qemu_eventfd(int pipefd[2]);
>  int qemu_pipe(int pipefd[2]);
>  #endif
>  
>
malc - Nov. 10, 2012, 12:17 a.m.
On Sat, 10 Nov 2012, Paolo Bonzini wrote:

> Il 09/11/2012 21:43, malc ha scritto:
> > 
> > Breaks (at least) SB16. Apparently doesn't do what commit message says
> > it does.
> > 
> > This reverts commit 22bfa75eafc21522afbb265091faa9cc0649e9fb.
> 
> NACK in this shape, but reverting just the async.c parts (and making
> aio_bh_update_timeout static otherwise you get a warning) is fine.

Please make a patch then, i'm just try to cure the symptoms here.

[..snip..]

Patch

diff --git a/async.c b/async.c
index 04f9dcb..564526f 100644
--- a/async.c
+++ b/async.c
@@ -117,20 +117,16 @@  void qemu_bh_delete(QEMUBH *bh)
     bh->deleted = 1;
 }
 
-static gboolean
-aio_ctx_prepare(GSource *source, gint    *timeout)
+void aio_bh_update_timeout(AioContext *ctx, uint32_t *timeout)
 {
-    AioContext *ctx = (AioContext *) source;
     QEMUBH *bh;
-    bool scheduled = false;
 
     for (bh = ctx->first_bh; bh; bh = bh->next) {
         if (!bh->deleted && bh->scheduled) {
-            scheduled = true;
             if (bh->idle) {
                 /* idle bottom halves will be polled at least
                  * every 10ms */
-                *timeout = 10;
+                *timeout = MIN(10, *timeout);
             } else {
                 /* non-idle bottom halves will be executed
                  * immediately */
@@ -139,8 +135,21 @@  aio_ctx_prepare(GSource *source, gint    *timeout)
             }
         }
     }
+}
+
+static gboolean
+aio_ctx_prepare(GSource *source, gint    *timeout)
+{
+    AioContext *ctx = (AioContext *) source;
+    uint32_t wait = -1;
+    aio_bh_update_timeout(ctx, &wait);
+
+    if (wait != -1) {
+        *timeout = MIN(*timeout, wait);
+        return wait == 0;
+    }
 
-    return scheduled;
+    return false;
 }
 
 static gboolean
diff --git a/oslib-posix.c b/oslib-posix.c
index 9db9c3d..dbeb627 100644
--- a/oslib-posix.c
+++ b/oslib-posix.c
@@ -61,6 +61,9 @@  static int running_on_valgrind = -1;
 #ifdef CONFIG_LINUX
 #include <sys/syscall.h>
 #endif
+#ifdef CONFIG_EVENTFD
+#include <sys/eventfd.h>
+#endif
 
 int qemu_get_thread_id(void)
 {
@@ -180,6 +183,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;
+        fds[1] = dup(ret);
+        if (fds[1] == -1) {
+            close(ret);
+            return -1;
+        }
+        qemu_set_cloexec(ret);
+        qemu_set_cloexec(fds[1]);
+        return 0;
+    }
+    if (errno != ENOSYS) {
+        return -1;
+    }
+#endif
+
+    return qemu_pipe(fds);
+}
+
 int qemu_utimens(const char *path, const struct timespec *times)
 {
     struct timeval tv[2], tv_now;
diff --git a/qemu-aio.h b/qemu-aio.h
index 1b7eb6e..2354617 100644
--- a/qemu-aio.h
+++ b/qemu-aio.h
@@ -125,6 +125,7 @@  void aio_notify(AioContext *ctx);
  * These are internal functions used by the QEMU main loop.
  */
 int aio_bh_poll(AioContext *ctx);
+void aio_bh_update_timeout(AioContext *ctx, uint32_t *timeout);
 
 /**
  * qemu_bh_schedule: Schedule a bottom half.
diff --git a/qemu-common.h b/qemu-common.h
index ac9985c..5059a97 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -218,6 +218,7 @@  ssize_t qemu_recv_full(int fd, void *buf, size_t count, int flags)
     QEMU_WARN_UNUSED_RESULT;
 
 #ifndef _WIN32
+int qemu_eventfd(int pipefd[2]);
 int qemu_pipe(int pipefd[2]);
 #endif