OpenE2K/qemu-e2k
13
4
Fork 0
Code Issues 4 Pull Requests Projects 2 Releases Activity

mirror: Don't call job_pause_point() under graph lock

Browse Source 
Calling job_pause_point() while holding the graph reader lock
potentially results in a deadlock: bdrv_graph_wrlock() first drains
everything, including the mirror job, which pauses it. The job is only
unpaused at the end of the drain section, which is when the graph writer
lock has been successfully taken. However, if the job happens to be
paused at a pause point where it still holds the reader lock, the writer
lock can't be taken as long as the job is still paused.

Mark job_pause_point() as GRAPH_UNLOCKED and fix mirror accordingly.

Cc: qemu-stable@nongnu.org
Buglink: https://issues.redhat.com/browse/RHEL-28125
Fixes: 004915a96a ("block: Protect bs->backing with graph_lock")
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-ID: <20240313153000.33121-1-kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
This commit is contained in:
Kevin Wolf 2024-03-13 16:30:00 +01:00
parent ba49d760eb
commit ae5a40e858
2 changed files with 7 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
block/mirror.c
View File

@ -479,9 +479,9 @@ static unsigned mirror_perform(MirrorBlockJob *s, int64_t offset,
return bytes_handled; return bytes_handled;
} }
static void coroutine_fn GRAPH_RDLOCK mirror_iteration(MirrorBlockJob *s) static void coroutine_fn GRAPH_UNLOCKED mirror_iteration(MirrorBlockJob *s)
{ {
BlockDriverState *source = s->mirror_top_bs->backing->bs; BlockDriverState *source;
MirrorOp *pseudo_op; MirrorOp *pseudo_op;
int64_t offset; int64_t offset;
/* At least the first dirty chunk is mirrored in one iteration. */ /* At least the first dirty chunk is mirrored in one iteration. */
@ -489,6 +489,10 @@ static void coroutine_fn GRAPH_RDLOCK mirror_iteration(MirrorBlockJob *s)
bool write_zeroes_ok = bdrv_can_write_zeroes_with_unmap(blk_bs(s->target)); bool write_zeroes_ok = bdrv_can_write_zeroes_with_unmap(blk_bs(s->target));
int max_io_bytes = MAX(s->buf_size / MAX_IN_FLIGHT, MAX_IO_BYTES); int max_io_bytes = MAX(s->buf_size / MAX_IN_FLIGHT, MAX_IO_BYTES);
bdrv_graph_co_rdlock();
source = s->mirror_top_bs->backing->bs;
bdrv_graph_co_rdunlock();
bdrv_dirty_bitmap_lock(s->dirty_bitmap); bdrv_dirty_bitmap_lock(s->dirty_bitmap);
offset = bdrv_dirty_iter_next(s->dbi); offset = bdrv_dirty_iter_next(s->dbi);
if (offset < 0) { if (offset < 0) {
@ -1066,9 +1070,7 @@ static int coroutine_fn mirror_run(Job *job, Error **errp)
mirror_wait_for_free_in_flight_slot(s); mirror_wait_for_free_in_flight_slot(s);
continue; continue;
} else if (cnt != 0) { } else if (cnt != 0) {
bdrv_graph_co_rdlock();
mirror_iteration(s); mirror_iteration(s);
bdrv_graph_co_rdunlock();
} }
} }

2
include/qemu/job.h
View File

@ -483,7 +483,7 @@ void job_enter(Job *job);
* *
* Called with job_mutex *not* held. * Called with job_mutex *not* held.
*/ */
void coroutine_fn job_pause_point(Job *job); void coroutine_fn GRAPH_UNLOCKED job_pause_point(Job *job);
/** /**
* @job: The job that calls the function. * @job: The job that calls the function.