Re: [Qemu-devel] [PATCH 00/13] linux-aio/virtio-scsi: support AioContext wide IO submission as batch

[Ming Lei]

On Tue, Nov 18, 2014 at 9:57 PM, Paolo Bonzini <address@hidden> wrote:
>
>
> On 09/11/2014 08:42, Ming Lei wrote:
>> This patch implements AioContext wide IO submission as batch, and
>> the idea behind is very simple:
>>
>>       - linux native aio(io_submit) supports to enqueue read/write requests
>>       to different files
>>
>>       - in one AioContext, I/O requests from VM can be submitted to different
>>       backend in host, one typical example is multi-lun scsi
>>
>> This patch changes 'struct qemu_laio_state' as per AioContext, and
>> multiple 'bs' can be associted with one single instance of
>> 'struct qemu_laio_state', then AioContext wide IO submission as batch
>> becomes easy to implement.
>>
>> One simple test in my laptop shows ~20% throughput improvement
>> on randread from VM(using AioContext wide IO batch vs. not using io batch)
>> with below config:
>>
>>       -drive 
>> id=drive_scsi1-0-0-0,if=none,format=raw,cache=none,aio=native,file=/dev/nullb2
>>  \
>>       -drive 
>> id=drive_scsi1-0-0-1,if=none,format=raw,cache=none,aio=native,file=/dev/nullb3
>>  \
>>       -device 
>> virtio-scsi-pci,num_queues=4,id=scsi1,addr=07,iothread=iothread0 \
>>       -device 
>> scsi-disk,bus=scsi1.0,channel=0,scsi-id=1,lun=0,drive=drive_scsi1-0-0-0,id=scsi1-0-0-0
>>  \
>>       -device 
>> scsi-disk,bus=scsi1.0,channel=0,scsi-id=1,lun=1,drive=drive_scsi1-0-0-1,id=scsi1-0-0-1
>>  \
>>
>> BTW, maybe more boost can be obtained since ~33K/sec write() system call
>> can be observed when this test case is running, and it might be a recent
>> regression(BH?).
>
> Ming,
>
> these patches are interesting.  I would like to compare them with the
> opposite approach (and, I think, more similar to your old work) where
> the qemu_laio_state API is moved entirely into AioContext, with lazy
> allocation (reference-counted too, probably).

Yes, it can be done in that way, but the feature is linux native aio
specific, so it might not be good to put it into AioContext.

Basically most of the implementation should be same, and the
difference should be where the io queue is put.

>
> Most of the patches would be the same, but you would replace
> aio_attach_aio_bs/aio_detach_aio_bs with something like
> aio_native_get/aio_native_unref.  Ultimately block/{linux,win32}-aio.c
> could be merged into block/aio-{posix,win32}.c, but you do not have to
> do that now.
>
> Could you try that?  This way we can see which API turns out to be nicer.

I can try that, could you share what the APIs you prefer to?

IMO, the APIs can be defined flexiably in this patchset, and only
the AioContext parameter is enough.

Thanks,