[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 01/16] aio: introduce aio_co_schedule and aio_co
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH 01/16] aio: introduce aio_co_schedule and aio_co_wake |
Date: |
Wed, 18 Jan 2017 16:40:29 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 |
On 18/01/2017 15:33, Stefan Hajnoczi wrote:
> On Fri, Jan 13, 2017 at 02:17:16PM +0100, Paolo Bonzini wrote:
>> +static void co_schedule_bh_cb(void *opaque)
>> +{
>> + AioContext *ctx = opaque;
>> + QSLIST_HEAD(, Coroutine) straight, reversed;
>> +
>> + QSLIST_MOVE_ATOMIC(&reversed, &ctx->scheduled_coroutines);
>> + QSLIST_INIT(&straight);
>> +
>> + while (!QSLIST_EMPTY(&reversed)) {
>> + Coroutine *co = QSLIST_FIRST(&reversed);
>> + QSLIST_REMOVE_HEAD(&reversed, co_scheduled_next);
>> + QSLIST_INSERT_HEAD(&straight, co, co_scheduled_next);
>> + }
>> +
>> + while (!QSLIST_EMPTY(&straight)) {
>> + Coroutine *co = QSLIST_FIRST(&straight);
>> + QSLIST_REMOVE_HEAD(&straight, co_scheduled_next);
>> + trace_aio_co_schedule_bh_cb(ctx, co);
>> + qemu_coroutine_enter(co);
>> + }
>> +}
>
> ctx->scheduled_coroutines is a specialized CoQueue. Was there no way to
> modify and then use CoQueue instead of open coding it?
First of all, I'm trying to avoid a circular dependency when CoQueue can
use aio_co_schedule (indirectly through aio_co_wake) after patch 7.
Secondarily, co_schedule_bh_cb can perform a single pass on
ctx->scheduled_coroutines because it will be rescheduled by
aio_co_schedule. The same is not true for qemu_co_queue_restart_all.
Also, CoQueue can have multiple consumers, while scheduled_coroutines
cannot. Currently, CoQueue needs no thread-safety because it's
protected by AioContext and/or by the non-preemptive nature of
coroutines. Later, it's going to be protected by an external CoMutex,
just like a mutex/condvar pair. scheduled_coroutines is different in
this respect.
>> +void aio_co_wake(struct Coroutine *co)
>> +{
>> + AioContext *ctx;
>> +
>> + /* Read coroutine before co->ctx. Matches smp_wmb in
>> + * qemu_coroutine_enter.
>> + */
>> + smp_read_barrier_depends();
>> + ctx = atomic_read(&co->ctx);
>> +
>> + if (ctx != qemu_get_current_aio_context()) {
>> + aio_co_schedule(ctx, co);
>> + return;
>> + }
>> +
>> + if (qemu_in_coroutine()) {
>> + Coroutine *self = qemu_coroutine_self();
>> + assert(self != co);
>> + QSIMPLEQ_INSERT_TAIL(&self->co_queue_wakeup, co, co_queue_next);
>> + } else {
>> + aio_context_acquire(ctx);
>> + qemu_coroutine_enter(co);
>> + aio_context_release(ctx);
>
> Why is it necessary to acquire AioContext here? We're already in ctx.
We're in its thread, but we've not necessarily acquired it yet.
aio_co_wake is called "aio_*" because it's a central place for
AioContext to acquire itself for coroutines. This way, coroutines only
care about CoMutexes, and not about AioContext. This was "highly
recommended" :) by Kevin last year and it's the main change since the
previous posting
(https://lists.gnu.org/archive/html/qemu-devel/2015-11/msg05416.html for
example).
Paolo
- Re: [Qemu-devel] [PATCH 04/16] io: add methods to set I/O handlers on AioContext, (continued)
[Qemu-devel] [PATCH 06/16] nbd: do not block on partial reply header reads, Paolo Bonzini, 2017/01/13
[Qemu-devel] [PATCH 08/16] qed: introduce qed_aio_start_io and qed_aio_next_io_cb, Paolo Bonzini, 2017/01/13
[Qemu-devel] [PATCH 09/16] aio: push aio_context_acquire/release down to dispatching, Paolo Bonzini, 2017/01/13