From: "Denis V. Lunev" <[email protected]>
job_pause_point_locked() sets job->paused before yielding and clears it
unconditionally on wake, before re-checking whether a pause is still
pending. job_pause() re-enters a parked job only while it is not yet
paused, so the wake that resumes one comes from a drain *ending*
(job_resume() -> job_enter_cond()). If the next drain begins before that
wake runs, the woken coroutine clears job->paused while pause_count is
already > 0 again:
AioContext change (BQL thread) job coroutine (iothread)
----------------------------- ------------------------
parked in job_pause_point():
paused=1, pause_count=1, yielded
drain ends -> job_resume():
pause_count = 0
job_enter_cond(): queue wake ..> (wake pending)
bdrv_try_change_aio_context():
bdrv_drain_all_begin():
job_pause() per node
pause_count = N (> 0)
wake runs, leaves job_do_yield():
paused = 0 (pause_count == N)
tran_commit -> job_set_aio_context():
assert(paused || completed) --> abort: paused == 0
bdrv_try_change_aio_context() drains precisely to quiesce the job before
changing its AioContext, but that brief paused==0 window trips the
assertion. It is guest-triggerable: a virtio-blk reset
(virtio_blk_stop_ioeventfd() -> blk_set_aio_context()) racing a running
mirror/blockCopy job hits it, as do x-blockdev-set-iothread, blockdev
hot-plug/unplug and job completion.
Keep job->paused set while a pause is still pending: loop the yield
until job_should_pause_locked() is false (or the job is cancelled), and
only then clear job->paused. Drained-state consumers then never observe
a pending-pause job as unpaused.
Signed-off-by: Denis V. Lunev <[email protected]>
Message-ID: <[email protected]>
Reviewed-by: Vladimir Sementsov-Ogievskiy <[email protected]>
Signed-off-by: Vladimir Sementsov-Ogievskiy <[email protected]>
---
job.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/job.c b/job.c
index e7479084726..d7220aaf30c 100644
--- a/job.c
+++ b/job.c
@@ -629,7 +629,14 @@ static void coroutine_fn job_pause_point_locked(Job *job)
? JOB_STATUS_STANDBY
: JOB_STATUS_PAUSED);
job->paused = true;
- job_do_yield_locked(job, -1);
+ /*
+ * Stay paused across back-to-back pause requests: a transient
+ * paused == false while pause_count > 0 would be observed as
+ * "not paused" by job_set_aio_context() and other drain consumers.
+ */
+ do {
+ job_do_yield_locked(job, -1);
+ } while (job_should_pause_locked(job) &&
!job_is_cancelled_locked(job));
job->paused = false;
job_state_transition_locked(job, status);
}
--
2.43.0
