Re: [PATCH 2/3] block/io: wait for serialising requests when a request becomes serialising

[Kevin Wolf]

Am 18.12.2019 um 14:17 hat Paolo Bonzini geschrieben:
> Marking without waiting would not result in actual serialising behavior.
> Thus, make a call bdrv_mark_request_serialising sufficient for
> serialisation to happen.
> 
> Signed-off-by: Paolo Bonzini <address@hidden>

> @@ -1851,13 +1841,18 @@ bdrv_co_write_req_prepare(BdrvChild *child, int64_t 
> offset, uint64_t bytes,
>      assert(!(flags & ~BDRV_REQ_MASK));
>  
>      if (flags & BDRV_REQ_SERIALISING) {
> -        bdrv_mark_request_serialising(req, bdrv_get_cluster_size(bs));
> +        waited = bdrv_mark_request_serialising(req, 
> bdrv_get_cluster_size(bs));
> +        /*
> +         * For a misaligned request, we should have already waited
> +         * in bdrv_padding_rmw_read and locked out concurrent writers.

Actually, bdrv_padding_rmw_read() only asserts that the request is
already serialised. bdrv_mark_request_serialising() is already called by
the callers of bdrv_padding_rmw_read().

> +         */
> +        assert(!waited ||
> +               (req->offset == req->overlap_offset &&
> +                req->bytes == req->overlap_bytes));
> +    } else {
> +        bdrv_wait_serialising_requests(req);
>      }
>  
> -    waited = bdrv_wait_serialising_requests(req);
> -
> -    assert(!waited || !req->serialising ||
> -           is_request_serialising_and_aligned(req));
>      assert(req->overlap_offset <= offset);
>      assert(offset + bytes <= req->overlap_offset + req->overlap_bytes);
>      assert(end_sector <= bs->total_sectors || child->perm & BLK_PERM_RESIZE);

Other than the comment, the patch looks fine to me.

Kevin