Re: [Qemu-devel] [PATCH v4 3/3] dataplane: submit I/O as a batch

[Paolo Bonzini <pbonzini@redhat.com>]

Il 03/07/2014 11:44, Kevin Wolf ha scritto:
> Am 02.07.2014 um 14:18 hat Ming Lei geschrieben:
>> Before commit 580b6b2aa2(dataplane: use the QEMU block
>> layer for I/O), dataplane for virtio-blk submits block
>> I/O as a batch.
>>
>> This commit 580b6b2aa2 replaces the custom linux AIO
>> implementation(including submit I/O as a batch) with QEMU
>> block layer, but this commit causes ~40% throughput regression
>> on virtio-blk performance, and removing submitting I/O
>> as a batch is one of the causes.
>>
>> This patch applies the newly introduced bdrv_io_plug() and
>> bdrv_io_unplug() interfaces to support submitting I/O
>> at batch for Qemu block layer, and in my test, the change
>> can improve throughput by ~30% with 'aio=native'.
>>
>> Following my fio test script:
>>
>> 	[global]
>> 	direct=1
>> 	size=4G
>> 	bsrange=4k-4k
>> 	timeout=40
>> 	numjobs=4
>> 	ioengine=libaio
>> 	iodepth=64
>> 	filename=/dev/vdc
>> 	group_reporting=1
>>
>> 	[f]
>> 	rw=randread
>>
>> Result on one of my small machine(host: x86_64, 2cores, 4thread, guest: 4cores):
>> 	- qemu master: 59K IOPS
>> 	- qemu master with these patches: 81K IOPS
>> 	- 2.0.0 release(dataplane using custom linux aio): 104K IOPS
>>
>> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
>> Signed-off-by: Ming Lei <ming.lei@canonical.com>
>
> Reviewing this one doesn't make sense because commit b002254d
> ('virtio-blk: Unify {non-,}dataplane's request handlings') removes the
> patched code. You need to patch common virtio-blk code now (which should
> improve non-dataplane virtio-blk as well).

Actually no, the second and third hunk still apply.  The patch you 
mentioned unifies request processing, but the loops that fetches 
requests from the virtqueue are still separate.  This is because 
virtio-blk-dataplane still uses the vring API instead of the generic 
virtio API.  address_space_map/unmap is not thread-safe yet, vring 
avoids it.

Regarding the first hunk, it is not needed at all.

Paolo