Re: [PATCH v6 09/17] virtio: add minimal support for virtqueues

[Andrew Jones <drjones@redhat.com>]

On Fri, Jul 11, 2014 at 11:32:20AM +0200, Paolo Bonzini wrote:
> Il 11/07/2014 11:23, Paolo Bonzini ha scritto:
> >
> >>+static struct virtqueue *vm_setup_vq(struct virtio_device *vdev,
> >>+                     unsigned index,
> >>+                     void (*callback)(struct virtqueue *vq),
> >>+                     const char *name)
> >>+{
> >>+    struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev);
> >>+    struct vring_virtqueue *vq;
> >>+    void *queue;
> >>+    unsigned num = VIRTIO_MMIO_QUEUE_NUM_MIN;
> >>+
> >>+    vq = alloc(sizeof(*vq));
> >
> >You can move the allocation of vq to vm_find_vqs (interleaving alloc and
> >alloc_aligned causes some fragmentation), allocating a single block
> >instead of one per vq.
> 
> ... or even use static storage for the "struct virtqueue".  You can just
> merge "struct vring_virtqueue" and "struct virtqueue", as the idea of other
> virtqueue transport never materialized.  Then:
> 

I'd rather keep the implementation as close as possible to the Linux
implementation, allowing new features to be easily lifted from there.
The fragmentation was expected with the early allocator (memregions),
but didn't bother me much, as it's unlikely to ever be more than a
handful of pages, and we're just writing unit tests. If a particular
unit test or arch wants a better allocator than memregions provides,
one that does a better job of limiting fragmentation, then it's just
a matter of installing one with alloc_ops.

drew