[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] scsi-disk: Don't enlarge min_io_size to max_io_
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH] scsi-disk: Don't enlarge min_io_size to max_io_size |
Date: |
Thu, 22 Mar 2018 10:11:01 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 |
On 22/03/2018 08:38, Fam Zheng wrote:
> Some backends report big max_io_sectors. Making min_io_size the same
> value in this case will make it impossible for guest to align memory,
> therefore the disk may not be usable at all.
>
> Change the default behavior (when min_io_size and opt_io_size are not
> specified in the command line), do not assume max_io_sectors is a good
> value for opt_io_size and min_io_size, use 512 instead.
>
> Reported-by: David Gibson <address@hidden>
> Signed-off-by: Fam Zheng <address@hidden>
> ---
> hw/scsi/scsi-disk.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
> index 5b7a48f5a5..76e3c9eaa4 100644
> --- a/hw/scsi/scsi-disk.c
> +++ b/hw/scsi/scsi-disk.c
> @@ -714,10 +714,8 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req,
> uint8_t *outbuf)
>
> /* min_io_size and opt_io_size can't be greater than
> * max_io_sectors */
> - min_io_size =
> - MIN_NON_ZERO(min_io_size, max_io_sectors);
> - opt_io_size =
> - MIN_NON_ZERO(opt_io_size, max_io_sectors);
> + min_io_size = MIN(min_io_size ? : 512, max_io_sectors);
> + opt_io_size = MIN(opt_io_size ? : 512, max_io_sectors);
There are a few easily fixed issues with your chosen defaults, though
the problem obviously makes sense:
1) the values are in sectors - since you chose 512, it's not clear if
you meant it to be 512 bytes or 512 sectors. :) 512 sectors (256 KiB
or 2 MiB depending on logical block size) is still too much for the
min_io_size. The min_io_size default (if it is 0) is the physical block
size, so I think we should make the min_io_size either 0 or the physical
block size.
2) For the opt_io_size, 256 KiB on the other hand is probably too
little. On my laptop (NVMe disk) a transfer size of 8 MiB is twice as
fast compared to a transfer size of 256 KiB, and 16 MiB or 32 MiB is a
little faster too. I would either leave zero as the default, or pick
something around 16-32 MiB.
Thanks,
Paolo
> }
> /* required VPD size with unmap support */
> buflen = 0x40;
>