[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 09/23] job: call job_enter from job_pause
From: |
Max Reitz |
Subject: |
Re: [PATCH v4 09/23] job: call job_enter from job_pause |
Date: |
Mon, 18 Jan 2021 14:45:58 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.5.0 |
On 16.01.21 22:46, Vladimir Sementsov-Ogievskiy wrote:
If main job coroutine called job_yield (while some background process
is in progress), we should give it a chance to call job_pause_point().
It will be used in backup, when moved on async block-copy.
Note, that job_user_pause is not enough: we want to handle
child_job_drained_begin() as well, which call job_pause().
OK.
Still, if job is already in job_do_yield() in job_pause_point() we
should not enter it.
Agreed.
iotest 109 output is modified: on stop we do bdrv_drain_all() which now
triggers job pause immediately (and pause after ready is standby).
Sounds like a good thing.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
job.c | 3 +++
tests/qemu-iotests/109.out | 24 ++++++++++++++++++++++++
2 files changed, 27 insertions(+)
diff --git a/job.c b/job.c
index 8fecf38960..3aaaebafe2 100644
--- a/job.c
+++ b/job.c
@@ -553,6 +553,9 @@ static bool job_timer_not_pending(Job *job)
void job_pause(Job *job)
{
job->pause_count++;
+ if (!job->paused) {
+ job_enter(job);
+ }
}
I see job_pause is also called from block_job_error_action() – should we
reenter the job there, too?
(It looks to me like e.g. mirror would basically just continue to run,
then, until it needs to yield because of some other issue. I don’t know
whether that’s a problem, because I suppose we don’t guarantee to stop
immediately on an error, though I suspect users would expect us to do
that as early as possible (i.e., not to launch new requests).
[Quite some time later]
I’ve now tested a mirror job that stops due to a target error, and it
actually does not make any progress; or at least it doesn’t report any.
So it looks like my concern is unjustified. I don’t know why it’s
unjustified, though, so perhaps you can explain it before I give my R-b O:))
Max
- [PATCH v4 00/23] backup performance: block_status + async, Vladimir Sementsov-Ogievskiy, 2021/01/16
- [PATCH v4 01/23] qapi: backup: add perf.use-copy-range parameter, Vladimir Sementsov-Ogievskiy, 2021/01/16
- [PATCH v4 02/23] block/block-copy: More explicit call_state, Vladimir Sementsov-Ogievskiy, 2021/01/16
- [PATCH v4 03/23] block/block-copy: implement block_copy_async, Vladimir Sementsov-Ogievskiy, 2021/01/16
- [PATCH v4 04/23] block/block-copy: add max_chunk and max_workers parameters, Vladimir Sementsov-Ogievskiy, 2021/01/16
- [PATCH v4 09/23] job: call job_enter from job_pause, Vladimir Sementsov-Ogievskiy, 2021/01/16
- Re: [PATCH v4 09/23] job: call job_enter from job_pause,
Max Reitz <=
- [PATCH v4 07/23] block/block-copy: add block_copy_cancel, Vladimir Sementsov-Ogievskiy, 2021/01/16
- [PATCH v4 05/23] block/block-copy: add list of all call-states, Vladimir Sementsov-Ogievskiy, 2021/01/16
- [PATCH v4 11/23] iotests: 56: prepare for backup over block-copy, Vladimir Sementsov-Ogievskiy, 2021/01/16
- [PATCH v4 06/23] block/block-copy: add ratelimit to block-copy, Vladimir Sementsov-Ogievskiy, 2021/01/16
- [PATCH v4 10/23] qapi: backup: add max-chunk and max-workers to x-perf struct, Vladimir Sementsov-Ogievskiy, 2021/01/16
- [PATCH v4 12/23] iotests: 185: prepare for backup over block-copy, Vladimir Sementsov-Ogievskiy, 2021/01/16
- [PATCH v4 19/23] block/block-copy: drop unused block_copy_set_progress_callback(), Vladimir Sementsov-Ogievskiy, 2021/01/16
- [PATCH v4 08/23] blockjob: add set_speed to BlockJobDriver, Vladimir Sementsov-Ogievskiy, 2021/01/16
- [PATCH v4 22/23] simplebench: bench_block_job: add cmd_options argument, Vladimir Sementsov-Ogievskiy, 2021/01/16