| Message ID | 1305254409-9079-1-git-send-email-asias.hejun@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 5/13/11 5:40 AM, Asias He wrote: > commit b764422bb0b46b00b896f6d4538ac3d3dde9e56b > (kvm tools: Add support for multiple virtio-blk) > removed the VIRTIO_BLK_F_SEG_MAX publishment to guest. > > There is no reason we should not support it. Just bring it back. > > Signed-off-by: Asias He<asias.hejun@gmail.com> Sasha? > --- > tools/kvm/virtio/blk.c | 19 +++++++++++++++---- > 1 files changed, 15 insertions(+), 4 deletions(-) > > diff --git a/tools/kvm/virtio/blk.c b/tools/kvm/virtio/blk.c > index 5085f1b..8740bc4 100644 > --- a/tools/kvm/virtio/blk.c > +++ b/tools/kvm/virtio/blk.c > @@ -21,6 +21,10 @@ > #define NUM_VIRT_QUEUES 1 > > #define VIRTIO_BLK_QUEUE_SIZE 128 > +/* > + * the header and status consume too entries > + */ > +#define DISK_SEG_MAX (VIRTIO_BLK_QUEUE_SIZE - 2) > > struct blk_dev_job { > struct virt_queue *vq; > @@ -278,11 +282,12 @@ void virtio_blk__init(struct kvm *kvm, struct disk_image *disk) > blk_dev_base_addr = IOPORT_VIRTIO_BLK + new_dev_idx * IOPORT_VIRTIO_BLK_SIZE; > > *bdev = (struct blk_dev) { > - .mutex = PTHREAD_MUTEX_INITIALIZER, > - .disk = disk, > - .idx = new_dev_idx, > - .blk_config = (struct virtio_blk_config) { > + .mutex = PTHREAD_MUTEX_INITIALIZER, > + .disk = disk, > + .idx = new_dev_idx, > + .blk_config = (struct virtio_blk_config) { > .capacity = disk->size / SECTOR_SIZE, > + .seg_max = DISK_SEG_MAX, > }, > .pci_hdr = (struct pci_device_header) { > .vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET, > @@ -294,6 +299,12 @@ void virtio_blk__init(struct kvm *kvm, struct disk_image *disk) > .subsys_id = PCI_SUBSYSTEM_ID_VIRTIO_BLK, > .bar[0] = blk_dev_base_addr | PCI_BASE_ADDRESS_SPACE_IO, > }, > + /* > + * Note we don't set VIRTIO_BLK_F_GEOMETRY here so the > + * guest kernel will compute disk geometry by own, the > + * same applies to VIRTIO_BLK_F_BLK_SIZE > + */ > + .host_features = (1UL<< VIRTIO_BLK_F_SEG_MAX), > }; > > if (irq__register_device(PCI_DEVICE_ID_VIRTIO_BLK,&dev,&pin,&line)< 0) -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2011-05-13 at 15:46 +0300, Pekka Enberg wrote: > On 5/13/11 5:40 AM, Asias He wrote: > > commit b764422bb0b46b00b896f6d4538ac3d3dde9e56b > > (kvm tools: Add support for multiple virtio-blk) > > removed the VIRTIO_BLK_F_SEG_MAX publishment to guest. > > > > There is no reason we should not support it. Just bring it back. > > > > Signed-off-by: Asias He<asias.hejun@gmail.com> > > Sasha? I'm not sure why it was removed, must have missed it when updating struct blk_dev. On the other hand, why do we need to limit max segment size?
On 05/13/2011 10:07 PM, Sasha Levin wrote: > On Fri, 2011-05-13 at 15:46 +0300, Pekka Enberg wrote: >> On 5/13/11 5:40 AM, Asias He wrote: >>> commit b764422bb0b46b00b896f6d4538ac3d3dde9e56b >>> (kvm tools: Add support for multiple virtio-blk) >>> removed the VIRTIO_BLK_F_SEG_MAX publishment to guest. >>> >>> There is no reason we should not support it. Just bring it back. >>> >>> Signed-off-by: Asias He<asias.hejun@gmail.com> >> >> Sasha? > > I'm not sure why it was removed, must have missed it when updating > struct blk_dev. > > On the other hand, why do we need to limit max segment size? That's because the number of entries in a scatter-gather operation must be less than the entries in the virt queue we can offer.
On Fri, 2011-05-13 at 22:34 +0800, Asias He wrote: > On 05/13/2011 10:07 PM, Sasha Levin wrote: > > On Fri, 2011-05-13 at 15:46 +0300, Pekka Enberg wrote: > >> On 5/13/11 5:40 AM, Asias He wrote: > >>> commit b764422bb0b46b00b896f6d4538ac3d3dde9e56b > >>> (kvm tools: Add support for multiple virtio-blk) > >>> removed the VIRTIO_BLK_F_SEG_MAX publishment to guest. > >>> > >>> There is no reason we should not support it. Just bring it back. > >>> > >>> Signed-off-by: Asias He<asias.hejun@gmail.com> > >> > >> Sasha? > > > > I'm not sure why it was removed, must have missed it when updating > > struct blk_dev. > > > > On the other hand, why do we need to limit max segment size? > > That's because the number of entries in a scatter-gather operation must > be less than the entries in the virt queue we can offer. > I thought that since it's the guest who allocates vq buffers and it's the guest who does the sg ops, it won't attempt to do that. But if thats the concern, then yes - lets add it to be safe.
On 05/13/2011 11:26 PM, Sasha Levin wrote: > On Fri, 2011-05-13 at 22:34 +0800, Asias He wrote: >> On 05/13/2011 10:07 PM, Sasha Levin wrote: >>> On Fri, 2011-05-13 at 15:46 +0300, Pekka Enberg wrote: >>>> On 5/13/11 5:40 AM, Asias He wrote: >>>>> commit b764422bb0b46b00b896f6d4538ac3d3dde9e56b >>>>> (kvm tools: Add support for multiple virtio-blk) >>>>> removed the VIRTIO_BLK_F_SEG_MAX publishment to guest. >>>>> >>>>> There is no reason we should not support it. Just bring it back. >>>>> >>>>> Signed-off-by: Asias He<asias.hejun@gmail.com> >>>> >>>> Sasha? >>> >>> I'm not sure why it was removed, must have missed it when updating >>> struct blk_dev. >>> >>> On the other hand, why do we need to limit max segment size? >> >> That's because the number of entries in a scatter-gather operation must >> be less than the entries in the virt queue we can offer. >> > > I thought that since it's the guest who allocates vq buffers and it's > the guest who does the sg ops, it won't attempt to do that. The size of the virt queue (number of entries) is determined by the device (host). > > But if thats the concern, then yes - lets add it to be safe. >
diff --git a/tools/kvm/virtio/blk.c b/tools/kvm/virtio/blk.c index 5085f1b..8740bc4 100644 --- a/tools/kvm/virtio/blk.c +++ b/tools/kvm/virtio/blk.c @@ -21,6 +21,10 @@ #define NUM_VIRT_QUEUES 1 #define VIRTIO_BLK_QUEUE_SIZE 128 +/* + * the header and status consume too entries + */ +#define DISK_SEG_MAX (VIRTIO_BLK_QUEUE_SIZE - 2) struct blk_dev_job { struct virt_queue *vq; @@ -278,11 +282,12 @@ void virtio_blk__init(struct kvm *kvm, struct disk_image *disk) blk_dev_base_addr = IOPORT_VIRTIO_BLK + new_dev_idx * IOPORT_VIRTIO_BLK_SIZE; *bdev = (struct blk_dev) { - .mutex = PTHREAD_MUTEX_INITIALIZER, - .disk = disk, - .idx = new_dev_idx, - .blk_config = (struct virtio_blk_config) { + .mutex = PTHREAD_MUTEX_INITIALIZER, + .disk = disk, + .idx = new_dev_idx, + .blk_config = (struct virtio_blk_config) { .capacity = disk->size / SECTOR_SIZE, + .seg_max = DISK_SEG_MAX, }, .pci_hdr = (struct pci_device_header) { .vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET, @@ -294,6 +299,12 @@ void virtio_blk__init(struct kvm *kvm, struct disk_image *disk) .subsys_id = PCI_SUBSYSTEM_ID_VIRTIO_BLK, .bar[0] = blk_dev_base_addr | PCI_BASE_ADDRESS_SPACE_IO, }, + /* + * Note we don't set VIRTIO_BLK_F_GEOMETRY here so the + * guest kernel will compute disk geometry by own, the + * same applies to VIRTIO_BLK_F_BLK_SIZE + */ + .host_features = (1UL << VIRTIO_BLK_F_SEG_MAX), }; if (irq__register_device(PCI_DEVICE_ID_VIRTIO_BLK, &dev, &pin, &line) < 0)
commit b764422bb0b46b00b896f6d4538ac3d3dde9e56b (kvm tools: Add support for multiple virtio-blk) removed the VIRTIO_BLK_F_SEG_MAX publishment to guest. There is no reason we should not support it. Just bring it back. Signed-off-by: Asias He <asias.hejun@gmail.com> --- tools/kvm/virtio/blk.c | 19 +++++++++++++++---- 1 files changed, 15 insertions(+), 4 deletions(-)