On Wed, Jun 15, 2016 at 9:53 AM, Paolo Bonzini <pbonz...@redhat.com> wrote: > On 14/06/2016 20:17, Stefan Hajnoczi wrote: >> Block jobs are coroutines that usually perform I/O but sometimes also >> sleep or yield. Currently only sleeping or yielded block jobs can be >> paused. This means jobs that do not sleep or yield (using >> block_job_yield()) are unaffected by block_job_pause(). >> >> Add block_job_pause_point() so that block jobs can mark quiescent points >> that are suitable for pausing. This solves the problem that it can take >> a block job a long time to pause if it is performing a long series of >> I/O operations. >> >> Transitioning to paused state involves a .pause()/.resume() callback. >> These callbacks are used to ensure that I/O and event loop activity has >> ceased while the job is at a pause point. >> >> Note that this patch introduces a stricter pause state than previously. >> The job->busy flag was incorrectly documented as a quiescent state >> without I/O pending. This is violated by any job that has I/O pending >> across sleep or block_job_yield(), like the mirror block job. > > Right, we should document job->busy as a quiescent state where no one > will re-enter the coroutine.
That statement doesn't correspond with how it's used: block_job_sleep_ns() leaves a timer pending and the job will re-enter when the timer expires. So "no one will re-enter the coroutine" is too strict. The important thing is it's safe to call block_job_enter(). In the block_job_sleep_ns() case the timer is cancelled to prevent doubly re-entry. The doc comment I have in v4 allows the block_job_sleep_ns() case: /* * Set to false by the job while the coroutine has yielded and may be * re-entered by block_job_enter(). There may still be I/O or event loop * activity pending. */ bool busy; Stefan
