Re: [Qemu-block] [PATCH] block: Swap request limit definitions

[Max Reitz]

On 15.02.2017 17:44, Kevin Wolf wrote:
> Am 15.02.2017 um 14:42 hat Max Reitz geschrieben:
>> On 14.02.2017 10:52, Alberto Garcia wrote:
>>> On Mon 13 Feb 2017 06:13:38 PM CET, Max Reitz wrote:
>>>
>>>>>> -#define BDRV_REQUEST_MAX_SECTORS MIN(SIZE_MAX >> BDRV_SECTOR_BITS, \
>>>>>> -                                     INT_MAX >> BDRV_SECTOR_BITS)
>>>>>> -#define BDRV_REQUEST_MAX_BYTES (BDRV_REQUEST_MAX_SECTORS << 
>>>>>> BDRV_SECTOR_BITS)
>>>>>> +#define BDRV_REQUEST_MAX_BYTES      MIN(SIZE_MAX, INT_MAX)
>>>>>> +#define BDRV_REQUEST_MAX_SECTORS    (BDRV_REQUEST_MAX_BYTES >> 
>>>>>> BDRV_SECTOR_BITS)
>>>>>
>>>>> I'm just pointing it out because I don't know if this can cause
>>>>> problems, but this patch would make BDRV_REQUEST_MAX_BYTES not a
>>>>> multiple of the sector size (INT_MAX is actually a prime number).
>>>>
>>>> Very good point. I don't think this could be an issue, though. For one
>>>> thing, the use of BDRV_REQUEST_MAX_BYTES is very limited.
>>>
>>> Ok, but then I wonder what's the benefit of increasing
>>> BDRV_REQUEST_MAX_BYTES.
>>
>> The benefit is that the definition looks cleaner.
> 
> Whatever way we want to write it, I think MAX_BYTES = MAX_SECTORS * 512
> should be a given. Everything else is bound to confuse people and
> introduce bugs sooner or later.

Probably only sooner and not later, considering we are switching to byte
granularity overall anyway. And if something confuses people, I'd argue
it's the fact that we still have sector granularity all over the place
and not that your requests can be a bit bigger if you submit them in
bytes than if you submit them in sectors.

Anyway, if MAX_BYTES should be a multiple of the sector size, then I
can't think of a much better way to write this than what we currently
have and this patch is unneeded.

Max

signature.asc
Description: OpenPGP digital signature