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

[Paolo Bonzini <pbonzini@redhat.com>]

Il 31/07/2014 04:07, Ming Lei ha scritto:
> On Wed, Jul 30, 2014 at 9:51 PM, Paolo Bonzini <pbonzini@redhat.com> 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).

> Also this kind of change requires almost all users of elem to be changed,
> that need lots of work.
> 
> That is why I choose to take the simple approach to ease memory
> preallocation for obj pool.

Yeah, that's simpler.

Paolo