Re: [Qemu-devel] [PATCH 07/15] dataplane: use object pool to speed up allocation for virtio blk request

[Ming Lei]

On Mon, Aug 4, 2014 at 6:21 PM, Stefan Hajnoczi <address@hidden> wrote:
> On Fri, Aug 01, 2014 at 03:42:05PM +0800, Ming Lei wrote:
>> On Thu, Jul 31, 2014 at 5:18 PM, Paolo Bonzini <address@hidden> wrote:
>> > Il 31/07/2014 05:22, Ming Lei ha scritto:
>> >>> >
>> >>> > The problem is that g_slice here is not using the slab-style allocator
>> >>> > because the object is larger than roughly 500 bytes.  One solution 
>> >>> > would
>> >>> > be to make virtqueue_pop/vring_pop allocate a VirtQueueElement of the
>> >>> > right size (and virtqueue_push/vring_push free it), as mentioned in the
>> >>> > review of patch 8.
>> >> Unluckily both iovec and addr array can't be fitted into 500 bytes, :-(
>> >> Not mention all users of VirtQueueElement need to be changed too,
>> >> I hate to make that work involved in this patchset, :-)
>> >
>> > Well, the point of dataplane was not just to get maximum iops.  It was
>> > also to provide guidance in the work necessary to improve the code and
>> > get maximum iops without special-casing everything.  This can be a lot
>> > of work indeed.
>> >
>> >>> >
>> >>> > However, I now remembered that VirtQueueElement is a mess because it's
>> >>> > serialized directly into the migration state. :(  So you basically
>> >>> > cannot change it without mucking with migration.  Please leave out 
>> >>> > patch
>> >>> > 8 for now.
>> >> save_device code serializes elem in this way:
>> >>
>> >>  qemu_put_buffer(f, (unsigned char *)&req->elem,
>> >>                         sizeof(VirtQueueElement));
>> >>
>> >> so I am wondering why this patch may break migration.
>> >
>> > Because you change the on-wire format and break migration from 2.1 to
>> > 2.2.  Sorry, I wasn't clear enough.
>>
>> That is really a mess, but in future we still may convert VirtQueueElement
>> into a smart one, and keep the original structure only for save/load, but
>> a conversion between the two structures is required in save/load.
>
> This is a good idea.  The VirtQueueElement structure isn't complicated,
> it's just big.  Just use the current layout as the serialization format.

The patch shouldn't be complicated too, and the only annoying part is to
find each virtio device's load/save , and add the conversion in  the two
functions if VirtQueueElement is involved.

I suggest to do the conversion in another standalone patchset.

Thanks,