qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Stefan Hajnoczi <stefanha@redhat.com>
To: qemu-devel@nongnu.org
Cc: Peter Maydell <peter.maydell@linaro.org>,
	Roman Pen <roman.penyaev@profitbricks.com>,
	Stefan Hajnoczi <stefanha@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>
Subject: [Qemu-devel] [PULL 03/17] linux-aio: process completions from ioq_submit()
Date: Mon, 12 Sep 2016 15:08:40 +0100	[thread overview]
Message-ID: <1473689334-29138-4-git-send-email-stefanha@redhat.com> (raw)
In-Reply-To: <1473689334-29138-1-git-send-email-stefanha@redhat.com>

From: Roman Pen <roman.penyaev@profitbricks.com>

In order to reduce completion latency it makes sense to harvest completed
requests ASAP.  Very fast backend device can complete requests just after
submission, so it is worth trying to check ring buffer in order to peek
completed requests directly after io_submit() has been called.

Indeed, this patch reduces the completions latencies and increases the
overall throughput, e.g. the following is the percentiles of number of
completed requests at once:

        1th 10th  20th  30th  40th  50th  60th  70th  80th  90th  99.99th
Before    2    4    42   112   128   128   128   128   128   128    128
 After    1    1     4    14    33    45    47    48    50    51    108

That means, that before the current patch is applied the ring buffer is
observed as full (128 requests were consumed at once) in 60% of calls.

After patch is applied the distribution of number of completed requests
is "smoother" and the queue (requests in-flight) is almost never full.

The fio read results are the following (write results are almost the
same and are not showed here):

  Before
  ------
job: (groupid=0, jobs=8): err= 0: pid=2227: Tue Jul 19 11:29:50 2016
  Description  : [Emulation of Storage Server Access Pattern]
  read : io=54681MB, bw=1822.7MB/s, iops=179779, runt= 30001msec
    slat (usec): min=172, max=16883, avg=338.35, stdev=109.66
    clat (usec): min=1, max=21977, avg=1051.45, stdev=299.29
     lat (usec): min=317, max=22521, avg=1389.83, stdev=300.73
    clat percentiles (usec):
     |  1.00th=[  346],  5.00th=[  596], 10.00th=[  708], 20.00th=[  852],
     | 30.00th=[  932], 40.00th=[  996], 50.00th=[ 1048], 60.00th=[ 1112],
     | 70.00th=[ 1176], 80.00th=[ 1256], 90.00th=[ 1384], 95.00th=[ 1496],
     | 99.00th=[ 1800], 99.50th=[ 1928], 99.90th=[ 2320], 99.95th=[ 2672],
     | 99.99th=[ 4704]
    bw (KB  /s): min=205229, max=553181, per=12.50%, avg=233278.26, stdev=18383.51

  After
  ------
job: (groupid=0, jobs=8): err= 0: pid=2220: Tue Jul 19 11:31:51 2016
  Description  : [Emulation of Storage Server Access Pattern]
  read : io=57637MB, bw=1921.2MB/s, iops=189529, runt= 30002msec
    slat (usec): min=169, max=20636, avg=329.61, stdev=124.18
    clat (usec): min=2, max=19592, avg=988.78, stdev=251.04
     lat (usec): min=381, max=21067, avg=1318.42, stdev=243.58
    clat percentiles (usec):
     |  1.00th=[  310],  5.00th=[  580], 10.00th=[  748], 20.00th=[  876],
     | 30.00th=[  908], 40.00th=[  948], 50.00th=[ 1012], 60.00th=[ 1064],
     | 70.00th=[ 1080], 80.00th=[ 1128], 90.00th=[ 1224], 95.00th=[ 1288],
     | 99.00th=[ 1496], 99.50th=[ 1608], 99.90th=[ 1960], 99.95th=[ 2256],
     | 99.99th=[ 5408]
    bw (KB  /s): min=212149, max=390160, per=12.49%, avg=245746.04, stdev=11606.75

Throughput increased from 1822MB/s to 1921MB/s, average completion latencies
decreased from 1051us to 988us.

Signed-off-by: Roman Pen <roman.penyaev@profitbricks.com>
Message-id: 1468931263-32667-4-git-send-email-roman.penyaev@profitbricks.com
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: qemu-devel@nongnu.org
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/linux-aio.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/block/linux-aio.c b/block/linux-aio.c
index 252ebef..d4e19d4 100644
--- a/block/linux-aio.c
+++ b/block/linux-aio.c
@@ -94,7 +94,11 @@ static void qemu_laio_process_completion(struct qemu_laiocb *laiocb)
 
     laiocb->ret = ret;
     if (laiocb->co) {
-        qemu_coroutine_enter(laiocb->co);
+        /* Jump and continue completion for foreign requests, don't do
+         * anything for current request, it will be completed shortly. */
+        if (laiocb->co != qemu_coroutine_self()) {
+            qemu_coroutine_enter(laiocb->co);
+        }
     } else {
         laiocb->common.cb(laiocb->common.opaque, ret);
         qemu_aio_unref(laiocb);
@@ -320,6 +324,19 @@ static void ioq_submit(LinuxAioState *s)
         QSIMPLEQ_SPLIT_AFTER(&s->io_q.pending, aiocb, next, &completed);
     } while (ret == len && !QSIMPLEQ_EMPTY(&s->io_q.pending));
     s->io_q.blocked = (s->io_q.in_queue > 0);
+
+    if (s->io_q.in_flight) {
+        /* We can try to complete something just right away if there are
+         * still requests in-flight. */
+        qemu_laio_process_completions(s);
+        /*
+         * Even we have completed everything (in_flight == 0), the queue can
+         * have still pended requests (in_queue > 0).  We do not attempt to
+         * repeat submission to avoid IO hang.  The reason is simple: s->e is
+         * still set and completion callback will be called shortly and all
+         * pended requests will be submitted from there.
+         */
+    }
 }
 
 void laio_io_plug(BlockDriverState *bs, LinuxAioState *s)
@@ -377,6 +394,7 @@ int coroutine_fn laio_co_submit(BlockDriverState *bs, LinuxAioState *s, int fd,
         .co         = qemu_coroutine_self(),
         .nbytes     = qiov->size,
         .ctx        = s,
+        .ret        = -EINPROGRESS,
         .is_read    = (type == QEMU_AIO_READ),
         .qiov       = qiov,
     };
@@ -386,7 +404,9 @@ int coroutine_fn laio_co_submit(BlockDriverState *bs, LinuxAioState *s, int fd,
         return ret;
     }
 
-    qemu_coroutine_yield();
+    if (laiocb.ret == -EINPROGRESS) {
+        qemu_coroutine_yield();
+    }
     return laiocb.ret;
 }
 
-- 
2.7.4

  parent reply	other threads:[~2016-09-12 14:09 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-12 14:08 [Qemu-devel] [PULL 00/17] Block patches Stefan Hajnoczi
2016-09-12 14:08 ` [Qemu-devel] [PULL 01/17] linux-aio: consume events in userspace instead of calling io_getevents Stefan Hajnoczi
2016-09-12 14:08 ` [Qemu-devel] [PULL 02/17] linux-aio: split processing events function Stefan Hajnoczi
2016-09-12 14:08 ` Stefan Hajnoczi [this message]
2016-09-12 14:08 ` [Qemu-devel] [PULL 04/17] virtio-blk: rename virtio_device_info to virtio_blk_info Stefan Hajnoczi
2016-09-12 14:08 ` [Qemu-devel] [PULL 05/17] block: unblock backup operations in backing file Stefan Hajnoczi
2016-09-12 14:08 ` [Qemu-devel] [PULL 06/17] Backup: clear all bitmap when doing block checkpoint Stefan Hajnoczi
2016-09-12 14:08 ` [Qemu-devel] [PULL 07/17] Backup: export interfaces for extra serialization Stefan Hajnoczi
2016-09-12 14:08 ` [Qemu-devel] [PULL 08/17] block: Link backup into block core Stefan Hajnoczi
2016-09-12 14:08 ` [Qemu-devel] [PULL 09/17] docs: block replication's description Stefan Hajnoczi
2016-09-12 14:08 ` [Qemu-devel] [PULL 10/17] mirror: auto complete active commit Stefan Hajnoczi
2016-09-12 14:08 ` [Qemu-devel] [PULL 11/17] configure: support replication Stefan Hajnoczi
2016-09-12 14:08 ` [Qemu-devel] [PULL 12/17] replication: Introduce new APIs to do replication operation Stefan Hajnoczi
2016-09-12 14:08 ` [Qemu-devel] [PULL 13/17] replication: Implement new driver for block replication Stefan Hajnoczi
2016-09-12 14:08 ` [Qemu-devel] [PULL 14/17] tests: add unit test case for replication Stefan Hajnoczi
2016-09-12 14:08 ` [Qemu-devel] [PULL 15/17] support replication driver in blockdev-add Stefan Hajnoczi
2016-10-25 21:31   ` Eric Blake
2016-09-12 14:08 ` [Qemu-devel] [PULL 16/17] MAINTAINERS: add maintainer for replication Stefan Hajnoczi
2016-09-12 14:08 ` [Qemu-devel] [PULL 17/17] tests: fix qvirtqueue_kick Stefan Hajnoczi
2016-09-12 15:12 ` [Qemu-devel] [PULL 00/17] Block patches Peter Maydell
2016-09-12 15:56   ` Peter Maydell
2016-09-13  1:11     ` Fam Zheng
2016-09-13  8:34       ` Stefan Hajnoczi
2016-09-13  8:53     ` Stefan Hajnoczi

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=1473689334-29138-4-git-send-email-stefanha@redhat.com \
    --to=stefanha@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=roman.penyaev@profitbricks.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).