Re: [Qemu-devel] [PATCH 18/42] job: Move coroutine and related code to Job

[Max Reitz]

On 2018-05-09 18:26, Kevin Wolf wrote:
> This commit moves some core functions for dealing with the job coroutine
> from BlockJob to Job. This includes primarily entering the coroutine
> (both for the first and reentering) and yielding explicitly and at pause
> points.
> 
> Signed-off-by: Kevin Wolf <address@hidden>
> ---
>  include/block/blockjob.h     |  40 ---------
>  include/block/blockjob_int.h |  26 ------
>  include/qemu/job.h           |  76 ++++++++++++++++
>  block/backup.c               |   2 +-
>  block/commit.c               |   4 +-
>  block/mirror.c               |  22 ++---
>  block/replication.c          |   2 +-
>  block/stream.c               |   4 +-
>  blockdev.c                   |   8 +-
>  blockjob.c                   | 201 
> +++++++------------------------------------
>  job.c                        | 137 +++++++++++++++++++++++++++++
>  tests/test-bdrv-drain.c      |  38 ++++----
>  tests/test-blockjob-txn.c    |  12 +--
>  tests/test-blockjob.c        |  14 +--
>  14 files changed, 296 insertions(+), 290 deletions(-)

[...]

> diff --git a/job.c b/job.c
> index b074b3ffd7..b4cece1a82 100644
> --- a/job.c
> +++ b/job.c

[...]

> @@ -172,6 +205,110 @@ void job_unref(Job *job)
>      }
>  }
>  
> +void job_enter_cond(Job *job, bool(*fn)(Job *job))
> +{
> +    if (!job_started(job)) {
> +        return;
> +    }
> +    if (job->deferred_to_main_loop) {
> +        return;
> +    }
> +
> +    job_lock();
> +    if (job->busy) {
> +        job_unlock();
> +        return;
> +    }
> +
> +    if (fn && !fn(job)) {
> +        job_unlock();
> +        return;
> +    }
> +
> +    assert(!job->deferred_to_main_loop);
> +    timer_del(&job->sleep_timer);
> +    job->busy = true;
> +    job_unlock();
> +    aio_co_wake(job->co);
> +}
> +
> +/* Yield, and schedule a timer to reenter the coroutine after @ns 
> nanoseconds.
> + * Reentering the job coroutine with block_job_enter() before the timer has

There is no job_enter() yet, but you do reference it below, so this
should probably be adjusted here...

> + * expired is allowed and cancels the timer.
> + *
> + * If @ns is (uint64_t) -1, no timer is scheduled and block_job_enter() must 
> be

...and here.  Or maybe you want to change the job_enter below to a
block_job_enter until there is a job_enter().

With that done (or not, because I don't really care about mid-series
comments);

Reviewed-by: Max Reitz <address@hidden>

> + * called explicitly. */
> +void coroutine_fn job_do_yield(Job *job, uint64_t ns)
> +{
> +    job_lock();
> +    if (ns != -1) {
> +        timer_mod(&job->sleep_timer, ns);
> +    }
> +    job->busy = false;
> +    job_unlock();
> +    qemu_coroutine_yield();
> +
> +    /* Set by job_enter before re-entering the coroutine.  */
> +    assert(job->busy);
> +}

signature.asc
Description: OpenPGP digital signature