[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v10 5/9] block: treat BDRV_REQ_ALLOCATE as seria
From: |
Vladimir Sementsov-Ogievskiy |
Subject: |
Re: [Qemu-devel] [PATCH v10 5/9] block: treat BDRV_REQ_ALLOCATE as serialising |
Date: |
Wed, 5 Dec 2018 13:14:03 +0000 |
03.12.2018 13:14, Anton Nefedov wrote:
> The idea is that ALLOCATE requests may overlap with other requests.
please, describe why
> Reuse the existing block layer infrastructure for serialising requests.
> Use the following approach:
> - mark ALLOCATE also SERIALISING, so subsequent requests to the area wait
> - ALLOCATE request itself must never wait if another request is in flight
> already. Return EAGAIN, let the caller reconsider.
>
> Signed-off-by: Anton Nefedov <address@hidden>
> ---
> block/io.c | 31 ++++++++++++++++++++++++-------
> 1 file changed, 24 insertions(+), 7 deletions(-)
>
> diff --git a/block/io.c b/block/io.c
> index d9d7644858..6ff946f63d 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -720,12 +720,13 @@ void bdrv_dec_in_flight(BlockDriverState *bs)
> bdrv_wakeup(bs);
> }
>
> -static bool coroutine_fn wait_serialising_requests(BdrvTrackedRequest *self)
> +static bool coroutine_fn find_or_wait_serialising_requests(
> + BdrvTrackedRequest *self, bool wait)
> {
> BlockDriverState *bs = self->bs;
> BdrvTrackedRequest *req;
> bool retry;
> - bool waited = false;
> + bool found = false;
>
> if (!atomic_read(&bs->serialising_in_flight)) {
> return false;
> @@ -751,11 +752,14 @@ static bool coroutine_fn
> wait_serialising_requests(BdrvTrackedRequest *self)
> * will wait for us as soon as it wakes up, then just go on
> * (instead of producing a deadlock in the former case). */
> if (!req->waiting_for) {
> + found = true;
> + if (!wait) {
> + break;
> + }
> self->waiting_for = req;
> qemu_co_queue_wait(&req->wait_queue, &bs->reqs_lock);
> self->waiting_for = NULL;
> retry = true;
> - waited = true;
> break;
> }
> }
> @@ -763,7 +767,12 @@ static bool coroutine_fn
> wait_serialising_requests(BdrvTrackedRequest *self)
> qemu_co_mutex_unlock(&bs->reqs_lock);
> } while (retry);
>
> - return waited;
> + return found;
> +}
> +
> +static bool coroutine_fn wait_serialising_requests(BdrvTrackedRequest *self)
> +{
> + return find_or_wait_serialising_requests(self, true);
> }
>
> static int bdrv_check_byte_request(BlockDriverState *bs, int64_t offset,
> @@ -1585,7 +1594,7 @@ bdrv_co_write_req_prepare(BdrvChild *child, int64_t
> offset, uint64_t bytes,
> BdrvTrackedRequest *req, int flags)
> {
> BlockDriverState *bs = child->bs;
> - bool waited;
> + bool found;
> int64_t end_sector = DIV_ROUND_UP(offset + bytes, BDRV_SECTOR_SIZE);
>
> if (bs->read_only) {
> @@ -1602,9 +1611,13 @@ bdrv_co_write_req_prepare(BdrvChild *child, int64_t
> offset, uint64_t bytes,
> mark_request_serialising(req, bdrv_get_cluster_size(bs));
> }
>
> - waited = wait_serialising_requests(req);
> + found = find_or_wait_serialising_requests(req,
> + !(flags & BDRV_REQ_ALLOCATE));
> + if (found && (flags & BDRV_REQ_ALLOCATE)) {
> + return -EAGAIN;
> + }
>
> - assert(!waited || !req->serialising ||
> + assert(!found || !req->serialising ||
> is_request_serialising_and_aligned(req));
> assert(req->overlap_offset <= offset);
> assert(offset + bytes <= req->overlap_offset + req->overlap_bytes);
> @@ -1866,6 +1879,10 @@ int coroutine_fn bdrv_co_pwritev(BdrvChild *child,
> assert(!((flags & BDRV_REQ_ALLOCATE) && (flags & BDRV_REQ_MAY_UNMAP)));
> assert(!((flags & BDRV_REQ_ALLOCATE) && !(flags &
> BDRV_REQ_ZERO_WRITE)));
>
> + if (flags & BDRV_REQ_ALLOCATE) {
> + flags |= BDRV_REQ_SERIALISING;
> + }
> +
> trace_bdrv_co_pwritev(child->bs, offset, bytes, flags);
>
> if (!bs->drv) {
>
patch looks correct technically, I just don't know the reasoning..
the only thing, is that it would be good to add a comment in BDRV flags
definition section, that _ALLOCATE implies _SERIALIZE.
--
Best regards,
Vladimir
- Re: [Qemu-devel] [PATCH v10 3/9] quorum: set supported write flags, (continued)
Re: [Qemu-devel] [PATCH v10 3/9] quorum: set supported write flags, Vladimir Sementsov-Ogievskiy, 2018/12/12
[Qemu-devel] [PATCH v10 6/9] file-posix: support BDRV_REQ_ALLOCATE, Anton Nefedov, 2018/12/03
Re: [Qemu-devel] [PATCH v10 6/9] file-posix: support BDRV_REQ_ALLOCATE, Alberto Garcia, 2018/12/07
[Qemu-devel] [PATCH v10 5/9] block: treat BDRV_REQ_ALLOCATE as serialising, Anton Nefedov, 2018/12/03
- Re: [Qemu-devel] [PATCH v10 5/9] block: treat BDRV_REQ_ALLOCATE as serialising,
Vladimir Sementsov-Ogievskiy <=
[Qemu-devel] [PATCH v10 8/9] qcow2: skip writing zero buffers to empty COW areas, Anton Nefedov, 2018/12/03
Re: [Qemu-devel] [PATCH v10 8/9] qcow2: skip writing zero buffers to empty COW areas, Vladimir Sementsov-Ogievskiy, 2018/12/13