Message ID | 1450867706-19860-4-git-send-email-pbonzini@redhat.com |
---|---|
State | New |
Headers | show |
On 23/12/2015 11:48, Paolo Bonzini wrote: > Because block_job_sleep_ns marks the job as non-busy, it is > possible to enter it again from block_job_enter. However, the > same coroutine may then be re-entered from co_sleep_cb, and this > time the CoSleepCB is not on the stack anymore. > > Fix this by open coding the sleep in block_job_sleep_ns, and > deleting the timer immediately after the coroutine is re-entered. > > Cc: qemu-stable@nongnu.org > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Forget about this, the current code does exactly the same as what I wrote, and is correct. /me needs vacation. :) Paolo > --- > blockjob.c | 27 +++++++++++++++++++++++---- > include/block/blockjob.h | 3 +-- > 2 files changed, 24 insertions(+), 6 deletions(-) > > diff --git a/blockjob.c b/blockjob.c > index 80adb9d..5e59a8f 100644 > --- a/blockjob.c > +++ b/blockjob.c > @@ -329,8 +329,21 @@ int block_job_complete_sync(BlockJob *job, Error **errp) > return block_job_finish_sync(job, &block_job_complete, errp); > } > > +typedef struct CoSleepCB { > + QEMUTimer *ts; > + Coroutine *co; > +} CoSleepCB; > + > +static void co_sleep_cb(void *opaque) > +{ > + CoSleepCB *sleep_cb = opaque; > + > + qemu_coroutine_enter(sleep_cb->co, NULL); > +} > + > void block_job_sleep_ns(BlockJob *job, QEMUClockType type, int64_t ns) > { > + CoSleepCB sleep_cb = { .ts = NULL }; > assert(job->busy); > > /* Check cancellation *before* setting busy = false, too! */ > @@ -339,10 +352,16 @@ void block_job_sleep_ns(BlockJob *job, QEMUClockType type, int64_t ns) > } > > job->busy = false; > - if (block_job_is_paused(job)) { > - qemu_coroutine_yield(); > - } else { > - co_aio_sleep_ns(bdrv_get_aio_context(job->bs), type, ns); > + if (!block_job_is_paused(job)) { > + sleep_cb.co = qemu_coroutine_self(), > + sleep_cb.ts = aio_timer_new(bdrv_get_aio_context(job->bs), type, > + SCALE_NS, co_sleep_cb, &sleep_cb); > + timer_mod(sleep_cb.ts, qemu_clock_get_ns(type) + ns); > + } > + qemu_coroutine_yield(); > + if (sleep_cb.ts) { > + timer_del(sleep_cb.ts); > + timer_free(sleep_cb.ts); > } > job->busy = true; > } > diff --git a/include/block/blockjob.h b/include/block/blockjob.h > index d84ccd8..82063f2 100644 > --- a/include/block/blockjob.h > +++ b/include/block/blockjob.h > @@ -120,8 +120,7 @@ struct BlockJob { > > /** > * Set to false by the job while it is in a quiescent state, where > - * no I/O is pending and the job has yielded on any condition > - * that is not detected by #aio_poll, such as a timer. > + * no I/O is pending. The job can be reentered with qemu_coroutine_enter. > */ > bool busy; > >
diff --git a/blockjob.c b/blockjob.c index 80adb9d..5e59a8f 100644 --- a/blockjob.c +++ b/blockjob.c @@ -329,8 +329,21 @@ int block_job_complete_sync(BlockJob *job, Error **errp) return block_job_finish_sync(job, &block_job_complete, errp); } +typedef struct CoSleepCB { + QEMUTimer *ts; + Coroutine *co; +} CoSleepCB; + +static void co_sleep_cb(void *opaque) +{ + CoSleepCB *sleep_cb = opaque; + + qemu_coroutine_enter(sleep_cb->co, NULL); +} + void block_job_sleep_ns(BlockJob *job, QEMUClockType type, int64_t ns) { + CoSleepCB sleep_cb = { .ts = NULL }; assert(job->busy); /* Check cancellation *before* setting busy = false, too! */ @@ -339,10 +352,16 @@ void block_job_sleep_ns(BlockJob *job, QEMUClockType type, int64_t ns) } job->busy = false; - if (block_job_is_paused(job)) { - qemu_coroutine_yield(); - } else { - co_aio_sleep_ns(bdrv_get_aio_context(job->bs), type, ns); + if (!block_job_is_paused(job)) { + sleep_cb.co = qemu_coroutine_self(), + sleep_cb.ts = aio_timer_new(bdrv_get_aio_context(job->bs), type, + SCALE_NS, co_sleep_cb, &sleep_cb); + timer_mod(sleep_cb.ts, qemu_clock_get_ns(type) + ns); + } + qemu_coroutine_yield(); + if (sleep_cb.ts) { + timer_del(sleep_cb.ts); + timer_free(sleep_cb.ts); } job->busy = true; } diff --git a/include/block/blockjob.h b/include/block/blockjob.h index d84ccd8..82063f2 100644 --- a/include/block/blockjob.h +++ b/include/block/blockjob.h @@ -120,8 +120,7 @@ struct BlockJob { /** * Set to false by the job while it is in a quiescent state, where - * no I/O is pending and the job has yielded on any condition - * that is not detected by #aio_poll, such as a timer. + * no I/O is pending. The job can be reentered with qemu_coroutine_enter. */ bool busy;
Because block_job_sleep_ns marks the job as non-busy, it is possible to enter it again from block_job_enter. However, the same coroutine may then be re-entered from co_sleep_cb, and this time the CoSleepCB is not on the stack anymore. Fix this by open coding the sleep in block_job_sleep_ns, and deleting the timer immediately after the coroutine is re-entered. Cc: qemu-stable@nongnu.org Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- blockjob.c | 27 +++++++++++++++++++++++---- include/block/blockjob.h | 3 +-- 2 files changed, 24 insertions(+), 6 deletions(-)