Message ID | 1334232076-19018-6-git-send-email-pbonzini@redhat.com |
---|---|
State | New |
Headers | show |
Am 12.04.2012 14:00, schrieb Paolo Bonzini: > The definition of when qemu_aio_flush should loop is much simpler > than it looks. It just has to call qemu_aio_wait until it makes > no progress and all flush callbacks return false. qemu_aio_wait > is the logical place to tell the caller about this. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > aio.c | 44 ++++++++++++++++++-------------------------- > qemu-aio.h | 6 ++++-- > 2 files changed, 22 insertions(+), 28 deletions(-) > > diff --git a/aio.c b/aio.c > index f19b3c6..5fcc0c6 100644 > --- a/aio.c > +++ b/aio.c > @@ -99,41 +99,26 @@ int qemu_aio_set_fd_handler(int fd, > > void qemu_aio_flush(void) > { > - AioHandler *node; > - int ret; > - > - do { > - ret = 0; > - > - /* > - * If there are pending emulated aio start them now so flush > - * will be able to return 1. > - */ > - qemu_aio_wait(); > - > - QLIST_FOREACH(node, &aio_handlers, node) { > - if (node->io_flush) { > - ret |= node->io_flush(node->opaque); > - } > - } > - } while (qemu_bh_poll() || ret > 0); > + while (qemu_aio_wait()); > } > > -void qemu_aio_wait(void) > +bool qemu_aio_wait(void) > { > int ret; > > /* > * If there are callbacks left that have been queued, we need to call then. > - * Return afterwards to avoid waiting needlessly in select(). > + * 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 (qemu_bh_poll()) { > - return; > + return true; > } > > do { > AioHandler *node; > fd_set rdfds, wrfds; > + bool busy; > int max_fd = -1; > > walking_handlers = 1; > @@ -142,14 +127,18 @@ void qemu_aio_wait(void) > FD_ZERO(&wrfds); > > /* fill fd sets */ > + busy = false; > QLIST_FOREACH(node, &aio_handlers, node) { > /* If there aren't pending AIO operations, don't invoke callbacks. > * Otherwise, if there are no AIO requests, qemu_aio_wait() would > * wait indefinitely. > */ > - if (node->io_flush && node->io_flush(node->opaque) == 0) > - continue; > - > + if (node->io_flush) { > + if (node->io_flush(node->opaque) == 0) { > + continue; > + } > + busy = true; > + } > if (!node->deleted && node->io_read) { > FD_SET(node->fd, &rdfds); > max_fd = MAX(max_fd, node->fd + 1); > @@ -163,8 +152,9 @@ void qemu_aio_wait(void) > walking_handlers = 0; > > /* No AIO operations? Get us out of here */ > - if (max_fd == -1) > - break; > + if (!busy) { > + return false; > + } This can change the behaviour for aio_handlers that don't have a flush callback. Previously we would run into the select, now we don't. Hm, okay, such handlers don't exist. Maybe we should assert() it somewhere, but I guess then it's not a problem with this patch. Kevin
diff --git a/aio.c b/aio.c index f19b3c6..5fcc0c6 100644 --- a/aio.c +++ b/aio.c @@ -99,41 +99,26 @@ int qemu_aio_set_fd_handler(int fd, void qemu_aio_flush(void) { - AioHandler *node; - int ret; - - do { - ret = 0; - - /* - * If there are pending emulated aio start them now so flush - * will be able to return 1. - */ - qemu_aio_wait(); - - QLIST_FOREACH(node, &aio_handlers, node) { - if (node->io_flush) { - ret |= node->io_flush(node->opaque); - } - } - } while (qemu_bh_poll() || ret > 0); + while (qemu_aio_wait()); } -void qemu_aio_wait(void) +bool qemu_aio_wait(void) { int ret; /* * If there are callbacks left that have been queued, we need to call then. - * Return afterwards to avoid waiting needlessly in select(). + * 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 (qemu_bh_poll()) { - return; + return true; } do { AioHandler *node; fd_set rdfds, wrfds; + bool busy; int max_fd = -1; walking_handlers = 1; @@ -142,14 +127,18 @@ void qemu_aio_wait(void) FD_ZERO(&wrfds); /* fill fd sets */ + busy = false; QLIST_FOREACH(node, &aio_handlers, node) { /* If there aren't pending AIO operations, don't invoke callbacks. * Otherwise, if there are no AIO requests, qemu_aio_wait() would * wait indefinitely. */ - if (node->io_flush && node->io_flush(node->opaque) == 0) - continue; - + if (node->io_flush) { + if (node->io_flush(node->opaque) == 0) { + continue; + } + busy = true; + } if (!node->deleted && node->io_read) { FD_SET(node->fd, &rdfds); max_fd = MAX(max_fd, node->fd + 1); @@ -163,8 +152,9 @@ void qemu_aio_wait(void) walking_handlers = 0; /* No AIO operations? Get us out of here */ - if (max_fd == -1) - break; + if (!busy) { + return false; + } /* wait until next event */ ret = select(max_fd, &rdfds, &wrfds, NULL, NULL); @@ -204,4 +194,6 @@ void qemu_aio_wait(void) walking_handlers = 0; } } while (ret == 0); + + return true; } diff --git a/qemu-aio.h b/qemu-aio.h index 0fc8409..bfdd35f 100644 --- a/qemu-aio.h +++ b/qemu-aio.h @@ -48,8 +48,10 @@ void qemu_aio_flush(void); /* Wait for a single AIO completion to occur. This function will wait * until a single AIO event has completed and it will ensure something * has moved before returning. This can issue new pending aio as - * result of executing I/O completion or bh callbacks. */ -void qemu_aio_wait(void); + * result of executing I/O completion or bh callbacks. + * + * Return whether there is still any pending AIO operation. */ +bool qemu_aio_wait(void); /* Register a file descriptor and associated callbacks. Behaves very similarly * to qemu_set_fd_handler2. Unlike qemu_set_fd_handler2, these callbacks will
The definition of when qemu_aio_flush should loop is much simpler than it looks. It just has to call qemu_aio_wait until it makes no progress and all flush callbacks return false. qemu_aio_wait is the logical place to tell the caller about this. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- aio.c | 44 ++++++++++++++++++-------------------------- qemu-aio.h | 6 ++++-- 2 files changed, 22 insertions(+), 28 deletions(-)