On 30/11/2016 14:36, Paolo Bonzini wrote:
>
>
> On 30/11/2016 14:31, Stefan Hajnoczi wrote:
>> On Tue, Nov 29, 2016 at 12:47:03PM +0100, Paolo Bonzini wrote:
>>> @@ -272,22 +275,32 @@ bool aio_prepare(AioContext *ctx)
>>> bool aio_pending(AioContext *ctx)
>>> {
>>> AioHandler *node;
>>> + bool result = false;
>>>
>>> - QLIST_FOREACH(node, &ctx->aio_handlers, node) {
>>> + /*
>>> + * We have to walk very carefully in case aio_set_fd_handler is
>>> + * called while we're walking.
>>> + */
>>> + qemu_lockcnt_inc(&ctx->list_lock);
>>> +
>>> + QLIST_FOREACH_RCU(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 &&
>>> aio_node_check(ctx, node->is_external)) {
>>> - return true;
>>> + result = true;
>>> + break;
>>> }
>>> if (revents & (G_IO_OUT | G_IO_ERR) && node->io_write &&
>>> aio_node_check(ctx, node->is_external)) {
>>> - return true;
>>> + result = true;
>>> + break;
>>> }
>>> }
>>> + qemu_lockcnt_dec(&ctx->list_lock);
>>>
>>> - return false;
>>> + return result;
>>> }
>>>
>>> bool aio_dispatch(AioContext *ctx)
>>> @@ -308,13 +321,12 @@ bool aio_dispatch(AioContext *ctx)
>>> * We have to walk very carefully in case aio_set_fd_handler is
>>> * called while we're walking.
>>> */
>>> - ctx->walking_handlers++;
>>> + qemu_lockcnt_inc(&ctx->list_lock);
>>>
>>> - QLIST_FOREACH_SAFE(node, &ctx->aio_handlers, node, tmp) {
>>> + QLIST_FOREACH_SAFE_RCU(node, &ctx->aio_handlers, node, tmp) {
>>> int revents;
>>>
>>> - revents = node->pfd.revents & node->pfd.events;
>>> - node->pfd.revents = 0;
>>> + revents = atomic_xchg(&node->pfd.revents, 0) & node->pfd.events;
>>
>> Why is node->pfd.revents accessed with atomic_*() here and in aio_poll()
>> but not in aio_pending()?
>
> It could use atomic_read there, indeed.
Actually, thanks to the (already committed) patches that limit aio_poll
to the I/O thread, these atomic accesses are not needed anymore.
Paolo
