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

[Paolo Bonzini]

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;
> >