diff mbox

[v3,08/10] aio: extract aio_dispatch() from aio_poll()

Message ID 1359979973-31338-9-git-send-email-stefanha@redhat.com
State New
Headers show

Commit Message

Stefan Hajnoczi Feb. 4, 2013, 12:12 p.m. UTC 
We will need to loop over AioHandlers calling ->io_read()/->io_write()
when aio_poll() is converted from select(2) to g_poll(2).

Luckily the code for this already exists, extract it into the new
aio_dispatch() function.

Two small changes:

 * aio_poll() checks !node->deleted to avoid calling handlers that have
   been deleted.

 * Fix typo 'then' -> 'them' in aio_poll() comment.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 aio-posix.c | 57 +++++++++++++++++++++++++++++++++++----------------------
 1 file changed, 35 insertions(+), 22 deletions(-)

Comments

Laszlo Ersek Feb. 6, 2013, 6:24 p.m. UTC | #1 
On 02/04/13 13:12, Stefan Hajnoczi wrote:
> We will need to loop over AioHandlers calling ->io_read()/->io_write()
> when aio_poll() is converted from select(2) to g_poll(2).
> 
> Luckily the code for this already exists, extract it into the new
> aio_dispatch() function.
> 
> Two small changes:
> 
>  * aio_poll() checks !node->deleted to avoid calling handlers that have
>    been deleted.

This is actually an unrelated bugfix, isn't it?

> 
>  * Fix typo 'then' -> 'them' in aio_poll() comment.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  aio-posix.c | 57 +++++++++++++++++++++++++++++++++++----------------------
>  1 file changed, 35 insertions(+), 22 deletions(-)

Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Stefan Hajnoczi Feb. 7, 2013, 8:24 a.m. UTC | #2 
On Wed, Feb 06, 2013 at 07:24:42PM +0100, Laszlo Ersek wrote:
> On 02/04/13 13:12, Stefan Hajnoczi wrote:
> > We will need to loop over AioHandlers calling ->io_read()/->io_write()
> > when aio_poll() is converted from select(2) to g_poll(2).
> > 
> > Luckily the code for this already exists, extract it into the new
> > aio_dispatch() function.
> > 
> > Two small changes:
> > 
> >  * aio_poll() checks !node->deleted to avoid calling handlers that have
> >    been deleted.
> 
> This is actually an unrelated bugfix, isn't it?

It is related because there are two copies of the aio handler dispatch
code - one for glib and one for select.  The !node->deleted check is
present in the select version of the loop but not in the glib version.

In the next patch I get rid of select version and reuse the function
extracted from the glib version by this patch.  I wouldn't be confident
in reusing the extracted function without !node->deleted.

Stefan
diff mbox

Patch

diff --git a/aio-posix.c b/aio-posix.c
index fe4dbb4..35131a3 100644
--- a/aio-posix.c
+++ b/aio-posix.c
@@ -129,30 +129,12 @@  bool aio_pending(AioContext *ctx)
     return false;
 }
 
-bool aio_poll(AioContext *ctx, bool blocking)
+static bool aio_dispatch(AioContext *ctx)
 {
-    static struct timeval tv0;
     AioHandler *node;
-    fd_set rdfds, wrfds;
-    int max_fd = -1;
-    int ret;
-    bool busy, progress;
-
-    progress = false;
-
-    /*
-     * If there are callbacks left that have been queued, we need to call then.
-     * Do not call select in this case, because it is possible that the caller
-     * does not need a complete flush (as is the case for qemu_aio_wait loops).
-     */
-    if (aio_bh_poll(ctx)) {
-        blocking = false;
-        progress = true;
-    }
+    bool progress = false;
 
     /*
-     * Then dispatch any pending callbacks from the GSource.
-     *
      * We have to walk very carefully in case qemu_aio_set_fd_handler is
      * called while we're walking.
      */
@@ -167,11 +149,15 @@  bool aio_poll(AioContext *ctx, bool blocking)
         node->pfd.revents = 0;
 
         /* See comment in aio_pending.  */
-        if (revents & (G_IO_IN | G_IO_HUP | G_IO_ERR) && node->io_read) {
+        if (!node->deleted &&
+            (revents & (G_IO_IN | G_IO_HUP | G_IO_ERR)) &&
+            node->io_read) {
             node->io_read(node->opaque);
             progress = true;
         }
-        if (revents & (G_IO_OUT | G_IO_ERR) && node->io_write) {
+        if (!node->deleted &&
+            (revents & (G_IO_OUT | G_IO_ERR)) &&
+            node->io_write) {
             node->io_write(node->opaque);
             progress = true;
         }
@@ -186,6 +172,33 @@  bool aio_poll(AioContext *ctx, bool blocking)
             g_free(tmp);
         }
     }
+    return progress;
+}
+
+bool aio_poll(AioContext *ctx, bool blocking)
+{
+    static struct timeval tv0;
+    AioHandler *node;
+    fd_set rdfds, wrfds;
+    int max_fd = -1;
+    int ret;
+    bool busy, progress;
+
+    progress = false;
+
+    /*
+     * If there are callbacks left that have been queued, we need to call them.
+     * Do not call select in this case, because it is possible that the caller
+     * does not need a complete flush (as is the case for qemu_aio_wait loops).
+     */
+    if (aio_bh_poll(ctx)) {
+        blocking = false;
+        progress = true;
+    }
+
+    if (aio_dispatch(ctx)) {
+        progress = true;
+    }
 
     if (progress && !blocking) {
         return true;