[Qemu-devel] [PATCH 0/7] timer: move APIs together according to their category

[Qemu-devel] [PATCH 0/7] timer: move APIs together according to their category

[Xuebing Wang]

There are 6 categories of APIs:
- QEMUClockType
- QEMUTimerList
- QEMUTimerListGroup
- QEMUTimer
- General utility functions
- Low level clock functions

Problems / solutions with previous API implementations:
1) Can not think of cases that we need QEMUTimerList APIs.
   Reference:  kernel_tree/Documentation/SubmittingPatches
               -- Section "4) Don't over-design"
   Purpose:    simplify API
   Solution:   remove them from APIs
2) Implementations of some APIs are interwined
   Purpose:    put them together according to their categories, thus new
               engineers are easy to understand them
   Solution:   put them together
3) Unnecessary header files are included
   Purpose:    simplify the included header files
   Solution:   only include the minimally required header file

This patchset includes below changes:
- put QEMUTimerList related functions together and make them private
- put QEMUClockType related functions together
- move QEMUTimerListGroup function to be below QEMUClockType
- put QEMUTimer related functions together
- put general utility functions together
- clean unnecessary #include and use minimal required #include

Xuebing Wang (7):
  timer: move QEMUTimerList functions together
  timer: make QEMUTimerList functions private (remove from APIs)
  timer: move QEMUClockType related functions together
  timer: move QEMUTimerListGroup function to be below QEMUClockType
  timer: move QEMUTimer related functions together
  timer: move general utility functions together
  timer: clean unnecessary #include and use minimal required #include

 include/qemu/timer.h |   94 --------
 qemu-timer.c         |  595 +++++++++++++++++++++++++++++---------------------
 2 files changed, 341 insertions(+), 348 deletions(-)

-- 
1.7.9.5

[Qemu-devel] [PATCH 1/7] timer: move QEMUTimerList functions together

[Xuebing Wang]

This patch does below:
-   Move function timerlist_has_timers() to be first QEMUTimerList function
    because there is a chance that we will make it private
-   Move QEMUTimerList related functions together
-   Remove below comments for function timerlist_deadline_ns(), because:
    --  There is already information in the header file include/qemu/timer.h
    --  "As above" part of the information is wrong
/*
 * As above, but return -1 for no deadline, and do not cap to 2^32
 * as we know the result is always positive.
 */

Signed-off-by: Xuebing Wang <xbing6@gmail.com>
---
 include/qemu/timer.h |   28 +++---
 qemu-timer.c         |  231 +++++++++++++++++++++++++-------------------------
 2 files changed, 129 insertions(+), 130 deletions(-)

diff --git a/include/qemu/timer.h b/include/qemu/timer.h
index 7f9a074..55cd3a7 100644
--- a/include/qemu/timer.h
+++ b/include/qemu/timer.h
@@ -254,6 +254,20 @@ bool qemu_clock_run_all_timers(void);
  */
 
 /**
+ * timerlist_has_timers:
+ * @timer_list: the timer list to operate on
+ *
+ * Determine whether a timer list has active timers
+ *
+ * Note that this function should not be used when other threads also access
+ * the timer list.  The return value may be outdated by the time it is acted
+ * upon.
+ *
+ * Returns: true if the timer list has timers.
+ */
+bool timerlist_has_timers(QEMUTimerList *timer_list);
+
+/**
  * timerlist_new:
  * @type: the clock type to associate with the timerlist
  * @cb: the callback to call on notification
@@ -276,20 +290,6 @@ QEMUTimerList *timerlist_new(QEMUClockType type,
 void timerlist_free(QEMUTimerList *timer_list);
 
 /**
- * timerlist_has_timers:
- * @timer_list: the timer list to operate on
- *
- * Determine whether a timer list has active timers
- *
- * Note that this function should not be used when other threads also access
- * the timer list.  The return value may be outdated by the time it is acted
- * upon.
- *
- * Returns: true if the timer list has timers.
- */
-bool timerlist_has_timers(QEMUTimerList *timer_list);
-
-/**
  * timerlist_expired:
  * @timer_list: the timer list to operate on
  *
diff --git a/qemu-timer.c b/qemu-timer.c
index e15ce47..4929f8b 100644
--- a/qemu-timer.c
+++ b/qemu-timer.c
@@ -95,6 +95,15 @@ static bool timer_expired_ns(QEMUTimer *timer_head, int64_t current_time)
     return timer_head && (timer_head->expire_time <= current_time);
 }
 
+/*
+ * QEMUTimerList
+ */
+
+bool timerlist_has_timers(QEMUTimerList *timer_list)
+{
+    return !!timer_list->active_timers;
+}
+
 QEMUTimerList *timerlist_new(QEMUClockType type,
                              QEMUTimerListNotifyCB *cb,
                              void *opaque)
@@ -122,6 +131,112 @@ void timerlist_free(QEMUTimerList *timer_list)
     g_free(timer_list);
 }
 
+bool timerlist_expired(QEMUTimerList *timer_list)
+{
+    int64_t expire_time;
+
+    qemu_mutex_lock(&timer_list->active_timers_lock);
+    if (!timer_list->active_timers) {
+        qemu_mutex_unlock(&timer_list->active_timers_lock);
+        return false;
+    }
+    expire_time = timer_list->active_timers->expire_time;
+    qemu_mutex_unlock(&timer_list->active_timers_lock);
+
+    return expire_time < qemu_clock_get_ns(timer_list->clock->type);
+}
+
+int64_t timerlist_deadline_ns(QEMUTimerList *timer_list)
+{
+    int64_t delta;
+    int64_t expire_time;
+
+    if (!timer_list->clock->enabled) {
+        return -1;
+    }
+
+    /* The active timers list may be modified before the caller uses our return
+     * value but ->notify_cb() is called when the deadline changes.  Therefore
+     * the caller should notice the change and there is no race condition.
+     */
+    qemu_mutex_lock(&timer_list->active_timers_lock);
+    if (!timer_list->active_timers) {
+        qemu_mutex_unlock(&timer_list->active_timers_lock);
+        return -1;
+    }
+    expire_time = timer_list->active_timers->expire_time;
+    qemu_mutex_unlock(&timer_list->active_timers_lock);
+
+    delta = expire_time - qemu_clock_get_ns(timer_list->clock->type);
+
+    if (delta <= 0) {
+        return 0;
+    }
+
+    return delta;
+}
+
+QEMUClockType timerlist_get_clock(QEMUTimerList *timer_list)
+{
+    return timer_list->clock->type;
+}
+
+bool timerlist_run_timers(QEMUTimerList *timer_list)
+{
+    QEMUTimer *ts;
+    int64_t current_time;
+    bool progress = false;
+    QEMUTimerCB *cb;
+    void *opaque;
+
+    qemu_event_reset(&timer_list->timers_done_ev);
+    if (!timer_list->clock->enabled) {
+        goto out;
+    }
+
+    current_time = qemu_clock_get_ns(timer_list->clock->type);
+    for(;;) {
+        qemu_mutex_lock(&timer_list->active_timers_lock);
+        ts = timer_list->active_timers;
+        if (!timer_expired_ns(ts, current_time)) {
+            qemu_mutex_unlock(&timer_list->active_timers_lock);
+            break;
+        }
+
+        /* remove timer from the list before calling the callback */
+        timer_list->active_timers = ts->next;
+        ts->next = NULL;
+        ts->expire_time = -1;
+        cb = ts->cb;
+        opaque = ts->opaque;
+        qemu_mutex_unlock(&timer_list->active_timers_lock);
+
+        /* run the callback (the timer list can be modified) */
+        cb(opaque);
+        progress = true;
+    }
+
+out:
+    qemu_event_set(&timer_list->timers_done_ev);
+    return progress;
+}
+
+void timerlist_notify(QEMUTimerList *timer_list)
+{
+    if (timer_list->notify_cb) {
+        timer_list->notify_cb(timer_list->notify_opaque);
+    } else {
+        qemu_notify_event();
+    }
+}
+
+static void timerlist_rearm(QEMUTimerList *timer_list)
+{
+    /* Interrupt execution to force deadline recalculation.  */
+    qemu_clock_warp(timer_list->clock->type);
+    timerlist_notify(timer_list);
+}
+
 static void qemu_clock_init(QEMUClockType type)
 {
     QEMUClock *clock = qemu_clock_ptr(type);
@@ -170,73 +285,18 @@ void qemu_clock_enable(QEMUClockType type, bool enabled)
     }
 }
 
-bool timerlist_has_timers(QEMUTimerList *timer_list)
-{
-    return !!timer_list->active_timers;
-}
-
 bool qemu_clock_has_timers(QEMUClockType type)
 {
     return timerlist_has_timers(
         main_loop_tlg.tl[type]);
 }
 
-bool timerlist_expired(QEMUTimerList *timer_list)
-{
-    int64_t expire_time;
-
-    qemu_mutex_lock(&timer_list->active_timers_lock);
-    if (!timer_list->active_timers) {
-        qemu_mutex_unlock(&timer_list->active_timers_lock);
-        return false;
-    }
-    expire_time = timer_list->active_timers->expire_time;
-    qemu_mutex_unlock(&timer_list->active_timers_lock);
-
-    return expire_time < qemu_clock_get_ns(timer_list->clock->type);
-}
-
 bool qemu_clock_expired(QEMUClockType type)
 {
     return timerlist_expired(
         main_loop_tlg.tl[type]);
 }
 
-/*
- * As above, but return -1 for no deadline, and do not cap to 2^32
- * as we know the result is always positive.
- */
-
-int64_t timerlist_deadline_ns(QEMUTimerList *timer_list)
-{
-    int64_t delta;
-    int64_t expire_time;
-
-    if (!timer_list->clock->enabled) {
-        return -1;
-    }
-
-    /* The active timers list may be modified before the caller uses our return
-     * value but ->notify_cb() is called when the deadline changes.  Therefore
-     * the caller should notice the change and there is no race condition.
-     */
-    qemu_mutex_lock(&timer_list->active_timers_lock);
-    if (!timer_list->active_timers) {
-        qemu_mutex_unlock(&timer_list->active_timers_lock);
-        return -1;
-    }
-    expire_time = timer_list->active_timers->expire_time;
-    qemu_mutex_unlock(&timer_list->active_timers_lock);
-
-    delta = expire_time - qemu_clock_get_ns(timer_list->clock->type);
-
-    if (delta <= 0) {
-        return 0;
-    }
-
-    return delta;
-}
-
 /* Calculate the soonest deadline across all timerlists attached
  * to the clock. This is used for the icount timeout so we
  * ignore whether or not the clock should be used in deadline
@@ -254,25 +314,11 @@ int64_t qemu_clock_deadline_ns_all(QEMUClockType type)
     return deadline;
 }
 
-QEMUClockType timerlist_get_clock(QEMUTimerList *timer_list)
-{
-    return timer_list->clock->type;
-}
-
 QEMUTimerList *qemu_clock_get_main_loop_timerlist(QEMUClockType type)
 {
     return main_loop_tlg.tl[type];
 }
 
-void timerlist_notify(QEMUTimerList *timer_list)
-{
-    if (timer_list->notify_cb) {
-        timer_list->notify_cb(timer_list->notify_opaque);
-    } else {
-        qemu_notify_event();
-    }
-}
-
 /* Transition function to convert a nanosecond timeout to ms
  * This is used where a system does not support ppoll
  */
@@ -376,13 +422,6 @@ static bool timer_mod_ns_locked(QEMUTimerList *timer_list,
     return pt == &timer_list->active_timers;
 }
 
-static void timerlist_rearm(QEMUTimerList *timer_list)
-{
-    /* Interrupt execution to force deadline recalculation.  */
-    qemu_clock_warp(timer_list->clock->type);
-    timerlist_notify(timer_list);
-}
-
 /* stop a timer, but do not dealloc it */
 void timer_del(QEMUTimer *ts)
 {
@@ -454,46 +493,6 @@ bool timer_expired(QEMUTimer *timer_head, int64_t current_time)
     return timer_expired_ns(timer_head, current_time * timer_head->scale);
 }
 
-bool timerlist_run_timers(QEMUTimerList *timer_list)
-{
-    QEMUTimer *ts;
-    int64_t current_time;
-    bool progress = false;
-    QEMUTimerCB *cb;
-    void *opaque;
-
-    qemu_event_reset(&timer_list->timers_done_ev);
-    if (!timer_list->clock->enabled) {
-        goto out;
-    }
-
-    current_time = qemu_clock_get_ns(timer_list->clock->type);
-    for(;;) {
-        qemu_mutex_lock(&timer_list->active_timers_lock);
-        ts = timer_list->active_timers;
-        if (!timer_expired_ns(ts, current_time)) {
-            qemu_mutex_unlock(&timer_list->active_timers_lock);
-            break;
-        }
-
-        /* remove timer from the list before calling the callback */
-        timer_list->active_timers = ts->next;
-        ts->next = NULL;
-        ts->expire_time = -1;
-        cb = ts->cb;
-        opaque = ts->opaque;
-        qemu_mutex_unlock(&timer_list->active_timers_lock);
-
-        /* run the callback (the timer list can be modified) */
-        cb(opaque);
-        progress = true;
-    }
-
-out:
-    qemu_event_set(&timer_list->timers_done_ev);
-    return progress;
-}
-
 bool qemu_clock_run_timers(QEMUClockType type)
 {
     return timerlist_run_timers(main_loop_tlg.tl[type]);
-- 
1.7.9.5

[Qemu-devel] [PATCH 2/7] timer: make QEMUTimerList functions private (remove from APIs)

[Xuebing Wang]

This patch does below:
-   Make QEMUTimerList functions private (remove from APIs)
-   Comment out function QEMUTimerList timerlist_get_clock() in order
    to fix compile error

Signed-off-by: Xuebing Wang <xbing6@gmail.com>
---
 include/qemu/timer.h |   94 ------------------------------------------------
 qemu-timer.c         |   97 ++++++++++++++++++++++++++++++++++++++++++++------
 2 files changed, 87 insertions(+), 104 deletions(-)

diff --git a/include/qemu/timer.h b/include/qemu/timer.h
index 55cd3a7..8644913 100644
--- a/include/qemu/timer.h
+++ b/include/qemu/timer.h
@@ -250,100 +250,6 @@ bool qemu_clock_run_timers(QEMUClockType type);
 bool qemu_clock_run_all_timers(void);
 
 /*
- * QEMUTimerList
- */
-
-/**
- * timerlist_has_timers:
- * @timer_list: the timer list to operate on
- *
- * Determine whether a timer list has active timers
- *
- * Note that this function should not be used when other threads also access
- * the timer list.  The return value may be outdated by the time it is acted
- * upon.
- *
- * Returns: true if the timer list has timers.
- */
-bool timerlist_has_timers(QEMUTimerList *timer_list);
-
-/**
- * timerlist_new:
- * @type: the clock type to associate with the timerlist
- * @cb: the callback to call on notification
- * @opaque: the opaque pointer to pass to the callback
- *
- * Create a new timerlist associated with the clock of
- * type @type.
- *
- * Returns: a pointer to the QEMUTimerList created
- */
-QEMUTimerList *timerlist_new(QEMUClockType type,
-                             QEMUTimerListNotifyCB *cb, void *opaque);
-
-/**
- * timerlist_free:
- * @timer_list: the timer list to free
- *
- * Frees a timer_list. It must have no active timers.
- */
-void timerlist_free(QEMUTimerList *timer_list);
-
-/**
- * timerlist_expired:
- * @timer_list: the timer list to operate on
- *
- * Determine whether a timer list has any timers which
- * are expired.
- *
- * Returns: true if the timer list has timers which
- * have expired.
- */
-bool timerlist_expired(QEMUTimerList *timer_list);
-
-/**
- * timerlist_deadline_ns:
- * @timer_list: the timer list to operate on
- *
- * Determine the deadline for a timer_list, i.e.
- * the number of nanoseconds until the first timer
- * expires. Return -1 if there are no timers.
- *
- * Returns: the number of nanoseconds until the earliest
- * timer expires -1 if none
- */
-int64_t timerlist_deadline_ns(QEMUTimerList *timer_list);
-
-/**
- * timerlist_get_clock:
- * @timer_list: the timer list to operate on
- *
- * Determine the clock type associated with a timer list.
- *
- * Returns: the clock type associated with the
- * timer list.
- */
-QEMUClockType timerlist_get_clock(QEMUTimerList *timer_list);
-
-/**
- * timerlist_run_timers:
- * @timer_list: the timer list to use
- *
- * Call all expired timers associated with the timer list.
- *
- * Returns: true if any timer expired
- */
-bool timerlist_run_timers(QEMUTimerList *timer_list);
-
-/**
- * timerlist_notify:
- * @timer_list: the timer list to use
- *
- * call the notifier callback associated with the timer list.
- */
-void timerlist_notify(QEMUTimerList *timer_list);
-
-/*
  * QEMUTimerListGroup
  */
 
diff --git a/qemu-timer.c b/qemu-timer.c
index 4929f8b..2db87ba 100644
--- a/qemu-timer.c
+++ b/qemu-timer.c
@@ -99,14 +99,37 @@ static bool timer_expired_ns(QEMUTimer *timer_head, int64_t current_time)
  * QEMUTimerList
  */
 
-bool timerlist_has_timers(QEMUTimerList *timer_list)
+/**
+ * timerlist_has_timers:
+ * @timer_list: the timer list to operate on
+ *
+ * Determine whether a timer list has active timers
+ *
+ * Note that this function should not be used when other threads also access
+ * the timer list.  The return value may be outdated by the time it is acted
+ * upon.
+ *
+ * Returns: true if the timer list has timers.
+ */
+static bool timerlist_has_timers(QEMUTimerList *timer_list)
 {
     return !!timer_list->active_timers;
 }
 
-QEMUTimerList *timerlist_new(QEMUClockType type,
-                             QEMUTimerListNotifyCB *cb,
-                             void *opaque)
+/**
+ * timerlist_new:
+ * @type: the clock type to associate with the timerlist
+ * @cb: the callback to call on notification
+ * @opaque: the opaque pointer to pass to the callback
+ *
+ * Create a new timerlist associated with the clock of
+ * type @type.
+ *
+ * Returns: a pointer to the QEMUTimerList created
+ */
+static QEMUTimerList *timerlist_new(QEMUClockType type,
+                                    QEMUTimerListNotifyCB *cb,
+                                    void *opaque)
 {
     QEMUTimerList *timer_list;
     QEMUClock *clock = qemu_clock_ptr(type);
@@ -121,7 +144,13 @@ QEMUTimerList *timerlist_new(QEMUClockType type,
     return timer_list;
 }
 
-void timerlist_free(QEMUTimerList *timer_list)
+/**
+ * timerlist_free:
+ * @timer_list: the timer list to free
+ *
+ * Frees a timer_list. It must have no active timers.
+ */
+static void timerlist_free(QEMUTimerList *timer_list)
 {
     assert(!timerlist_has_timers(timer_list));
     if (timer_list->clock) {
@@ -131,7 +160,17 @@ void timerlist_free(QEMUTimerList *timer_list)
     g_free(timer_list);
 }
 
-bool timerlist_expired(QEMUTimerList *timer_list)
+/**
+ * timerlist_expired:
+ * @timer_list: the timer list to operate on
+ *
+ * Determine whether a timer list has any timers which
+ * are expired.
+ *
+ * Returns: true if the timer list has timers which
+ * have expired.
+ */
+static bool timerlist_expired(QEMUTimerList *timer_list)
 {
     int64_t expire_time;
 
@@ -146,7 +185,18 @@ bool timerlist_expired(QEMUTimerList *timer_list)
     return expire_time < qemu_clock_get_ns(timer_list->clock->type);
 }
 
-int64_t timerlist_deadline_ns(QEMUTimerList *timer_list)
+/**
+ * timerlist_deadline_ns:
+ * @timer_list: the timer list to operate on
+ *
+ * Determine the deadline for a timer_list, i.e.
+ * the number of nanoseconds until the first timer
+ * expires. Return -1 if there are no timers.
+ *
+ * Returns: the number of nanoseconds until the earliest
+ * timer expires -1 if none
+ */
+static int64_t timerlist_deadline_ns(QEMUTimerList *timer_list)
 {
     int64_t delta;
     int64_t expire_time;
@@ -176,12 +226,33 @@ int64_t timerlist_deadline_ns(QEMUTimerList *timer_list)
     return delta;
 }
 
-QEMUClockType timerlist_get_clock(QEMUTimerList *timer_list)
+#if 0 /* TODO: unused, consider to remove */
+/* Comment out in order to fix compile error as below:
+   ‘timerlist_get_clock’ defined but not used [-Werror=unused-function] */
+/**
+ * timerlist_get_clock:
+ * @timer_list: the timer list to operate on
+ *
+ * Determine the clock type associated with a timer list.
+ *
+ * Returns: the clock type associated with the
+ * timer list.
+ */
+static QEMUClockType timerlist_get_clock(QEMUTimerList *timer_list)
 {
     return timer_list->clock->type;
 }
+#endif
 
-bool timerlist_run_timers(QEMUTimerList *timer_list)
+/**
+ * timerlist_run_timers:
+ * @timer_list: the timer list to use
+ *
+ * Call all expired timers associated with the timer list.
+ *
+ * Returns: true if any timer expired
+ */
+static bool timerlist_run_timers(QEMUTimerList *timer_list)
 {
     QEMUTimer *ts;
     int64_t current_time;
@@ -221,7 +292,13 @@ out:
     return progress;
 }
 
-void timerlist_notify(QEMUTimerList *timer_list)
+/**
+ * timerlist_notify:
+ * @timer_list: the timer list to use
+ *
+ * call the notifier callback associated with the timer list.
+ */
+static void timerlist_notify(QEMUTimerList *timer_list)
 {
     if (timer_list->notify_cb) {
         timer_list->notify_cb(timer_list->notify_opaque);
-- 
1.7.9.5

[Qemu-devel] [PATCH 3/7] timer: move QEMUClockType related functions together

[Xuebing Wang]

Signed-off-by: Xuebing Wang <xbing6@gmail.com>
---
 qemu-timer.c |  160 ++++++++++++++++++++++++++++++----------------------------
 1 file changed, 82 insertions(+), 78 deletions(-)

diff --git a/qemu-timer.c b/qemu-timer.c
index 2db87ba..471150c 100644
--- a/qemu-timer.c
+++ b/qemu-timer.c
@@ -314,6 +314,10 @@ static void timerlist_rearm(QEMUTimerList *timer_list)
     timerlist_notify(timer_list);
 }
 
+/*
+ * QEMUClockType
+ */
+
 static void qemu_clock_init(QEMUClockType type)
 {
     QEMUClock *clock = qemu_clock_ptr(type);
@@ -326,11 +330,71 @@ static void qemu_clock_init(QEMUClockType type)
     main_loop_tlg.tl[type] = timerlist_new(type, NULL, NULL);
 }
 
+int64_t qemu_clock_get_ns(QEMUClockType type)
+{
+    int64_t now, last;
+    QEMUClock *clock = qemu_clock_ptr(type);
+
+    switch (type) {
+    case QEMU_CLOCK_REALTIME:
+        return get_clock();
+    default:
+    case QEMU_CLOCK_VIRTUAL:
+        if (use_icount) {
+            return cpu_get_icount();
+        } else {
+            return cpu_get_clock();
+        }
+    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;
+    }
+}
+
+bool qemu_clock_has_timers(QEMUClockType type)
+{
+    return timerlist_has_timers(
+        main_loop_tlg.tl[type]);
+}
+
+bool qemu_clock_expired(QEMUClockType type)
+{
+    return timerlist_expired(
+        main_loop_tlg.tl[type]);
+}
+
 bool qemu_clock_use_for_deadline(QEMUClockType type)
 {
     return !(use_icount && (type == QEMU_CLOCK_VIRTUAL));
 }
 
+/* Calculate the soonest deadline across all timerlists attached
+ * to the clock. This is used for the icount timeout so we
+ * ignore whether or not the clock should be used in deadline
+ * calculations.
+ */
+int64_t qemu_clock_deadline_ns_all(QEMUClockType type)
+{
+    int64_t deadline = -1;
+    QEMUTimerList *timer_list;
+    QEMUClock *clock = qemu_clock_ptr(type);
+    QLIST_FOREACH(timer_list, &clock->timerlists, list) {
+        deadline = qemu_soonest_timeout(deadline,
+                                        timerlist_deadline_ns(timer_list));
+    }
+    return deadline;
+}
+
+QEMUTimerList *qemu_clock_get_main_loop_timerlist(QEMUClockType type)
+{
+    return main_loop_tlg.tl[type];
+}
+
 void qemu_clock_notify(QEMUClockType type)
 {
     QEMUTimerList *timer_list;
@@ -362,38 +426,34 @@ void qemu_clock_enable(QEMUClockType type, bool enabled)
     }
 }
 
-bool qemu_clock_has_timers(QEMUClockType type)
+void qemu_clock_register_reset_notifier(QEMUClockType type,
+                                        Notifier *notifier)
 {
-    return timerlist_has_timers(
-        main_loop_tlg.tl[type]);
+    QEMUClock *clock = qemu_clock_ptr(type);
+    notifier_list_add(&clock->reset_notifiers, notifier);
 }
 
-bool qemu_clock_expired(QEMUClockType type)
+void qemu_clock_unregister_reset_notifier(QEMUClockType type,
+                                          Notifier *notifier)
 {
-    return timerlist_expired(
-        main_loop_tlg.tl[type]);
+    notifier_remove(notifier);
 }
 
-/* Calculate the soonest deadline across all timerlists attached
- * to the clock. This is used for the icount timeout so we
- * ignore whether or not the clock should be used in deadline
- * calculations.
- */
-int64_t qemu_clock_deadline_ns_all(QEMUClockType type)
+bool qemu_clock_run_timers(QEMUClockType type)
 {
-    int64_t deadline = -1;
-    QEMUTimerList *timer_list;
-    QEMUClock *clock = qemu_clock_ptr(type);
-    QLIST_FOREACH(timer_list, &clock->timerlists, list) {
-        deadline = qemu_soonest_timeout(deadline,
-                                        timerlist_deadline_ns(timer_list));
-    }
-    return deadline;
+    return timerlist_run_timers(main_loop_tlg.tl[type]);
 }
 
-QEMUTimerList *qemu_clock_get_main_loop_timerlist(QEMUClockType type)
+bool qemu_clock_run_all_timers(void)
 {
-    return main_loop_tlg.tl[type];
+    bool progress = false;
+    QEMUClockType type;
+
+    for (type = 0; type < QEMU_CLOCK_MAX; type++) {
+        progress |= qemu_clock_run_timers(type);
+    }
+
+    return progress;
 }
 
 /* Transition function to convert a nanosecond timeout to ms
@@ -570,11 +630,6 @@ bool timer_expired(QEMUTimer *timer_head, int64_t current_time)
     return timer_expired_ns(timer_head, current_time * timer_head->scale);
 }
 
-bool qemu_clock_run_timers(QEMUClockType type)
-{
-    return timerlist_run_timers(main_loop_tlg.tl[type]);
-}
-
 void timerlistgroup_init(QEMUTimerListGroup *tlg,
                          QEMUTimerListNotifyCB *cb, void *opaque)
 {
@@ -616,45 +671,6 @@ int64_t timerlistgroup_deadline_ns(QEMUTimerListGroup *tlg)
     return deadline;
 }
 
-int64_t qemu_clock_get_ns(QEMUClockType type)
-{
-    int64_t now, last;
-    QEMUClock *clock = qemu_clock_ptr(type);
-
-    switch (type) {
-    case QEMU_CLOCK_REALTIME:
-        return get_clock();
-    default:
-    case QEMU_CLOCK_VIRTUAL:
-        if (use_icount) {
-            return cpu_get_icount();
-        } else {
-            return cpu_get_clock();
-        }
-    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;
-    }
-}
-
-void qemu_clock_register_reset_notifier(QEMUClockType type,
-                                        Notifier *notifier)
-{
-    QEMUClock *clock = qemu_clock_ptr(type);
-    notifier_list_add(&clock->reset_notifiers, notifier);
-}
-
-void qemu_clock_unregister_reset_notifier(QEMUClockType type,
-                                          Notifier *notifier)
-{
-    notifier_remove(notifier);
-}
-
 void init_clocks(void)
 {
     QEMUClockType type;
@@ -671,15 +687,3 @@ uint64_t timer_expire_time_ns(QEMUTimer *ts)
 {
     return timer_pending(ts) ? ts->expire_time : -1;
 }
-
-bool qemu_clock_run_all_timers(void)
-{
-    bool progress = false;
-    QEMUClockType type;
-
-    for (type = 0; type < QEMU_CLOCK_MAX; type++) {
-        progress |= qemu_clock_run_timers(type);
-    }
-
-    return progress;
-}
-- 
1.7.9.5

[Qemu-devel] [PATCH 4/7] timer: move QEMUTimerListGroup function to be below QEMUClockType

[Xuebing Wang]

Signed-off-by: Xuebing Wang <xbing6@gmail.com>
---
 qemu-timer.c |   86 ++++++++++++++++++++++++++++++----------------------------
 1 file changed, 45 insertions(+), 41 deletions(-)

diff --git a/qemu-timer.c b/qemu-timer.c
index 471150c..6f13a76 100644
--- a/qemu-timer.c
+++ b/qemu-timer.c
@@ -456,6 +456,51 @@ bool qemu_clock_run_all_timers(void)
     return progress;
 }
 
+/*
+ * QEMUTimerListGroup
+ */
+
+void timerlistgroup_init(QEMUTimerListGroup *tlg,
+                         QEMUTimerListNotifyCB *cb, void *opaque)
+{
+    QEMUClockType type;
+    for (type = 0; type < QEMU_CLOCK_MAX; type++) {
+        tlg->tl[type] = timerlist_new(type, cb, opaque);
+    }
+}
+
+void timerlistgroup_deinit(QEMUTimerListGroup *tlg)
+{
+    QEMUClockType type;
+    for (type = 0; type < QEMU_CLOCK_MAX; type++) {
+        timerlist_free(tlg->tl[type]);
+    }
+}
+
+bool timerlistgroup_run_timers(QEMUTimerListGroup *tlg)
+{
+    QEMUClockType type;
+    bool progress = false;
+    for (type = 0; type < QEMU_CLOCK_MAX; type++) {
+        progress |= timerlist_run_timers(tlg->tl[type]);
+    }
+    return progress;
+}
+
+int64_t timerlistgroup_deadline_ns(QEMUTimerListGroup *tlg)
+{
+    int64_t deadline = -1;
+    QEMUClockType type;
+    for (type = 0; type < QEMU_CLOCK_MAX; type++) {
+        if (qemu_clock_use_for_deadline(tlg->tl[type]->clock->type)) {
+            deadline = qemu_soonest_timeout(deadline,
+                                            timerlist_deadline_ns(
+                                                tlg->tl[type]));
+        }
+    }
+    return deadline;
+}
+
 /* Transition function to convert a nanosecond timeout to ms
  * This is used where a system does not support ppoll
  */
@@ -630,47 +675,6 @@ bool timer_expired(QEMUTimer *timer_head, int64_t current_time)
     return timer_expired_ns(timer_head, current_time * timer_head->scale);
 }
 
-void timerlistgroup_init(QEMUTimerListGroup *tlg,
-                         QEMUTimerListNotifyCB *cb, void *opaque)
-{
-    QEMUClockType type;
-    for (type = 0; type < QEMU_CLOCK_MAX; type++) {
-        tlg->tl[type] = timerlist_new(type, cb, opaque);
-    }
-}
-
-void timerlistgroup_deinit(QEMUTimerListGroup *tlg)
-{
-    QEMUClockType type;
-    for (type = 0; type < QEMU_CLOCK_MAX; type++) {
-        timerlist_free(tlg->tl[type]);
-    }
-}
-
-bool timerlistgroup_run_timers(QEMUTimerListGroup *tlg)
-{
-    QEMUClockType type;
-    bool progress = false;
-    for (type = 0; type < QEMU_CLOCK_MAX; type++) {
-        progress |= timerlist_run_timers(tlg->tl[type]);
-    }
-    return progress;
-}
-
-int64_t timerlistgroup_deadline_ns(QEMUTimerListGroup *tlg)
-{
-    int64_t deadline = -1;
-    QEMUClockType type;
-    for (type = 0; type < QEMU_CLOCK_MAX; type++) {
-        if (qemu_clock_use_for_deadline(tlg->tl[type]->clock->type)) {
-            deadline = qemu_soonest_timeout(deadline,
-                                            timerlist_deadline_ns(
-                                                tlg->tl[type]));
-        }
-    }
-    return deadline;
-}
-
 void init_clocks(void)
 {
     QEMUClockType type;
-- 
1.7.9.5

[Qemu-devel] [PATCH 5/7] timer: move QEMUTimer related functions together

[Xuebing Wang]

Signed-off-by: Xuebing Wang <xbing6@gmail.com>
---
 qemu-timer.c |   13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/qemu-timer.c b/qemu-timer.c
index 6f13a76..c9801da 100644
--- a/qemu-timer.c
+++ b/qemu-timer.c
@@ -548,6 +548,9 @@ int qemu_poll_ns(GPollFD *fds, guint nfds, int64_t timeout)
 #endif
 }
 
+/*
+ * QEMUTimer
+ */
 
 void timer_init(QEMUTimer *ts,
                 QEMUTimerList *timer_list, int scale,
@@ -675,6 +678,11 @@ bool timer_expired(QEMUTimer *timer_head, int64_t current_time)
     return timer_expired_ns(timer_head, current_time * timer_head->scale);
 }
 
+uint64_t timer_expire_time_ns(QEMUTimer *ts)
+{
+    return timer_pending(ts) ? ts->expire_time : -1;
+}
+
 void init_clocks(void)
 {
     QEMUClockType type;
@@ -686,8 +694,3 @@ void init_clocks(void)
     prctl(PR_SET_TIMERSLACK, 1, 0, 0, 0);
 #endif
 }
-
-uint64_t timer_expire_time_ns(QEMUTimer *ts)
-{
-    return timer_pending(ts) ? ts->expire_time : -1;
-}
-- 
1.7.9.5

[Qemu-devel] [PATCH 6/7] timer: move general utility functions together

[Xuebing Wang]

Signed-off-by: Xuebing Wang <xbing6@gmail.com>
---
 qemu-timer.c |   97 ++++++++++++++++++++++++++++++----------------------------
 1 file changed, 50 insertions(+), 47 deletions(-)

diff --git a/qemu-timer.c b/qemu-timer.c
index c9801da..e592c14 100644
--- a/qemu-timer.c
+++ b/qemu-timer.c
@@ -501,53 +501,6 @@ int64_t timerlistgroup_deadline_ns(QEMUTimerListGroup *tlg)
     return deadline;
 }
 
-/* Transition function to convert a nanosecond timeout to ms
- * This is used where a system does not support ppoll
- */
-int qemu_timeout_ns_to_ms(int64_t ns)
-{
-    int64_t ms;
-    if (ns < 0) {
-        return -1;
-    }
-
-    if (!ns) {
-        return 0;
-    }
-
-    /* Always round up, because it's better to wait too long than to wait too
-     * little and effectively busy-wait
-     */
-    ms = (ns + SCALE_MS - 1) / SCALE_MS;
-
-    /* To avoid overflow problems, limit this to 2^31, i.e. approx 25 days */
-    if (ms > (int64_t) INT32_MAX) {
-        ms = INT32_MAX;
-    }
-
-    return (int) ms;
-}
-
-
-/* qemu implementation of g_poll which uses a nanosecond timeout but is
- * otherwise identical to g_poll
- */
-int qemu_poll_ns(GPollFD *fds, guint nfds, int64_t timeout)
-{
-#ifdef CONFIG_PPOLL
-    if (timeout < 0) {
-        return ppoll((struct pollfd *)fds, nfds, NULL, NULL);
-    } else {
-        struct timespec ts;
-        ts.tv_sec = timeout / 1000000000LL;
-        ts.tv_nsec = timeout % 1000000000LL;
-        return ppoll((struct pollfd *)fds, nfds, &ts, NULL);
-    }
-#else
-    return g_poll(fds, nfds, qemu_timeout_ns_to_ms(timeout));
-#endif
-}
-
 /*
  * QEMUTimer
  */
@@ -683,6 +636,56 @@ uint64_t timer_expire_time_ns(QEMUTimer *ts)
     return timer_pending(ts) ? ts->expire_time : -1;
 }
 
+/*
+ * General utility functions
+ */
+
+/* Transition function to convert a nanosecond timeout to ms
+ * This is used where a system does not support ppoll
+ */
+int qemu_timeout_ns_to_ms(int64_t ns)
+{
+    int64_t ms;
+    if (ns < 0) {
+        return -1;
+    }
+
+    if (!ns) {
+        return 0;
+    }
+
+    /* Always round up, because it's better to wait too long than to wait too
+     * little and effectively busy-wait
+     */
+    ms = (ns + SCALE_MS - 1) / SCALE_MS;
+
+    /* To avoid overflow problems, limit this to 2^31, i.e. approx 25 days */
+    if (ms > (int64_t) INT32_MAX) {
+        ms = INT32_MAX;
+    }
+
+    return (int) ms;
+}
+
+/* qemu implementation of g_poll which uses a nanosecond timeout but is
+ * otherwise identical to g_poll
+ */
+int qemu_poll_ns(GPollFD *fds, guint nfds, int64_t timeout)
+{
+#ifdef CONFIG_PPOLL
+    if (timeout < 0) {
+        return ppoll((struct pollfd *)fds, nfds, NULL, NULL);
+    } else {
+        struct timespec ts;
+        ts.tv_sec = timeout / 1000000000LL;
+        ts.tv_nsec = timeout % 1000000000LL;
+        return ppoll((struct pollfd *)fds, nfds, &ts, NULL);
+    }
+#else
+    return g_poll(fds, nfds, qemu_timeout_ns_to_ms(timeout));
+#endif
+}
+
 void init_clocks(void)
 {
     QEMUClockType type;
-- 
1.7.9.5

[Qemu-devel] [PATCH 7/7] timer: clean unnecessary #include and use minimal required #include

[Xuebing Wang]

Signed-off-by: Xuebing Wang <xbing6@gmail.com>
---
 qemu-timer.c |    9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/qemu-timer.c b/qemu-timer.c
index e592c14..21aff82 100644
--- a/qemu-timer.c
+++ b/qemu-timer.c
@@ -22,13 +22,10 @@
  * THE SOFTWARE.
  */
 
-#include "sysemu/sysemu.h"
-#include "monitor/monitor.h"
-#include "ui/console.h"
-
-#include "hw/hw.h"
-
+#include "qemu/thread.h" /* for qemu_event_init() etc */
+#include "qemu/main-loop.h" /* for qemu_notify_event() */
 #include "qemu/timer.h"
+
 #ifdef CONFIG_POSIX
 #include <pthread.h>
 #endif
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH 0/7] timer: move APIs together according to their category

[Alex Bligh]

> 
> This patchset includes below changes:
> - put QEMUTimerList related functions together and make them private
> - put QEMUClockType related functions together
> - move QEMUTimerListGroup function to be below QEMUClockType
> - put QEMUTimer related functions together
> - put general utility functions together
> - clean unnecessary #include and use minimal required #include
> 
> Xuebing Wang (7):
>  timer: move QEMUTimerList functions together
>  timer: make QEMUTimerList functions private (remove from APIs)
>  timer: move QEMUClockType related functions together
>  timer: move QEMUTimerListGroup function to be below QEMUClockType
>  timer: move QEMUTimer related functions together
>  timer: move general utility functions together
>  timer: clean unnecessary #include and use minimal required #include


I'm generally in favour of this patch set or something like it. It's
nicely constrained to 2 include files. It appears to touch no code.
And it appears to makes things tidier.

Stefan / Paolo? If you agree I will review in greater detail.

-- 
Alex Bligh

Re: [Qemu-devel] [PATCH 0/7] timer: move APIs together according to their category

[Paolo Bonzini]

Il 25/02/2014 17:25, Alex Bligh ha scritto:
>
> I'm generally in favour of this patch set or something like it. It's
> nicely constrained to 2 include files. It appears to touch no code.
> And it appears to makes things tidier.
>
> Stefan / Paolo? If you agree I will review in greater detail.

Sure, any help with reviews is welcome.

Paolo

Re: [Qemu-devel] [PATCH 0/7] timer: move APIs together according to their category

[Alex Bligh]

On 25 Feb 2014, at 13:36, Xuebing Wang wrote:

> There are 6 categories of APIs:
> - QEMUClockType
> - QEMUTimerList
> - QEMUTimerListGroup
> - QEMUTimer
> - General utility functions
> - Low level clock functions
> 
> Problems / solutions with previous API implementations:
> 1) Can not think of cases that we need QEMUTimerList APIs.
>   Reference:  kernel_tree/Documentation/SubmittingPatches
>               -- Section "4) Don't over-design"
>   Purpose:    simplify API
>   Solution:   remove them from APIs
> 2) Implementations of some APIs are interwined
>   Purpose:    put them together according to their categories, thus new
>               engineers are easy to understand them
>   Solution:   put them together
> 3) Unnecessary header files are included
>   Purpose:    simplify the included header files
>   Solution:   only include the minimally required header file
> 
> This patchset includes below changes:
> - put QEMUTimerList related functions together and make them private
> - put QEMUClockType related functions together
> - move QEMUTimerListGroup function to be below QEMUClockType
> - put QEMUTimer related functions together
> - put general utility functions together
> - clean unnecessary #include and use minimal required #include
> 
> Xuebing Wang (7):
>  timer: move QEMUTimerList functions together
>  timer: make QEMUTimerList functions private (remove from APIs)
>  timer: move QEMUClockType related functions together
>  timer: move QEMUTimerListGroup function to be below QEMUClockType
>  timer: move QEMUTimer related functions together
>  timer: move general utility functions together
>  timer: clean unnecessary #include and use minimal required #include
> 
> include/qemu/timer.h |   94 --------
> qemu-timer.c         |  595 +++++++++++++++++++++++++++++---------------------
> 2 files changed, 341 insertions(+), 348 deletions(-)


As far as I can see, this patch set reorders qemu-timer.c in a more
logical way, and removes some functions from timer.h that need not
be exposed.

Therefore, this seems to me a good idea in principle, and:

Reviewed-By: Alex Bligh <alex@alex.org.uk>

on the lot, subject to the proviso I haven't compile tested them.

-- 
Alex Bligh