Re: [PATCH v3 3/7] block: add max_hw_transfer to BlockLimits

[Paolo Bonzini <pbonzini@redhat.com>]

On 03/06/21 19:33, Eric Blake wrote:
>> +/* Returns the maximum hardware transfer length, in bytes; guaranteed nonzero */
>> +uint64_t blk_get_max_hw_transfer(BlockBackend *blk)
>> +{
>> +    BlockDriverState *bs = blk_bs(blk);
>> +    uint64_t max = INT_MAX;
> 
> This is an unaligned value; should we instead round it down to the
> request_alignment granularity?

See below...

>> +++ b/include/block/block_int.h
>> @@ -695,6 +695,13 @@ typedef struct BlockLimits {
>>        * clamped down. */
>>       uint32_t max_transfer;
>>   
>> +    /* Maximal hardware transfer length in bytes.  Applies whenever
> 
> Leading /* on its own line, per our style.

The whole file still uses this style, I can change it if desired or do 
it later for the whole file or even the whole block subsystem.

>> +     * transfers to the device bypass the kernel I/O scheduler, for
>> +     * example with SG_IO.  If larger than max_transfer or if zero,
>> +     * blk_get_max_hw_transfer will fall back to max_transfer.
>> +     */
> 
> Should we mandate any additional requirements on this value such as
> multiple of request_alignment or even power-of-2?

Certainly not power of 2.  Multiple of request_alignment probably makes 
sense, but max_transfer doesn't have that limit.

Paolo

>> +    uint64_t max_hw_transfer;
>> +
>>       /* memory alignment, in bytes so that no bounce buffer is needed */
>>       size_t min_mem_alignment;
>>   
>