| Message ID | 1461322564-17440-5-git-send-email-famz@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Am 22.04.2016 um 12:56 hat Fam Zheng geschrieben: > aio_poll doesn't poll the external nodes so this should never be true, > but aio_ctx_dispatch may get notified by the events from GSource. To > make bdrv_drained_begin effective in main loop, we should check the > is_external flag here too. > > This could result in a few busy polls because the fd is left unhandled, > but the drained section is only transient and shouldn't be longer than > one or two event loop iterations. Are the busy polls because aio_ctx_check() calls aio_pending(), which still returns true, even if only disabled AioHandlers are ready? If so, should we just add the is_external check to aio_pending(), too? Kevin > Signed-off-by: Fam Zheng <famz@redhat.com> > --- > aio-posix.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/aio-posix.c b/aio-posix.c > index 7fd565f..a7c9304 100644 > --- a/aio-posix.c > +++ b/aio-posix.c > @@ -323,6 +323,7 @@ bool aio_dispatch(AioContext *ctx) > > if (!node->deleted && > (revents & (G_IO_IN | G_IO_HUP | G_IO_ERR)) && > + aio_node_check(ctx, node->is_external) && > node->io_read) { > node->io_read(node->opaque); > > @@ -333,6 +334,7 @@ bool aio_dispatch(AioContext *ctx) > } > if (!node->deleted && > (revents & (G_IO_OUT | G_IO_ERR)) && > + aio_node_check(ctx, node->is_external) && > node->io_write) { > node->io_write(node->opaque); > progress = true; > -- > 2.8.0 >
On Fri, 04/22 13:38, Kevin Wolf wrote: > Am 22.04.2016 um 12:56 hat Fam Zheng geschrieben: > > aio_poll doesn't poll the external nodes so this should never be true, > > but aio_ctx_dispatch may get notified by the events from GSource. To > > make bdrv_drained_begin effective in main loop, we should check the > > is_external flag here too. > > > > This could result in a few busy polls because the fd is left unhandled, > > but the drained section is only transient and shouldn't be longer than > > one or two event loop iterations. > > Are the busy polls because aio_ctx_check() calls aio_pending(), which > still returns true, even if only disabled AioHandlers are ready? If so, > should we just add the is_external check to aio_pending(), too? Good point, that is certainly better! Will add that. Fam > > Kevin > > > Signed-off-by: Fam Zheng <famz@redhat.com> > > --- > > aio-posix.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/aio-posix.c b/aio-posix.c > > index 7fd565f..a7c9304 100644 > > --- a/aio-posix.c > > +++ b/aio-posix.c > > @@ -323,6 +323,7 @@ bool aio_dispatch(AioContext *ctx) > > > > if (!node->deleted && > > (revents & (G_IO_IN | G_IO_HUP | G_IO_ERR)) && > > + aio_node_check(ctx, node->is_external) && > > node->io_read) { > > node->io_read(node->opaque); > > > > @@ -333,6 +334,7 @@ bool aio_dispatch(AioContext *ctx) > > } > > if (!node->deleted && > > (revents & (G_IO_OUT | G_IO_ERR)) && > > + aio_node_check(ctx, node->is_external) && > > node->io_write) { > > node->io_write(node->opaque); > > progress = true; > > -- > > 2.8.0 > >
diff --git a/aio-posix.c b/aio-posix.c index 7fd565f..a7c9304 100644 --- a/aio-posix.c +++ b/aio-posix.c @@ -323,6 +323,7 @@ bool aio_dispatch(AioContext *ctx) if (!node->deleted && (revents & (G_IO_IN | G_IO_HUP | G_IO_ERR)) && + aio_node_check(ctx, node->is_external) && node->io_read) { node->io_read(node->opaque); @@ -333,6 +334,7 @@ bool aio_dispatch(AioContext *ctx) } if (!node->deleted && (revents & (G_IO_OUT | G_IO_ERR)) && + aio_node_check(ctx, node->is_external) && node->io_write) { node->io_write(node->opaque); progress = true;
aio_poll doesn't poll the external nodes so this should never be true, but aio_ctx_dispatch may get notified by the events from GSource. To make bdrv_drained_begin effective in main loop, we should check the is_external flag here too. This could result in a few busy polls because the fd is left unhandled, but the drained section is only transient and shouldn't be longer than one or two event loop iterations. Signed-off-by: Fam Zheng <famz@redhat.com> --- aio-posix.c | 2 ++ 1 file changed, 2 insertions(+)