* [Qemu-devel] [PATCH 1/2] main: add high resolution GSource based timer
@ 2011-08-22 19:21 Anthony Liguori
2011-08-22 19:21 ` [Qemu-devel] [PATCH 2/2] [RFC] time: refactor QEMU timer to use GHRTimer Anthony Liguori
2011-08-22 19:26 ` [Qemu-devel] [PATCH 1/2] main: add high resolution GSource based timer Anthony Liguori
0 siblings, 2 replies; 13+ messages in thread
From: Anthony Liguori @ 2011-08-22 19:21 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Anthony Liguori, Jan Kiszka
This was originally written by Paolo Bonzini.
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
Makefile.objs | 1 +
configure | 4 +-
ghrtimer.c | 334 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
ghrtimer.h | 49 +++++++++
4 files changed, 386 insertions(+), 2 deletions(-)
create mode 100644 ghrtimer.c
create mode 100644 ghrtimer.h
diff --git a/Makefile.objs b/Makefile.objs
index d1f3e5d..e4267ed 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -9,6 +9,7 @@ qobject-obj-y += qerror.o error.o
oslib-obj-y = osdep.o
oslib-obj-$(CONFIG_WIN32) += oslib-win32.o qemu-thread-win32.o
oslib-obj-$(CONFIG_POSIX) += oslib-posix.o qemu-thread-posix.o
+oslib-obj-y += ghrtimer.o
#######################################################################
# coroutines
diff --git a/configure b/configure
index 8bbd694..6339c45 100755
--- a/configure
+++ b/configure
@@ -1843,8 +1843,8 @@ fi
##########################################
# glib support probe
if $pkg_config --modversion gthread-2.0 > /dev/null 2>&1 ; then
- glib_cflags=`$pkg_config --cflags gthread-2.0 2>/dev/null`
- glib_libs=`$pkg_config --libs gthread-2.0 2>/dev/null`
+ glib_cflags=`$pkg_config --cflags gthread-2.0 gobject-2.0 2>/dev/null`
+ glib_libs=`$pkg_config --libs gthread-2.0 gobject-2.0 2>/dev/null`
LIBS="$glib_libs $LIBS"
libs_qga="$glib_libs $libs_qga"
else
diff --git a/ghrtimer.c b/ghrtimer.c
new file mode 100644
index 0000000..1e7e1c2
--- /dev/null
+++ b/ghrtimer.c
@@ -0,0 +1,334 @@
+/*
+ * timerfd GSource wrapper
+ *
+ * Copyright IBM, Corp. 2011
+ * Copyright Red Hat, Inc. 2011
+ *
+ * Authors:
+ * Anthony Liguori <aliguori@us.ibm.com>
+ * Paolo Bonzini <pbonzini@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#include "config-host.h"
+#include <stdlib.h>
+#include "ghrtimer.h"
+#include <glib-object.h>
+#include <fcntl.h>
+#include <unistd.h>
+
+#ifdef CONFIG_TIMERFD
+#include <sys/timerfd.h>
+#endif
+
+struct _GHRTimer {
+ GSource source;
+ gint64 deadline;
+ GPollFD poll;
+ char pending;
+};
+
+#define MIN_TIMER_REARM_NS 250000
+
+#if GLIB_MAJOR_VERSION == 2 && GLIB_MINOR_VERSION <= 26
+static inline guint64 muldiv64(guint64 a, guint32 b, guint32 c)
+{
+ guint64 rl = (a & 0xffffffff) * (guint64)b;
+ guint64 rh = (a >> 32) * (guint64)b + (rl >> 32);
+ rl &= 0xffffffff;
+ return ((rh / c) << 32) | ((((rh % c) << 32) + rl) / c);
+}
+
+gint64
+g_get_monotonic_time_ns (void)
+{
+#if defined(__linux__) || (defined(__FreeBSD__) && __FreeBSD_version >= 500000) \
+ || defined(__DragonFly__) || defined(__FreeBSD_kernel__) \
+ || defined(__OpenBSD__)
+ struct timespec ts;
+ clock_gettime(CLOCK_MONOTONIC, &ts);
+ return ts.tv_sec * 1000000000LL + ts.tv_nsec;
+
+#elif defined _WIN32
+ LARGE_INTEGER ti;
+ static LARGE_INTEGER freq;
+ if (freq.QuadPart == 0) {
+ QueryPerformanceFrequency(&freq);
+ }
+ QueryPerformanceCounter(&ti);
+ return muldiv64(ti.QuadPart, 1000000000, freq.QuadPart);
+
+#else
+#ifdef CONFIG_TIMERFD
+#error configuration problem, timerfd uses CLOCK_MONOTONIC
+#endif
+ GTimeVal tv;
+ g_get_current_time (&tv);
+ return ((guint64) tv.tv_sec) * 1000000000 + tv.tv_usec * 1000;
+#endif
+}
+
+gint64
+g_source_get_time_ns (GSource *source)
+{
+ return g_get_monotonic_time_ns ();
+}
+
+#else
+gint64
+g_get_monotonic_time_ns (void)
+{
+ return g_get_monotonic_time () * 1000;
+}
+
+gint64
+g_source_get_time_ns (GSource *source)
+{
+ return g_source_get_time (source) * 1000;
+}
+#endif
+
+gboolean
+g_hrtimer_pending (GHRTimer *timer)
+{
+ return timer->pending;
+}
+
+void
+g_hrtimer_rearm (GHRTimer *timer,
+ gint64 us)
+{
+ return g_hrtimer_rearm_ns (timer, us * 1000);
+}
+
+void
+g_hrtimer_rearm_ns (GHRTimer *timer,
+ gint64 ns)
+{
+ gint64 time = g_get_monotonic_time_ns ();
+ timer->deadline = ns;
+ if (ns < time) {
+ timer->pending = TRUE;
+ return;
+ }
+
+ timer->pending = FALSE;
+ if (ns == G_HRTIMER_QUIESCE) {
+ return;
+ }
+
+ if (ns - time < MIN_TIMER_REARM_NS) {
+ timer->deadline = ns = time + MIN_TIMER_REARM_NS;
+ }
+#ifdef CONFIG_TIMERFD
+ if (timer->poll.fd != -1) {
+ struct itimerspec new = {
+ .it_interval = { 0, 0 },
+ .it_value = { ns / 1000000000, ns % 1000000000 }
+ };
+ timerfd_settime(timer->poll.fd, TFD_TIMER_ABSTIME, &new, NULL);
+ }
+#endif
+}
+
+static gboolean
+g_hrtimer_prepare (GSource *source,
+ gint *timeout)
+{
+ GHRTimer *timer = (GHRTimer *) source;
+
+ if (timer->deadline == G_HRTIMER_QUIESCE) {
+ g_assert (!timer->pending);
+ *timeout = -1;
+ return FALSE;
+ }
+
+ if (timer->poll.fd == -1) {
+ gint64 timeout_ns = timer->deadline - g_get_monotonic_time_ns ();
+ if (timeout_ns < 0) {
+ *timeout = 0;
+ timer->pending = TRUE;
+ } else {
+ *timeout = timeout_ns / 1000000;
+ }
+ } else {
+ *timeout = -1;
+#ifndef CONFIG_TIMERFD
+ abort ();
+#endif
+ }
+ return timer->pending;
+}
+
+static gboolean
+g_hrtimer_check (GSource *source)
+{
+ GHRTimer *timer = (GHRTimer *) source;
+
+ if (timer->deadline == G_HRTIMER_QUIESCE) {
+ g_assert (!timer->pending);
+ return FALSE;
+ }
+
+ if (timer->poll.fd == -1) {
+ timer->pending |= (timer->deadline <= g_source_get_time_ns (source));
+ } else {
+ long long overrun;
+ timer->pending |= (timer->poll.revents & G_IO_IN) != 0;
+ if (timer->pending) {
+ if (read (timer->poll.fd, (char *) &overrun, sizeof (overrun))) {
+ /* do nothing */
+ }
+ }
+#ifndef CONFIG_TIMERFD
+ abort ();
+#endif
+ }
+
+ return timer->pending;
+}
+
+static gboolean
+g_hrtimer_dispatch (GSource *source,
+ GSourceFunc callback,
+ gpointer user_data)
+{
+ GHRTimer *timer = (GHRTimer *) source;
+
+ if (!callback) {
+ g_warning ("Timer source dispatched without callback\n"
+ "You must call g_source_set_callback().");
+ return TRUE;
+ }
+
+ timer->pending = FALSE;
+ timer->deadline = G_HRTIMER_QUIESCE;
+ if (user_data == NULL)
+ user_data = timer;
+ callback (user_data);
+ return TRUE;
+}
+
+static void
+g_hrtimer_finalize (GSource *source)
+{
+ GHRTimer *timer = (GHRTimer *) source;
+
+ if (timer->poll.fd != -1) {
+ close (timer->poll.fd);
+#ifndef CONFIG_TIMERFD
+ abort ();
+#endif
+ }
+}
+
+static void
+g_hrtimer_closure_callback (gpointer data)
+{
+ GClosure *closure = data;
+ g_closure_invoke (closure, NULL, 0, NULL, NULL);
+}
+
+static GSourceFuncs hrtimer_source_funcs = {
+ g_hrtimer_prepare,
+ g_hrtimer_check,
+ g_hrtimer_dispatch,
+ g_hrtimer_finalize,
+ (GSourceFunc) g_hrtimer_closure_callback,
+ (gpointer) g_cclosure_marshal_VOID__VOID
+};
+
+GHRTimer *
+g_hrtimer_new (void)
+{
+ GHRTimer *timer;
+
+ timer = (GHRTimer *) g_source_new (&hrtimer_source_funcs,
+ sizeof (GHRTimer));
+
+#ifdef CONFIG_TIMERFD
+ timer->poll.fd = timerfd_create (CLOCK_MONOTONIC, 0);
+ if (timer->poll.fd != -1) {
+ fcntl(timer->poll.fd, F_SETFD, fcntl (timer->poll.fd, F_GETFD) | FD_CLOEXEC);
+ fcntl(timer->poll.fd, F_SETFL, fcntl (timer->poll.fd, F_GETFL) | O_NONBLOCK);
+ timer->poll.events = G_IO_IN;
+ g_source_add_poll (&timer->source, &timer->poll);
+ }
+#else
+ timer->poll.fd = -1;
+#endif
+ timer->deadline = G_HRTIMER_QUIESCE;
+ return timer;
+}
+guint
+g_hrtimer_add (GHRTimer **timer,
+ GSourceFunc func,
+ gpointer user_data)
+{
+ return g_hrtimer_add_full (G_PRIORITY_DEFAULT, timer, func, user_data, NULL);
+}
+
+guint
+g_hrtimer_add_full (gint priority,
+ GHRTimer **timer,
+ GSourceFunc func,
+ gpointer user_data,
+ GDestroyNotify notify)
+{
+ GHRTimer *hrtimer;
+ guint id;
+
+ hrtimer = g_hrtimer_new ();
+ if (priority != G_PRIORITY_DEFAULT)
+ g_source_set_priority (&hrtimer->source, priority);
+
+ g_source_set_callback (&hrtimer->source, (GSourceFunc) func,
+ user_data, notify);
+ id = g_source_attach (&hrtimer->source, NULL);
+
+ *timer = hrtimer;
+ return id;
+}
+
+#ifdef MAIN
+#include <stdio.h>
+
+static int i = 3;
+static GMainLoop *loop;
+
+void
+rearm_timer (GHRTimer *timer)
+{
+ printf (".");
+ fflush (stdout);
+ g_hrtimer_rearm_ns (timer, g_get_monotonic_time_ns () + 1000000000);
+}
+
+void
+hrtimer_callback (gpointer user_data)
+{
+ GHRTimer *timer = user_data;
+
+ if (--i == 0) {
+ printf ("\n");
+ fflush (stdout);
+ g_main_loop_quit (loop);
+ } else {
+ rearm_timer (timer);
+ }
+}
+
+int main()
+{
+ GHRTimer *timer;
+ loop = g_main_loop_new (NULL, FALSE);
+ g_hrtimer_add (&timer, (GSourceFunc) hrtimer_callback, NULL);
+ rearm_timer (timer);
+ g_main_loop_run (loop);
+ g_source_unref ((GSource *) timer);
+}
+#endif
+
diff --git a/ghrtimer.h b/ghrtimer.h
new file mode 100644
index 0000000..2cf6961
--- /dev/null
+++ b/ghrtimer.h
@@ -0,0 +1,49 @@
+/*
+ * timerfd GSource wrapper
+ *
+ * Copyright IBM, Corp. 2011
+ * Copyright Red Hat, Inc. 2011
+ *
+ * Authors:
+ * Anthony Liguori <aliguori@us.ibm.com>
+ * Paolo Bonzini <pbonzini@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#ifndef G_HRTIMER_H
+#define G_HRTIMER_H 1
+
+#include <glib.h>
+
+#define G_HRTIMER_QUIESCE ((gint64) 0x7FFFFFFFFFFFFFFF)
+
+typedef struct _GHRTimer GHRTimer;
+
+gint64 g_get_monotonic_time_ns (void);
+
+gint64 g_source_get_time_ns (GSource *source);
+
+GHRTimer *g_hrtimer_new (void);
+
+gboolean g_hrtimer_pending (GHRTimer *timer);
+
+void g_hrtimer_rearm (GHRTimer *timer,
+ gint64 usec);
+
+void g_hrtimer_rearm_ns (GHRTimer *timer,
+ gint64 nsec);
+
+guint g_hrtimer_add (GHRTimer **timer,
+ GSourceFunc callback,
+ gpointer user_data);
+
+guint g_hrtimer_add_full (int priority,
+ GHRTimer **timer,
+ GSourceFunc callback,
+ gpointer user_data,
+ GDestroyNotify notify);
+
+#endif
--
1.7.4.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH 2/2] [RFC] time: refactor QEMU timer to use GHRTimer
2011-08-22 19:21 [Qemu-devel] [PATCH 1/2] main: add high resolution GSource based timer Anthony Liguori
@ 2011-08-22 19:21 ` Anthony Liguori
2011-08-22 20:28 ` Jan Kiszka
2011-08-23 7:43 ` Paolo Bonzini
2011-08-22 19:26 ` [Qemu-devel] [PATCH 1/2] main: add high resolution GSource based timer Anthony Liguori
1 sibling, 2 replies; 13+ messages in thread
From: Anthony Liguori @ 2011-08-22 19:21 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Anthony Liguori, Jan Kiszka
This replaces all of the QEMU timer code with GHRTimer, dramatically simplifying
time keeping in QEMU while making it possible to use QEMUTimer code outside of
the main loop. The later is critical to building unit tests.
This is an RFC because I'm sure this breaks things as it changes things. QEMU
time keeping is quite a mess today. Here's what we do today:
1) We have three clocks:
a) the real time clock, based on system time, not monotonic
b) the host clock, based on the real time clock, monotonic by detecting
movements backward in time
c) the vm clock, based on real time clock but may start/stop with the guest
2) A "cpu ticks" clock that uses platform specific mechanisms (inline asm)
3) Various clock source implementations that may use a periodic timer or a
a dynamic time source. We have different implementations for different
platforms
4) Time events are delivered via SIGALRM which means we end up getting EINTRs
very often in QEMU. This is fairly annoying. Signals also race against
select leading to a very ugly set of work arounds involving writing data to
pipes. This is the sort of stuff in Unix programming that I wish I never had
to learn about and am very eager to eliminate in QEMU :-)
(2) is just plain broken. In modern operating systems, gettimeofday() is
optimized CPU instructions when they can be used safely. Often they can't be
used safely and we ignore that in QEMU. For instance, on x86, RDTSC races with
the scheduler (not to mention that the TSC is infamously unstable across cores).
The kernel does the right thing here and provides the fastest method that's
correct.
(1.a) seems like a bug more than a feature. I don't see a lot of disadvantages
to using a monotonic time source.
(1.b) is a bit naive in its current form. Modern kernels export a truly
monotonic time source which has a reliable frequency. Even though (1.b) detects
backwards jumps, it doesn't do anything about large forward jumps which can also
be problematic.
(3) results in tons of bitrot. The only implementation that really should be
used these days is dynticks.
(4) is just painful
I've tested TCG, KVM, and icount. They all seem to be working as well as they
did before. I've only tested a Linux host though. I'm sure I may be breaking
something here and am hoping folks can help identify it.
This depends on the I/O thread and glib patches I posted earlier today.
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
qemu-timer.c | 593 +++-------------------------------------------------------
qemu-timer.h | 196 +-------------------
vl.c | 4 +-
3 files changed, 33 insertions(+), 760 deletions(-)
diff --git a/qemu-timer.c b/qemu-timer.c
index 46dd483..aafb7e8 100644
--- a/qemu-timer.c
+++ b/qemu-timer.c
@@ -29,22 +29,8 @@
#include "hw/hw.h"
-#include <unistd.h>
-#include <fcntl.h>
-#include <time.h>
-#include <errno.h>
-#include <sys/time.h>
-#include <signal.h>
-#ifdef __FreeBSD__
-#include <sys/param.h>
-#endif
-
-#ifdef _WIN32
-#include <windows.h>
-#include <mmsystem.h>
-#endif
-
#include "qemu-timer.h"
+#include "ghrtimer.h"
/* Conversion factor from emulated instructions to virtual clock ticks. */
int icount_time_shift;
@@ -78,12 +64,7 @@ int64_t cpu_get_ticks(void)
return timers_state.cpu_ticks_offset;
} else {
int64_t ticks;
- ticks = cpu_get_real_ticks();
- if (timers_state.cpu_ticks_prev > ticks) {
- /* Note: non increasing ticks may happen if the host uses
- software suspend */
- timers_state.cpu_ticks_offset += timers_state.cpu_ticks_prev - ticks;
- }
+ ticks = get_clock();
timers_state.cpu_ticks_prev = ticks;
return ticks + timers_state.cpu_ticks_offset;
}
@@ -105,7 +86,7 @@ static int64_t cpu_get_clock(void)
void cpu_enable_ticks(void)
{
if (!timers_state.cpu_ticks_enabled) {
- timers_state.cpu_ticks_offset -= cpu_get_real_ticks();
+ timers_state.cpu_ticks_offset -= cpu_get_ticks();
timers_state.cpu_clock_offset -= get_clock();
timers_state.cpu_ticks_enabled = 1;
}
@@ -148,22 +129,9 @@ struct QEMUTimer {
struct QEMUTimer *next;
};
-struct qemu_alarm_timer {
- char const *name;
- int (*start)(struct qemu_alarm_timer *t);
- void (*stop)(struct qemu_alarm_timer *t);
- void (*rearm)(struct qemu_alarm_timer *t);
-#if defined(__linux__)
- int fd;
- timer_t timer;
-#elif defined(_WIN32)
- HANDLE timer;
-#endif
- char expired;
- char pending;
-};
+static GHRTimer *alarm_timer;
-static struct qemu_alarm_timer *alarm_timer;
+static int64_t qemu_next_alarm_deadline(void);
static bool qemu_timer_expired_ns(QEMUTimer *timer_head, int64_t current_time)
{
@@ -172,51 +140,15 @@ static bool qemu_timer_expired_ns(QEMUTimer *timer_head, int64_t current_time)
int qemu_alarm_pending(void)
{
- return alarm_timer->pending;
+ return g_hrtimer_pending(alarm_timer);
}
-static inline int alarm_has_dynticks(struct qemu_alarm_timer *t)
+static void qemu_rearm_alarm_timer(void)
{
- return !!t->rearm;
+ int64_t next = g_get_monotonic_time_ns() + qemu_next_alarm_deadline();
+ g_hrtimer_rearm_ns(alarm_timer, next);
}
-static void qemu_rearm_alarm_timer(struct qemu_alarm_timer *t)
-{
- if (!alarm_has_dynticks(t))
- return;
-
- t->rearm(t);
-}
-
-/* TODO: MIN_TIMER_REARM_NS should be optimized */
-#define MIN_TIMER_REARM_NS 250000
-
-#ifdef _WIN32
-
-static int mm_start_timer(struct qemu_alarm_timer *t);
-static void mm_stop_timer(struct qemu_alarm_timer *t);
-static void mm_rearm_timer(struct qemu_alarm_timer *t);
-
-static int win32_start_timer(struct qemu_alarm_timer *t);
-static void win32_stop_timer(struct qemu_alarm_timer *t);
-static void win32_rearm_timer(struct qemu_alarm_timer *t);
-
-#else
-
-static int unix_start_timer(struct qemu_alarm_timer *t);
-static void unix_stop_timer(struct qemu_alarm_timer *t);
-static void unix_rearm_timer(struct qemu_alarm_timer *t);
-
-#ifdef __linux__
-
-static int dynticks_start_timer(struct qemu_alarm_timer *t);
-static void dynticks_stop_timer(struct qemu_alarm_timer *t);
-static void dynticks_rearm_timer(struct qemu_alarm_timer *t);
-
-#endif /* __linux__ */
-
-#endif /* _WIN32 */
-
/* Correlation between real and virtual time is always going to be
fairly approximate, so ignore small variation.
When the guest is idle real and virtual time will be aligned in
@@ -272,86 +204,6 @@ int64_t qemu_icount_round(int64_t count)
return (count + (1 << icount_time_shift) - 1) >> icount_time_shift;
}
-static struct qemu_alarm_timer alarm_timers[] = {
-#ifndef _WIN32
-#ifdef __linux__
- {"dynticks", dynticks_start_timer,
- dynticks_stop_timer, dynticks_rearm_timer},
-#endif
- {"unix", unix_start_timer, unix_stop_timer, unix_rearm_timer},
-#else
- {"mmtimer", mm_start_timer, mm_stop_timer, NULL},
- {"mmtimer2", mm_start_timer, mm_stop_timer, mm_rearm_timer},
- {"dynticks", win32_start_timer, win32_stop_timer, win32_rearm_timer},
- {"win32", win32_start_timer, win32_stop_timer, NULL},
-#endif
- {NULL, }
-};
-
-static void show_available_alarms(void)
-{
- int i;
-
- printf("Available alarm timers, in order of precedence:\n");
- for (i = 0; alarm_timers[i].name; i++)
- printf("%s\n", alarm_timers[i].name);
-}
-
-void configure_alarms(char const *opt)
-{
- int i;
- int cur = 0;
- int count = ARRAY_SIZE(alarm_timers) - 1;
- char *arg;
- char *name;
- struct qemu_alarm_timer tmp;
-
- if (!strcmp(opt, "?")) {
- show_available_alarms();
- exit(0);
- }
-
- arg = g_strdup(opt);
-
- /* Reorder the array */
- name = strtok(arg, ",");
- while (name) {
- for (i = 0; i < count && alarm_timers[i].name; i++) {
- if (!strcmp(alarm_timers[i].name, name))
- break;
- }
-
- if (i == count) {
- fprintf(stderr, "Unknown clock %s\n", name);
- goto next;
- }
-
- if (i < cur)
- /* Ignore */
- goto next;
-
- /* Swap */
- tmp = alarm_timers[i];
- alarm_timers[i] = alarm_timers[cur];
- alarm_timers[cur] = tmp;
-
- cur++;
-next:
- name = strtok(NULL, ",");
- }
-
- g_free(arg);
-
- if (cur) {
- /* Disable remaining timers */
- for (i = cur; i < count; i++)
- alarm_timers[i].name = NULL;
- } else {
- show_available_alarms();
- exit(1);
- }
-}
-
#define QEMU_NUM_CLOCKS 3
QEMUClock *rt_clock;
@@ -527,8 +379,8 @@ static void qemu_mod_timer_ns(QEMUTimer *ts, int64_t expire_time)
/* Rearm if necessary */
if (pt == &active_timers[ts->clock->type]) {
- if (!alarm_timer->pending) {
- qemu_rearm_alarm_timer(alarm_timer);
+ if (g_hrtimer_pending(alarm_timer)) {
+ qemu_rearm_alarm_timer();
}
/* Interrupt execution to force deadline recalculation. */
qemu_clock_warp(ts->clock);
@@ -586,26 +438,17 @@ static void qemu_run_timers(QEMUClock *clock)
int64_t qemu_get_clock_ns(QEMUClock *clock)
{
- int64_t now, last;
-
switch(clock->type) {
+ case QEMU_CLOCK_HOST:
case QEMU_CLOCK_REALTIME:
- return get_clock();
+ return g_get_monotonic_time_ns();
default:
case QEMU_CLOCK_VIRTUAL:
if (use_icount) {
return cpu_get_icount();
} else {
- return cpu_get_clock();
+ return g_get_monotonic_time_ns();
}
- case QEMU_CLOCK_HOST:
- now = get_clock_realtime();
- last = clock->last;
- clock->last = now;
- if (now < last) {
- notifier_list_notify(&clock->reset_notifiers, &now);
- }
- return now;
}
}
@@ -699,74 +542,6 @@ void configure_icount(const char *option)
qemu_get_clock_ns(vm_clock) + get_ticks_per_sec() / 10);
}
-void qemu_run_all_timers(void)
-{
- alarm_timer->pending = 0;
-
- /* rearm timer, if not periodic */
- if (alarm_timer->expired) {
- alarm_timer->expired = 0;
- qemu_rearm_alarm_timer(alarm_timer);
- }
-
- /* vm time timers */
- if (vm_running) {
- qemu_run_timers(vm_clock);
- }
-
- qemu_run_timers(rt_clock);
- qemu_run_timers(host_clock);
-}
-
-static int64_t qemu_next_alarm_deadline(void);
-
-#ifdef _WIN32
-static void CALLBACK host_alarm_handler(PVOID lpParam, BOOLEAN unused)
-#else
-static void host_alarm_handler(int host_signum)
-#endif
-{
- struct qemu_alarm_timer *t = alarm_timer;
- if (!t)
- return;
-
-#if 0
-#define DISP_FREQ 1000
- {
- static int64_t delta_min = INT64_MAX;
- static int64_t delta_max, delta_cum, last_clock, delta, ti;
- static int count;
- ti = qemu_get_clock_ns(vm_clock);
- if (last_clock != 0) {
- delta = ti - last_clock;
- if (delta < delta_min)
- delta_min = delta;
- if (delta > delta_max)
- delta_max = delta;
- delta_cum += delta;
- if (++count == DISP_FREQ) {
- printf("timer: min=%" PRId64 " us max=%" PRId64 " us avg=%" PRId64 " us avg_freq=%0.3f Hz\n",
- muldiv64(delta_min, 1000000, get_ticks_per_sec()),
- muldiv64(delta_max, 1000000, get_ticks_per_sec()),
- muldiv64(delta_cum, 1000000 / DISP_FREQ, get_ticks_per_sec()),
- (double)get_ticks_per_sec() / ((double)delta_cum / DISP_FREQ));
- count = 0;
- delta_min = INT64_MAX;
- delta_max = 0;
- delta_cum = 0;
- }
- }
- last_clock = ti;
- }
-#endif
- if (alarm_has_dynticks(t) ||
- qemu_next_alarm_deadline () <= 0) {
- t->expired = alarm_has_dynticks(t);
- t->pending = 1;
- qemu_notify_event();
- }
-}
-
int64_t qemu_next_icount_deadline(void)
{
/* To avoid problems with overflow limit this to 2^32. */
@@ -811,351 +586,39 @@ static int64_t qemu_next_alarm_deadline(void)
return delta;
}
-#if defined(__linux__)
-
-#include "compatfd.h"
-
-static int dynticks_start_timer(struct qemu_alarm_timer *t)
-{
- struct sigevent ev;
- timer_t host_timer;
- struct sigaction act;
-
- sigfillset(&act.sa_mask);
- act.sa_flags = 0;
- act.sa_handler = host_alarm_handler;
-
- sigaction(SIGALRM, &act, NULL);
-
- /*
- * Initialize ev struct to 0 to avoid valgrind complaining
- * about uninitialized data in timer_create call
- */
- memset(&ev, 0, sizeof(ev));
- ev.sigev_value.sival_int = 0;
- ev.sigev_notify = SIGEV_SIGNAL;
-#ifdef SIGEV_THREAD_ID
- if (qemu_signalfd_available()) {
- ev.sigev_notify = SIGEV_THREAD_ID;
- ev._sigev_un._tid = qemu_get_thread_id();
- }
-#endif /* SIGEV_THREAD_ID */
- ev.sigev_signo = SIGALRM;
-
- if (timer_create(CLOCK_REALTIME, &ev, &host_timer)) {
- perror("timer_create");
-
- /* disable dynticks */
- fprintf(stderr, "Dynamic Ticks disabled\n");
-
- return -1;
- }
-
- t->timer = host_timer;
-
- return 0;
-}
-
-static void dynticks_stop_timer(struct qemu_alarm_timer *t)
-{
- timer_t host_timer = t->timer;
-
- timer_delete(host_timer);
-}
-
-static void dynticks_rearm_timer(struct qemu_alarm_timer *t)
+static void alarm_timer_on_change_state_rearm(void *opaque, int running,
+ int reason)
{
- timer_t host_timer = t->timer;
- struct itimerspec timeout;
- int64_t nearest_delta_ns = INT64_MAX;
- int64_t current_ns;
-
- assert(alarm_has_dynticks(t));
- if (!active_timers[QEMU_CLOCK_REALTIME] &&
- !active_timers[QEMU_CLOCK_VIRTUAL] &&
- !active_timers[QEMU_CLOCK_HOST])
- return;
-
- nearest_delta_ns = qemu_next_alarm_deadline();
- if (nearest_delta_ns < MIN_TIMER_REARM_NS)
- nearest_delta_ns = MIN_TIMER_REARM_NS;
-
- /* check whether a timer is already running */
- if (timer_gettime(host_timer, &timeout)) {
- perror("gettime");
- fprintf(stderr, "Internal timer error: aborting\n");
- exit(1);
- }
- current_ns = timeout.it_value.tv_sec * 1000000000LL + timeout.it_value.tv_nsec;
- if (current_ns && current_ns <= nearest_delta_ns)
- return;
-
- timeout.it_interval.tv_sec = 0;
- timeout.it_interval.tv_nsec = 0; /* 0 for one-shot timer */
- timeout.it_value.tv_sec = nearest_delta_ns / 1000000000;
- timeout.it_value.tv_nsec = nearest_delta_ns % 1000000000;
- if (timer_settime(host_timer, 0 /* RELATIVE */, &timeout, NULL)) {
- perror("settime");
- fprintf(stderr, "Internal timer error: aborting\n");
- exit(1);
- }
-}
-
-#endif /* defined(__linux__) */
-
-#if !defined(_WIN32)
-
-static int unix_start_timer(struct qemu_alarm_timer *t)
-{
- struct sigaction act;
-
- /* timer signal */
- sigfillset(&act.sa_mask);
- act.sa_flags = 0;
- act.sa_handler = host_alarm_handler;
-
- sigaction(SIGALRM, &act, NULL);
- return 0;
-}
-
-static void unix_rearm_timer(struct qemu_alarm_timer *t)
-{
- struct itimerval itv;
- int64_t nearest_delta_ns = INT64_MAX;
- int err;
-
- assert(alarm_has_dynticks(t));
- if (!active_timers[QEMU_CLOCK_REALTIME] &&
- !active_timers[QEMU_CLOCK_VIRTUAL] &&
- !active_timers[QEMU_CLOCK_HOST])
- return;
-
- nearest_delta_ns = qemu_next_alarm_deadline();
- if (nearest_delta_ns < MIN_TIMER_REARM_NS)
- nearest_delta_ns = MIN_TIMER_REARM_NS;
-
- itv.it_interval.tv_sec = 0;
- itv.it_interval.tv_usec = 0; /* 0 for one-shot timer */
- itv.it_value.tv_sec = nearest_delta_ns / 1000000000;
- itv.it_value.tv_usec = (nearest_delta_ns % 1000000000) / 1000;
- err = setitimer(ITIMER_REAL, &itv, NULL);
- if (err) {
- perror("setitimer");
- fprintf(stderr, "Internal timer error: aborting\n");
- exit(1);
- }
-}
-
-static void unix_stop_timer(struct qemu_alarm_timer *t)
-{
- struct itimerval itv;
-
- memset(&itv, 0, sizeof(itv));
- setitimer(ITIMER_REAL, &itv, NULL);
-}
-
-#endif /* !defined(_WIN32) */
-
-
-#ifdef _WIN32
-
-static MMRESULT mm_timer;
-static unsigned mm_period;
-
-static void CALLBACK mm_alarm_handler(UINT uTimerID, UINT uMsg,
- DWORD_PTR dwUser, DWORD_PTR dw1,
- DWORD_PTR dw2)
-{
- struct qemu_alarm_timer *t = alarm_timer;
- if (!t) {
- return;
- }
- if (alarm_has_dynticks(t) || qemu_next_alarm_deadline() <= 0) {
- t->expired = alarm_has_dynticks(t);
- t->pending = 1;
- qemu_notify_event();
- }
-}
-
-static int mm_start_timer(struct qemu_alarm_timer *t)
-{
- TIMECAPS tc;
- UINT flags;
-
- memset(&tc, 0, sizeof(tc));
- timeGetDevCaps(&tc, sizeof(tc));
-
- mm_period = tc.wPeriodMin;
- timeBeginPeriod(mm_period);
-
- flags = TIME_CALLBACK_FUNCTION;
- if (alarm_has_dynticks(t)) {
- flags |= TIME_ONESHOT;
- } else {
- flags |= TIME_PERIODIC;
- }
-
- mm_timer = timeSetEvent(1, /* interval (ms) */
- mm_period, /* resolution */
- mm_alarm_handler, /* function */
- (DWORD_PTR)t, /* parameter */
- flags);
-
- if (!mm_timer) {
- fprintf(stderr, "Failed to initialize win32 alarm timer: %ld\n",
- GetLastError());
- timeEndPeriod(mm_period);
- return -1;
- }
-
- return 0;
-}
-
-static void mm_stop_timer(struct qemu_alarm_timer *t)
-{
- timeKillEvent(mm_timer);
- timeEndPeriod(mm_period);
-}
-
-static void mm_rearm_timer(struct qemu_alarm_timer *t)
-{
- int nearest_delta_ms;
-
- assert(alarm_has_dynticks(t));
- if (!active_timers[QEMU_CLOCK_REALTIME] &&
- !active_timers[QEMU_CLOCK_VIRTUAL] &&
- !active_timers[QEMU_CLOCK_HOST]) {
- return;
- }
-
- timeKillEvent(mm_timer);
-
- nearest_delta_ms = (qemu_next_alarm_deadline() + 999999) / 1000000;
- if (nearest_delta_ms < 1) {
- nearest_delta_ms = 1;
- }
- mm_timer = timeSetEvent(nearest_delta_ms,
- mm_period,
- mm_alarm_handler,
- (DWORD_PTR)t,
- TIME_ONESHOT | TIME_CALLBACK_FUNCTION);
-
- if (!mm_timer) {
- fprintf(stderr, "Failed to re-arm win32 alarm timer %ld\n",
- GetLastError());
-
- timeEndPeriod(mm_period);
- exit(1);
- }
-}
-
-static int win32_start_timer(struct qemu_alarm_timer *t)
-{
- HANDLE hTimer;
- BOOLEAN success;
-
- /* If you call ChangeTimerQueueTimer on a one-shot timer (its period
- is zero) that has already expired, the timer is not updated. Since
- creating a new timer is relatively expensive, set a bogus one-hour
- interval in the dynticks case. */
- success = CreateTimerQueueTimer(&hTimer,
- NULL,
- host_alarm_handler,
- t,
- 1,
- alarm_has_dynticks(t) ? 3600000 : 1,
- WT_EXECUTEINTIMERTHREAD);
-
- if (!success) {
- fprintf(stderr, "Failed to initialize win32 alarm timer: %ld\n",
- GetLastError());
- return -1;
- }
-
- t->timer = hTimer;
- return 0;
-}
-
-static void win32_stop_timer(struct qemu_alarm_timer *t)
-{
- HANDLE hTimer = t->timer;
-
- if (hTimer) {
- DeleteTimerQueueTimer(NULL, hTimer, NULL);
+ if (running) {
+ qemu_rearm_alarm_timer();
}
}
-static void win32_rearm_timer(struct qemu_alarm_timer *t)
+static gboolean alarm_timer_fire(gpointer opaque)
{
- HANDLE hTimer = t->timer;
- int nearest_delta_ms;
- BOOLEAN success;
-
- assert(alarm_has_dynticks(t));
- if (!active_timers[QEMU_CLOCK_REALTIME] &&
- !active_timers[QEMU_CLOCK_VIRTUAL] &&
- !active_timers[QEMU_CLOCK_HOST])
- return;
-
- nearest_delta_ms = (qemu_next_alarm_deadline() + 999999) / 1000000;
- if (nearest_delta_ms < 1) {
- nearest_delta_ms = 1;
- }
- success = ChangeTimerQueueTimer(NULL,
- hTimer,
- nearest_delta_ms,
- 3600000);
-
- if (!success) {
- fprintf(stderr, "Failed to rearm win32 alarm timer: %ld\n",
- GetLastError());
- exit(-1);
+ if (vm_running) {
+ qemu_run_timers(vm_clock);
}
-}
+ qemu_run_timers(rt_clock);
+ qemu_run_timers(host_clock);
-#endif /* _WIN32 */
+ qemu_rearm_alarm_timer();
-static void alarm_timer_on_change_state_rearm(void *opaque, int running, int reason)
-{
- if (running)
- qemu_rearm_alarm_timer((struct qemu_alarm_timer *) opaque);
+ return TRUE;
}
int init_timer_alarm(void)
{
- struct qemu_alarm_timer *t = NULL;
- int i, err = -1;
-
- for (i = 0; alarm_timers[i].name; i++) {
- t = &alarm_timers[i];
-
- err = t->start(t);
- if (!err)
- break;
- }
-
- if (err) {
- err = -ENOENT;
- goto fail;
- }
-
- /* first event is at time 0 */
- t->pending = 1;
- alarm_timer = t;
- qemu_add_vm_change_state_handler(alarm_timer_on_change_state_rearm, t);
+ g_hrtimer_add(&alarm_timer, alarm_timer_fire, NULL);
+ qemu_add_vm_change_state_handler(alarm_timer_on_change_state_rearm,
+ alarm_timer);
return 0;
-
-fail:
- return err;
}
void quit_timers(void)
{
- struct qemu_alarm_timer *t = alarm_timer;
- alarm_timer = NULL;
- t->stop(t);
}
int qemu_calculate_timeout(void)
diff --git a/qemu-timer.h b/qemu-timer.h
index 0a43469..436cf41 100644
--- a/qemu-timer.h
+++ b/qemu-timer.h
@@ -5,10 +5,7 @@
#include "notify.h"
#include <time.h>
#include <sys/time.h>
-
-#ifdef _WIN32
-#include <windows.h>
-#endif
+#include "ghrtimer.h"
/* timers */
@@ -56,7 +53,6 @@ int qemu_timer_expired(QEMUTimer *timer_head, int64_t current_time);
void qemu_run_all_timers(void);
int qemu_alarm_pending(void);
int64_t qemu_next_icount_deadline(void);
-void configure_alarms(char const *opt);
void configure_icount(const char *option);
int qemu_calculate_timeout(void);
void init_clocks(void);
@@ -92,47 +88,17 @@ static inline int64_t get_ticks_per_sec(void)
/* real time host monotonic timer */
static inline int64_t get_clock_realtime(void)
{
- struct timeval tv;
-
- gettimeofday(&tv, NULL);
- return tv.tv_sec * 1000000000LL + (tv.tv_usec * 1000);
+ return g_get_monotonic_time_ns();
}
/* Warning: don't insert tracepoints into these functions, they are
also used by simpletrace backend and tracepoints would cause
an infinite recursion! */
-#ifdef _WIN32
-extern int64_t clock_freq;
-
static inline int64_t get_clock(void)
{
- LARGE_INTEGER ti;
- QueryPerformanceCounter(&ti);
- return muldiv64(ti.QuadPart, get_ticks_per_sec(), clock_freq);
+ return g_get_monotonic_time_ns();
}
-#else
-
-extern int use_rt_clock;
-
-static inline int64_t get_clock(void)
-{
-#if defined(__linux__) || (defined(__FreeBSD__) && __FreeBSD_version >= 500000) \
- || defined(__DragonFly__) || defined(__FreeBSD_kernel__)
- if (use_rt_clock) {
- struct timespec ts;
- clock_gettime(CLOCK_MONOTONIC, &ts);
- return ts.tv_sec * 1000000000LL + ts.tv_nsec;
- } else
-#endif
- {
- /* XXX: using gettimeofday leads to problems if the date
- changes, so it should be avoided. */
- return get_clock_realtime();
- }
-}
-#endif
-
void qemu_get_timer(QEMUFile *f, QEMUTimer *ts);
void qemu_put_timer(QEMUFile *f, QEMUTimer *ts);
@@ -157,160 +123,6 @@ extern int icount_time_shift;
extern int64_t qemu_icount_bias;
int64_t cpu_get_icount(void);
-/*******************************************/
-/* host CPU ticks (if available) */
-
-#if defined(_ARCH_PPC)
-
-static inline int64_t cpu_get_real_ticks(void)
-{
- int64_t retval;
-#ifdef _ARCH_PPC64
- /* This reads timebase in one 64bit go and includes Cell workaround from:
- http://ozlabs.org/pipermail/linuxppc-dev/2006-October/027052.html
- */
- __asm__ __volatile__ ("mftb %0\n\t"
- "cmpwi %0,0\n\t"
- "beq- $-8"
- : "=r" (retval));
-#else
- /* http://ozlabs.org/pipermail/linuxppc-dev/1999-October/003889.html */
- unsigned long junk;
- __asm__ __volatile__ ("mfspr %1,269\n\t" /* mftbu */
- "mfspr %L0,268\n\t" /* mftb */
- "mfspr %0,269\n\t" /* mftbu */
- "cmpw %0,%1\n\t"
- "bne $-16"
- : "=r" (retval), "=r" (junk));
-#endif
- return retval;
-}
-
-#elif defined(__i386__)
-
-static inline int64_t cpu_get_real_ticks(void)
-{
- int64_t val;
- asm volatile ("rdtsc" : "=A" (val));
- return val;
-}
-
-#elif defined(__x86_64__)
-
-static inline int64_t cpu_get_real_ticks(void)
-{
- uint32_t low,high;
- int64_t val;
- asm volatile("rdtsc" : "=a" (low), "=d" (high));
- val = high;
- val <<= 32;
- val |= low;
- return val;
-}
-
-#elif defined(__hppa__)
-
-static inline int64_t cpu_get_real_ticks(void)
-{
- int val;
- asm volatile ("mfctl %%cr16, %0" : "=r"(val));
- return val;
-}
-
-#elif defined(__ia64)
-
-static inline int64_t cpu_get_real_ticks(void)
-{
- int64_t val;
- asm volatile ("mov %0 = ar.itc" : "=r"(val) :: "memory");
- return val;
-}
-
-#elif defined(__s390__)
-
-static inline int64_t cpu_get_real_ticks(void)
-{
- int64_t val;
- asm volatile("stck 0(%1)" : "=m" (val) : "a" (&val) : "cc");
- return val;
-}
-
-#elif defined(__sparc_v8plus__) || defined(__sparc_v8plusa__) || defined(__sparc_v9__)
-
-static inline int64_t cpu_get_real_ticks (void)
-{
-#if defined(_LP64)
- uint64_t rval;
- asm volatile("rd %%tick,%0" : "=r"(rval));
- return rval;
-#else
- union {
- uint64_t i64;
- struct {
- uint32_t high;
- uint32_t low;
- } i32;
- } rval;
- asm volatile("rd %%tick,%1; srlx %1,32,%0"
- : "=r"(rval.i32.high), "=r"(rval.i32.low));
- return rval.i64;
-#endif
-}
-
-#elif defined(__mips__) && \
- ((defined(__mips_isa_rev) && __mips_isa_rev >= 2) || defined(__linux__))
-/*
- * binutils wants to use rdhwr only on mips32r2
- * but as linux kernel emulate it, it's fine
- * to use it.
- *
- */
-#define MIPS_RDHWR(rd, value) { \
- __asm__ __volatile__ (".set push\n\t" \
- ".set mips32r2\n\t" \
- "rdhwr %0, "rd"\n\t" \
- ".set pop" \
- : "=r" (value)); \
- }
-
-static inline int64_t cpu_get_real_ticks(void)
-{
- /* On kernels >= 2.6.25 rdhwr <reg>, $2 and $3 are emulated */
- uint32_t count;
- static uint32_t cyc_per_count = 0;
-
- if (!cyc_per_count) {
- MIPS_RDHWR("$3", cyc_per_count);
- }
-
- MIPS_RDHWR("$2", count);
- return (int64_t)(count * cyc_per_count);
-}
-
-#elif defined(__alpha__)
-
-static inline int64_t cpu_get_real_ticks(void)
-{
- uint64_t cc;
- uint32_t cur, ofs;
-
- asm volatile("rpcc %0" : "=r"(cc));
- cur = cc;
- ofs = cc >> 32;
- return cur - ofs;
-}
-
-#else
-/* The host CPU doesn't have an easily accessible cycle counter.
- Just return a monotonically increasing value. This will be
- totally wrong, but hopefully better than nothing. */
-static inline int64_t cpu_get_real_ticks (void)
-{
- static int64_t ticks = 0;
- return ticks++;
-}
-#endif
-
#ifdef NEED_CPU_H
/* Deterministic execution requires that IO only be performed on the last
instruction of a TB so that interrupts take effect immediately. */
@@ -330,7 +142,7 @@ static inline int can_do_io(CPUState *env)
#ifdef CONFIG_PROFILER
static inline int64_t profile_getclock(void)
{
- return cpu_get_real_ticks();
+ return g_get_monotonic_time_ns();
}
extern int64_t qemu_time, qemu_time_start;
diff --git a/vl.c b/vl.c
index 912d546..5744ed2 100644
--- a/vl.c
+++ b/vl.c
@@ -1430,8 +1430,6 @@ void main_loop_wait(int nonblocking)
slirp_select_poll(&rfds, &wfds, &xfds, (ret < 0));
glib_select_poll(&rfds, &wfds, &xfds, (ret < 0));
- qemu_run_all_timers();
-
/* Check bottom-halves last in case any of the earlier events triggered
them. */
qemu_bh_poll();
@@ -2918,7 +2916,7 @@ int main(int argc, char **argv, char **envp)
old_param = 1;
break;
case QEMU_OPTION_clock:
- configure_alarms(optarg);
+ printf("WARNING: this option is deprecated and no longer needed.\n");
break;
case QEMU_OPTION_startdate:
configure_rtc_date_offset(optarg, 1);
--
1.7.4.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] main: add high resolution GSource based timer
2011-08-22 19:21 [Qemu-devel] [PATCH 1/2] main: add high resolution GSource based timer Anthony Liguori
2011-08-22 19:21 ` [Qemu-devel] [PATCH 2/2] [RFC] time: refactor QEMU timer to use GHRTimer Anthony Liguori
@ 2011-08-22 19:26 ` Anthony Liguori
1 sibling, 0 replies; 13+ messages in thread
From: Anthony Liguori @ 2011-08-22 19:26 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Paolo Bonzini, qemu-devel, Stefan Hajnoczi, Jan Kiszka
On 08/22/2011 02:21 PM, Anthony Liguori wrote:
> This was originally written by Paolo Bonzini.
>
> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com>
FYI, all of these glib integration patches are available at:
http://repo.or.cz/w/qemu/aliguori.git/shortlog/refs/heads/glib-main
But be forewarned, they may eat your lunch and steal your stapler.
Regards,
Anthony Liguori
> ---
> Makefile.objs | 1 +
> configure | 4 +-
> ghrtimer.c | 334 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> ghrtimer.h | 49 +++++++++
> 4 files changed, 386 insertions(+), 2 deletions(-)
> create mode 100644 ghrtimer.c
> create mode 100644 ghrtimer.h
>
> diff --git a/Makefile.objs b/Makefile.objs
> index d1f3e5d..e4267ed 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -9,6 +9,7 @@ qobject-obj-y += qerror.o error.o
> oslib-obj-y = osdep.o
> oslib-obj-$(CONFIG_WIN32) += oslib-win32.o qemu-thread-win32.o
> oslib-obj-$(CONFIG_POSIX) += oslib-posix.o qemu-thread-posix.o
> +oslib-obj-y += ghrtimer.o
>
> #######################################################################
> # coroutines
> diff --git a/configure b/configure
> index 8bbd694..6339c45 100755
> --- a/configure
> +++ b/configure
> @@ -1843,8 +1843,8 @@ fi
> ##########################################
> # glib support probe
> if $pkg_config --modversion gthread-2.0> /dev/null 2>&1 ; then
> - glib_cflags=`$pkg_config --cflags gthread-2.0 2>/dev/null`
> - glib_libs=`$pkg_config --libs gthread-2.0 2>/dev/null`
> + glib_cflags=`$pkg_config --cflags gthread-2.0 gobject-2.0 2>/dev/null`
> + glib_libs=`$pkg_config --libs gthread-2.0 gobject-2.0 2>/dev/null`
> LIBS="$glib_libs $LIBS"
> libs_qga="$glib_libs $libs_qga"
> else
> diff --git a/ghrtimer.c b/ghrtimer.c
> new file mode 100644
> index 0000000..1e7e1c2
> --- /dev/null
> +++ b/ghrtimer.c
> @@ -0,0 +1,334 @@
> +/*
> + * timerfd GSource wrapper
> + *
> + * Copyright IBM, Corp. 2011
> + * Copyright Red Hat, Inc. 2011
> + *
> + * Authors:
> + * Anthony Liguori<aliguori@us.ibm.com>
> + * Paolo Bonzini<pbonzini@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
> + * See the COPYING.LIB file in the top-level directory.
> + *
> + */
> +
> +#include "config-host.h"
> +#include<stdlib.h>
> +#include "ghrtimer.h"
> +#include<glib-object.h>
> +#include<fcntl.h>
> +#include<unistd.h>
> +
> +#ifdef CONFIG_TIMERFD
> +#include<sys/timerfd.h>
> +#endif
> +
> +struct _GHRTimer {
> + GSource source;
> + gint64 deadline;
> + GPollFD poll;
> + char pending;
> +};
> +
> +#define MIN_TIMER_REARM_NS 250000
> +
> +#if GLIB_MAJOR_VERSION == 2&& GLIB_MINOR_VERSION<= 26
> +static inline guint64 muldiv64(guint64 a, guint32 b, guint32 c)
> +{
> + guint64 rl = (a& 0xffffffff) * (guint64)b;
> + guint64 rh = (a>> 32) * (guint64)b + (rl>> 32);
> + rl&= 0xffffffff;
> + return ((rh / c)<< 32) | ((((rh % c)<< 32) + rl) / c);
> +}
> +
> +gint64
> +g_get_monotonic_time_ns (void)
> +{
> +#if defined(__linux__) || (defined(__FreeBSD__)&& __FreeBSD_version>= 500000) \
> + || defined(__DragonFly__) || defined(__FreeBSD_kernel__) \
> + || defined(__OpenBSD__)
> + struct timespec ts;
> + clock_gettime(CLOCK_MONOTONIC,&ts);
> + return ts.tv_sec * 1000000000LL + ts.tv_nsec;
> +
> +#elif defined _WIN32
> + LARGE_INTEGER ti;
> + static LARGE_INTEGER freq;
> + if (freq.QuadPart == 0) {
> + QueryPerformanceFrequency(&freq);
> + }
> + QueryPerformanceCounter(&ti);
> + return muldiv64(ti.QuadPart, 1000000000, freq.QuadPart);
> +
> +#else
> +#ifdef CONFIG_TIMERFD
> +#error configuration problem, timerfd uses CLOCK_MONOTONIC
> +#endif
> + GTimeVal tv;
> + g_get_current_time (&tv);
> + return ((guint64) tv.tv_sec) * 1000000000 + tv.tv_usec * 1000;
> +#endif
> +}
> +
> +gint64
> +g_source_get_time_ns (GSource *source)
> +{
> + return g_get_monotonic_time_ns ();
> +}
> +
> +#else
> +gint64
> +g_get_monotonic_time_ns (void)
> +{
> + return g_get_monotonic_time () * 1000;
> +}
> +
> +gint64
> +g_source_get_time_ns (GSource *source)
> +{
> + return g_source_get_time (source) * 1000;
> +}
> +#endif
> +
> +gboolean
> +g_hrtimer_pending (GHRTimer *timer)
> +{
> + return timer->pending;
> +}
> +
> +void
> +g_hrtimer_rearm (GHRTimer *timer,
> + gint64 us)
> +{
> + return g_hrtimer_rearm_ns (timer, us * 1000);
> +}
> +
> +void
> +g_hrtimer_rearm_ns (GHRTimer *timer,
> + gint64 ns)
> +{
> + gint64 time = g_get_monotonic_time_ns ();
> + timer->deadline = ns;
> + if (ns< time) {
> + timer->pending = TRUE;
> + return;
> + }
> +
> + timer->pending = FALSE;
> + if (ns == G_HRTIMER_QUIESCE) {
> + return;
> + }
> +
> + if (ns - time< MIN_TIMER_REARM_NS) {
> + timer->deadline = ns = time + MIN_TIMER_REARM_NS;
> + }
> +#ifdef CONFIG_TIMERFD
> + if (timer->poll.fd != -1) {
> + struct itimerspec new = {
> + .it_interval = { 0, 0 },
> + .it_value = { ns / 1000000000, ns % 1000000000 }
> + };
> + timerfd_settime(timer->poll.fd, TFD_TIMER_ABSTIME,&new, NULL);
> + }
> +#endif
> +}
> +
> +static gboolean
> +g_hrtimer_prepare (GSource *source,
> + gint *timeout)
> +{
> + GHRTimer *timer = (GHRTimer *) source;
> +
> + if (timer->deadline == G_HRTIMER_QUIESCE) {
> + g_assert (!timer->pending);
> + *timeout = -1;
> + return FALSE;
> + }
> +
> + if (timer->poll.fd == -1) {
> + gint64 timeout_ns = timer->deadline - g_get_monotonic_time_ns ();
> + if (timeout_ns< 0) {
> + *timeout = 0;
> + timer->pending = TRUE;
> + } else {
> + *timeout = timeout_ns / 1000000;
> + }
> + } else {
> + *timeout = -1;
> +#ifndef CONFIG_TIMERFD
> + abort ();
> +#endif
> + }
> + return timer->pending;
> +}
> +
> +static gboolean
> +g_hrtimer_check (GSource *source)
> +{
> + GHRTimer *timer = (GHRTimer *) source;
> +
> + if (timer->deadline == G_HRTIMER_QUIESCE) {
> + g_assert (!timer->pending);
> + return FALSE;
> + }
> +
> + if (timer->poll.fd == -1) {
> + timer->pending |= (timer->deadline<= g_source_get_time_ns (source));
> + } else {
> + long long overrun;
> + timer->pending |= (timer->poll.revents& G_IO_IN) != 0;
> + if (timer->pending) {
> + if (read (timer->poll.fd, (char *)&overrun, sizeof (overrun))) {
> + /* do nothing */
> + }
> + }
> +#ifndef CONFIG_TIMERFD
> + abort ();
> +#endif
> + }
> +
> + return timer->pending;
> +}
> +
> +static gboolean
> +g_hrtimer_dispatch (GSource *source,
> + GSourceFunc callback,
> + gpointer user_data)
> +{
> + GHRTimer *timer = (GHRTimer *) source;
> +
> + if (!callback) {
> + g_warning ("Timer source dispatched without callback\n"
> + "You must call g_source_set_callback().");
> + return TRUE;
> + }
> +
> + timer->pending = FALSE;
> + timer->deadline = G_HRTIMER_QUIESCE;
> + if (user_data == NULL)
> + user_data = timer;
> + callback (user_data);
> + return TRUE;
> +}
> +
> +static void
> +g_hrtimer_finalize (GSource *source)
> +{
> + GHRTimer *timer = (GHRTimer *) source;
> +
> + if (timer->poll.fd != -1) {
> + close (timer->poll.fd);
> +#ifndef CONFIG_TIMERFD
> + abort ();
> +#endif
> + }
> +}
> +
> +static void
> +g_hrtimer_closure_callback (gpointer data)
> +{
> + GClosure *closure = data;
> + g_closure_invoke (closure, NULL, 0, NULL, NULL);
> +}
> +
> +static GSourceFuncs hrtimer_source_funcs = {
> + g_hrtimer_prepare,
> + g_hrtimer_check,
> + g_hrtimer_dispatch,
> + g_hrtimer_finalize,
> + (GSourceFunc) g_hrtimer_closure_callback,
> + (gpointer) g_cclosure_marshal_VOID__VOID
> +};
> +
> +GHRTimer *
> +g_hrtimer_new (void)
> +{
> + GHRTimer *timer;
> +
> + timer = (GHRTimer *) g_source_new (&hrtimer_source_funcs,
> + sizeof (GHRTimer));
> +
> +#ifdef CONFIG_TIMERFD
> + timer->poll.fd = timerfd_create (CLOCK_MONOTONIC, 0);
> + if (timer->poll.fd != -1) {
> + fcntl(timer->poll.fd, F_SETFD, fcntl (timer->poll.fd, F_GETFD) | FD_CLOEXEC);
> + fcntl(timer->poll.fd, F_SETFL, fcntl (timer->poll.fd, F_GETFL) | O_NONBLOCK);
> + timer->poll.events = G_IO_IN;
> + g_source_add_poll (&timer->source,&timer->poll);
> + }
> +#else
> + timer->poll.fd = -1;
> +#endif
> + timer->deadline = G_HRTIMER_QUIESCE;
> + return timer;
> +}
> +guint
> +g_hrtimer_add (GHRTimer **timer,
> + GSourceFunc func,
> + gpointer user_data)
> +{
> + return g_hrtimer_add_full (G_PRIORITY_DEFAULT, timer, func, user_data, NULL);
> +}
> +
> +guint
> +g_hrtimer_add_full (gint priority,
> + GHRTimer **timer,
> + GSourceFunc func,
> + gpointer user_data,
> + GDestroyNotify notify)
> +{
> + GHRTimer *hrtimer;
> + guint id;
> +
> + hrtimer = g_hrtimer_new ();
> + if (priority != G_PRIORITY_DEFAULT)
> + g_source_set_priority (&hrtimer->source, priority);
> +
> + g_source_set_callback (&hrtimer->source, (GSourceFunc) func,
> + user_data, notify);
> + id = g_source_attach (&hrtimer->source, NULL);
> +
> + *timer = hrtimer;
> + return id;
> +}
> +
> +#ifdef MAIN
> +#include<stdio.h>
> +
> +static int i = 3;
> +static GMainLoop *loop;
> +
> +void
> +rearm_timer (GHRTimer *timer)
> +{
> + printf (".");
> + fflush (stdout);
> + g_hrtimer_rearm_ns (timer, g_get_monotonic_time_ns () + 1000000000);
> +}
> +
> +void
> +hrtimer_callback (gpointer user_data)
> +{
> + GHRTimer *timer = user_data;
> +
> + if (--i == 0) {
> + printf ("\n");
> + fflush (stdout);
> + g_main_loop_quit (loop);
> + } else {
> + rearm_timer (timer);
> + }
> +}
> +
> +int main()
> +{
> + GHRTimer *timer;
> + loop = g_main_loop_new (NULL, FALSE);
> + g_hrtimer_add (&timer, (GSourceFunc) hrtimer_callback, NULL);
> + rearm_timer (timer);
> + g_main_loop_run (loop);
> + g_source_unref ((GSource *) timer);
> +}
> +#endif
> +
> diff --git a/ghrtimer.h b/ghrtimer.h
> new file mode 100644
> index 0000000..2cf6961
> --- /dev/null
> +++ b/ghrtimer.h
> @@ -0,0 +1,49 @@
> +/*
> + * timerfd GSource wrapper
> + *
> + * Copyright IBM, Corp. 2011
> + * Copyright Red Hat, Inc. 2011
> + *
> + * Authors:
> + * Anthony Liguori<aliguori@us.ibm.com>
> + * Paolo Bonzini<pbonzini@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later
> + * See the COPYING.LIB file in the top-level directory.
> + *
> + */
> +
> +#ifndef G_HRTIMER_H
> +#define G_HRTIMER_H 1
> +
> +#include<glib.h>
> +
> +#define G_HRTIMER_QUIESCE ((gint64) 0x7FFFFFFFFFFFFFFF)
> +
> +typedef struct _GHRTimer GHRTimer;
> +
> +gint64 g_get_monotonic_time_ns (void);
> +
> +gint64 g_source_get_time_ns (GSource *source);
> +
> +GHRTimer *g_hrtimer_new (void);
> +
> +gboolean g_hrtimer_pending (GHRTimer *timer);
> +
> +void g_hrtimer_rearm (GHRTimer *timer,
> + gint64 usec);
> +
> +void g_hrtimer_rearm_ns (GHRTimer *timer,
> + gint64 nsec);
> +
> +guint g_hrtimer_add (GHRTimer **timer,
> + GSourceFunc callback,
> + gpointer user_data);
> +
> +guint g_hrtimer_add_full (int priority,
> + GHRTimer **timer,
> + GSourceFunc callback,
> + gpointer user_data,
> + GDestroyNotify notify);
> +
> +#endif
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] [RFC] time: refactor QEMU timer to use GHRTimer
2011-08-22 19:21 ` [Qemu-devel] [PATCH 2/2] [RFC] time: refactor QEMU timer to use GHRTimer Anthony Liguori
@ 2011-08-22 20:28 ` Jan Kiszka
2011-08-22 20:36 ` Anthony Liguori
2011-08-23 8:12 ` Paolo Bonzini
2011-08-23 7:43 ` Paolo Bonzini
1 sibling, 2 replies; 13+ messages in thread
From: Jan Kiszka @ 2011-08-22 20:28 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Paolo Bonzini, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 3851 bytes --]
On 2011-08-22 21:21, Anthony Liguori wrote:
> This replaces all of the QEMU timer code with GHRTimer, dramatically simplifying
> time keeping in QEMU while making it possible to use QEMUTimer code outside of
> the main loop. The later is critical to building unit tests.
>
> This is an RFC because I'm sure this breaks things as it changes things. QEMU
> time keeping is quite a mess today. Here's what we do today:
>
> 1) We have three clocks:
> a) the real time clock, based on system time, not monotonic
> b) the host clock, based on the real time clock, monotonic by detecting
> movements backward in time
> c) the vm clock, based on real time clock but may start/stop with the guest
Not quite correct. We have:
- QEMU_CLOCK_REALTIME: Based on monotonic source *if* the host
supports it (there were probably once some stone-old Linuxes or
BSDs), otherwise based on gettimeofday, i.e. non-monotonic. Always
monotonic on Windows.
- QEMU_CLOCK_VIRTUAL: Without -icount, same as above, but stops when
the guest is stopped. The offset to compensate for stopped
times is based on TSC, not sure why. With -icount, things get more
complicated, Paolo had some nice explanations for the details.
- QEMU_CLOCK_HOST: That's the one always based on the host's system
time (CLOCK_REALTIME)
+ it takes potentially configured offsets into acount
+ users of that clock can register callbacks on time warps into the
past (to adjust pending timers)
>
> 2) A "cpu ticks" clock that uses platform specific mechanisms (inline asm)
>
> 3) Various clock source implementations that may use a periodic timer or a
> a dynamic time source. We have different implementations for different
> platforms
>
> 4) Time events are delivered via SIGALRM which means we end up getting EINTRs
> very often in QEMU. This is fairly annoying. Signals also race against
> select leading to a very ugly set of work arounds involving writing data to
> pipes. This is the sort of stuff in Unix programming that I wish I never had
> to learn about and am very eager to eliminate in QEMU :-)
>
> (2) is just plain broken. In modern operating systems, gettimeofday() is
> optimized CPU instructions when they can be used safely. Often they can't be
> used safely and we ignore that in QEMU. For instance, on x86, RDTSC races with
> the scheduler (not to mention that the TSC is infamously unstable across cores).
> The kernel does the right thing here and provides the fastest method that's
> correct.
I basically agree. Likely, these optimizations date back to the days
Linux had no fast gettimeofday syscalls. Not sure what the state on
other UNIXes is, but it's likely not worth keeping these optimizations.
Let's drop that one first and separately.
>
> (1.a) seems like a bug more than a feature. I don't see a lot of disadvantages
> to using a monotonic time source.
>
> (1.b) is a bit naive in its current form. Modern kernels export a truly
> monotonic time source which has a reliable frequency. Even though (1.b) detects
> backwards jumps, it doesn't do anything about large forward jumps which can also
> be problematic.
These two assessments are partly just wrong, partly fail to see the real
use case. QEMU_CLOCK_HOST serves the very valid scenarios where a guest
clock shall be kept synchronized on the host time, also following its
jumps accordingly without stalling timers.
I haven't looked at the timer parts yet, but the clock assessments
indicate that some more careful thoughts are required. Strong NACK for
breaking QEMU_CLOCK_HOST in any case.
I do agree that there is likely room for cleanups, specifically when
demanding a sane POSIX/WIN32 host and/or reusing CLOCK_MONOTONIC
abstractions.
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] [RFC] time: refactor QEMU timer to use GHRTimer
2011-08-22 20:28 ` Jan Kiszka
@ 2011-08-22 20:36 ` Anthony Liguori
2011-08-22 20:49 ` Jan Kiszka
2011-08-23 8:12 ` Paolo Bonzini
1 sibling, 1 reply; 13+ messages in thread
From: Anthony Liguori @ 2011-08-22 20:36 UTC (permalink / raw)
To: Jan Kiszka; +Cc: Paolo Bonzini, Anthony Liguori, qemu-devel
On 08/22/2011 03:28 PM, Jan Kiszka wrote:
> On 2011-08-22 21:21, Anthony Liguori wrote:
>> This replaces all of the QEMU timer code with GHRTimer, dramatically simplifying
>> time keeping in QEMU while making it possible to use QEMUTimer code outside of
>> the main loop. The later is critical to building unit tests.
>>
>> This is an RFC because I'm sure this breaks things as it changes things. QEMU
>> time keeping is quite a mess today. Here's what we do today:
>>
>> 1) We have three clocks:
>> a) the real time clock, based on system time, not monotonic
>> b) the host clock, based on the real time clock, monotonic by detecting
>> movements backward in time
>> c) the vm clock, based on real time clock but may start/stop with the guest
>
> Not quite correct. We have:
>
> - QEMU_CLOCK_REALTIME: Based on monotonic source *if* the host
> supports it (there were probably once some stone-old Linuxes or
> BSDs), otherwise based on gettimeofday, i.e. non-monotonic. Always
> monotonic on Windows.
The only clock on Linux that is truly monotonic is CLOCK_MONOTONIC_RAW
which is very new (2.6.28+). CLOCK_MONOTONIC is not actually monotonic
as it's subject to adjustments.
> - QEMU_CLOCK_VIRTUAL: Without -icount, same as above, but stops when
> the guest is stopped. The offset to compensate for stopped
> times is based on TSC, not sure why. With -icount, things get more
> complicated, Paolo had some nice explanations for the details.
>
> - QEMU_CLOCK_HOST: That's the one always based on the host's system
> time (CLOCK_REALTIME)
> + it takes potentially configured offsets into acount
> + users of that clock can register callbacks on time warps into the
> past (to adjust pending timers)
Right, my assertion is that time warps are a bug as far as QEMU is
concerned. Is there any reason why the guest should care at all about
time warping in the host?
>> 2) A "cpu ticks" clock that uses platform specific mechanisms (inline asm)
>>
>> 3) Various clock source implementations that may use a periodic timer or a
>> a dynamic time source. We have different implementations for different
>> platforms
>>
>> 4) Time events are delivered via SIGALRM which means we end up getting EINTRs
>> very often in QEMU. This is fairly annoying. Signals also race against
>> select leading to a very ugly set of work arounds involving writing data to
>> pipes. This is the sort of stuff in Unix programming that I wish I never had
>> to learn about and am very eager to eliminate in QEMU :-)
>>
>> (2) is just plain broken. In modern operating systems, gettimeofday() is
>> optimized CPU instructions when they can be used safely. Often they can't be
>> used safely and we ignore that in QEMU. For instance, on x86, RDTSC races with
>> the scheduler (not to mention that the TSC is infamously unstable across cores).
>> The kernel does the right thing here and provides the fastest method that's
>> correct.
>
> I basically agree. Likely, these optimizations date back to the days
> Linux had no fast gettimeofday syscalls. Not sure what the state on
> other UNIXes is, but it's likely not worth keeping these optimizations.
> Let's drop that one first and separately.
>
>>
>> (1.a) seems like a bug more than a feature. I don't see a lot of disadvantages
>> to using a monotonic time source.
>>
>> (1.b) is a bit naive in its current form. Modern kernels export a truly
>> monotonic time source which has a reliable frequency. Even though (1.b) detects
>> backwards jumps, it doesn't do anything about large forward jumps which can also
>> be problematic.
>
> These two assessments are partly just wrong, partly fail to see the real
> use case. QEMU_CLOCK_HOST serves the very valid scenarios where a guest
> clock shall be kept synchronized on the host time, also following its
> jumps accordingly without stalling timers.
The only reason we see jumps at all is because we're using
CLOCK_MONOTONIC or CLOCK_REALTIME. If we used CLOCK_MONOTONIC_RAW, we
don't see any jumps at all.
Regards,
Anthony Liguori
> I haven't looked at the timer parts yet, but the clock assessments
> indicate that some more careful thoughts are required. Strong NACK for
> breaking QEMU_CLOCK_HOST in any case.
>
> I do agree that there is likely room for cleanups, specifically when
> demanding a sane POSIX/WIN32 host and/or reusing CLOCK_MONOTONIC
> abstractions.
>
> Jan
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] [RFC] time: refactor QEMU timer to use GHRTimer
2011-08-22 20:36 ` Anthony Liguori
@ 2011-08-22 20:49 ` Jan Kiszka
2011-08-22 21:55 ` Anthony Liguori
0 siblings, 1 reply; 13+ messages in thread
From: Jan Kiszka @ 2011-08-22 20:49 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Paolo Bonzini, Anthony Liguori, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 5075 bytes --]
On 2011-08-22 22:36, Anthony Liguori wrote:
> On 08/22/2011 03:28 PM, Jan Kiszka wrote:
>> On 2011-08-22 21:21, Anthony Liguori wrote:
>>> This replaces all of the QEMU timer code with GHRTimer, dramatically
>>> simplifying
>>> time keeping in QEMU while making it possible to use QEMUTimer code
>>> outside of
>>> the main loop. The later is critical to building unit tests.
>>>
>>> This is an RFC because I'm sure this breaks things as it changes
>>> things. QEMU
>>> time keeping is quite a mess today. Here's what we do today:
>>>
>>> 1) We have three clocks:
>>> a) the real time clock, based on system time, not monotonic
>>> b) the host clock, based on the real time clock, monotonic by
>>> detecting
>>> movements backward in time
>>> c) the vm clock, based on real time clock but may start/stop with
>>> the guest
>>
>> Not quite correct. We have:
>>
>> - QEMU_CLOCK_REALTIME: Based on monotonic source *if* the host
>> supports it (there were probably once some stone-old Linuxes or
>> BSDs), otherwise based on gettimeofday, i.e. non-monotonic. Always
>> monotonic on Windows.
>
> The only clock on Linux that is truly monotonic is CLOCK_MONOTONIC_RAW
> which is very new (2.6.28+). CLOCK_MONOTONIC is not actually monotonic
> as it's subject to adjustments.
CLOCK_MONOTONIC may be subject to frequency tuning while
CLOCK_MONOTONIC_RAW is not. That does not and must not (for POSIX
compliance) make the former non-monotonic.
>
>> - QEMU_CLOCK_VIRTUAL: Without -icount, same as above, but stops when
>> the guest is stopped. The offset to compensate for stopped
>> times is based on TSC, not sure why. With -icount, things get more
>> complicated, Paolo had some nice explanations for the details.
>>
>> - QEMU_CLOCK_HOST: That's the one always based on the host's system
>> time (CLOCK_REALTIME)
>> + it takes potentially configured offsets into acount
>> + users of that clock can register callbacks on time warps into the
>> past (to adjust pending timers)
>
> Right, my assertion is that time warps are a bug as far as QEMU is
> concerned. Is there any reason why the guest should care at all about
> time warping in the host?
Yes, to synchronize on an accurate host clock source without the need to
add NTP-like services (or kvmclock) to the guest. So the warps are
conceptionally valid, though still not very welcome.
>
>>> 2) A "cpu ticks" clock that uses platform specific mechanisms (inline
>>> asm)
>>>
>>> 3) Various clock source implementations that may use a periodic timer
>>> or a
>>> a dynamic time source. We have different implementations for
>>> different
>>> platforms
>>>
>>> 4) Time events are delivered via SIGALRM which means we end up
>>> getting EINTRs
>>> very often in QEMU. This is fairly annoying. Signals also race
>>> against
>>> select leading to a very ugly set of work arounds involving
>>> writing data to
>>> pipes. This is the sort of stuff in Unix programming that I wish
>>> I never had
>>> to learn about and am very eager to eliminate in QEMU :-)
>>>
>>> (2) is just plain broken. In modern operating systems,
>>> gettimeofday() is
>>> optimized CPU instructions when they can be used safely. Often they
>>> can't be
>>> used safely and we ignore that in QEMU. For instance, on x86, RDTSC
>>> races with
>>> the scheduler (not to mention that the TSC is infamously unstable
>>> across cores).
>>> The kernel does the right thing here and provides the fastest method
>>> that's
>>> correct.
>>
>> I basically agree. Likely, these optimizations date back to the days
>> Linux had no fast gettimeofday syscalls. Not sure what the state on
>> other UNIXes is, but it's likely not worth keeping these optimizations.
>> Let's drop that one first and separately.
>>
>>>
>>> (1.a) seems like a bug more than a feature. I don't see a lot of
>>> disadvantages
>>> to using a monotonic time source.
>>>
>>> (1.b) is a bit naive in its current form. Modern kernels export a truly
>>> monotonic time source which has a reliable frequency. Even though
>>> (1.b) detects
>>> backwards jumps, it doesn't do anything about large forward jumps
>>> which can also
>>> be problematic.
>>
>> These two assessments are partly just wrong, partly fail to see the real
>> use case. QEMU_CLOCK_HOST serves the very valid scenarios where a guest
>> clock shall be kept synchronized on the host time, also following its
>> jumps accordingly without stalling timers.
>
> The only reason we see jumps at all is because we're using
> CLOCK_MONOTONIC or CLOCK_REALTIME. If we used CLOCK_MONOTONIC_RAW, we
> don't see any jumps at all.
CLOCK_MONOTONIC will not jump backward as well, so is perfectly fine and
better portable. Backward jumps cannot be avoided when using a host
system clock that is subject to follow a more accurate external source.
But having such source for RTC emulation e.g. is a useful feature.
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] [RFC] time: refactor QEMU timer to use GHRTimer
2011-08-22 20:49 ` Jan Kiszka
@ 2011-08-22 21:55 ` Anthony Liguori
2011-08-22 23:48 ` Jan Kiszka
0 siblings, 1 reply; 13+ messages in thread
From: Anthony Liguori @ 2011-08-22 21:55 UTC (permalink / raw)
To: Jan Kiszka; +Cc: Paolo Bonzini, qemu-devel
On 08/22/2011 03:49 PM, Jan Kiszka wrote:
> On 2011-08-22 22:36, Anthony Liguori wrote:
>> On 08/22/2011 03:28 PM, Jan Kiszka wrote:
>>> On 2011-08-22 21:21, Anthony Liguori wrote:
>>>> This replaces all of the QEMU timer code with GHRTimer, dramatically
>>>> simplifying
>>>> time keeping in QEMU while making it possible to use QEMUTimer code
>>>> outside of
>>>> the main loop. The later is critical to building unit tests.
>>>>
>>>> This is an RFC because I'm sure this breaks things as it changes
>>>> things. QEMU
>>>> time keeping is quite a mess today. Here's what we do today:
>>>>
>>>> 1) We have three clocks:
>>>> a) the real time clock, based on system time, not monotonic
>>>> b) the host clock, based on the real time clock, monotonic by
>>>> detecting
>>>> movements backward in time
>>>> c) the vm clock, based on real time clock but may start/stop with
>>>> the guest
>>>
>>> Not quite correct. We have:
>>>
>>> - QEMU_CLOCK_REALTIME: Based on monotonic source *if* the host
>>> supports it (there were probably once some stone-old Linuxes or
>>> BSDs), otherwise based on gettimeofday, i.e. non-monotonic. Always
>>> monotonic on Windows.
>>
>> The only clock on Linux that is truly monotonic is CLOCK_MONOTONIC_RAW
>> which is very new (2.6.28+). CLOCK_MONOTONIC is not actually monotonic
>> as it's subject to adjustments.
>
> CLOCK_MONOTONIC may be subject to frequency tuning while
> CLOCK_MONOTONIC_RAW is not. That does not and must not (for POSIX
> compliance) make the former non-monotonic.
Yes, you are right. I got myself confused.
>>> These two assessments are partly just wrong, partly fail to see the real
>>> use case. QEMU_CLOCK_HOST serves the very valid scenarios where a guest
>>> clock shall be kept synchronized on the host time, also following its
>>> jumps accordingly without stalling timers.
>>
>> The only reason we see jumps at all is because we're using
>> CLOCK_MONOTONIC or CLOCK_REALTIME. If we used CLOCK_MONOTONIC_RAW, we
>> don't see any jumps at all.
>
> CLOCK_MONOTONIC will not jump backward as well, so is perfectly fine and
> better portable. Backward jumps cannot be avoided when using a host
> system clock that is subject to follow a more accurate external source.
> But having such source for RTC emulation e.g. is a useful feature.
I think its of limited utility. The RTC isn't universally used for time
keeping. There's also no guarantee that the guest isn't going to be
upset by this.
I think a better approach is to simply have a verb in qemu-ga to set/get
the guest time. That let's you implement clock adjustment without
having to worry about NTP. I'm happy to add that as part of this series.
I don't think messing around with this stuff belongs in the QEMU clock
layer though.
Regards,
Anthony Liguori
> Jan
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] [RFC] time: refactor QEMU timer to use GHRTimer
2011-08-22 21:55 ` Anthony Liguori
@ 2011-08-22 23:48 ` Jan Kiszka
0 siblings, 0 replies; 13+ messages in thread
From: Jan Kiszka @ 2011-08-22 23:48 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Paolo Bonzini, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1865 bytes --]
On 2011-08-22 23:55, Anthony Liguori wrote:
>>>> These two assessments are partly just wrong, partly fail to see the
>>>> real
>>>> use case. QEMU_CLOCK_HOST serves the very valid scenarios where a guest
>>>> clock shall be kept synchronized on the host time, also following its
>>>> jumps accordingly without stalling timers.
>>>
>>> The only reason we see jumps at all is because we're using
>>> CLOCK_MONOTONIC or CLOCK_REALTIME. If we used CLOCK_MONOTONIC_RAW, we
>>> don't see any jumps at all.
>>
>> CLOCK_MONOTONIC will not jump backward as well, so is perfectly fine and
>> better portable. Backward jumps cannot be avoided when using a host
>> system clock that is subject to follow a more accurate external source.
>> But having such source for RTC emulation e.g. is a useful feature.
>
> I think its of limited utility. The RTC isn't universally used for time
> keeping. There's also no guarantee that the guest isn't going to be
> upset by this.
That's why it's configurable. On the other hand, most guests are
well-prepared to deal with the RTC and the host runtime clock source not
being in sync. That's why we had no bug reports after making this mode
default.
>
> I think a better approach is to simply have a verb in qemu-ga to set/get
> the guest time. That let's you implement clock adjustment without
> having to worry about NTP. I'm happy to add that as part of this series.
That's not that easy in legacy guests, even more when they aren't Linux.
>
> I don't think messing around with this stuff belongs in the QEMU clock
> layer though.
It's really not messing, the impact is almost minimal.
Let's focus on the real big pieces first and then see what CLOCK_HOST
actually contributes. That also means you should split up you patches a
bit more, already for better bisectability.
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] [RFC] time: refactor QEMU timer to use GHRTimer
2011-08-22 19:21 ` [Qemu-devel] [PATCH 2/2] [RFC] time: refactor QEMU timer to use GHRTimer Anthony Liguori
2011-08-22 20:28 ` Jan Kiszka
@ 2011-08-23 7:43 ` Paolo Bonzini
2011-08-23 12:33 ` Anthony Liguori
1 sibling, 1 reply; 13+ messages in thread
From: Paolo Bonzini @ 2011-08-23 7:43 UTC (permalink / raw)
To: Anthony Liguori; +Cc: qemu-devel, Jan Kiszka
On 08/22/2011 09:21 PM, Anthony Liguori wrote:
> - ticks = cpu_get_real_ticks();
> - if (timers_state.cpu_ticks_prev > ticks) {
> - /* Note: non increasing ticks may happen if the host uses
> - software suspend */
> - timers_state.cpu_ticks_offset += timers_state.cpu_ticks_prev - ticks;
> - }
> + ticks = get_clock();
>
> [...]
>
> -static inline int64_t cpu_get_real_ticks(void)
> -{
> - int64_t val;
> - asm volatile ("rdtsc" : "=A" (val));
> - return val;
> -}
> -
cpu_get_ticks is used also to emulate the guest TSC, are you sure you
want to change that uniformly to a 1 GHz rate?
I had some more cleanups in this area, I'll try to get them tested and
submitted but I have little time for this right now.
Paolo
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] [RFC] time: refactor QEMU timer to use GHRTimer
2011-08-22 20:28 ` Jan Kiszka
2011-08-22 20:36 ` Anthony Liguori
@ 2011-08-23 8:12 ` Paolo Bonzini
2011-08-23 9:07 ` Edgar E. Iglesias
1 sibling, 1 reply; 13+ messages in thread
From: Paolo Bonzini @ 2011-08-23 8:12 UTC (permalink / raw)
To: Jan Kiszka; +Cc: Anthony Liguori, qemu-devel
On 08/22/2011 10:28 PM, Jan Kiszka wrote:
> - QEMU_CLOCK_VIRTUAL: Without -icount, same as above, but stops when
> the guest is stopped. The offset to compensate for stopped
> times is based on TSC, not sure why. With -icount, things get more
> complicated, Paolo had some nice explanations for the details.
The TSC is actually out of the picture. However, it is easy to get
confused, because the same code handles stopping both the vm_clock and
the TSC when the guest is paused. cpu_get_ticks handles the TSC,
cpu_get_clock handles the clock:
Removing some "uninteresting" details, you have:
/* return the host CPU cycle counter and handle stop/restart */
int64_t cpu_get_ticks(void)
{
if (!vm_running) {
return timers_state.cpu_ticks_offset;
} else {
int64_t ticks;
ticks = cpu_get_real_ticks();
return ticks + timers_state.cpu_ticks_offset;
}
}
/* return the host CPU monotonic timer and handle stop/restart */
static int64_t cpu_get_clock(void)
{
int64_t ti;
if (!vm_running) {
return timers_state.cpu_clock_offset;
} else {
ti = get_clock();
return ti + timers_state.cpu_clock_offset;
}
}
which use the same algorithm but with different base clocks (TSC vs.
CLOCK_MONOTONIC).
With -icount, things get indeed more complicated. I'll cover only the
iothread case since all my attempts at understanding the non-iothread
case failed. :) The "-icount N" vm_clock has nanosecond resolution just
like the normal vm_clock, but those are not real nanoseconds. While the
CPU is running, each instruction increments the vm_clock by 2^N
nanoseconds (yes, this is completely bollocks for SMP. :). When the CPU
is not running, instead, the vm_clock follows CLOCK_MONOTONIC; the
rationale is there in qemu-timer.c.
"-icount auto" is the same, except that we try to keep vm_clock roughly
the same as CLOCK_MONOTONIC by oscillating the clock frequency between
"-icount N" values that are more representative of the actual execution
frequency.
On top of this, the VM has to do two things in icount mode. The first is
to stop execution at the next vm_clock deadline, which means breaking
translation blocks after executing the appropriate number of
instructions; this is quite obvious. The second is to stop execution of
a TB after any MMIO instruction, in order to recalculate deadlines if
necessary. The latter is responsible for most of the icount black magic
spread all over the tree. However, it's not that bad: long term, it
sounds at least plausible to reuse this machinery to run the CPU threads
outside the iothread lock (and only take it when doing MMIO, just like
KVM does).
Paolo
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] [RFC] time: refactor QEMU timer to use GHRTimer
2011-08-23 8:12 ` Paolo Bonzini
@ 2011-08-23 9:07 ` Edgar E. Iglesias
0 siblings, 0 replies; 13+ messages in thread
From: Edgar E. Iglesias @ 2011-08-23 9:07 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Anthony Liguori, Jan Kiszka, qemu-devel
On Tue, Aug 23, 2011 at 10:12:05AM +0200, Paolo Bonzini wrote:
> On 08/22/2011 10:28 PM, Jan Kiszka wrote:
> > - QEMU_CLOCK_VIRTUAL: Without -icount, same as above, but stops when
> > the guest is stopped. The offset to compensate for stopped
> > times is based on TSC, not sure why. With -icount, things get more
> > complicated, Paolo had some nice explanations for the details.
>
> The TSC is actually out of the picture. However, it is easy to get
> confused, because the same code handles stopping both the vm_clock and
> the TSC when the guest is paused. cpu_get_ticks handles the TSC,
> cpu_get_clock handles the clock:
>
> Removing some "uninteresting" details, you have:
>
> /* return the host CPU cycle counter and handle stop/restart */
> int64_t cpu_get_ticks(void)
> {
> if (!vm_running) {
> return timers_state.cpu_ticks_offset;
> } else {
> int64_t ticks;
> ticks = cpu_get_real_ticks();
> return ticks + timers_state.cpu_ticks_offset;
> }
> }
>
> /* return the host CPU monotonic timer and handle stop/restart */
> static int64_t cpu_get_clock(void)
> {
> int64_t ti;
> if (!vm_running) {
> return timers_state.cpu_clock_offset;
> } else {
> ti = get_clock();
> return ti + timers_state.cpu_clock_offset;
> }
> }
>
> which use the same algorithm but with different base clocks (TSC vs.
> CLOCK_MONOTONIC).
>
> With -icount, things get indeed more complicated. I'll cover only the
> iothread case since all my attempts at understanding the non-iothread
> case failed. :) The "-icount N" vm_clock has nanosecond resolution just
> like the normal vm_clock, but those are not real nanoseconds. While the
> CPU is running, each instruction increments the vm_clock by 2^N
> nanoseconds (yes, this is completely bollocks for SMP. :). When the CPU
> is not running, instead, the vm_clock follows CLOCK_MONOTONIC; the
> rationale is there in qemu-timer.c.
>
> "-icount auto" is the same, except that we try to keep vm_clock roughly
> the same as CLOCK_MONOTONIC by oscillating the clock frequency between
> "-icount N" values that are more representative of the actual execution
> frequency.
>
> On top of this, the VM has to do two things in icount mode. The first is
> to stop execution at the next vm_clock deadline, which means breaking
> translation blocks after executing the appropriate number of
> instructions; this is quite obvious. The second is to stop execution of
> a TB after any MMIO instruction, in order to recalculate deadlines if
> necessary. The latter is responsible for most of the icount black magic
> spread all over the tree. However, it's not that bad: long term, it
> sounds at least plausible to reuse this machinery to run the CPU threads
> outside the iothread lock (and only take it when doing MMIO, just like
> KVM does).
Interesting idea :)
Cheers
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] [RFC] time: refactor QEMU timer to use GHRTimer
2011-08-23 7:43 ` Paolo Bonzini
@ 2011-08-23 12:33 ` Anthony Liguori
2011-08-23 12:44 ` Paolo Bonzini
0 siblings, 1 reply; 13+ messages in thread
From: Anthony Liguori @ 2011-08-23 12:33 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, Jan Kiszka
On 08/23/2011 02:43 AM, Paolo Bonzini wrote:
> On 08/22/2011 09:21 PM, Anthony Liguori wrote:
>> - ticks = cpu_get_real_ticks();
>> - if (timers_state.cpu_ticks_prev > ticks) {
>> - /* Note: non increasing ticks may happen if the host uses
>> - software suspend */
>> - timers_state.cpu_ticks_offset += timers_state.cpu_ticks_prev - ticks;
>> - }
>> + ticks = get_clock();
>>
>> [...]
>>
>> -static inline int64_t cpu_get_real_ticks(void)
>> -{
>> - int64_t val;
>> - asm volatile ("rdtsc" : "=A" (val));
>> - return val;
>> -}
>> -
>
> cpu_get_ticks is used also to emulate the guest TSC, are you sure you
> want to change that uniformly to a 1 GHz rate?
Where possible, CLOCK_MONOTONIC_RAW should be implemented in terms or
rdtscp. It will provide the highest accuracy time source that you can get.
So I don't think there's any different in terms of timer granularity
from using CLOCK_MONOTONIC_RAW and rdtsc directly other than the former
is more correct.
Regards,
Anthony Liguori
>
> I had some more cleanups in this area, I'll try to get them tested and
> submitted but I have little time for this right now.
>
> Paolo
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] [RFC] time: refactor QEMU timer to use GHRTimer
2011-08-23 12:33 ` Anthony Liguori
@ 2011-08-23 12:44 ` Paolo Bonzini
0 siblings, 0 replies; 13+ messages in thread
From: Paolo Bonzini @ 2011-08-23 12:44 UTC (permalink / raw)
To: Anthony Liguori; +Cc: qemu-devel, Jan Kiszka
On 08/23/2011 02:33 PM, Anthony Liguori wrote:
>> cpu_get_ticks is used also to emulate the guest TSC, are you sure you
>> want to change that uniformly to a 1 GHz rate?
>
> Where possible, CLOCK_MONOTONIC_RAW should be implemented in terms or
> rdtscp. It will provide the highest accuracy time source that you can get.
Not all machines have rdtscp. People do not want to see their rdtsc
suddenly 20 times slower or so (that's more or less the speed difference
between HPET access and rdtsc). Many also would likely be surprised to
see a guest TSC that is 1 GHz. Xen did the same at some point, and
backed it out.
And you're not using CLOCK_MONOTONIC_RAW anyway (yet?). If you used it,
you should also change the CPUID features since QEMU would be providing
a constant, invariant TSC to all guests.
> So I don't think there's any different in terms of timer granularity
> from using CLOCK_MONOTONIC_RAW and rdtsc directly other than the former
> is more correct.
CLOCK_MONOTONIC_RAW has its resolution capped to 1 ns. It is as correct
and as precise, but less accurate than the TSC if your machine's TSC is
constant and invariant.
Paolo
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2011-08-23 12:44 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-22 19:21 [Qemu-devel] [PATCH 1/2] main: add high resolution GSource based timer Anthony Liguori
2011-08-22 19:21 ` [Qemu-devel] [PATCH 2/2] [RFC] time: refactor QEMU timer to use GHRTimer Anthony Liguori
2011-08-22 20:28 ` Jan Kiszka
2011-08-22 20:36 ` Anthony Liguori
2011-08-22 20:49 ` Jan Kiszka
2011-08-22 21:55 ` Anthony Liguori
2011-08-22 23:48 ` Jan Kiszka
2011-08-23 8:12 ` Paolo Bonzini
2011-08-23 9:07 ` Edgar E. Iglesias
2011-08-23 7:43 ` Paolo Bonzini
2011-08-23 12:33 ` Anthony Liguori
2011-08-23 12:44 ` Paolo Bonzini
2011-08-22 19:26 ` [Qemu-devel] [PATCH 1/2] main: add high resolution GSource based timer Anthony Liguori
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.