[Qemu-devel] [PATCH v3 0/2] Make simpletrace work on Windows

[Qemu-devel] [PATCH v3 0/2] Make simpletrace work on Windows

[stefanha]

From: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>

The 'simple' trace backend uses pthreads and does not work on Windows.  These
patches switch from pthreads to glib so that the code builds on all platforms
supported by glib.

v3:
 * Explain that glib threads need to be used directly
 * Explain the signal masking

v2:
 * Block signals in trace write-out thread

Stefan Hajnoczi (2):
  trace: portable simple trace backend using glib
  trace: use binary file open mode in simpletrace

 trace/simple.c |   76 ++++++++++++++++++++++++++++++++++++--------------------
 1 files changed, 49 insertions(+), 27 deletions(-)

-- 
1.7.5.4

[Qemu-devel] [PATCH v3 1/2] trace: portable simple trace backend using glib

[stefanha]

From: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>

Convert the simple trace backend to glib so that it works under Windows.
We cannot use pthread directly but glib provides portable abstractions.
Also use glib atomics instead of newish gcc builtins which may not be
supported on Windows toolchains.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 trace/simple.c |   74 ++++++++++++++++++++++++++++++++++++-------------------
 1 files changed, 48 insertions(+), 26 deletions(-)

diff --git a/trace/simple.c b/trace/simple.c
index a609368..885764a 100644
--- a/trace/simple.c
+++ b/trace/simple.c
@@ -12,8 +12,10 @@
 #include <stdint.h>
 #include <stdio.h>
 #include <time.h>
+#ifndef _WIN32
 #include <signal.h>
 #include <pthread.h>
+#endif
 #include "qemu-timer.h"
 #include "trace.h"
 #include "trace/control.h"
@@ -54,9 +56,9 @@ enum {
  * Trace records are written out by a dedicated thread.  The thread waits for
  * records to become available, writes them out, and then waits again.
  */
-static pthread_mutex_t trace_lock = PTHREAD_MUTEX_INITIALIZER;
-static pthread_cond_t trace_available_cond = PTHREAD_COND_INITIALIZER;
-static pthread_cond_t trace_empty_cond = PTHREAD_COND_INITIALIZER;
+static GStaticMutex trace_lock = G_STATIC_MUTEX_INIT;
+static GCond *trace_available_cond;
+static GCond *trace_empty_cond;
 static bool trace_available;
 static bool trace_writeout_enabled;
 
@@ -93,29 +95,30 @@ static bool get_trace_record(unsigned int idx, TraceRecord *record)
  */
 static void flush_trace_file(bool wait)
 {
-    pthread_mutex_lock(&trace_lock);
+    g_static_mutex_lock(&trace_lock);
     trace_available = true;
-    pthread_cond_signal(&trace_available_cond);
+    g_cond_signal(trace_available_cond);
 
     if (wait) {
-        pthread_cond_wait(&trace_empty_cond, &trace_lock);
+        g_cond_wait(trace_empty_cond, g_static_mutex_get_mutex(&trace_lock));
     }
 
-    pthread_mutex_unlock(&trace_lock);
+    g_static_mutex_unlock(&trace_lock);
 }
 
 static void wait_for_trace_records_available(void)
 {
-    pthread_mutex_lock(&trace_lock);
+    g_static_mutex_lock(&trace_lock);
     while (!(trace_available && trace_writeout_enabled)) {
-        pthread_cond_signal(&trace_empty_cond);
-        pthread_cond_wait(&trace_available_cond, &trace_lock);
+        g_cond_signal(trace_empty_cond);
+        g_cond_wait(trace_available_cond,
+                    g_static_mutex_get_mutex(&trace_lock));
     }
     trace_available = false;
-    pthread_mutex_unlock(&trace_lock);
+    g_static_mutex_unlock(&trace_lock);
 }
 
-static void *writeout_thread(void *opaque)
+static gpointer writeout_thread(gpointer opaque)
 {
     TraceRecord record;
     unsigned int writeout_idx = 0;
@@ -159,7 +162,7 @@ static void trace(TraceEventID event, uint64_t x1, uint64_t x2, uint64_t x3,
 
     timestamp = get_clock();
 
-    idx = __sync_fetch_and_add(&trace_idx, 1) % TRACE_BUF_LEN;
+    idx = g_atomic_int_exchange_and_add((gint *)&trace_idx, 1) % TRACE_BUF_LEN;
     trace_buf[idx] = (TraceRecord){
         .event = event,
         .timestamp_ns = timestamp,
@@ -331,28 +334,47 @@ bool trace_event_set_state(const char *name, bool state)
     return false;
 }
 
-bool trace_backend_init(const char *events, const char *file)
+/* Helper function to create a thread with signals blocked.  Use glib's
+ * portable threads since QEMU abstractions cannot be used due to reentrancy in
+ * the tracer.  Also note the signal masking on POSIX hosts so that the thread
+ * does not steal signals when the rest of the program wants them blocked.
+ */
+static GThread *trace_thread_create(GThreadFunc fn)
 {
-    pthread_t thread;
-    pthread_attr_t attr;
+    GThread *thread;
+#ifndef _WIN32
     sigset_t set, oldset;
-    int ret;
-
-    pthread_attr_init(&attr);
-    pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED);
 
     sigfillset(&set);
     pthread_sigmask(SIG_SETMASK, &set, &oldset);
-    ret = pthread_create(&thread, &attr, writeout_thread, NULL);
+#endif
+    thread = g_thread_create(writeout_thread, NULL, FALSE, NULL);
+#ifndef _WIN32
     pthread_sigmask(SIG_SETMASK, &oldset, NULL);
+#endif
 
-    if (ret != 0) {
+    return thread;
+}
+
+bool trace_backend_init(const char *events, const char *file)
+{
+    GThread *thread;
+
+    if (!g_thread_supported()) {
+        g_thread_init(NULL);
+    }
+
+    trace_available_cond = g_cond_new();
+    trace_empty_cond = g_cond_new();
+
+    thread = trace_thread_create(writeout_thread);
+    if (!thread) {
         fprintf(stderr, "warning: unable to initialize simple trace backend\n");
-    } else {
-        atexit(st_flush_trace_buffer);
-        trace_backend_init_events(events);
-        st_set_trace_file(file);
+        return false;
     }
 
+    atexit(st_flush_trace_buffer);
+    trace_backend_init_events(events);
+    st_set_trace_file(file);
     return true;
 }
-- 
1.7.5.4

[Qemu-devel] [PATCH v3 2/2] trace: use binary file open mode in simpletrace

[stefanha]

From: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>

For Windows portability the simple trace backend must use the 'b' file
open mode.  This prevents the stdio library from mangling 0x0a/0x0d
newline characters.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 trace/simple.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/trace/simple.c b/trace/simple.c
index 885764a..b639dda 100644
--- a/trace/simple.c
+++ b/trace/simple.c
@@ -234,7 +234,7 @@ void st_set_trace_file_enabled(bool enable)
             .x1 = HEADER_VERSION,
         };
 
-        trace_fp = fopen(trace_file_name, "w");
+        trace_fp = fopen(trace_file_name, "wb");
         if (!trace_fp) {
             return;
         }
-- 
1.7.5.4