Re: [PATCH v3 02/12] block/io.c: drop assertion on double waiting for request serialisation

[Stefan Hajnoczi]

On Mon, Aug 17, 2020 at 12:15:43PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> The comments states, that on misaligned request we should have already
> been waiting. But for bdrv_padding_rmw_read, we called
> bdrv_mark_request_serialising with align = request_alignment, and now
> we serialise with align = cluster_size. So we may have to wait again
> with larger alignment.
> 
> Note, that the only user of BDRV_REQ_SERIALISING is backup which issues
> cluster-aligned requests, so seems the assertion should not fire for
> now. But it's wrong anyway.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  block/io.c | 11 +----------
>  1 file changed, 1 insertion(+), 10 deletions(-)

This code was added by Paolo, CCing him.

> diff --git a/block/io.c b/block/io.c
> index ad3a51ed53..b18680a842 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -1881,7 +1881,6 @@ bdrv_co_write_req_prepare(BdrvChild *child, int64_t 
> offset, uint64_t bytes,
>                            BdrvTrackedRequest *req, int flags)
>  {
>      BlockDriverState *bs = child->bs;
> -    bool waited;
>      int64_t end_sector = DIV_ROUND_UP(offset + bytes, BDRV_SECTOR_SIZE);
>  
>      if (bs->read_only) {
> @@ -1893,15 +1892,7 @@ bdrv_co_write_req_prepare(BdrvChild *child, int64_t 
> offset, uint64_t bytes,
>      assert(!(flags & ~BDRV_REQ_MASK));
>  
>      if (flags & BDRV_REQ_SERIALISING) {
> -        waited = bdrv_mark_request_serialising(req, 
> bdrv_get_cluster_size(bs));
> -        /*
> -         * For a misaligned request we should have already waited earlier,
> -         * because we come after bdrv_padding_rmw_read which must be called
> -         * with the request already marked as serialising.
> -         */
> -        assert(!waited ||
> -               (req->offset == req->overlap_offset &&
> -                req->bytes == req->overlap_bytes));
> +        bdrv_mark_request_serialising(req, bdrv_get_cluster_size(bs));
>      } else {
>          bdrv_wait_serialising_requests(req);
>      }
> -- 
> 2.18.0
>

signature.asc
Description: PGP signature