Patchwork [11/17] aio: make AioContexts GSources

login
register
mail settings
Submitter Paolo Bonzini
Date Sept. 25, 2012, 12:55 p.m.
Message ID <1348577763-12920-12-git-send-email-pbonzini@redhat.com>
Download mbox | patch
Permalink /patch/186816/
State New
Headers show

Comments

Paolo Bonzini - Sept. 25, 2012, 12:55 p.m.
This lets AioContexts be used (optionally) with a glib main loop.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 aio-posix.c |  4 ++++
 aio-win32.c |  4 ++++
 async.c     | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 qemu-aio.h  | 23 ++++++++++++++++++++++
 4 file modificati, 95 inserzioni(+). 1 rimozione(-)
Anthony Liguori - Sept. 25, 2012, 10:06 p.m.
Paolo Bonzini <pbonzini@redhat.com> writes:

> This lets AioContexts be used (optionally) with a glib main loop.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  aio-posix.c |  4 ++++
>  aio-win32.c |  4 ++++
>  async.c     | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  qemu-aio.h  | 23 ++++++++++++++++++++++
>  4 file modificati, 95 inserzioni(+). 1 rimozione(-)
>
> diff --git a/aio-posix.c b/aio-posix.c
> index c848a9f..e29ece9 100644
> --- a/aio-posix.c
> +++ b/aio-posix.c
> @@ -56,6 +56,8 @@ void aio_set_fd_handler(AioContext *ctx,
>      /* Are we deleting the fd handler? */
>      if (!io_read && !io_write) {
>          if (node) {
> +            g_source_remove_poll(&ctx->source, &node->pfd);
> +
>              /* If the lock is held, just mark the node as deleted */
>              if (ctx->walking_handlers) {
>                  node->deleted = 1;
> @@ -75,6 +77,8 @@ void aio_set_fd_handler(AioContext *ctx,
>              node = g_malloc0(sizeof(AioHandler));
>              node->pfd.fd = fd;
>              QLIST_INSERT_HEAD(&ctx->aio_handlers, node, node);
> +
> +            g_source_add_poll(&ctx->source, &node->pfd);
>          }
>          /* Update handler with latest information */
>          node->io_read = io_read;
> diff --git a/aio-win32.c b/aio-win32.c
> index c46dfb2..5057371 100644
> --- a/aio-win32.c
> +++ b/aio-win32.c
> @@ -45,6 +45,8 @@ void aio_set_event_notifier(AioContext *ctx,
>      /* Are we deleting the fd handler? */
>      if (!io_notify) {
>          if (node) {
> +            g_source_remove_poll(&ctx->source, &node->pfd);
> +

Why remove vs. setting events = 0?

add_poll/remove_poll also comes with an event loop notify which I don't
think is strictly necessary here.

>              /* If the lock is held, just mark the node as deleted */
>              if (ctx->walking_handlers) {
>                  node->deleted = 1;
> @@ -66,6 +68,8 @@ void aio_set_event_notifier(AioContext *ctx,
>              node->pfd.fd = (uintptr_t)event_notifier_get_handle(e);
>              node->pfd.events = G_IO_IN;
>              QLIST_INSERT_HEAD(&ctx->aio_handlers, node, node);
> +
> +            g_source_add_poll(&ctx->source, &node->pfd);
>          }
>          /* Update handler with latest information */
>          node->io_notify = io_notify;
> diff --git a/async.c b/async.c
> index 513bdd7..ed2bd3f 100644
> --- a/async.c
> +++ b/async.c
> @@ -136,10 +136,73 @@ void aio_bh_update_timeout(AioContext *ctx, uint32_t *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 FALSE;
> +}
> +
> +static gboolean
> +aio_ctx_check(GSource *source)
> +{
> +    AioContext *ctx = (AioContext *) source;
> +    QEMUBH *bh;
> +
> +    for (bh = ctx->first_bh; bh; bh = bh->next) {
> +        if (!bh->deleted && bh->scheduled) {
> +            return true;
> +	}
> +    }
> +    return aio_pending(ctx);
> +}

Think you've got some copy/paste leftover glib coding style.  Probably
should use TRUE/true consistently too.  I think using TRUE/FALSE for
gboolean and true/false for bool is reasonable.

> +
> +static gboolean
> +aio_ctx_dispatch(GSource     *source,
> +                 GSourceFunc  callback,
> +                 gpointer     user_data)
> +{
> +    AioContext *ctx = (AioContext *) source;
> +
> +    assert(callback == NULL);
> +    aio_poll(ctx, false);
> +    return TRUE;
> +}
> +
> +static GSourceFuncs aio_source_funcs = {
> +    aio_ctx_prepare,
> +    aio_ctx_check,
> +    aio_ctx_dispatch,
> +    NULL
> +};
> +
> +GSource *aio_get_g_source(AioContext *ctx)
> +{
> +    g_source_ref(&ctx->source);
> +    return &ctx->source;
> +}
>  
>  AioContext *aio_context_new(void)
>  {
> -    return g_new0(AioContext, 1);
> +    return (AioContext *) g_source_new(&aio_source_funcs, sizeof(AioContext));
> +}
> +
> +void aio_context_ref(AioContext *ctx)
> +{
> +    g_source_ref(&ctx->source);
> +}
> +
> +void aio_context_unref(AioContext *ctx)
> +{
> +    g_source_unref(&ctx->source);
>  }
>  
>  void aio_flush(AioContext *ctx)
> diff --git a/qemu-aio.h b/qemu-aio.h
> index ac24896..aedf66c 100644
> --- a/qemu-aio.h
> +++ b/qemu-aio.h
> @@ -44,6 +44,8 @@ typedef void QEMUBHFunc(void *opaque);
>  typedef void IOHandler(void *opaque);
>  
>  typedef struct AioContext {
> +    GSource source;
> +
>      /* The list of registered AIO handlers */
>      QLIST_HEAD(, AioHandler) aio_handlers;
>  
> @@ -75,6 +77,22 @@ typedef int (AioFlushEventNotifierHandler)(EventNotifier *e);
>  AioContext *aio_context_new(void);
>  
>  /**
> + * aio_context_ref:
> + * @ctx: The AioContext to operate on.
> + *
> + * Add a reference to an AioContext.
> + */
> +void aio_context_ref(AioContext *ctx);
> +
> +/**
> + * aio_context_unref:
> + * @ctx: The AioContext to operate on.
> + *
> + * Drop a reference to an AioContext.
> + */
> +void aio_context_unref(AioContext *ctx);
> +
> +/**
>   * aio_bh_new: Allocate a new bottom half structure.
>   *
>   * Bottom halves are lightweight callbacks whose invocation is guaranteed
> @@ -188,6 +206,11 @@ void aio_set_event_notifier(AioContext *ctx,
>                              EventNotifierHandler *io_read,
>                              AioFlushEventNotifierHandler *io_flush);
>  
> +/* Return a GSource that lets the main loop poll the file descriptors attached
> + * to this AioContext.
> + */
> +GSource *aio_get_g_source(AioContext *ctx);
> +
>  /* Functions to operate on the main QEMU AioContext.  */
>  
>  void qemu_aio_flush(void);
> -- 
> 1.7.12

I kind of dislike the fact that we've got a single source for all bottom
halves but this is definitely a good starting point.

The GSource implementation looks right to me.

Regards,

Anthony Liguori
Paolo Bonzini - Sept. 26, 2012, 6:40 a.m.
Il 26/09/2012 00:06, Anthony Liguori ha scritto:
>> >          if (node) {
>> > +            g_source_remove_poll(&ctx->source, &node->pfd);
>> > +
> Why remove vs. setting events = 0?

Because otherwise you'd get a dangling pointer to node->pfd. :)

Paolo

> add_poll/remove_poll also comes with an event loop notify which I don't
> think is strictly necessary here.
>

Patch

diff --git a/aio-posix.c b/aio-posix.c
index c848a9f..e29ece9 100644
--- a/aio-posix.c
+++ b/aio-posix.c
@@ -56,6 +56,8 @@  void aio_set_fd_handler(AioContext *ctx,
     /* Are we deleting the fd handler? */
     if (!io_read && !io_write) {
         if (node) {
+            g_source_remove_poll(&ctx->source, &node->pfd);
+
             /* If the lock is held, just mark the node as deleted */
             if (ctx->walking_handlers) {
                 node->deleted = 1;
@@ -75,6 +77,8 @@  void aio_set_fd_handler(AioContext *ctx,
             node = g_malloc0(sizeof(AioHandler));
             node->pfd.fd = fd;
             QLIST_INSERT_HEAD(&ctx->aio_handlers, node, node);
+
+            g_source_add_poll(&ctx->source, &node->pfd);
         }
         /* Update handler with latest information */
         node->io_read = io_read;
diff --git a/aio-win32.c b/aio-win32.c
index c46dfb2..5057371 100644
--- a/aio-win32.c
+++ b/aio-win32.c
@@ -45,6 +45,8 @@  void aio_set_event_notifier(AioContext *ctx,
     /* Are we deleting the fd handler? */
     if (!io_notify) {
         if (node) {
+            g_source_remove_poll(&ctx->source, &node->pfd);
+
             /* If the lock is held, just mark the node as deleted */
             if (ctx->walking_handlers) {
                 node->deleted = 1;
@@ -66,6 +68,8 @@  void aio_set_event_notifier(AioContext *ctx,
             node->pfd.fd = (uintptr_t)event_notifier_get_handle(e);
             node->pfd.events = G_IO_IN;
             QLIST_INSERT_HEAD(&ctx->aio_handlers, node, node);
+
+            g_source_add_poll(&ctx->source, &node->pfd);
         }
         /* Update handler with latest information */
         node->io_notify = io_notify;
diff --git a/async.c b/async.c
index 513bdd7..ed2bd3f 100644
--- a/async.c
+++ b/async.c
@@ -136,10 +136,73 @@  void aio_bh_update_timeout(AioContext *ctx, uint32_t *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 FALSE;
+}
+
+static gboolean
+aio_ctx_check(GSource *source)
+{
+    AioContext *ctx = (AioContext *) source;
+    QEMUBH *bh;
+
+    for (bh = ctx->first_bh; bh; bh = bh->next) {
+        if (!bh->deleted && bh->scheduled) {
+            return true;
+	}
+    }
+    return aio_pending(ctx);
+}
+
+static gboolean
+aio_ctx_dispatch(GSource     *source,
+                 GSourceFunc  callback,
+                 gpointer     user_data)
+{
+    AioContext *ctx = (AioContext *) source;
+
+    assert(callback == NULL);
+    aio_poll(ctx, false);
+    return TRUE;
+}
+
+static GSourceFuncs aio_source_funcs = {
+    aio_ctx_prepare,
+    aio_ctx_check,
+    aio_ctx_dispatch,
+    NULL
+};
+
+GSource *aio_get_g_source(AioContext *ctx)
+{
+    g_source_ref(&ctx->source);
+    return &ctx->source;
+}
 
 AioContext *aio_context_new(void)
 {
-    return g_new0(AioContext, 1);
+    return (AioContext *) g_source_new(&aio_source_funcs, sizeof(AioContext));
+}
+
+void aio_context_ref(AioContext *ctx)
+{
+    g_source_ref(&ctx->source);
+}
+
+void aio_context_unref(AioContext *ctx)
+{
+    g_source_unref(&ctx->source);
 }
 
 void aio_flush(AioContext *ctx)
diff --git a/qemu-aio.h b/qemu-aio.h
index ac24896..aedf66c 100644
--- a/qemu-aio.h
+++ b/qemu-aio.h
@@ -44,6 +44,8 @@  typedef void QEMUBHFunc(void *opaque);
 typedef void IOHandler(void *opaque);
 
 typedef struct AioContext {
+    GSource source;
+
     /* The list of registered AIO handlers */
     QLIST_HEAD(, AioHandler) aio_handlers;
 
@@ -75,6 +77,22 @@  typedef int (AioFlushEventNotifierHandler)(EventNotifier *e);
 AioContext *aio_context_new(void);
 
 /**
+ * aio_context_ref:
+ * @ctx: The AioContext to operate on.
+ *
+ * Add a reference to an AioContext.
+ */
+void aio_context_ref(AioContext *ctx);
+
+/**
+ * aio_context_unref:
+ * @ctx: The AioContext to operate on.
+ *
+ * Drop a reference to an AioContext.
+ */
+void aio_context_unref(AioContext *ctx);
+
+/**
  * aio_bh_new: Allocate a new bottom half structure.
  *
  * Bottom halves are lightweight callbacks whose invocation is guaranteed
@@ -188,6 +206,11 @@  void aio_set_event_notifier(AioContext *ctx,
                             EventNotifierHandler *io_read,
                             AioFlushEventNotifierHandler *io_flush);
 
+/* Return a GSource that lets the main loop poll the file descriptors attached
+ * to this AioContext.
+ */
+GSource *aio_get_g_source(AioContext *ctx);
+
 /* Functions to operate on the main QEMU AioContext.  */
 
 void qemu_aio_flush(void);