From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34460) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YHdzu-0007mR-N4 for qemu-devel@nongnu.org; Sat, 31 Jan 2015 14:46:31 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YHdzt-0004KM-L6 for qemu-devel@nongnu.org; Sat, 31 Jan 2015 14:46:26 -0500 Received: from mx-v6.kamp.de ([2a02:248:0:51::16]:51518 helo=mx01.kamp.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YHdzt-0004Jv-9Z for qemu-devel@nongnu.org; Sat, 31 Jan 2015 14:46:25 -0500 Message-ID: <54CD310D.103@kamp.de> Date: Sat, 31 Jan 2015 20:46:21 +0100 From: Peter Lieven MIME-Version: 1.0 References: <1422628382-3937-1-git-send-email-pl@kamp.de> <1422628382-3937-5-git-send-email-pl@kamp.de> <54CBBC6C.5030205@redhat.com> <54CBF21C.6040102@kamp.de> <20150130211501.GG24537@noname.redhat.com> In-Reply-To: <20150130211501.GG24537@noname.redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v3 4/4] virtio-blk: introduce multiread List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: famz@redhat.com, benoit@irqsave.net, ming.lei@canonical.com, armbru@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com, pbonzini@redhat.com, Max Reitz Am 30.01.2015 um 22:15 schrieb Kevin Wolf: > Am 30.01.2015 um 22:05 hat Peter Lieven geschrieben: >> Am 30.01.2015 um 18:16 schrieb Max Reitz: >>> On 2015-01-30 at 09:33, Peter Lieven wrote: >>>> this patch finally introduces multiread support to virtio-blk. While >>>> multiwrite support was there for a long time, read support was missing. >>>> >>>> The complete merge logic is moved into virtio-blk.c which has >>>> been the only user of request merging ever since. This is required >>>> to be able to merge chunks of requests and immediately invoke callbacks >>>> for those requests. Secondly, this is required to switch to >>>> direct invocation of coroutines which is planned at a later stage. >>>> >>>> The following benchmarks show the performance of running fio with >>>> 4 worker threads on a local ram disk. The numbers show the average >>>> of 10 test runs after 1 run as warmup phase. >>>> >>>> | 4k | 64k | 4k >>>> MB/s | rd seq | rd rand | rd seq | rd rand | wr seq | wr rand >>>> --------------+--------+---------+--------+---------+--------+-------- >>>> master | 1221 | 1187 | 4178 | 4114 | 1745 | 1213 >>>> multiread | 1829 | 1189 | 4639 | 4110 | 1894 | 1216 >>>> >>>> Signed-off-by: Peter Lieven >>>> --- >>>> hw/block/dataplane/virtio-blk.c | 8 +- >>>> hw/block/virtio-blk.c | 288 +++++++++++++++++++++++++++------------- >>>> include/hw/virtio/virtio-blk.h | 19 +-- >>>> trace-events | 1 + >>>> 4 files changed, 210 insertions(+), 106 deletions(-) >>>> + int64_t sector_num = 0; >>>> + >>>> + if (mrb->num_reqs == 1) { >>>> + virtio_submit_multireq2(blk, mrb, 0, 1, -1); >>>> + mrb->num_reqs = 0; >>>> return; >>>> } >>>> - ret = blk_aio_multiwrite(blk, mrb->blkreq, mrb->num_writes); >>>> - if (ret != 0) { >>>> - for (i = 0; i < mrb->num_writes; i++) { >>>> - if (mrb->blkreq[i].error) { >>>> - virtio_blk_rw_complete(mrb->blkreq[i].opaque, -EIO); >>>> + max_xfer_len = blk_get_max_transfer_length(mrb->reqs[0]->dev->blk); >>>> + max_xfer_len = MIN_NON_ZERO(max_xfer_len, INT_MAX); >>>> + >>>> + qsort(mrb->reqs, mrb->num_reqs, sizeof(*mrb->reqs), >>>> + &virtio_multireq_compare); >>>> + >>>> + for (i = 0; i < mrb->num_reqs; i++) { >>>> + VirtIOBlockReq *req = mrb->reqs[i]; >>>> + if (num_reqs > 0) { >>>> + bool merge = true; >>>> + >>>> + /* merge would exceed maximum number of IOVs */ >>>> + if (niov + req->qiov.niov + 1 > IOV_MAX) { >>> Hm, why the +1? >> A really good question. I copied this piece from the old merge routine. It seems >> definetely wrong. > The old code merged requests even if they were overlapping. This could > result in one area being split in two. > > I think you don't support this here, so removing the + 1 is probably > okay. I don't support it because it was a good source for bugs in the past and I think a good guest should not create overlapping requests at all. Peter