[Qemu-devel] [RFC PATCH] qemu-io-cmds: use clock_gettime for benchmarking

Alex Bennée posted 1 patch 4 years, 10 months ago
Test s390x passed
Test asan passed
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Test FreeBSD passed
Test checkpatch failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190529162438.22653-1-alex.bennee@linaro.org
qemu-io-cmds.c | 69 +++++++++++++++++++++++++-------------------------
1 file changed, 35 insertions(+), 34 deletions(-)
[Qemu-devel] [RFC PATCH] qemu-io-cmds: use clock_gettime for benchmarking
Posted by Alex Bennée 4 years, 10 months ago
The previous use of gettimeofday() ran into undefined behaviour when
we ended up doing a div 0 for a very short operation. This is because
gettimeofday only works at the microsecond level as well as being
prone to discontinuous jumps in system time. Using clock_gettime with
CLOCK_MONOTONIC gives greater precision and alleviates some of the
potential problems with time jumping around.

We could use CLOCK_MONOTONIC_RAW to avoid being tripped up by NTP and
adjtime but that is Linux specific so I decided it would do for now.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 qemu-io-cmds.c | 69 +++++++++++++++++++++++++-------------------------
 1 file changed, 35 insertions(+), 34 deletions(-)

diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index 30a7d9a13bf..4ace2969a9e 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -248,20 +248,21 @@ static void cvtstr(double value, char *str, size_t size)
 
 
 
-static struct timeval tsub(struct timeval t1, struct timeval t2)
+static struct timespec tsub(struct timespec t1, struct timespec t2)
 {
-    t1.tv_usec -= t2.tv_usec;
-    if (t1.tv_usec < 0) {
-        t1.tv_usec += 1000000;
+    t1.tv_nsec -= t2.tv_nsec;
+    if (t1.tv_nsec < 0) {
+        t1.tv_nsec += 1000000000;
         t1.tv_sec--;
     }
     t1.tv_sec -= t2.tv_sec;
     return t1;
 }
 
-static double tdiv(double value, struct timeval tv)
+static double tdiv(double value, struct timespec tv)
 {
-    return value / ((double)tv.tv_sec + ((double)tv.tv_usec / 1000000.0));
+    double time = (double)tv.tv_sec + ((double)tv.tv_nsec / 1000000000.0);
+    return value / time;
 }
 
 #define HOURS(sec)      ((sec) / (60 * 60))
@@ -274,16 +275,16 @@ enum {
     VERBOSE_FIXED_TIME  = 0x2,
 };
 
-static void timestr(struct timeval *tv, char *ts, size_t size, int format)
+static void timestr(struct timespec *tv, char *ts, size_t size, int format)
 {
-    double usec = (double)tv->tv_usec / 1000000.0;
+    double nsec = (double)tv->tv_nsec / 1000000000.0;
 
     if (format & TERSE_FIXED_TIME) {
         if (!HOURS(tv->tv_sec)) {
             snprintf(ts, size, "%u:%02u.%02u",
                     (unsigned int) MINUTES(tv->tv_sec),
                     (unsigned int) SECONDS(tv->tv_sec),
-                    (unsigned int) (usec * 100));
+                    (unsigned int) (nsec * 100000));
             return;
         }
         format |= VERBOSE_FIXED_TIME; /* fallback if hours needed */
@@ -294,9 +295,9 @@ static void timestr(struct timeval *tv, char *ts, size_t size, int format)
                 (unsigned int) HOURS(tv->tv_sec),
                 (unsigned int) MINUTES(tv->tv_sec),
                 (unsigned int) SECONDS(tv->tv_sec),
-                (unsigned int) (usec * 100));
+                (unsigned int) (nsec * 100000));
     } else {
-        snprintf(ts, size, "0.%04u sec", (unsigned int) (usec * 10000));
+        snprintf(ts, size, "0.%04u sec", (unsigned int) (nsec * 10000000));
     }
 }
 
@@ -376,7 +377,7 @@ static void dump_buffer(const void *buffer, int64_t offset, int64_t len)
     }
 }
 
-static void print_report(const char *op, struct timeval *t, int64_t offset,
+static void print_report(const char *op, struct timespec *t, int64_t offset,
                          int64_t count, int64_t total, int cnt, bool Cflag)
 {
     char s1[64], s2[64], ts[64];
@@ -649,7 +650,7 @@ static const cmdinfo_t read_cmd = {
 
 static int read_f(BlockBackend *blk, int argc, char **argv)
 {
-    struct timeval t1, t2;
+    struct timespec t1, t2;
     bool Cflag = false, qflag = false, vflag = false;
     bool Pflag = false, sflag = false, lflag = false, bflag = false;
     int c, cnt, ret;
@@ -758,13 +759,13 @@ static int read_f(BlockBackend *blk, int argc, char **argv)
 
     buf = qemu_io_alloc(blk, count, 0xab);
 
-    gettimeofday(&t1, NULL);
+    clock_gettime(CLOCK_MONOTONIC, &t1);
     if (bflag) {
         ret = do_load_vmstate(blk, buf, offset, count, &total);
     } else {
         ret = do_pread(blk, buf, offset, count, &total);
     }
-    gettimeofday(&t2, NULL);
+    clock_gettime(CLOCK_MONOTONIC, &t2);
 
     if (ret < 0) {
         printf("read failed: %s\n", strerror(-ret));
@@ -836,7 +837,7 @@ static const cmdinfo_t readv_cmd = {
 
 static int readv_f(BlockBackend *blk, int argc, char **argv)
 {
-    struct timeval t1, t2;
+    struct timespec t1, t2;
     bool Cflag = false, qflag = false, vflag = false;
     int c, cnt, ret;
     char *buf;
@@ -891,9 +892,9 @@ static int readv_f(BlockBackend *blk, int argc, char **argv)
         return -EINVAL;
     }
 
-    gettimeofday(&t1, NULL);
+    clock_gettime(CLOCK_MONOTONIC,&t1);
     ret = do_aio_readv(blk, &qiov, offset, &total);
-    gettimeofday(&t2, NULL);
+    clock_gettime(CLOCK_MONOTONIC,&t2);
 
     if (ret < 0) {
         printf("readv failed: %s\n", strerror(-ret));
@@ -972,7 +973,7 @@ static const cmdinfo_t write_cmd = {
 
 static int write_f(BlockBackend *blk, int argc, char **argv)
 {
-    struct timeval t1, t2;
+    struct timespec t1, t2;
     bool Cflag = false, qflag = false, bflag = false;
     bool Pflag = false, zflag = false, cflag = false;
     int flags = 0;
@@ -1091,7 +1092,7 @@ static int write_f(BlockBackend *blk, int argc, char **argv)
         buf = qemu_io_alloc(blk, count, pattern);
     }
 
-    gettimeofday(&t1, NULL);
+    clock_gettime(CLOCK_MONOTONIC,&t1);
     if (bflag) {
         ret = do_save_vmstate(blk, buf, offset, count, &total);
     } else if (zflag) {
@@ -1101,7 +1102,7 @@ static int write_f(BlockBackend *blk, int argc, char **argv)
     } else {
         ret = do_pwrite(blk, buf, offset, count, flags, &total);
     }
-    gettimeofday(&t2, NULL);
+    clock_gettime(CLOCK_MONOTONIC,&t2);
 
     if (ret < 0) {
         printf("write failed: %s\n", strerror(-ret));
@@ -1160,7 +1161,7 @@ static const cmdinfo_t writev_cmd = {
 
 static int writev_f(BlockBackend *blk, int argc, char **argv)
 {
-    struct timeval t1, t2;
+    struct timespec t1, t2;
     bool Cflag = false, qflag = false;
     int flags = 0;
     int c, cnt, ret;
@@ -1213,9 +1214,9 @@ static int writev_f(BlockBackend *blk, int argc, char **argv)
         return -EINVAL;
     }
 
-    gettimeofday(&t1, NULL);
+    clock_gettime(CLOCK_MONOTONIC,&t1);
     ret = do_aio_writev(blk, &qiov, offset, flags, &total);
-    gettimeofday(&t2, NULL);
+    clock_gettime(CLOCK_MONOTONIC,&t2);
 
     if (ret < 0) {
         printf("writev failed: %s\n", strerror(-ret));
@@ -1250,15 +1251,15 @@ struct aio_ctx {
     bool zflag;
     BlockAcctCookie acct;
     int pattern;
-    struct timeval t1;
+    struct timespec t1;
 };
 
 static void aio_write_done(void *opaque, int ret)
 {
     struct aio_ctx *ctx = opaque;
-    struct timeval t2;
+    struct timespec t2;
 
-    gettimeofday(&t2, NULL);
+    clock_gettime(CLOCK_MONOTONIC,&t2);
 
 
     if (ret < 0) {
@@ -1288,9 +1289,9 @@ out:
 static void aio_read_done(void *opaque, int ret)
 {
     struct aio_ctx *ctx = opaque;
-    struct timeval t2;
+    struct timespec t2;
 
-    gettimeofday(&t2, NULL);
+    clock_gettime(CLOCK_MONOTONIC,&t2);
 
     if (ret < 0) {
         printf("readv failed: %s\n", strerror(-ret));
@@ -1425,7 +1426,7 @@ static int aio_read_f(BlockBackend *blk, int argc, char **argv)
         return -EINVAL;
     }
 
-    gettimeofday(&ctx->t1, NULL);
+    clock_gettime(CLOCK_MONOTONIC,&ctx->t1);
     block_acct_start(blk_get_stats(blk), &ctx->acct, ctx->qiov.size,
                      BLOCK_ACCT_READ);
     blk_aio_preadv(blk, ctx->offset, &ctx->qiov, 0, aio_read_done, ctx);
@@ -1570,7 +1571,7 @@ static int aio_write_f(BlockBackend *blk, int argc, char **argv)
             return -EINVAL;
         }
 
-        gettimeofday(&ctx->t1, NULL);
+        clock_gettime(CLOCK_MONOTONIC,&ctx->t1);
         block_acct_start(blk_get_stats(blk), &ctx->acct, ctx->qiov.size,
                          BLOCK_ACCT_WRITE);
 
@@ -1746,7 +1747,7 @@ static const cmdinfo_t discard_cmd = {
 
 static int discard_f(BlockBackend *blk, int argc, char **argv)
 {
-    struct timeval t1, t2;
+    struct timespec t1, t2;
     bool Cflag = false, qflag = false;
     int c, ret;
     int64_t offset, bytes;
@@ -1787,9 +1788,9 @@ static int discard_f(BlockBackend *blk, int argc, char **argv)
         return -EINVAL;
     }
 
-    gettimeofday(&t1, NULL);
+    clock_gettime(CLOCK_MONOTONIC,&t1);
     ret = blk_pdiscard(blk, offset, bytes);
-    gettimeofday(&t2, NULL);
+    clock_gettime(CLOCK_MONOTONIC,&t2);
 
     if (ret < 0) {
         printf("discard failed: %s\n", strerror(-ret));
-- 
2.20.1


Re: [Qemu-devel] [RFC PATCH] qemu-io-cmds: use clock_gettime for benchmarking
Posted by no-reply@patchew.org 4 years, 10 months ago
Patchew URL: https://patchew.org/QEMU/20190529162438.22653-1-alex.bennee@linaro.org/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [Qemu-devel] [RFC PATCH] qemu-io-cmds: use clock_gettime for benchmarking
Type: series
Message-id: 20190529162438.22653-1-alex.bennee@linaro.org

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

From https://github.com/patchew-project/qemu
 * [new tag]               patchew/20190529162438.22653-1-alex.bennee@linaro.org -> patchew/20190529162438.22653-1-alex.bennee@linaro.org
Switched to a new branch 'test'
d6ebcb635a qemu-io-cmds: use clock_gettime for benchmarking

=== OUTPUT BEGIN ===
ERROR: space required after that ',' (ctx:VxO)
#132: FILE: qemu-io-cmds.c:895:
+    clock_gettime(CLOCK_MONOTONIC,&t1);
                                  ^

ERROR: space required before that '&' (ctx:OxV)
#132: FILE: qemu-io-cmds.c:895:
+    clock_gettime(CLOCK_MONOTONIC,&t1);
                                   ^

ERROR: space required after that ',' (ctx:VxO)
#135: FILE: qemu-io-cmds.c:897:
+    clock_gettime(CLOCK_MONOTONIC,&t2);
                                  ^

ERROR: space required before that '&' (ctx:OxV)
#135: FILE: qemu-io-cmds.c:897:
+    clock_gettime(CLOCK_MONOTONIC,&t2);
                                   ^

ERROR: space required after that ',' (ctx:VxO)
#153: FILE: qemu-io-cmds.c:1095:
+    clock_gettime(CLOCK_MONOTONIC,&t1);
                                  ^

ERROR: space required before that '&' (ctx:OxV)
#153: FILE: qemu-io-cmds.c:1095:
+    clock_gettime(CLOCK_MONOTONIC,&t1);
                                   ^

ERROR: space required after that ',' (ctx:VxO)
#162: FILE: qemu-io-cmds.c:1105:
+    clock_gettime(CLOCK_MONOTONIC,&t2);
                                  ^

ERROR: space required before that '&' (ctx:OxV)
#162: FILE: qemu-io-cmds.c:1105:
+    clock_gettime(CLOCK_MONOTONIC,&t2);
                                   ^

ERROR: space required after that ',' (ctx:VxO)
#180: FILE: qemu-io-cmds.c:1217:
+    clock_gettime(CLOCK_MONOTONIC,&t1);
                                  ^

ERROR: space required before that '&' (ctx:OxV)
#180: FILE: qemu-io-cmds.c:1217:
+    clock_gettime(CLOCK_MONOTONIC,&t1);
                                   ^

ERROR: space required after that ',' (ctx:VxO)
#183: FILE: qemu-io-cmds.c:1219:
+    clock_gettime(CLOCK_MONOTONIC,&t2);
                                  ^

ERROR: space required before that '&' (ctx:OxV)
#183: FILE: qemu-io-cmds.c:1219:
+    clock_gettime(CLOCK_MONOTONIC,&t2);
                                   ^

ERROR: space required after that ',' (ctx:VxO)
#202: FILE: qemu-io-cmds.c:1262:
+    clock_gettime(CLOCK_MONOTONIC,&t2);
                                  ^

ERROR: space required before that '&' (ctx:OxV)
#202: FILE: qemu-io-cmds.c:1262:
+    clock_gettime(CLOCK_MONOTONIC,&t2);
                                   ^

ERROR: space required after that ',' (ctx:VxO)
#214: FILE: qemu-io-cmds.c:1294:
+    clock_gettime(CLOCK_MONOTONIC,&t2);
                                  ^

ERROR: space required before that '&' (ctx:OxV)
#214: FILE: qemu-io-cmds.c:1294:
+    clock_gettime(CLOCK_MONOTONIC,&t2);
                                   ^

ERROR: space required after that ',' (ctx:VxO)
#223: FILE: qemu-io-cmds.c:1429:
+    clock_gettime(CLOCK_MONOTONIC,&ctx->t1);
                                  ^

ERROR: space required before that '&' (ctx:OxV)
#223: FILE: qemu-io-cmds.c:1429:
+    clock_gettime(CLOCK_MONOTONIC,&ctx->t1);
                                   ^

ERROR: space required after that ',' (ctx:VxO)
#232: FILE: qemu-io-cmds.c:1574:
+        clock_gettime(CLOCK_MONOTONIC,&ctx->t1);
                                      ^

ERROR: space required before that '&' (ctx:OxV)
#232: FILE: qemu-io-cmds.c:1574:
+        clock_gettime(CLOCK_MONOTONIC,&ctx->t1);
                                       ^

ERROR: space required after that ',' (ctx:VxO)
#250: FILE: qemu-io-cmds.c:1791:
+    clock_gettime(CLOCK_MONOTONIC,&t1);
                                  ^

ERROR: space required before that '&' (ctx:OxV)
#250: FILE: qemu-io-cmds.c:1791:
+    clock_gettime(CLOCK_MONOTONIC,&t1);
                                   ^

ERROR: space required after that ',' (ctx:VxO)
#253: FILE: qemu-io-cmds.c:1793:
+    clock_gettime(CLOCK_MONOTONIC,&t2);
                                  ^

ERROR: space required before that '&' (ctx:OxV)
#253: FILE: qemu-io-cmds.c:1793:
+    clock_gettime(CLOCK_MONOTONIC,&t2);
                                   ^

total: 24 errors, 0 warnings, 214 lines checked

Commit d6ebcb635ac6 (qemu-io-cmds: use clock_gettime for benchmarking) has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20190529162438.22653-1-alex.bennee@linaro.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com