changeset 638e865d70c6 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=638e865d70c6
description:
        kvm: Fix a case where the run timers weren't armed properly

        There is a possibility that the timespec used to arm a timer becomes
        zero if the number of ticks used when arming a timer is close to the
        resolution of the timer. Due to the semantics of POSIX timers, this
        actually disarms the timer. This changeset fixes this issue by
        eliminating the rounding error (we always round away from zero
        now). It also reuses the minimum number of cycles, which were
        previously only used for cycle-based timers, to calculate a more
        useful resolution.

diffstat:

 src/cpu/kvm/timer.cc |  33 ++++++++++++++++++++++++++++-----
 1 files changed, 28 insertions(+), 5 deletions(-)

diffs (70 lines):

diff -r 3fda7e22041b -r 638e865d70c6 src/cpu/kvm/timer.cc
--- a/src/cpu/kvm/timer.cc      Thu Sep 19 17:30:26 2013 +0200
+++ b/src/cpu/kvm/timer.cc      Thu Sep 19 17:55:03 2013 +0200
@@ -37,6 +37,7 @@
  * Authors: Andreas Sandberg
  */
 
+#include <algorithm>
 #include <csignal>
 #include <ctime>
 
@@ -45,6 +46,14 @@
 #include "cpu/kvm/timer.hh"
 #include "debug/KvmTimer.hh"
 
+/**
+ * Minimum number of cycles that a host can spend in a KVM call (used
+ * to calculate the resolution of some timers).
+ *
+ * The value of this constant is a bit arbitrary, but in practice, we
+ * can't really do anything useful in less than ~1000 cycles.
+ */
+static const uint64_t MIN_HOST_CYCLES = 1000;
 
 PosixKvmTimer::PosixKvmTimer(int signo, clockid_t clockID,
                              float hostFactor, Tick hostFreq)
@@ -82,6 +91,8 @@
     ts.it_value.tv_sec = hostNs(ticks) / 1000000000ULL;
     ts.it_value.tv_nsec = hostNs(ticks) % 1000000000ULL;
 
+    assert(ts.it_value.tv_nsec > 0 || ts.it_value.tv_sec > 0);
+
     DPRINTF(KvmTimer, "Arming POSIX timer: %i ticks (%is%ins)\n",
             ticks, ts.it_value.tv_sec, ts.it_value.tv_nsec);
 
@@ -109,9 +120,23 @@
     if (clock_getres(clockID, &ts) == -1)
         panic("PosixKvmTimer: Failed to get timer resolution\n");
 
-    Tick resolution(ticksFromHostNs(ts.tv_sec * 1000000000ULL + ts.tv_nsec));
+    const uint64_t res_ns(ts.tv_sec * 1000000000ULL + ts.tv_nsec);
+    // We preferrably want ticksFromHostNs() to calculate the the
+    // ceiling rather than truncating the value. However, there are
+    // other cases where truncating is fine, so we just add 1 here to
+    // make sure that the actual resolution is strictly less than what
+    // we return. We could get all kinds of nasty behavior if
+    // arm(resolution) is called and the resulting time is 0 (which
+    // could happen if we truncate the results and the resolution is
+    // 1ns).
+    const Tick resolution(ticksFromHostNs(res_ns) + 1);
+    // It might not make sense to enter into KVM for less than a
+    // certain number of host cycles. In some systems (e.g., Linux)
+    // the resolution of the timer we use is 1ns (a few cycles on most
+    // CPUs), which isn't very useful.
+    const Tick min_cycles(ticksFromHostCycles(MIN_HOST_CYCLES));
 
-    return resolution;
+    return std::max(resolution, min_cycles);
 }
 
 
@@ -143,7 +168,5 @@
 Tick
 PerfKvmTimer::calcResolution()
 {
-    // This is a bit arbitrary, but in practice, we can't really do
-    // anything useful in less than ~1000 anyway.
-    return ticksFromHostCycles(1000);
+    return ticksFromHostCycles(MIN_HOST_CYCLES);
 }
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to