Re: [Qemu-devel] [RFC PATCH V2 4/4] virtio-blk: introduce multiread

[Eric Blake]

On 12/05/2014 04:50 AM, Peter Lieven wrote:
> this patch finally introduce multiread support to virtio-blk while

s/introduce/introduces/
s/virtio-blk while/virtio-blk. While/

> multiwrite support was there for a long time read support was missing.

s/time/time,/

> 
> To achieve this the patch does serveral things which might need futher

s/serveral/several/
s/futher/further/

> explaination:

s/explaination/explanation/

> 
>  - the whole merge and multireq logic is moved from block.c into
>    virtio-blk. This is move is a preparation for directly creating a
>    coroutine out of virtio-blk.

Can this move be done as a separate prerequisite patch? Mixing code
motion and new features in the same patch is harder to review.

> 
>  - requests are only merged if they are strictly sequential and no

s/sequential/sequential,/

>    longer sorted. This simplification decreases overhead and reduces
>    latency. It will also merge some requests which were unmergable before.
> 
>    The old algorithm took up to 32 requests sorted them and tried to merge

s/requests/requests,/

>    them. The outcome was anything between 1 and 32 requests. In case of
>    32 requests there were 31 requests unnecessarily delayed.
> 
>    On the other hand lets imagine e.g. 16 unmergeable requests followed

s/lets/let's/

>    by 32 mergable requests. The latter 32 requests would have been split
>    into two 16 byte requests.
> 
>    Last the simplified logic allows for a fast path if we have only a
>    single request in the multirequest. In this case the request is sent as
>    ordinary request without mulltireq callbacks.

s/mulltireq/multireq/

> 
> As a first benchmark I installed Ubuntu 14.04.1 on a ramdisk. The number of
> merged requests is in the same order while the latency is slightly decreased.
> One should not stick too much to the numbers because the number of wr_requests
> are highly fluctuant. I hope the numbers show that this patch is at least
> not causing too big harm:
> 

I'll leave the actual patch review to developers more knowledgeable
about block behavior.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature