All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Peter Lieven <pl@kamp.de>, qemu-devel@nongnu.org
Cc: kwolf@redhat.com, famz@redhat.com, benoit@irqsave.net,
	ming.lei@canonical.com, armbru@redhat.com, mreitz@redhat.com,
	stefanha@redhat.com, pbonzini@redhat.com
Subject: Re: [Qemu-devel] [RFC PATCH V2 4/4] virtio-blk: introduce multiread
Date: Fri, 05 Dec 2014 08:05:47 -0700	[thread overview]
Message-ID: <5481C9CB.70805@redhat.com> (raw)
In-Reply-To: <1417780229-6930-5-git-send-email-pl@kamp.de>

[-- Attachment #1: Type: text/plain, Size: 2269 bytes --]

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]

  reply	other threads:[~2014-12-05 15:45 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-05 11:50 [Qemu-devel] [RFC PATCH V2 0/4] virtio-blk: add multiread support Peter Lieven
2014-12-05 11:50 ` [Qemu-devel] [RFC PATCH V2 1/4] block: add accounting for merged requests Peter Lieven
2014-12-05 14:58   ` Eric Blake
2014-12-09 15:43     ` Peter Lieven
2014-12-05 11:50 ` [Qemu-devel] [RFC PATCH V2 2/4] hw/virtio-blk: add a constant for max number of " Peter Lieven
2014-12-05 14:59   ` Eric Blake
2014-12-05 11:50 ` [Qemu-devel] [RFC PATCH V2 3/4] block-backend: expose bs->bl.max_transfer_length Peter Lieven
2014-12-05 11:50 ` [Qemu-devel] [RFC PATCH V2 4/4] virtio-blk: introduce multiread Peter Lieven
2014-12-05 15:05   ` Eric Blake [this message]
2014-12-09 15:44     ` Peter Lieven

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5481C9CB.70805@redhat.com \
    --to=eblake@redhat.com \
    --cc=armbru@redhat.com \
    --cc=benoit@irqsave.net \
    --cc=famz@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=ming.lei@canonical.com \
    --cc=mreitz@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=pl@kamp.de \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.