Re: [PATCH v1 1/1] virtio-block: switch to blk_get_max_hw_transfer

[Kevin Wolf]

Am 12.01.2023 um 21:28 hat Ilya Dryomov geschrieben:
> On Thu, Dec 9, 2021 at 10:34 AM Or Ozeri <oro@il.ibm.com> wrote:
> >
> > The blk_get_max_hw_transfer API was recently added in 6.1.0.
> > It allows querying an underlying block device its max transfer capability.
> > This commit changes virtio-blk to use this.
> >
> > Signed-off-by: Or Ozeri <oro@il.ibm.com>
> > ---
> >  hw/block/virtio-blk.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> > index f139cd7cc9..1ba9a06888 100644
> > --- a/hw/block/virtio-blk.c
> > +++ b/hw/block/virtio-blk.c
> > @@ -458,7 +458,7 @@ static void virtio_blk_submit_multireq(BlockBackend 
> > *blk, MultiReqBuffer *mrb)
> >          return;
> >      }
> >
> > -    max_transfer = blk_get_max_transfer(mrb->reqs[0]->dev->blk);
> > +    max_transfer = blk_get_max_hw_transfer(mrb->reqs[0]->dev->blk);
> >
> >      qsort(mrb->reqs, mrb->num_reqs, sizeof(*mrb->reqs),
> >            &multireq_compare);
> 
> Hi Or,
> 
> Superficially, this makes sense to me.

I'm not sure I understand. This is not a passthrough device (unlike
scsi-generic), so why should we consider the hardware limits rather than
the kernel/other backend limits for read/write requests?

See the documentation of both fields:

    /*
     * Maximal transfer length in bytes.  Need not be power of 2, but
     * must be multiple of opt_transfer and bl.request_alignment, or 0
     * for no 32-bit limit.  For now, anything larger than INT_MAX is
     * clamped down.
     */
    uint32_t max_transfer;

    /*
     * Maximal hardware transfer length in bytes.  Applies whenever
     * 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.
     */
    uint64_t max_hw_transfer;

Is the real problem that max_transfer isn't right?

Kevin