Re: [Qemu-devel] [PULL 06/49] virtio: move allocation to virtqueue_pop/vring_pop

From: Peter Maydell <peter.maydell@linaro.org>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>, Gerd Hoffmann <kraxel@redhat.com>,
	Eduardo Habkost <ehabkost@redhat.com>,
	Qemu-block <qemu-block@nongnu.org>,
	Jason Wang <jasowang@redhat.com>,
	QEMU Developers <qemu-devel@nongnu.org>,
	Amit Shah <amit.shah@redhat.com>,
	"Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>,
	Stefan Hajnoczi <stefanha@redhat.com>,
	Cornelia Huck <cornelia.huck@de.ibm.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Greg Kurz <gkurz@linux.vnet.ibm.com>
Subject: Re: [Qemu-devel] [PULL 06/49] virtio: move allocation to virtqueue_pop/vring_pop
Date: Fri, 5 Feb 2016 12:52:55 +0000	[thread overview]
Message-ID: <CAFEAcA9-+bx0r=f8KmKkDAigLUpv=B73KWwJQ-azU+PoWfA64Q@mail.gmail.com> (raw)
In-Reply-To: <1454612376-7072-7-git-send-email-mst@redhat.com>

On 4 February 2016 at 21:51, Michael S. Tsirkin <mst@redhat.com> wrote:
> From: Paolo Bonzini <pbonzini@redhat.com>
>
> The return code of virtqueue_pop/vring_pop is unused except to check for
> errors or 0.  We can thus easily move allocation inside the functions
> and just return a pointer to the VirtQueueElement.
>
> The advantage is that we will be able to allocate only the space that
> is needed for the actual size of the s/g list instead of the full
> VIRTQUEUE_MAX_SIZE items.  Currently VirtQueueElement takes about 48K
> of memory, and this kind of allocation puts a lot of stress on malloc.
> By cutting the size by two or three orders of magnitude, malloc can
> use much more efficient algorithms.
>
> The patch is pretty large, but changes to each device are testable
> more or less independently.  Splitting it would mostly add churn.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> ---
>  hw/9pfs/virtio-9p.h                 |  2 +-
>  include/hw/virtio/dataplane/vring.h |  2 +-
>  include/hw/virtio/virtio-balloon.h  |  2 +-
>  include/hw/virtio/virtio-blk.h      |  3 +-
>  include/hw/virtio/virtio-net.h      |  2 +-
>  include/hw/virtio/virtio-scsi.h     |  2 +-
>  include/hw/virtio/virtio-serial.h   |  2 +-
>  include/hw/virtio/virtio.h          |  2 +-
>  hw/9pfs/9p.c                        |  2 +-
>  hw/9pfs/virtio-9p-device.c          | 17 ++++----
>  hw/block/dataplane/virtio-blk.c     | 11 +++--
>  hw/block/virtio-blk.c               | 15 +++----
>  hw/char/virtio-serial-bus.c         | 80 +++++++++++++++++++++++--------------
>  hw/display/virtio-gpu.c             | 21 ++++++----
>  hw/input/virtio-input.c             | 24 +++++++----
>  hw/net/virtio-net.c                 | 69 ++++++++++++++++++++------------
>  hw/scsi/virtio-scsi-dataplane.c     | 15 +++----
>  hw/scsi/virtio-scsi.c               | 18 ++++-----
>  hw/virtio/dataplane/vring.c         | 18 +++++----
>  hw/virtio/virtio-balloon.c          | 22 ++++++----
>  hw/virtio/virtio-rng.c              | 10 +++--
>  hw/virtio/virtio.c                  | 12 ++++--
>  roms/seabios                        |  2 +-
>  23 files changed, 210 insertions(+), 143 deletions(-)

> --- a/roms/seabios
> +++ b/roms/seabios
> @@ -1 +1 @@
> -Subproject commit 01a84bea2d28a19d2405c1ecac4bdef17683cc0c
> +Subproject commit 33fbe13a3e2a01e0ba1087a8feed801a0451db21
> --
> MST

Hi. This commit in this pull request includes a seabios submodule
update, but the commit message says nothing about it. Is it
really supposed to be here?

thanks
-- PMM