Re: [Qemu-devel] [PATCH 02/20] blockjob: introduce .drain callback for jobs

[Vladimir Sementsov-Ogievskiy]

27.10.2016 13:48, Paolo Bonzini wrote:
> This is required to decouple block jobs from running in an
> AioContext.  With multiqueue block devices, a BlockDriverState
> does not really belong to a single AioContext.
>
> The solution is to first wait until all I/O operations are
> complete; then loop in the main thread for the block job to
> complete entirely.
>
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---

[...]

> @@ -413,14 +409,21 @@ static int block_job_finish_sync(BlockJob *job,
>       assert(blk_bs(job->blk)->job == job);
>   
>       block_job_ref(job);
> +
>       finish(job, &local_err);
>       if (local_err) {
>           error_propagate(errp, local_err);
>           block_job_unref(job);
>           return -EBUSY;
>       }
> +    /* block_job_drain calls block_job_enter, and it should be enough to
> +     * induce progress until the job completes or moves to the main thread.
> +    */

Hi Paolo!

Looks like I have a problem with this. block_job_drain enters the job 
only if job.busy = false. But what if job yielded with busy = true?

My case is the following: in the job I call co_aio_sleep_ns() for some 
time without setting job.busy to false, and it looks like timer doesn't 
work while we are in "while() { block_job_drain() }" loop. If I just set 
"job.busy = false" and "job.busy = true" around co_aio_sleep_ns() all 
start to work.

I don't want set job.busy to false, because actually job is working - 
several additional coroutines do their work, only the main one (job.co) 
do nothing. I can remove timer, and make other coroutines wake up the 
main one when it needed, and, anyway it looks like better way..

But the question is: is it ok, that we can't use sleep timer in the job, 
without setting busy = true? Is it right that only io can wake up block 
job coroutine, if it yielded without setting busy=false?

> +    while (!job->deferred_to_main_loop && !job->completed) {
> +        block_job_drain(job);
> +    }
>       while (!job->completed) {
> -        aio_poll(block_job_get_aio_context(job), true);
> +        aio_poll(qemu_get_aio_context(), true);
>       }
>       ret = (job->cancelled && job->ret == 0) ? -ECANCELED : job->ret;
>       block_job_unref(job);


--
Best regards,
Vladimir