[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 09/17] aio: prepare for introducing GSource-base
From: |
Anthony Liguori |
Subject: |
Re: [Qemu-devel] [PATCH 09/17] aio: prepare for introducing GSource-based dispatch |
Date: |
Tue, 25 Sep 2012 17:01:19 -0500 |
User-agent: |
Notmuch/0.13.2+93~ged93d79 (http://notmuchmail.org) Emacs/23.3.1 (x86_64-pc-linux-gnu) |
Paolo Bonzini <address@hidden> writes:
> This adds a GPollFD to each AioHandler. It will then be possible to
> attach these GPollFDs to a GSource, and from there to the main loop.
> aio_wait examines the GPollFDs and avoids calling select() if any
> is set (similar to what it does if bottom halves are available).
>
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
> aio.c | 82
> +++++++++++++++++++++++++++++++++++++++++++++++++++++---------
> qemu-aio.h | 7 ++++++
> 2 file modificati, 78 inserzioni(+), 11 rimozioni(-)
>
> diff --git a/aio.c b/aio.c
> index 95ad467..c848a9f 100644
> --- a/aio.c
> +++ b/aio.c
> @@ -20,7 +20,7 @@
>
> struct AioHandler
> {
> - int fd;
> + GPollFD pfd;
> IOHandler *io_read;
> IOHandler *io_write;
> AioFlushHandler *io_flush;
> @@ -34,7 +34,7 @@ static AioHandler *find_aio_handler(AioContext *ctx, int fd)
> AioHandler *node;
>
> QLIST_FOREACH(node, &ctx->aio_handlers, node) {
> - if (node->fd == fd)
> + if (node->pfd.fd == fd)
> if (!node->deleted)
> return node;
> }
> @@ -57,9 +57,10 @@ void aio_set_fd_handler(AioContext *ctx,
> if (!io_read && !io_write) {
> if (node) {
> /* If the lock is held, just mark the node as deleted */
> - if (ctx->walking_handlers)
> + if (ctx->walking_handlers) {
> node->deleted = 1;
> - else {
> + node->pfd.revents = 0;
> + } else {
> /* Otherwise, delete it for real. We can't just mark it as
> * deleted because deleted nodes are only cleaned up after
> * releasing the walking_handlers lock.
> @@ -72,7 +73,7 @@ void aio_set_fd_handler(AioContext *ctx,
> if (node == NULL) {
> /* Alloc and insert if it's not already there */
> node = g_malloc0(sizeof(AioHandler));
> - node->fd = fd;
> + node->pfd.fd = fd;
> QLIST_INSERT_HEAD(&ctx->aio_handlers, node, node);
> }
> /* Update handler with latest information */
> @@ -80,6 +81,10 @@ void aio_set_fd_handler(AioContext *ctx,
> node->io_write = io_write;
> node->io_flush = io_flush;
> node->opaque = opaque;
> +
> + node->pfd.events = G_IO_ERR;
> + node->pfd.events |= (io_read ? G_IO_IN | G_IO_HUP : 0);
> + node->pfd.events |= (io_write ? G_IO_OUT : 0);
> }
Should we even set G_IO_ERR? I think that corresponds to exceptfd in
select() but we've never set that historically. I know glib recommends
it but I don't think it's applicable to how we use it.
Moreover, the way you do dispatch, if G_IO_ERR did occur, we'd dispatch
both the read and write handlers which definitely isn't right.
I think it's easiest just to drop it.
> }
>
> @@ -93,6 +98,25 @@ void aio_set_event_notifier(AioContext *ctx,
> (AioFlushHandler *)io_flush, notifier);
> }
>
> +bool aio_pending(AioContext *ctx)
> +{
> + AioHandler *node;
> +
> + QLIST_FOREACH(node, &ctx->aio_handlers, node) {
> + int revents;
> +
> + revents = node->pfd.revents & node->pfd.events;
> + if (revents & (G_IO_IN | G_IO_HUP | G_IO_ERR) && node->io_read) {
> + return true;
> + }
> + if (revents & (G_IO_OUT | G_IO_ERR) && node->io_write) {
> + return true;
> + }
> + }
> +
> + return false;
> +}
> +
> bool aio_poll(AioContext *ctx, bool blocking)
> {
> static struct timeval tv0;
> @@ -114,6 +138,42 @@ bool aio_poll(AioContext *ctx, bool blocking)
> progress = true;
> }
>
> + /*
> + * 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.
> + */
> + node = QLIST_FIRST(&ctx->aio_handlers);
> + while (node) {
> + AioHandler *tmp;
> + int revents;
> +
> + ctx->walking_handlers++;
> +
> + revents = node->pfd.revents & node->pfd.events;
> + node->pfd.revents &= ~revents;
This is interesting and I must admit I don't understand why it's
necessary. What case are you trying to handle?
> +
> + if (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) {
> + node->io_write(node->opaque);
> + progress = true;
> + }
> +
> + tmp = node;
> + node = QLIST_NEXT(node, node);
> +
> + ctx->walking_handlers--;
> +
> + if (!ctx->walking_handlers && tmp->deleted) {
> + QLIST_REMOVE(tmp, node);
> + g_free(tmp);
> + }
> + }
> +
> if (progress && !blocking) {
> return true;
> }
> @@ -137,12 +197,12 @@ bool aio_poll(AioContext *ctx, bool blocking)
> busy = true;
> }
> if (!node->deleted && node->io_read) {
> - FD_SET(node->fd, &rdfds);
> - max_fd = MAX(max_fd, node->fd + 1);
> + FD_SET(node->pfd.fd, &rdfds);
> + max_fd = MAX(max_fd, node->pfd.fd + 1);
> }
> if (!node->deleted && node->io_write) {
> - FD_SET(node->fd, &wrfds);
> - max_fd = MAX(max_fd, node->fd + 1);
> + FD_SET(node->pfd.fd, &wrfds);
> + max_fd = MAX(max_fd, node->pfd.fd + 1);
> }
> }
>
> @@ -167,12 +227,12 @@ bool aio_poll(AioContext *ctx, bool blocking)
> ctx->walking_handlers++;
>
> if (!node->deleted &&
> - FD_ISSET(node->fd, &rdfds) &&
> + FD_ISSET(node->pfd.fd, &rdfds) &&
> node->io_read) {
> node->io_read(node->opaque);
> }
> if (!node->deleted &&
> - FD_ISSET(node->fd, &wrfds) &&
> + FD_ISSET(node->pfd.fd, &wrfds) &&
> node->io_write) {
> node->io_write(node->opaque);
> }
> diff --git a/qemu-aio.h b/qemu-aio.h
> index f19201e..ac24896 100644
> --- a/qemu-aio.h
> +++ b/qemu-aio.h
> @@ -133,6 +133,13 @@ void qemu_bh_delete(QEMUBH *bh);
> * outstanding AIO operations have been completed or cancelled. */
> void aio_flush(AioContext *ctx);
>
> +/* Return whether there are any pending callbacks from the GSource
> + * attached to the AioContext.
> + *
> + * This is used internally in the implementation of the GSource.
> + */
> +bool aio_pending(AioContext *ctx);
> +
> /* Progress in completing AIO work to occur. This can issue new pending
> * aio as a result of executing I/O completion or bh callbacks.
> *
> --
> 1.7.12
Regards,
Anthony Liguori
- [Qemu-devel] [PATCH 06/17] aio: introduce AioContext, move bottom halves there, (continued)
- [Qemu-devel] [PATCH 06/17] aio: introduce AioContext, move bottom halves there, Paolo Bonzini, 2012/09/25
- [Qemu-devel] [PATCH 05/17] aio: provide platform-independent API, Paolo Bonzini, 2012/09/25
- [Qemu-devel] [PATCH 07/17] aio: add I/O handlers to the AioContext interface, Paolo Bonzini, 2012/09/25
- [Qemu-devel] [PATCH 08/17] aio: add non-blocking variant of aio_wait, Paolo Bonzini, 2012/09/25
- [Qemu-devel] [PATCH 10/17] aio: add Win32 implementation, Paolo Bonzini, 2012/09/25
- [Qemu-devel] [PATCH 09/17] aio: prepare for introducing GSource-based dispatch, Paolo Bonzini, 2012/09/25
- [Qemu-devel] [PATCH 12/17] aio: add aio_notify, Paolo Bonzini, 2012/09/25
- [Qemu-devel] [PATCH 13/17] aio: call aio_notify after setting I/O handlers, Paolo Bonzini, 2012/09/25
- [Qemu-devel] [PATCH 14/17] main-loop: use GSource to poll AIO file descriptors, Paolo Bonzini, 2012/09/25