diff mbox

[1/1] virtio-mmio: return the max queue num of virtio-mmio with initial value

Message ID 1437071893-19457-1-git-send-email-wei@redhat.com
State New
Headers show

Commit Message

Wei Huang July 16, 2015, 6:38 p.m. UTC
Recently we found that virtio-console devices consumes lots AArch64 guest
memory, roughly 1GB with 8 devices. After debugging, it turns out that lots
of factors contribute to this problem: i) guest PAGE_SIZE=64KB, ii)
virtio-mmio based devices, and iii) virtio-console device. Here is the
detailed analysis:

1. First, during initialization, virtio-mmio driver in guest pokes vq
   size by reading VIRTIO_MMIO_QUEUE_NUM_MAX (see virtio_mmio.c file).
2. QEMU returns VIRTQUEUE_MAX_SIZE (1024) to guest VM; And virtio-mmio uses
   it as the default vq size.
3. virtio-console driver allocates vring buffers based on this value (see
   add_inbuf() function of virtio_console.c file). Because PAGE_SIZE=64KB,
   ~64MB is allocated for each virtio-console vq.

This patch addresses the problem by returning the iniatlized vring size
when VM queries QEMU about VIRTIO_MMIO_QUEUE_NUM_MAX. This is similar to
virtio-pci's approach. By doing this, the vq memory consumption is reduced
substantially.

Signed-off-by: Wei Huang <wei@redhat.com>
---
 hw/virtio/virtio-mmio.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

Comments

Peter Maydell July 20, 2015, 11:07 a.m. UTC | #1
On 16 July 2015 at 19:38, Wei Huang <wei@redhat.com> wrote:
> Recently we found that virtio-console devices consumes lots AArch64 guest
> memory, roughly 1GB with 8 devices. After debugging, it turns out that lots
> of factors contribute to this problem: i) guest PAGE_SIZE=64KB, ii)
> virtio-mmio based devices, and iii) virtio-console device. Here is the
> detailed analysis:
>
> 1. First, during initialization, virtio-mmio driver in guest pokes vq
>    size by reading VIRTIO_MMIO_QUEUE_NUM_MAX (see virtio_mmio.c file).
> 2. QEMU returns VIRTQUEUE_MAX_SIZE (1024) to guest VM; And virtio-mmio uses
>    it as the default vq size.
> 3. virtio-console driver allocates vring buffers based on this value (see
>    add_inbuf() function of virtio_console.c file). Because PAGE_SIZE=64KB,
>    ~64MB is allocated for each virtio-console vq.
>
> This patch addresses the problem by returning the iniatlized vring size
> when VM queries QEMU about VIRTIO_MMIO_QUEUE_NUM_MAX. This is similar to
> virtio-pci's approach. By doing this, the vq memory consumption is reduced
> substantially.

I don't know if this patch is sensible to apply anyway, but from
this description this really sounds like a guest kernel bug.
QEMU tells the kernel the maximum queue size it can cope with,
and if the guest kernel cares about not using insane amounts of
RAM on queues then it should not blindly use the maximum size
but restrict it itself...

thanks
-- PMM
Wei Huang July 20, 2015, 3:38 p.m. UTC | #2
On 07/20/2015 06:07 AM, Peter Maydell wrote:
> On 16 July 2015 at 19:38, Wei Huang <wei@redhat.com> wrote:
>> Recently we found that virtio-console devices consumes lots AArch64 guest
>> memory, roughly 1GB with 8 devices. After debugging, it turns out that lots
>> of factors contribute to this problem: i) guest PAGE_SIZE=64KB, ii)
>> virtio-mmio based devices, and iii) virtio-console device. Here is the
>> detailed analysis:
>>
>> 1. First, during initialization, virtio-mmio driver in guest pokes vq
>>    size by reading VIRTIO_MMIO_QUEUE_NUM_MAX (see virtio_mmio.c file).
>> 2. QEMU returns VIRTQUEUE_MAX_SIZE (1024) to guest VM; And virtio-mmio uses
>>    it as the default vq size.
>> 3. virtio-console driver allocates vring buffers based on this value (see
>>    add_inbuf() function of virtio_console.c file). Because PAGE_SIZE=64KB,
>>    ~64MB is allocated for each virtio-console vq.
>>
>> This patch addresses the problem by returning the iniatlized vring size
>> when VM queries QEMU about VIRTIO_MMIO_QUEUE_NUM_MAX. This is similar to
>> virtio-pci's approach. By doing this, the vq memory consumption is reduced
>> substantially.
> 
> I don't know if this patch is sensible to apply anyway, but from
> this description this really sounds like a guest kernel bug.
> QEMU tells the kernel the maximum queue size it can cope with,
> and if the guest kernel cares about not using insane amounts of
> RAM on queues then it should not blindly use the maximum size
> but restrict it itself...
Yes, this is another way of solving the problem. I think there are three
alternatives:

1. Fix the return value of MMIO_QUEUE_NUM_MAX in QEMU (my patch);
2. In guest VM, virtio-mmio shouldn't query MMIO_QUEUE_NUM_MAX. Instead,
it should behave similarly to virtio-pci driver which queries
VIRTIO_PCI_QUEUE_NUM instead. However this approach requires
modification of VIRTIO Specification as VIRTIO_PCI_QUEUE_NUM is
WRITE-ONLY in virtio-mmio.
3. Fix virtio_console driver in guest VM. This driver currently takes in
info->num and allocates memory based on its value. Apparently we can put
a upper-limit on it.

Your suggestion could fall in to (2) or (3). Any preference?

Thanks,
-Wei



> 
> thanks
> -- PMM
>
Andrew Jones July 21, 2015, 11:22 a.m. UTC | #3
On Mon, Jul 20, 2015 at 10:38:58AM -0500, Wei Huang wrote:
> 
> 
> On 07/20/2015 06:07 AM, Peter Maydell wrote:
> > On 16 July 2015 at 19:38, Wei Huang <wei@redhat.com> wrote:
> >> Recently we found that virtio-console devices consumes lots AArch64 guest
> >> memory, roughly 1GB with 8 devices. After debugging, it turns out that lots
> >> of factors contribute to this problem: i) guest PAGE_SIZE=64KB, ii)
> >> virtio-mmio based devices, and iii) virtio-console device. Here is the
> >> detailed analysis:
> >>
> >> 1. First, during initialization, virtio-mmio driver in guest pokes vq
> >>    size by reading VIRTIO_MMIO_QUEUE_NUM_MAX (see virtio_mmio.c file).
> >> 2. QEMU returns VIRTQUEUE_MAX_SIZE (1024) to guest VM; And virtio-mmio uses
> >>    it as the default vq size.
> >> 3. virtio-console driver allocates vring buffers based on this value (see
> >>    add_inbuf() function of virtio_console.c file). Because PAGE_SIZE=64KB,
> >>    ~64MB is allocated for each virtio-console vq.
> >>
> >> This patch addresses the problem by returning the iniatlized vring size
> >> when VM queries QEMU about VIRTIO_MMIO_QUEUE_NUM_MAX. This is similar to
> >> virtio-pci's approach. By doing this, the vq memory consumption is reduced
> >> substantially.
> > 
> > I don't know if this patch is sensible to apply anyway, but from
> > this description this really sounds like a guest kernel bug.
> > QEMU tells the kernel the maximum queue size it can cope with,
> > and if the guest kernel cares about not using insane amounts of
> > RAM on queues then it should not blindly use the maximum size
> > but restrict it itself...
> Yes, this is another way of solving the problem. I think there are three
> alternatives:
> 
> 1. Fix the return value of MMIO_QUEUE_NUM_MAX in QEMU (my patch);
> 2. In guest VM, virtio-mmio shouldn't query MMIO_QUEUE_NUM_MAX. Instead,
> it should behave similarly to virtio-pci driver which queries
> VIRTIO_PCI_QUEUE_NUM instead. However this approach requires
> modification of VIRTIO Specification as VIRTIO_PCI_QUEUE_NUM is
> WRITE-ONLY in virtio-mmio.
> 3. Fix virtio_console driver in guest VM. This driver currently takes in
> info->num and allocates memory based on its value. Apparently we can put
> a upper-limit on it.
> 
> Your suggestion could fall in to (2) or (3). Any preference?

I tend to agree with Peter here that (3) is the correct way to fix this.
The driver should be doing something like this in its setup-vq

  num = guest-preferred-queue-depth
  max-num = readl(VIRTIO_MMIO_QUEUE_NUM_MAX)
  if (num > max-num)
      num = max-num
  writel(num, VIRTIO_MMIO_QUEUE_NUM)

But I guess guest-preferred-queue-depth is currently the max, i.e.

  writel(readl(VIRTIO_MMIO_QUEUE_NUM_MAX), VIRTIO_MMIO_QUEUE_NUM)

which probably isn't necessary, and a more reasonable size should be
selected.

Thanks,
drew

> 
> Thanks,
> -Wei
> 
> 
> 
> > 
> > thanks
> > -- PMM
> > 
>
diff mbox

Patch

diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
index 10123f3..27840fe 100644
--- a/hw/virtio/virtio-mmio.c
+++ b/hw/virtio/virtio-mmio.c
@@ -93,6 +93,7 @@  static uint64_t virtio_mmio_read(void *opaque, hwaddr offset, unsigned size)
 {
     VirtIOMMIOProxy *proxy = (VirtIOMMIOProxy *)opaque;
     VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+    uint64_t queue_num;
 
     DPRINTF("virtio_mmio_read offset 0x%x\n", (int)offset);
 
@@ -149,10 +150,8 @@  static uint64_t virtio_mmio_read(void *opaque, hwaddr offset, unsigned size)
         }
         return proxy->host_features;
     case VIRTIO_MMIO_QUEUENUMMAX:
-        if (!virtio_queue_get_num(vdev, vdev->queue_sel)) {
-            return 0;
-        }
-        return VIRTQUEUE_MAX_SIZE;
+        queue_num = virtio_queue_get_num(vdev, vdev->queue_sel);
+        return queue_num;
     case VIRTIO_MMIO_QUEUEPFN:
         return virtio_queue_get_addr(vdev, vdev->queue_sel)
             >> proxy->guest_page_shift;