Re: [Qemu-devel] [PATCH 08/15] virtio: decrease size of VirtQueueElement

[Ming Lei]

On Thu, Jul 31, 2014 at 5:38 PM, Paolo Bonzini <address@hidden> wrote:
> Il 31/07/2014 04:07, Ming Lei ha scritto:
>> On Wed, Jul 30, 2014 at 9:51 PM, Paolo Bonzini <address@hidden> 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.
>>
>> Yes, that should be correct way to do, but can't avoid big chunk allocation
>> completely because 'num' is a bit big.
>
> For typical iops microbenchmarks, num will be 3 (4K I/O) to 18 (64K).

That is only in benchmark, in reality, wring is always big size from
file system, and read might be big too for readahead or by I/O
merge.

Also you need to walk virt queue first for figure out how many io
vectors in current request first, it is a bit expensive to walk the
list since lots of dcache miss is often generated.

Thanks,