On 19/08/20 17:28, Stefan Hajnoczi wrote: > 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 <vsement...@virtuozzo.com> >> --- >> block/io.c | 11 +---------- >> 1 file changed, 1 insertion(+), 10 deletions(-) > > This code was added by Paolo, CCing him.
Reviewed-by: Paolo Bonzini <pbonz...@redhat.com>
bdrv_mark_request_serialising already calls
bdrv_wait_serialising_requests_locked so it is okay.
Thanks,
Paolo
>> 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: OpenPGP digital signature
