[PATCH][QEMU] Make active-timers thread-safe

[PATCH][QEMU] Make active-timers thread-safe

[Ben Guthro]

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

Protect active_timers[] lists with mutexes so timers can be safely used from different threads.

Signed-off-by: Ben Guthro <bguthro@vrtualiron.com>
Signed-off-by: Gary Grebus <ggrebus@virtualiron.com>


[-- Attachment #2: qemu-threadsafe-timers.patch --]
[-- Type: text/x-patch, Size: 3089 bytes --]

diff -r 475eda44cd99 tools/ioemu/vl.c
--- a/tools/ioemu/vl.c	Mon Aug 27 14:14:56 2007 -0400
+++ b/tools/ioemu/vl.c	Mon Aug 27 14:14:56 2007 -0400
@@ -759,6 +759,7 @@ QEMUClock *vm_clock;
 QEMUClock *vm_clock;
 
 static QEMUTimer *active_timers[2];
+static pthread_mutex_t active_timers_lock[2];
 #ifdef _WIN32
 static MMRESULT timerID;
 static HANDLE host_alarm = NULL;
@@ -795,7 +796,7 @@ void qemu_free_timer(QEMUTimer *ts)
 }
 
 /* stop a timer, but do not dealloc it */
-void qemu_del_timer(QEMUTimer *ts)
+static void qemu_del_timer_nolock(QEMUTimer *ts)
 {
     QEMUTimer **pt, *t;
 
@@ -808,10 +809,18 @@ void qemu_del_timer(QEMUTimer *ts)
             break;
         if (t == ts) {
             *pt = t->next;
+	    t->next = NULL;
             break;
         }
         pt = &t->next;
     }
+}
+
+void qemu_del_timer(QEMUTimer *ts)
+{
+    pthread_mutex_lock(&active_timers_lock[ts->clock->type]);
+    qemu_del_timer_nolock(ts);
+    pthread_mutex_unlock(&active_timers_lock[ts->clock->type]);
 }
 
 void qemu_advance_timer(QEMUTimer *ts, int64_t expire_time)
@@ -826,11 +835,13 @@ void qemu_mod_timer(QEMUTimer *ts, int64
 {
     QEMUTimer **pt, *t;
 
-    qemu_del_timer(ts);
-
-    /* add the timer in the sorted list */
     /* NOTE: this code must be signal safe because
        qemu_timer_expired() can be called from a signal. */
+    pthread_mutex_lock(&active_timers_lock[ts->clock->type]);
+
+    qemu_del_timer_nolock(ts);
+
+    /* add the timer in the sorted list */
     pt = &active_timers[ts->clock->type];
     for(;;) {
         t = *pt;
@@ -843,16 +854,20 @@ void qemu_mod_timer(QEMUTimer *ts, int64
     ts->expire_time = expire_time;
     ts->next = *pt;
     *pt = ts;
+    pthread_mutex_unlock(&active_timers_lock[ts->clock->type]);
 }
 
 int qemu_timer_pending(QEMUTimer *ts)
 {
     QEMUTimer *t;
+    pthread_mutex_lock(&active_timers_lock[ts->clock->type]);
     for(t = active_timers[ts->clock->type]; t != NULL; t = t->next) {
         if (t == ts)
-            return 1;
-    }
-    return 0;
+            break;
+    }
+    pthread_mutex_unlock(&active_timers_lock[ts->clock->type]);
+    return !!t;
+
 }
 
 static inline int qemu_timer_expired(QEMUTimer *timer_head, int64_t current_time)
@@ -865,7 +880,12 @@ static void qemu_run_timers(QEMUTimer **
 static void qemu_run_timers(QEMUTimer **ptimer_head, int64_t current_time)
 {
     QEMUTimer *ts;
+    int clock_type;
     
+    if (*ptimer_head == NULL)
+	return;
+    clock_type = (*ptimer_head)->clock->type;
+    pthread_mutex_lock(&active_timers_lock[clock_type]);
     for(;;) {
         ts = *ptimer_head;
         if (!ts || ts->expire_time > current_time)
@@ -875,8 +895,11 @@ static void qemu_run_timers(QEMUTimer **
         ts->next = NULL;
         
         /* run the callback (the timer list can be modified) */
+	pthread_mutex_unlock(&active_timers_lock[clock_type]);
         ts->cb(ts->opaque);
-    }
+	pthread_mutex_lock(&active_timers_lock[clock_type]);
+    }
+    pthread_mutex_unlock(&active_timers_lock[clock_type]);
 }
 
 int64_t qemu_get_clock(QEMUClock *clock)

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH][QEMU] Make active-timers thread-safe

[Dave Lively]

Hi Daniel -
  While I agree this patch wasn't necessary, I was curious why it was
even compiling for us (it was "auto-ported" to our unstable tree
without human intervention) since the patch introduces uses of pthread
data structures but doesn't introduce an include of <pthread.h>.  I
notice tools/ioemu/vl.h is still including <pthread.h> for i386/x86_64
to implement a mapcache lock.  Isn't this now unnecessary for the same
reason?

Dave

On 8/27/07, Daniel P. Berrange <berrange@redhat.com> wrote:
> On Mon, Aug 27, 2007 at 02:58:26PM -0400, Ben Guthro wrote:
> > Protect active_timers[] lists with mutexes so timers can be safely used
> > from different threads.
>
> What code in QEMU is actually still using threads ? There used to be a Xen
> specific patch to the IDE layer to perform DMA ops in threads, but that was
> removed when xen-unstable synced with QEMU 0.9.0. Not aware of anything
> else using threads in the QEMU device model - its pretty dangerous because
> QEMU's internal data structures basically all assume single-threaded access.
>
> Regards,
> Dan.
> --
> |=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
> |=-           Perl modules: http://search.cpan.org/~danberr/              -=|
> |=-               Projects: http://freshmeat.net/~danielpb/               -=|
> |=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=|
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>