| Message ID | 1412048118-24834-2-git-send-email-famz@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Sep 30, 2014 at 11:35:17AM +0800, Fam Zheng wrote: > diff --git a/main-loop.c b/main-loop.c > index d2e64f1..4641ef4 100644 > --- a/main-loop.c > +++ b/main-loop.c > @@ -234,7 +234,8 @@ static int os_host_main_loop_wait(int64_t timeout) > spin_counter++; > } > > - ret = qemu_poll_ns((GPollFD *)gpollfds->data, gpollfds->len, timeout); > + ret = qemu_poll_ns(qemu_aio_context, (GPollFD *)gpollfds->data, > + gpollfds->len, timeout); > > if (timeout) { > qemu_mutex_lock_iothread(); > @@ -439,7 +440,8 @@ static int os_host_main_loop_wait(int64_t timeout) > poll_timeout_ns = qemu_soonest_timeout(poll_timeout_ns, timeout); > > qemu_mutex_unlock_iothread(); > - g_poll_ret = qemu_poll_ns(poll_fds, n_poll_fds + w->num, poll_timeout_ns); > + g_poll_ret = qemu_poll_ns(qemu_aio_context, > + poll_fds, n_poll_fds + w->num, poll_timeout_ns); > > qemu_mutex_lock_iothread(); > if (g_poll_ret > 0) { This is a hack. The immediate problem is that we're not holding the QEMU global mutex but are accessing qemu_aio_context. What if other threads touch qemu_aio_context while they hold the QEMU global mutex? You didn't indicate what thread-safety rules need to be obeyed so I wonder whether you looked into this. I'm also concerned that this is somewhat abusing AioContext. You want to stash fields in there but this poll call is about more than just AioContext, it also includes non-AioContext file descriptors. So it seems like a kludge to put the fields into AioContext. Can you put the state into a separate struct so it's easy to understand its thread-safety characteristics? Stefan
diff --git a/aio-posix.c b/aio-posix.c index d3ac06e..d7a3ce3 100644 --- a/aio-posix.c +++ b/aio-posix.c @@ -228,7 +228,7 @@ bool aio_poll(AioContext *ctx, bool blocking) ctx->walking_handlers--; /* wait until next event */ - ret = qemu_poll_ns((GPollFD *)ctx->pollfds->data, + ret = qemu_poll_ns(ctx, (GPollFD *)ctx->pollfds->data, ctx->pollfds->len, blocking ? aio_compute_timeout(ctx) : 0); diff --git a/include/qemu/timer.h b/include/qemu/timer.h index 5f5210d..66d20d0 100644 --- a/include/qemu/timer.h +++ b/include/qemu/timer.h @@ -647,6 +647,7 @@ int qemu_timeout_ns_to_ms(int64_t ns); /** * qemu_poll_ns: + * @ctx: Aio Context we are polling in * @fds: Array of file descriptors * @nfds: number of file descriptors * @timeout: timeout in nanoseconds @@ -656,7 +657,7 @@ int qemu_timeout_ns_to_ms(int64_t ns); * * Returns: number of fds ready */ -int qemu_poll_ns(GPollFD *fds, guint nfds, int64_t timeout); +int qemu_poll_ns(AioContext *ctx, GPollFD *fds, guint nfds, int64_t timeout); /** * qemu_soonest_timeout: diff --git a/main-loop.c b/main-loop.c index d2e64f1..4641ef4 100644 --- a/main-loop.c +++ b/main-loop.c @@ -234,7 +234,8 @@ static int os_host_main_loop_wait(int64_t timeout) spin_counter++; } - ret = qemu_poll_ns((GPollFD *)gpollfds->data, gpollfds->len, timeout); + ret = qemu_poll_ns(qemu_aio_context, (GPollFD *)gpollfds->data, + gpollfds->len, timeout); if (timeout) { qemu_mutex_lock_iothread(); @@ -439,7 +440,8 @@ static int os_host_main_loop_wait(int64_t timeout) poll_timeout_ns = qemu_soonest_timeout(poll_timeout_ns, timeout); qemu_mutex_unlock_iothread(); - g_poll_ret = qemu_poll_ns(poll_fds, n_poll_fds + w->num, poll_timeout_ns); + g_poll_ret = qemu_poll_ns(qemu_aio_context, + poll_fds, n_poll_fds + w->num, poll_timeout_ns); qemu_mutex_lock_iothread(); if (g_poll_ret > 0) { diff --git a/qemu-timer.c b/qemu-timer.c index 00a5d35..7336b20 100644 --- a/qemu-timer.c +++ b/qemu-timer.c @@ -307,7 +307,7 @@ int qemu_timeout_ns_to_ms(int64_t ns) /* qemu implementation of g_poll which uses a nanosecond timeout but is * otherwise identical to g_poll */ -int qemu_poll_ns(GPollFD *fds, guint nfds, int64_t timeout) +int qemu_poll_ns(AioContext *ctx, GPollFD *fds, guint nfds, int64_t timeout) { #ifdef CONFIG_PPOLL if (timeout < 0) {
qemu_poll_ns may ultilize the information in AioContext to achieve better performance. Signed-off-by: Fam Zheng <famz@redhat.com> --- aio-posix.c | 2 +- include/qemu/timer.h | 3 ++- main-loop.c | 6 ++++-- qemu-timer.c | 2 +- 4 files changed, 8 insertions(+), 5 deletions(-)