[Qemu-devel] [PATCH] qemu-timer: check active_timers outside lock/event

[Qemu-devel] [PATCH] qemu-timer: check active_timers outside lock/event

[Paolo Bonzini]

This avoids taking the active_timers_lock or resetting/setting the
timers_done_ev if there are no active timers.  This removes a small
(2-3%) source of overhead for dataplane.  The list is then checked
again inside the lock, or a NULL pointer could be dereferenced.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-timer.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/qemu-timer.c b/qemu-timer.c
index 9299cdc..ff620ec 100644
--- a/qemu-timer.c
+++ b/qemu-timer.c
@@ -174,7 +174,7 @@ void qemu_clock_enable(QEMUClockType type, bool enabled)
 
 bool timerlist_has_timers(QEMUTimerList *timer_list)
 {
-    return !!timer_list->active_timers;
+    return !!atomic_read(&timer_list->active_timers);
 }
 
 bool qemu_clock_has_timers(QEMUClockType type)
@@ -187,6 +187,10 @@ bool timerlist_expired(QEMUTimerList *timer_list)
 {
     int64_t expire_time;
 
+    if (!atomic_read(&timer_list->active_timers)) {
+        return false;
+    }
+
     qemu_mutex_lock(&timer_list->active_timers_lock);
     if (!timer_list->active_timers) {
         qemu_mutex_unlock(&timer_list->active_timers_lock);
@@ -214,6 +218,10 @@ int64_t timerlist_deadline_ns(QEMUTimerList *timer_list)
     int64_t delta;
     int64_t expire_time;
 
+    if (!atomic_read(&timer_list->active_timers)) {
+        return -1;
+    }
+
     if (!timer_list->clock->enabled) {
         return -1;
     }
@@ -363,7 +371,7 @@ static void timer_del_locked(QEMUTimerList *timer_list, QEMUTimer *ts)
         if (!t)
             break;
         if (t == ts) {
-            *pt = t->next;
+            atomic_set(pt, t->next);
             break;
         }
         pt = &t->next;
@@ -386,7 +394,7 @@ static bool timer_mod_ns_locked(QEMUTimerList *timer_list,
     }
     ts->expire_time = MAX(expire_time, 0);
     ts->next = *pt;
-    *pt = ts;
+    atomic_set(pt, ts);
 
     return pt == &timer_list->active_timers;
 }
@@ -481,8 +489,12 @@ bool timerlist_run_timers(QEMUTimerList *timer_list)
     QEMUTimerCB *cb;
     void *opaque;
 
+    if (!atomic_read(&timer_list->active_timers)) {
+        return false;
+    }
+
     qemu_event_reset(&timer_list->timers_done_ev);
-    if (!timer_list->clock->enabled || !timer_list->active_timers) {
+    if (!timer_list->clock->enabled) {
         goto out;
     }
 
-- 
2.9.3

Re: [Qemu-devel] [PATCH] qemu-timer: check active_timers outside lock/event

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 557 bytes --]

On Thu, Dec 01, 2016 at 10:03:43AM +0100, Paolo Bonzini wrote:
> This avoids taking the active_timers_lock or resetting/setting the
> timers_done_ev if there are no active timers.  This removes a small
> (2-3%) source of overhead for dataplane.  The list is then checked
> again inside the lock, or a NULL pointer could be dereferenced.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  qemu-timer.c | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]