From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53030) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XCV3l-00071h-9y for qemu-devel@nongnu.org; Wed, 30 Jul 2014 10:40:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XCV3f-000334-2c for qemu-devel@nongnu.org; Wed, 30 Jul 2014 10:40:53 -0400 Received: from mx1.redhat.com ([209.132.183.28]:47009) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XCV3e-00031M-GN for qemu-devel@nongnu.org; Wed, 30 Jul 2014 10:40:46 -0400 Date: Wed, 30 Jul 2014 16:40:57 +0200 From: "Michael S. Tsirkin" Message-ID: <20140730144057.GB26933@redhat.com> References: <1406720388-18671-1-git-send-email-ming.lei@canonical.com> <1406720388-18671-9-git-send-email-ming.lei@canonical.com> <53D8F85A.1090400@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <53D8F85A.1090400@redhat.com> Subject: Re: [Qemu-devel] [PATCH 08/15] virtio: decrease size of VirtQueueElement List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: Kevin Wolf , Peter Maydell , Fam Zheng , Ming Lei , qemu-devel@nongnu.org, Stefan Hajnoczi On Wed, Jul 30, 2014 at 03:51:22PM +0200, Paolo Bonzini wrote: > Il 30/07/2014 13:39, Ming Lei ha scritto: > > diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h > > index a60104c..943e72f 100644 > > --- a/include/hw/virtio/virtio.h > > +++ b/include/hw/virtio/virtio.h > > @@ -84,12 +84,17 @@ typedef struct VirtQueue VirtQueue; > > typedef struct VirtQueueElement > > { > > unsigned int index; > > + unsigned int num; > > unsigned int out_num; > > unsigned int in_num; > > - hwaddr in_addr[VIRTQUEUE_MAX_SIZE]; > > - hwaddr out_addr[VIRTQUEUE_MAX_SIZE]; > > - struct iovec in_sg[VIRTQUEUE_MAX_SIZE]; > > - struct iovec out_sg[VIRTQUEUE_MAX_SIZE]; > > + > > + hwaddr *in_addr; > > + hwaddr *out_addr; > > + struct iovec *in_sg; > > + struct iovec *out_sg; > > + > > + hwaddr addr[VIRTQUEUE_MAX_SIZE]; > > + struct iovec sg[VIRTQUEUE_MAX_SIZE]; > > } VirtQueueElement; > > > > #define VIRTIO_PCI_QUEUE_MAX 64 > > -- > > since addr and sg aren't used directly, allocate them flexibly like > > char *p; > VirtQueueElement *elem; > total_size = ROUND_UP(sizeof(struct VirtQueueElement), > __alignof__(elem->addr[0]); > addr_offset = total_size; > total_size = ROUND_UP(total_size + num * sizeof(elem->addr[0]), > __alignof__(elem->sg[0])); > sg_offset = total_size; > total_size += num * sizeof(elem->sg[0]); > > elem = p = g_slice_alloc(total_size); > elem->size = total_size; > elem->in_addr = p + addr_offset; > elem->out_addr = elem->in_addr + in_num; > elem->in_sg = p + sg_offset; > elem->out_sg = elem->in_sg + in_num; > > ... > > g_slice_free1(elem->size, elem); > > The small size will help glib do slab-style allocation and should remove > the need for an object pool. > > Paolo As long as you relying on this, why not allocate in_sg/out_sg separately? In any case, a bunch of code needs to be audited to make sure no one assumes we can always stick VIRTQUEUE_MAX_SIZE there. But what really worries me is this is an optimization patch without any numbers accompanying it. I would say split this out from your virtio blk work, work on this separately. -- MST