[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/3] block: eliminate BDRV_REQ_NO_SERIALISING
From: |
Paolo Bonzini |
Subject: |
Re: [PATCH 1/3] block: eliminate BDRV_REQ_NO_SERIALISING |
Date: |
Wed, 18 Dec 2019 17:43:58 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.1.1 |
On 18/12/19 17:37, Kevin Wolf wrote:
>> * passthrough flags. */
>> - assert(!(flags & ~(BDRV_REQ_NO_SERIALISING | BDRV_REQ_COPY_ON_READ |
>> - BDRV_REQ_PREFETCH)));
>> + assert(!(flags & ~(BDRV_REQ_COPY_ON_READ | BDRV_REQ_PREFETCH)));
>>
>> /* Handle Copy on Read and associated serialisation */
>> if (flags & BDRV_REQ_COPY_ON_READ) {
>> @@ -1458,12 +1457,7 @@ static int coroutine_fn bdrv_aligned_preadv(BdrvChild
>> *child,
>> bdrv_mark_request_serialising(req, bdrv_get_cluster_size(bs));
>> }
>>
>> - /* BDRV_REQ_SERIALISING is only for write operation */
>> - assert(!(flags & BDRV_REQ_SERIALISING));
> I think we shoud still keep this assertion as long as read requests
> don't mark themselves as serialising when BDRV_REQ_SERIALISING is given.
> Otherwise, someone might add the flag to a read request and will later
> be surprised that it didn't work.
I'm removing it because it's anyway tested by the earlier
assert(!(flags & ~(BDRV_REQ_COPY_ON_READ | BDRV_REQ_PREFETCH)));
>> @@ -3222,9 +3214,7 @@ static int coroutine_fn bdrv_co_copy_range_internal(
>>
>> /* BDRV_REQ_SERIALISING is only for write operation */
>> assert(!(read_flags & BDRV_REQ_SERIALISING));
> Here you kept the assertion, so apart from making sense anyway, it would
> also be more consistent to keep it above, too. :-)
... which is not present here.
Paolo