From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39423) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bAHMo-00078v-VO for qemu-devel@nongnu.org; Tue, 07 Jun 2016 09:48:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bAHMh-0006gA-Hu for qemu-devel@nongnu.org; Tue, 07 Jun 2016 09:48:25 -0400 References: <1465217162-21115-1-git-send-email-kwolf@redhat.com> <1465217162-21115-6-git-send-email-kwolf@redhat.com> From: "Denis V. Lunev" Message-ID: <5756CCE6.9060507@openvz.org> Date: Tue, 7 Jun 2016 16:32:22 +0300 MIME-Version: 1.0 In-Reply-To: <1465217162-21115-6-git-send-email-kwolf@redhat.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 5/5] qemu-img bench: Add --flush-interval List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf , qemu-block@nongnu.org Cc: mreitz@redhat.com, eblake@redhat.com, qemu-devel@nongnu.org On 06/06/2016 03:46 PM, Kevin Wolf wrote: > This options allows to flush the image periodically during write tests. > > Signed-off-by: Kevin Wolf > --- > qemu-img-cmds.hx | 4 +-- > qemu-img.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++++++------ > qemu-img.texi | 8 ++++- > 3 files changed, 95 insertions(+), 12 deletions(-) > > diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx > index 05a2991..7e95b2d 100644 > --- a/qemu-img-cmds.hx > +++ b/qemu-img-cmds.hx > @@ -10,9 +10,9 @@ STEXI > ETEXI > > DEF("bench", img_bench, > - "bench [-c count] [-d depth] [-f fmt] [-n] [-o offset] [--pattern=pattern] [-q] [-s buffer_size] [-S step_size] [-t cache] [-w] filename") > + "bench [-c count] [-d depth] [-f fmt] [--flush-interval=flush_interval] [-n] [--no-drain] [-o offset] [--pattern=pattern] [-q] [-s buffer_size] [-S step_size] [-t cache] [-w] filename") > STEXI > -@item bench [-c @var{count}] [-d @var{depth}] [-f @var{fmt}] [-n] [-o @var{offset}] [--pattern=@var{pattern}] [-q] [-s @var{buffer_size}] [-S @var{step_size}] [-t @var{cache}] [-w] @var{filename} > +@item bench [-c @var{count}] [-d @var{depth}] [-f @var{fmt}] [--flush-interval=@var{flush_interval}] [-n] [--no-drain] [-o @var{offset}] [--pattern=@var{pattern}] [-q] [-s @var{buffer_size}] [-S @var{step_size}] [-t @var{cache}] [-w] @var{filename} > ETEXI > > DEF("check", img_check, > diff --git a/qemu-img.c b/qemu-img.c > index c5e2638..04cddab 100644 > --- a/qemu-img.c > +++ b/qemu-img.c > @@ -54,6 +54,8 @@ enum { > OPTION_OBJECT = 258, > OPTION_IMAGE_OPTS = 259, > OPTION_PATTERN = 260, > + OPTION_FLUSH_INTERVAL = 261, > + OPTION_NO_DRAIN = 262, > }; > > typedef enum OutputFormat { > @@ -3468,13 +3470,24 @@ typedef struct BenchData { > int step; > int nrreq; > int n; > + int flush_interval; > + bool drain_on_flush; > uint8_t *buf; > QEMUIOVector *qiov; > > int in_flight; > + bool in_flush; > uint64_t offset; > } BenchData; > > +static void bench_undrained_flush_cb(void *opaque, int ret) > +{ > + if (ret < 0) { > + error_report("Failed flush request: %s\n", strerror(-ret)); > + exit(EXIT_FAILURE); > + } > +} > + > static void bench_cb(void *opaque, int ret) > { > BenchData *b = opaque; > @@ -3484,9 +3497,39 @@ static void bench_cb(void *opaque, int ret) > error_report("Failed request: %s\n", strerror(-ret)); > exit(EXIT_FAILURE); > } > - if (b->in_flight > 0) { > + > + if (b->in_flush) { > + /* Just finished a flush with drained queue: Start next requests */ > + assert(b->in_flight == 0); > + b->in_flush = false; > + } else if (b->in_flight > 0) { > + int remaining = b->n - b->in_flight; > + > b->n--; > b->in_flight--; > + > + /* Time for flush? Drain queue if requested, then flush */ > + if (b->flush_interval && remaining % b->flush_interval == 0) { > + if (!b->in_flight || !b->drain_on_flush) { > + BlockCompletionFunc *cb; > + > + if (b->drain_on_flush) { > + b->in_flush = true; > + cb = bench_cb; > + } else { > + cb = bench_undrained_flush_cb; > + } > + > + acb = blk_aio_flush(b->blk, cb, b); > + if (!acb) { > + error_report("Failed to issue flush request"); > + exit(EXIT_FAILURE); > + } > + } > + if (b->drain_on_flush) { > + return; > + } > + } > } > > while (b->n > b->in_flight && b->in_flight < b->nrreq) { > @@ -3520,6 +3563,8 @@ static int img_bench(int argc, char **argv) > size_t bufsize = 4096; > int pattern = 0; > size_t step = 0; > + int flush_interval = 0; > + bool drain_on_flush = true; > int64_t image_size; > BlockBackend *blk = NULL; > BenchData data = {}; > @@ -3531,8 +3576,10 @@ static int img_bench(int argc, char **argv) > for (;;) { > static const struct option long_options[] = { > {"help", no_argument, 0, 'h'}, > + {"flush-interval", required_argument, 0, OPTION_FLUSH_INTERVAL}, > {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS}, > {"pattern", required_argument, 0, OPTION_PATTERN}, > + {"no-drain", no_argument, 0, OPTION_NO_DRAIN}, > {0, 0, 0, 0} > }; > c = getopt_long(argc, argv, "hc:d:f:no:qs:S:t:w", long_options, NULL); > @@ -3640,6 +3687,20 @@ static int img_bench(int argc, char **argv) > } > break; > } > + case OPTION_FLUSH_INTERVAL: > + { > + char *end; > + errno = 0; > + flush_interval = strtoul(optarg, &end, 0); > + if (errno || *end || flush_interval > INT_MAX) { > + error_report("Invalid flush interval specified"); > + return 1; > + } > + break; > + } > + case OPTION_NO_DRAIN: > + drain_on_flush = false; > + break; > case OPTION_IMAGE_OPTS: > image_opts = true; > break; > @@ -3651,6 +3712,17 @@ static int img_bench(int argc, char **argv) > } > filename = argv[argc - 1]; > > + if (!is_write && flush_interval) { > + error_report("--flush-interval is only available in write tests"); > + ret = -1; > + goto out; > + } > + if (flush_interval && flush_interval < depth) { > + error_report("Flush interval can't be smaller than depth"); > + ret = -1; > + goto out; > + } > + > blk = img_open(image_opts, filename, fmt, flags, writethrough, quiet); > if (!blk) { > ret = -1; > @@ -3664,19 +3736,24 @@ static int img_bench(int argc, char **argv) > } > > data = (BenchData) { > - .blk = blk, > - .image_size = image_size, > - .bufsize = bufsize, > - .step = step ?: bufsize, > - .nrreq = depth, > - .n = count, > - .offset = offset, > - .write = is_write, > + .blk = blk, > + .image_size = image_size, > + .bufsize = bufsize, > + .step = step ?: bufsize, > + .nrreq = depth, > + .n = count, > + .offset = offset, > + .write = is_write, > + .flush_interval = flush_interval, > + .drain_on_flush = drain_on_flush, > }; > printf("Sending %d %s requests, %d bytes each, %d in parallel " > "(starting at offset %" PRId64 ", step size %d)\n", > data.n, data.write ? "write" : "read", data.bufsize, data.nrreq, > data.offset, data.step); > + if (flush_interval) { > + printf("Sending flush every %d requests\n", flush_interval); > + } > > data.buf = blk_blockalign(blk, data.nrreq * data.bufsize); > memset(data.buf, pattern, data.nrreq * data.bufsize); > diff --git a/qemu-img.texi b/qemu-img.texi > index ccc0b51..cbe50e9 100644 > --- a/qemu-img.texi > +++ b/qemu-img.texi > @@ -131,7 +131,7 @@ Skip the creation of the target volume > Command description: > > @table @option > -@item bench [-c @var{count}] [-d @var{depth}] [-f @var{fmt}] [-n] [-o @var{offset}] [--pattern=@var{pattern}] [-q] [-s @var{buffer_size}] [-S @var{step_size}] [-t @var{cache}] [-w] @var{filename} > +@item bench [-c @var{count}] [-d @var{depth}] [-f @var{fmt}] [--flush-interval=@var{flush_interval}] [-n] [--no-drain] [-o @var{offset}] [--pattern=@var{pattern}] [-q] [-s @var{buffer_size}] [-S @var{step_size}] [-t @var{cache}] [-w] @var{filename} > > Run a simple sequential I/O benchmark on the specified image. If @code{-w} is > specified, a write test is performed, otherwise a read test is performed. > @@ -142,6 +142,12 @@ starts at the position given by @var{offset}, each following request increases > the current position by @var{step_size}. If @var{step_size} is not given, > @var{buffer_size} is used for its value. > > +If @var{flush_interval} is specified for a write test, the request queue is > +drained and a flush is issued before new writes are made whenever the number of > +remaining requests is a multiple of @var{flush_interval}. If additionally > +@code{--no-drain} is specified, a flush is issued without draining the request > +queue first. > + > If @code{-n} is specified, the native AIO backend is used if possible. On > Linux, this option only works if @code{-t none} or @code{-t directsync} is > specified as well. Reviewed-by: Denis V. Lunev