@@ -49,6 +49,7 @@ struct VirtIOSCSIConf {
uint32_t num_queues;
uint32_t virtqueue_size;
uint32_t max_sectors;
+ uint32_t max_segments;
uint32_t cmd_per_lun;
#ifdef CONFIG_VHOST_SCSI
char *vhostfd;
@@ -736,7 +736,7 @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config)
blk_get_geometry(s->blk, &capacity);
memset(&blkcfg, 0, sizeof(blkcfg));
virtio_stq_p(vdev, &blkcfg.capacity, capacity);
- virtio_stl_p(vdev, &blkcfg.seg_max, 128 - 2);
+ virtio_stl_p(vdev, &blkcfg.seg_max, VIRTQUEUE_MAX_SIZE - 2);
virtio_stw_p(vdev, &blkcfg.geometry.cylinders, conf->cyls);
virtio_stl_p(vdev, &blkcfg.blk_size, blk_size);
virtio_stw_p(vdev, &blkcfg.min_io_size, conf->min_io_size / blk_size);
@@ -238,6 +238,8 @@ static Property vhost_scsi_properties[] = {
DEFINE_PROP_UINT32("max_sectors", VirtIOSCSICommon, conf.max_sectors,
0xFFFF),
DEFINE_PROP_UINT32("cmd_per_lun", VirtIOSCSICommon, conf.cmd_per_lun, 128),
+ DEFINE_PROP_UINT32("max_segments", VirtIOSCSICommon, conf.max_segments,
+ VIRTQUEUE_MAX_SIZE - 2),
DEFINE_PROP_END_OF_LIST(),
};
@@ -146,6 +146,8 @@ static Property vhost_user_scsi_properties[] = {
DEFINE_PROP_BIT64("param_change", VHostUserSCSI, host_features,
VIRTIO_SCSI_F_CHANGE,
true),
+ DEFINE_PROP_UINT32("max_segments", VirtIOSCSICommon, conf.max_segments,
+ VIRTQUEUE_MAX_SIZE - 2),
DEFINE_PROP_END_OF_LIST(),
};
@@ -644,7 +644,7 @@ static void virtio_scsi_get_config(VirtIODevice *vdev,
VirtIOSCSICommon *s = VIRTIO_SCSI_COMMON(vdev);
virtio_stl_p(vdev, &scsiconf->num_queues, s->conf.num_queues);
- virtio_stl_p(vdev, &scsiconf->seg_max, 128 - 2);
+ virtio_stl_p(vdev, &scsiconf->seg_max, s->conf.max_segments);
virtio_stl_p(vdev, &scsiconf->max_sectors, s->conf.max_sectors);
virtio_stl_p(vdev, &scsiconf->cmd_per_lun, s->conf.cmd_per_lun);
virtio_stl_p(vdev, &scsiconf->event_info_size, sizeof(VirtIOSCSIEvent));
@@ -929,6 +929,8 @@ static Property virtio_scsi_properties[] = {
VIRTIO_SCSI_F_CHANGE, true),
DEFINE_PROP_LINK("iothread", VirtIOSCSI, parent_obj.conf.iothread,
TYPE_IOTHREAD, IOThread *),
+ DEFINE_PROP_UINT32("max_segments", VirtIOSCSI, parent_obj.conf.max_segments,
+ VIRTQUEUE_MAX_SIZE - 2),
DEFINE_PROP_END_OF_LIST(),
};
Linux guests submit IO requests no longer than PAGE_SIZE * max_seg field reported by SCSI controler. Thus typical sequential read with 1 MB size results in the following pattern of the IO from the guest: 8,16 1 15754 2.766095122 2071 D R 2095104 + 1008 [dd] 8,16 1 15755 2.766108785 2071 D R 2096112 + 1008 [dd] 8,16 1 15756 2.766113486 2071 D R 2097120 + 32 [dd] 8,16 1 15757 2.767668961 0 C R 2095104 + 1008 [0] 8,16 1 15758 2.768534315 0 C R 2096112 + 1008 [0] 8,16 1 15759 2.768539782 0 C R 2097120 + 32 [0] The IO was generated by dd if=/dev/sda of=/dev/null bs=1024 iflag=direct This effectively means that on rotational disks we will observe 3 IOPS for each 2 MBs processed. This definitely negatively affects both guest and host IO performance. The cure is relatively simple - we should report lengthy scatter-gather ability of the SCSI controller. Fortunately the situation here is very good. VirtIO transport layer can accomodate 1024 items in one request while we are using only 128. This situation is present since almost very beginning. 2 items are dedicated for request metadata thus we should publish VIRTQUEUE_MAX_SIZE - 2 as max_seg. The following pattern is observed after the patch: 8,16 1 9921 2.662721340 2063 D R 2095104 + 1024 [dd] 8,16 1 9922 2.662737585 2063 D R 2096128 + 1024 [dd] 8,16 1 9923 2.665188167 0 C R 2095104 + 1024 [0] 8,16 1 9924 2.665198777 0 C R 2096128 + 1024 [0] which is much better. The dark side of this patch is that we are tweaking guest visible parameter, though this should be relatively safe as above transport layer support is present in QEMU/host Linux for a very long time. The patch adds configurable property for VirtIO SCSI with a new default and hardcode option for VirtBlock which does not provide good configurable framework. Signed-off-by: Denis V. Lunev <den@openvz.org> CC: "Michael S. Tsirkin" <mst@redhat.com> CC: Stefan Hajnoczi <stefanha@redhat.com> CC: Kevin Wolf <kwolf@redhat.com> CC: Max Reitz <mreitz@redhat.com> CC: Paolo Bonzini <pbonzini@redhat.com> --- Guys, we should make a decision whether this is good to make such guest visible change or we should link this to the machine type. We have made very short discussion with Stefan on IRC but without final solution. This patch is posted to start the discussion. include/hw/virtio/virtio-scsi.h | 1 + hw/block/virtio-blk.c | 2 +- hw/scsi/vhost-scsi.c | 2 ++ hw/scsi/vhost-user-scsi.c | 2 ++ hw/scsi/virtio-scsi.c | 4 +++- 5 files changed, 9 insertions(+), 2 deletions(-)