[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-commits] [qemu/qemu] d69a87: block/mirror: Do not wait for active
|
From: |
Paolo Bonzini |
|
Subject: |
[Qemu-commits] [qemu/qemu] d69a87: block/mirror: Do not wait for active writes |
|
Date: |
Tue, 15 Nov 2022 06:20:56 -0800 |
Branch: refs/heads/master
Home: https://github.com/qemu/qemu
Commit: d69a879bdf1aed586478eaa161ee064fe1b92f1a
https://github.com/qemu/qemu/commit/d69a879bdf1aed586478eaa161ee064fe1b92f1a
Author: Hanna Reitz <hreitz@redhat.com>
Date: 2022-11-10 (Thu, 10 Nov 2022)
Changed paths:
M block/mirror.c
Log Message:
-----------
block/mirror: Do not wait for active writes
Waiting for all active writes to settle before daring to create a
background copying operation means that we will never do background
operations while the guest does anything (in write-blocking mode), and
therefore cannot converge. Yes, we also will not diverge, but actually
converging would be even nicer.
It is unclear why we did decide to wait for all active writes to settle
before creating a background operation, but it just does not seem
necessary. Active writes will put themselves into the in_flight bitmap
and thus properly block actually conflicting background requests.
It is important for active requests to wait on overlapping background
requests, which we do in active_write_prepare(). However, so far it was
not documented why it is important. Add such documentation now, and
also to the other call of mirror_wait_on_conflicts(), so that it becomes
more clear why and when requests need to actively wait for other
requests to settle.
Another thing to note is that of course we need to ensure that there are
no active requests when the job completes, but that is done by virtue of
the BDS being drained anyway, so there cannot be any active requests at
that point.
With this change, we will need to explicitly keep track of how many
bytes are in flight in active requests so that
job_progress_set_remaining() in mirror_run() can set the correct number
of remaining bytes.
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2123297
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Message-Id: <20221109165452.67927-2-hreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit: eb994912993077f178ccb43b20e422ecf9ae4ac7
https://github.com/qemu/qemu/commit/eb994912993077f178ccb43b20e422ecf9ae4ac7
Author: Hanna Reitz <hreitz@redhat.com>
Date: 2022-11-10 (Thu, 10 Nov 2022)
Changed paths:
M block/mirror.c
Log Message:
-----------
block/mirror: Drop mirror_wait_for_any_operation()
mirror_wait_for_free_in_flight_slot() is the only remaining user of
mirror_wait_for_any_operation(), so inline the latter into the former.
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Message-Id: <20221109165452.67927-3-hreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit: da93d5c84e56e6b4e84aa8e98b6b984c9b6bb528
https://github.com/qemu/qemu/commit/da93d5c84e56e6b4e84aa8e98b6b984c9b6bb528
Author: Hanna Reitz <hreitz@redhat.com>
Date: 2022-11-10 (Thu, 10 Nov 2022)
Changed paths:
M block/mirror.c
Log Message:
-----------
block/mirror: Fix NULL s->job in active writes
There is a small gap in mirror_start_job() before putting the mirror
filter node into the block graph (bdrv_append() call) and the actual job
being created. Before the job is created, MirrorBDSOpaque.job is NULL.
It is possible that requests come in when bdrv_drained_end() is called,
and those requests would see MirrorBDSOpaque.job == NULL. Have our
filter node handle that case gracefully.
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Message-Id: <20221109165452.67927-4-hreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit: 38591290e7d8c9b5fdeb33eb2b438fef7915de22
https://github.com/qemu/qemu/commit/38591290e7d8c9b5fdeb33eb2b438fef7915de22
Author: Hanna Reitz <hreitz@redhat.com>
Date: 2022-11-10 (Thu, 10 Nov 2022)
Changed paths:
M tests/qemu-iotests/151
M tests/qemu-iotests/151.out
Log Message:
-----------
iotests/151: Test that active mirror progresses
Before this series, a mirror job in write-blocking mode would pause
issuing background requests while active requests are in flight. Thus,
if the source is constantly in use by active requests, no actual
progress can be made.
This series should have fixed that, making the mirror job issue
background requests even while active requests are in flight.
Have a new test case in 151 verify this.
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Message-Id: <20221109165452.67927-5-hreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit: 7b5929c73485b31ebc87c4c20328a6cb40519b71
https://github.com/qemu/qemu/commit/7b5929c73485b31ebc87c4c20328a6cb40519b71
Author: Hanna Reitz <hreitz@redhat.com>
Date: 2022-11-10 (Thu, 10 Nov 2022)
Changed paths:
M tests/qemu-iotests/151
M tests/qemu-iotests/151.out
Log Message:
-----------
iotests/151: Test active requests on mirror start
Have write requests happen to the source node right when we start a
mirror job. The mirror filter node may encounter MirrorBDSOpaque.job
being NULL, but this should not cause a segfault.
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Message-Id: <20221109165452.67927-6-hreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit: c78532652265f03a2a2fbc239821eda34b967272
https://github.com/qemu/qemu/commit/c78532652265f03a2a2fbc239821eda34b967272
Author: Alberto Faria <afaria@redhat.com>
Date: 2022-11-10 (Thu, 10 Nov 2022)
Changed paths:
M qapi/block-core.json
Log Message:
-----------
qapi/block-core: Fix BlockdevOptionsNvmeIoUring @path description
The nvme-io_uring BlockDriver's path option must point at the character
device of an NVMe namespace, not at an image file.
Fixes: fd66dbd424f5 ("blkio: add libblkio block driver")
Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Alberto Faria <afaria@redhat.com>
Message-Id: <20221108142347.1322674-1-afaria@redhat.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit: e56b0c66310feab14a14dd6a24fd58dae178e059
https://github.com/qemu/qemu/commit/e56b0c66310feab14a14dd6a24fd58dae178e059
Author: Alberto Faria <afaria@redhat.com>
Date: 2022-11-10 (Thu, 10 Nov 2022)
Changed paths:
M block/blkio.c
Log Message:
-----------
block/blkio: Set BlockDriver::has_variable_length to false
Setting it to true can cause the device size to be queried from libblkio
in otherwise fast paths, degrading performance. Set it to false and
require users to refresh the device size explicitly instead.
Fixes: 4c8f4fda0504 ("block/blkio: Tolerate device size changes")
Suggested-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Alberto Faria <afaria@redhat.com>
Message-Id: <20221108144433.1334074-1-afaria@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit: d5f8d79c2f1f22cb883ae404abff1ee8276d47f1
https://github.com/qemu/qemu/commit/d5f8d79c2f1f22cb883ae404abff1ee8276d47f1
Author: Hanna Reitz <hreitz@redhat.com>
Date: 2022-11-10 (Thu, 10 Nov 2022)
Changed paths:
M block.c
M block/block-backend.c
M blockjob.c
M include/block/block-global-state.h
M include/block/block-io.h
M include/block/block_int-common.h
Log Message:
-----------
block: Make bdrv_child_get_parent_aio_context I/O
We want to use bdrv_child_get_parent_aio_context() from
bdrv_parent_drained_{begin,end}_single(), both of which are "I/O or GS"
functions.
Prior to 3ed4f708fe1, all the implementations were I/O code anyway.
3ed4f708fe1 has put block jobs' AioContext field under the job mutex, so
to make child_job_get_parent_aio_context() work in an I/O context, we
need to take that lock there.
Furthermore, blk_root_get_parent_aio_context() is not marked as
anything, but is safe to run in an I/O context, so mark it that way now.
(blk_get_aio_context() is an I/O code function.)
With that done, all implementations explicitly are I/O code, so we can
mark bdrv_child_get_parent_aio_context() as I/O code, too, so callers
know it is safe to run from both GS and I/O contexts.
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Message-Id: <20221107151321.211175-2-hreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit: af5b6ebe5b887cb650434f9f7294af597a66314c
https://github.com/qemu/qemu/commit/af5b6ebe5b887cb650434f9f7294af597a66314c
Author: Hanna Reitz <hreitz@redhat.com>
Date: 2022-11-10 (Thu, 10 Nov 2022)
Changed paths:
M block/block-backend.c
Log Message:
-----------
block-backend: Update ctx immediately after root
blk_get_aio_context() asserts that blk->ctx is always equal to the root
BDS's context (if there is a root BDS). Therefore,
blk_do_set_aio_context() must update blk->ctx immediately after the root
BDS's context has changed.
Without this patch, the next patch would break iotest 238, because
bdrv_drained_begin() (called by blk_do_set_aio_context()) may then
invoke bdrv_child_get_parent_aio_context() on the root child, i.e.
blk_get_aio_context(). However, by this point, blk->ctx would not have
been updated and thus differ from the root node's context. This patch
fixes that.
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Message-Id: <20221107151321.211175-3-hreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit: ace5a161ea1c09d8eaa8b2a717528457dc924e83
https://github.com/qemu/qemu/commit/ace5a161ea1c09d8eaa8b2a717528457dc924e83
Author: Hanna Reitz <hreitz@redhat.com>
Date: 2022-11-10 (Thu, 10 Nov 2022)
Changed paths:
M block/io.c
Log Message:
-----------
block: Start/end drain on correct AioContext
bdrv_parent_drained_{begin,end}_single() are supposed to operate on the
parent, not on the child, so they should not attempt to get the context
to poll from the child but the parent instead. BDRV_POLL_WHILE(c->bs)
does get the context from the child, so we should replace it with
AIO_WAIT_WHILE() on the parent's context instead.
This problem becomes apparent when bdrv_replace_child_noperm() invokes
bdrv_parent_drained_end_single() after removing a child from a subgraph
that is in an I/O thread. By the time bdrv_parent_drained_end_single()
is called, child->bs is NULL, and so BDRV_POLL_WHILE(c->bs, ...) will
poll the main loop instead of the I/O thread; but anything that
bdrv_parent_drained_end_single_no_poll() may have scheduled is going to
want to run in the I/O thread, but because we poll the main loop, the
I/O thread is never unpaused, and nothing is run, resulting in a
deadlock.
Closes: https://gitlab.com/qemu-project/qemu/-/issues/1215
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Message-Id: <20221107151321.211175-4-hreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit: 46530d3560b6fd5de38b6f4e45ce7d7135b9add7
https://github.com/qemu/qemu/commit/46530d3560b6fd5de38b6f4e45ce7d7135b9add7
Author: Hanna Reitz <hreitz@redhat.com>
Date: 2022-11-14 (Mon, 14 Nov 2022)
Changed paths:
A tests/qemu-iotests/tests/stream-under-throttle
A tests/qemu-iotests/tests/stream-under-throttle.out
Log Message:
-----------
tests/stream-under-throttle: New test
Test streaming a base image into the top image underneath two throttle
nodes. This was reported to make qemu 7.1 hang
(https://gitlab.com/qemu-project/qemu/-/issues/1215), so this serves as
a regression test.
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Message-Id: <20221110160921.33158-1-hreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit: bb00d0aa620e821cc2fbf2e0c5de45a8c957626c
https://github.com/qemu/qemu/commit/bb00d0aa620e821cc2fbf2e0c5de45a8c957626c
Author: Stefan Hajnoczi <stefanha@redhat.com>
Date: 2022-11-14 (Mon, 14 Nov 2022)
Changed paths:
M block.c
M block/blkio.c
M block/block-backend.c
M block/io.c
M block/mirror.c
M blockjob.c
M include/block/block-global-state.h
M include/block/block-io.h
M include/block/block_int-common.h
M qapi/block-core.json
M tests/qemu-iotests/151
M tests/qemu-iotests/151.out
A tests/qemu-iotests/tests/stream-under-throttle
A tests/qemu-iotests/tests/stream-under-throttle.out
Log Message:
-----------
Merge tag 'for-upstream' of https://repo.or.cz/qemu/kevin into staging
Block layer patches
- Fix deadlock in graph modification with iothreads
- mirror: Fix non-converging cases for active mirror
- qapi: Fix BlockdevOptionsNvmeIoUring @path description
- blkio: Set BlockDriver::has_variable_length to false
# -----BEGIN PGP SIGNATURE-----
#
# iQJFBAABCAAvFiEE3D3rFZqa+V09dFb+fwmycsiPL9YFAmNyIF8RHGt3b2xmQHJl
# ZGhhdC5jb20ACgkQfwmycsiPL9ZcHRAAwcQ9cLu6Oh96iWvCCOIxqOsEzVYeCwxI
# yJrrOYSKvMabWms+gg3m5zYt/sU4CRvjzFMd/WDl4LXN4B1SNBdOjPXkswoLA6cU
# QvzbVNRPgZxodVXewjWw5fNFYkBvA+Jgx9ffEK0dYAWKFN3bT6I3NzjcKr2eJ2d2
# Y8RzltBspwwadyTH0lQxY8HfXE7UHukBCAVkcbqQQYuzKa2dR9ERKfRM10uDZwNI
# eNGWu1W0xvE3+nXqnGfXUXVO7R7Q5L0HfShr4Dhw0zyWbg6DBJRi7iY8cVV1VmCp
# M0C8ybODRdsMcRJh+k+Q+T33oRBnXytXDiNzNRHx2gOabuc6k/sc6aSfcIvgCMQf
# PLQsHI0a1o/N238N1Znhfn+M5S0+elTy/xwmzXN2rL3whNMJ9IRoqoxh7nH90CB2
# F7lMjp7FMmJVYtmy0FcBDUVfShgzqM1TsORAXUfdU5QXf4wA+FyZ16SN/WYYfg4B
# ZCsdu2vDimA4rNOiWpPEBNLnHv3S/cswTqobQUQ2QN0zzGPZxoKEWAuG4pqlmSGN
# nMgEiLGFL7Ztgpjw6ZQCisL5rh0P9g53JgY8+b68KfeDXG+R2bEHPtZotIVz7mT7
# JP5ydTyxozNGvMCKg/0Fp1HaHU1ADm9swnWm5cYm/ax9hq5rMNsaq6YTLap1o1PP
# e1Oe0rnq/Ys=
# =zRlt
# -----END PGP SIGNATURE-----
# gpg: Signature made Mon 14 Nov 2022 06:02:55 EST
# gpg: using RSA key DC3DEB159A9AF95D3D7456FE7F09B272C88F2FD6
# gpg: issuer "kwolf@redhat.com"
# gpg: Good signature from "Kevin Wolf <kwolf@redhat.com>" [full]
# Primary key fingerprint: DC3D EB15 9A9A F95D 3D74 56FE 7F09 B272 C88F 2FD6
* tag 'for-upstream' of https://repo.or.cz/qemu/kevin:
tests/stream-under-throttle: New test
block: Start/end drain on correct AioContext
block-backend: Update ctx immediately after root
block: Make bdrv_child_get_parent_aio_context I/O
block/blkio: Set BlockDriver::has_variable_length to false
qapi/block-core: Fix BlockdevOptionsNvmeIoUring @path description
iotests/151: Test active requests on mirror start
iotests/151: Test that active mirror progresses
block/mirror: Fix NULL s->job in active writes
block/mirror: Drop mirror_wait_for_any_operation()
block/mirror: Do not wait for active writes
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Compare: https://github.com/qemu/qemu/compare/98f10f0e2613...bb00d0aa620e