qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/5] Fixes for thread pool patches.
@ 2012-11-02 13:14 Paolo Bonzini
  2012-11-02 13:14 ` [Qemu-devel] [PATCH 1/5] compiler: support Darwin weak references Paolo Bonzini
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Paolo Bonzini @ 2012-11-02 13:14 UTC (permalink / raw)
  To: qemu-devel; +Cc: blauwirbel, jan.kiszka, peter.maydell

Three fixes: 1) Darwin does not support weak aliases, use weak
references instead.  2) Darwin, NetBSD and OpenBSD do not have
sem_timedwait, implement counting semaphores with a mutex and
cv there.  3) Daemonize was broken, fixes are in patches 3-5.

Paolo Bonzini (5):
  compiler: support Darwin weak references
  semaphore: implement fallback counting semaphores with mutex+condvar
  qemu-timer: reinitialize timers after fork
  vl: unify calls to init_timer_alarm
  vl: delay thread initialization after daemonization

 compiler.h          |  9 ++++++-
 main-loop.c         |  6 +++--
 osdep.c             | 56 +++++++++++++++++++++++-----------------
 oslib-win32.c       | 12 +++++----
 qemu-sockets.c      | 40 ++++++++++++++++-------------
 qemu-thread-posix.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 qemu-thread-posix.h |  6 +++++
 qemu-timer.c        | 15 ++++++++++-
 qmp.c               |  2 ++
 vl.c                |  9 +++----
 10 file modificati, 172 inserzioni(+), 57 rimozioni(-)

-- 
1.7.12.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH 1/5] compiler: support Darwin weak references
  2012-11-02 13:14 [Qemu-devel] [PATCH 0/5] Fixes for thread pool patches Paolo Bonzini
@ 2012-11-02 13:14 ` Paolo Bonzini
  2012-11-02 13:46   ` Peter Maydell
  2012-11-02 13:14 ` [Qemu-devel] [PATCH 2/5] semaphore: implement fallback counting semaphores with mutex+condvar Paolo Bonzini
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Paolo Bonzini @ 2012-11-02 13:14 UTC (permalink / raw)
  To: qemu-devel; +Cc: blauwirbel, jan.kiszka, peter.maydell

Weakrefs only tell you if the symbol was defined elsewhere, so you
need a further check at runtime to pick the default definition
when needed.

This could be automated by the compiler, but it does not do it.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 compiler.h     |  9 ++++++++-
 osdep.c        | 56 ++++++++++++++++++++++++++++++++------------------------
 oslib-win32.c  | 12 +++++++-----
 qemu-sockets.c | 40 ++++++++++++++++++++++------------------
 qmp.c          |  2 ++
 5 file modificati, 71 inserzioni(+), 48 rimozioni(-)

diff --git a/compiler.h b/compiler.h
index 58865d6..4d411be 100644
--- a/compiler.h
+++ b/compiler.h
@@ -50,8 +50,15 @@
 #   define __printf__ __gnu_printf__
 #  endif
 # endif
-# define QEMU_WEAK_ALIAS(newname, oldname) \
+# if defined(__APPLE__)
+#  define QEMU_WEAK_ALIAS(newname, oldname) \
+        static typeof(oldname) weak_##newname __attribute__((weakref(#oldname)))
+#  define QEMU_WEAK_REF(newname, oldname) (weak_##newname ? weak_##newname : oldname)
+# else
+#  define QEMU_WEAK_ALIAS(newname, oldname) \
         typeof(oldname) newname __attribute__((weak, alias (#oldname)))
+#  define QEMU_WEAK_REF(newname, oldname) newname
+# endif
 #else
 #define GCC_ATTR /**/
 #define GCC_FMT_ATTR(n, m)
diff --git a/osdep.c b/osdep.c
index a87d4a4..2f7a491 100644
--- a/osdep.c
+++ b/osdep.c
@@ -54,6 +54,38 @@ static bool fips_enabled = false;
 
 static const char *qemu_version = QEMU_VERSION;
 
+static int default_fdset_get_fd(int64_t fdset_id, int flags)
+{
+    return -1;
+}
+QEMU_WEAK_ALIAS(monitor_fdset_get_fd, default_fdset_get_fd);
+#define monitor_fdset_get_fd \
+    QEMU_WEAK_REF(monitor_fdset_get_fd, default_fdset_get_fd)
+
+static int default_fdset_dup_fd_add(int64_t fdset_id, int dup_fd)
+{
+    return -1;
+}
+QEMU_WEAK_ALIAS(monitor_fdset_dup_fd_add, default_fdset_dup_fd_add);
+#define monitor_fdset_dup_fd_add \
+    QEMU_WEAK_REF(monitor_fdset_dup_fd_add, default_fdset_dup_fd_add)
+
+static int default_fdset_dup_fd_remove(int dup_fd)
+{
+    return -1;
+}
+QEMU_WEAK_ALIAS(monitor_fdset_dup_fd_remove, default_fdset_dup_fd_remove);
+#define monitor_fdset_dup_fd_remove \
+    QEMU_WEAK_REF(monitor_fdset_dup_fd_remove, default_fdset_dup_fd_remove)
+
+static int default_fdset_dup_fd_find(int dup_fd)
+{
+    return -1;
+}
+QEMU_WEAK_ALIAS(monitor_fdset_dup_fd_find, default_fdset_dup_fd_find);
+#define monitor_fdset_dup_fd_find \
+    QEMU_WEAK_REF(monitor_fdset_dup_fd_remove, default_fdset_dup_fd_find)
+
 int socket_set_cork(int fd, int v)
 {
 #if defined(SOL_TCP) && defined(TCP_CORK)
@@ -400,27 +432,3 @@ bool fips_get_state(void)
     return fips_enabled;
 }
 
-
-static int default_fdset_get_fd(int64_t fdset_id, int flags)
-{
-    return -1;
-}
-QEMU_WEAK_ALIAS(monitor_fdset_get_fd, default_fdset_get_fd);
-
-static int default_fdset_dup_fd_add(int64_t fdset_id, int dup_fd)
-{
-    return -1;
-}
-QEMU_WEAK_ALIAS(monitor_fdset_dup_fd_add, default_fdset_dup_fd_add);
-
-static int default_fdset_dup_fd_remove(int dup_fd)
-{
-    return -1;
-}
-QEMU_WEAK_ALIAS(monitor_fdset_dup_fd_remove, default_fdset_dup_fd_remove);
-
-static int default_fdset_dup_fd_find(int dup_fd)
-{
-    return -1;
-}
-QEMU_WEAK_ALIAS(monitor_fdset_dup_fd_find, default_fdset_dup_fd_find);
diff --git a/oslib-win32.c b/oslib-win32.c
index 9ca83df..326a2bd 100644
--- a/oslib-win32.c
+++ b/oslib-win32.c
@@ -32,6 +32,13 @@
 #include "trace.h"
 #include "qemu_socket.h"
 
+static void default_qemu_fd_register(int fd)
+{
+}
+QEMU_WEAK_ALIAS(qemu_fd_register, default_qemu_fd_register);
+#define qemu_fd_register \
+    QEMU_WEAK_REF(qemu_fd_register, default_qemu_fd_register)
+
 void *qemu_oom_check(void *ptr)
 {
     if (ptr == NULL) {
@@ -150,8 +157,3 @@ int qemu_get_thread_id(void)
 {
     return GetCurrentThreadId();
 }
-
-static void default_qemu_fd_register(int fd)
-{
-}
-QEMU_WEAK_ALIAS(qemu_fd_register, default_qemu_fd_register);
diff --git a/qemu-sockets.c b/qemu-sockets.c
index f2a6371..abcd791 100644
--- a/qemu-sockets.c
+++ b/qemu-sockets.c
@@ -61,6 +61,28 @@ static QemuOptsList dummy_opts = {
     },
 };
 
+static int default_monitor_get_fd(Monitor *mon, const char *name, Error **errp)
+{
+    error_setg(errp, "only QEMU supports file descriptor passing");
+    return -1;
+}
+QEMU_WEAK_ALIAS(monitor_get_fd, default_monitor_get_fd);
+#define monitor_get_fd \
+    QEMU_WEAK_REF(monitor_get_fd, default_monitor_get_fd)
+
+static int default_qemu_set_fd_handler2(int fd,
+                                        IOCanReadHandler *fd_read_poll,
+                                        IOHandler *fd_read,
+                                        IOHandler *fd_write,
+                                        void *opaque)
+
+{
+    abort();
+}
+QEMU_WEAK_ALIAS(qemu_set_fd_handler2, default_qemu_set_fd_handler2);
+#define qemu_set_fd_handler2 \
+    QEMU_WEAK_REF(qemu_set_fd_handler2, default_qemu_set_fd_handler2)
+
 static int inet_getport(struct addrinfo *e)
 {
     struct sockaddr_in *i4;
@@ -967,21 +989,3 @@ int socket_init(void)
 #endif
     return 0;
 }
-
-static int default_monitor_get_fd(Monitor *mon, const char *name, Error **errp)
-{
-    error_setg(errp, "only QEMU supports file descriptor passing");
-    return -1;
-}
-QEMU_WEAK_ALIAS(monitor_get_fd, default_monitor_get_fd);
-
-static int default_qemu_set_fd_handler2(int fd,
-                                        IOCanReadHandler *fd_read_poll,
-                                        IOHandler *fd_read,
-                                        IOHandler *fd_write,
-                                        void *opaque)
-
-{
-    abort();
-}
-QEMU_WEAK_ALIAS(qemu_set_fd_handler2, default_qemu_set_fd_handler2);
diff --git a/qmp.c b/qmp.c
index 638888a..13e83a5 100644
--- a/qmp.c
+++ b/qmp.c
@@ -477,6 +477,8 @@ static CpuDefinitionInfoList *default_arch_query_cpu_definitions(Error **errp)
     return NULL;
 }
 QEMU_WEAK_ALIAS(arch_query_cpu_definitions, default_arch_query_cpu_definitions);
+#define arch_query_cpu_definitions \
+    QEMU_WEAK_REF(arch_query_cpu_definitions, default_arch_query_cpu_definitions)
 
 CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
 {
-- 
1.7.12.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH 2/5] semaphore: implement fallback counting semaphores with mutex+condvar
  2012-11-02 13:14 [Qemu-devel] [PATCH 0/5] Fixes for thread pool patches Paolo Bonzini
  2012-11-02 13:14 ` [Qemu-devel] [PATCH 1/5] compiler: support Darwin weak references Paolo Bonzini
@ 2012-11-02 13:14 ` Paolo Bonzini
  2012-11-02 13:50   ` Peter Maydell
  2012-11-18  9:09   ` Brad Smith
  2012-11-02 13:14 ` [Qemu-devel] [PATCH 3/5] qemu-timer: reinitialize timers after fork Paolo Bonzini
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 11+ messages in thread
From: Paolo Bonzini @ 2012-11-02 13:14 UTC (permalink / raw)
  To: qemu-devel; +Cc: blauwirbel, jan.kiszka, peter.maydell

OpenBSD and Darwin do not have sem_timedwait.  Implement a fallback
for them.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-thread-posix.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 qemu-thread-posix.h |  6 +++++
 2 file modificati, 80 inserzioni(+)

diff --git a/qemu-thread-posix.c b/qemu-thread-posix.c
index 6a3d3a1..048db8f 100644
--- a/qemu-thread-posix.c
+++ b/qemu-thread-posix.c
@@ -122,36 +122,100 @@ void qemu_sem_init(QemuSemaphore *sem, int init)
 {
     int rc;
 
+#if defined(__OpenBSD__) || defined(__APPLE__) || defined(__NetBSD__)
+    rc = pthread_mutex_init(&sem->lock, NULL);
+    if (rc != 0) {
+        error_exit(rc, __func__);
+    }
+    rc = pthread_cond_init(&sem->cond, NULL);
+    if (rc != 0) {
+        error_exit(rc, __func__);
+    }
+    if (init < 0) {
+        error_exit(EINVAL, __func__);
+    }
+    sem->count = init;
+#else
     rc = sem_init(&sem->sem, 0, init);
     if (rc < 0) {
         error_exit(errno, __func__);
     }
+#endif
 }
 
 void qemu_sem_destroy(QemuSemaphore *sem)
 {
     int rc;
 
+#if defined(__OpenBSD__) || defined(__APPLE__) || defined(__NetBSD__)
+    rc = pthread_cond_destroy(&sem->cond);
+    if (rc < 0) {
+        error_exit(rc, __func__);
+    }
+    rc = pthread_mutex_destroy(&sem->lock);
+    if (rc < 0) {
+        error_exit(rc, __func__);
+    }
+#else
     rc = sem_destroy(&sem->sem);
     if (rc < 0) {
         error_exit(errno, __func__);
     }
+#endif
 }
 
 void qemu_sem_post(QemuSemaphore *sem)
 {
     int rc;
 
+#if defined(__OpenBSD__) || defined(__APPLE__) || defined(__NetBSD__)
+    pthread_mutex_lock(&sem->lock);
+    if (sem->count == INT_MAX) {
+        rc = EINVAL;
+    } else if (sem->count++ < 0) {
+        rc = pthread_cond_signal(&sem->cond);
+    } else {
+        rc = 0;
+    }
+    pthread_mutex_unlock(&sem->lock);
+    if (rc != 0) {
+        error_exit(rc, __func__);
+    }
+#else
     rc = sem_post(&sem->sem);
     if (rc < 0) {
         error_exit(errno, __func__);
     }
+#endif
 }
 
 int qemu_sem_timedwait(QemuSemaphore *sem, int ms)
 {
     int rc;
 
+#if defined(__OpenBSD__) || defined(__APPLE__) || defined(__NetBSD__)
+    struct timespec ts;
+    clock_gettime(CLOCK_REALTIME, &ts);
+    if (ms) {
+        int nsec = ts.tv_nsec + (ms % 1000) * 1000000;
+        ts.tv_sec += ms / 1000 + nsec / 1000000000;
+        ts.tv_nsec = nsec % 1000000000;
+    }
+
+    pthread_mutex_lock(&sem->lock);
+    --sem->count;
+    while (sem->count < 0) {
+        rc = pthread_cond_timedwait(&sem->cond, &sem->lock, &ts);
+        if (rc == ETIMEDOUT) {
+            break;
+        }
+        if (rc != 0) {
+            error_exit(rc, __func__);
+        }
+    }
+    pthread_mutex_unlock(&sem->lock);
+    return (rc == ETIMEDOUT ? -1 : 0);
+#else
     if (ms <= 0) {
         /* This is cheaper than sem_timedwait.  */
         do {
@@ -181,10 +245,19 @@ int qemu_sem_timedwait(QemuSemaphore *sem, int ms)
         error_exit(errno, __func__);
     }
     return 0;
+#endif
 }
 
 void qemu_sem_wait(QemuSemaphore *sem)
 {
+#if defined(__OpenBSD__) || defined(__APPLE__) || defined(__NetBSD__)
+    pthread_mutex_lock(&sem->lock);
+    --sem->count;
+    while (sem->count < 0) {
+        pthread_cond_wait(&sem->cond, &sem->lock);
+    }
+    pthread_mutex_unlock(&sem->lock);
+#else
     int rc;
 
     do {
@@ -193,6 +266,7 @@ void qemu_sem_wait(QemuSemaphore *sem)
     if (rc < 0) {
         error_exit(errno, __func__);
     }
+#endif
 }
 
 void qemu_thread_create(QemuThread *thread,
diff --git a/qemu-thread-posix.h b/qemu-thread-posix.h
index 2542c15..1c098c2 100644
--- a/qemu-thread-posix.h
+++ b/qemu-thread-posix.h
@@ -12,7 +12,13 @@ struct QemuCond {
 };
 
 struct QemuSemaphore {
+#if defined(__OpenBSD__) || defined(__APPLE__) || defined(__NetBSD__)
+    pthread_mutex_t lock;
+    pthread_cond_t cond;
+    int count;
+#else
     sem_t sem;
+#endif
 };
 
 struct QemuThread {
-- 
1.7.12.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH 3/5] qemu-timer: reinitialize timers after fork
  2012-11-02 13:14 [Qemu-devel] [PATCH 0/5] Fixes for thread pool patches Paolo Bonzini
  2012-11-02 13:14 ` [Qemu-devel] [PATCH 1/5] compiler: support Darwin weak references Paolo Bonzini
  2012-11-02 13:14 ` [Qemu-devel] [PATCH 2/5] semaphore: implement fallback counting semaphores with mutex+condvar Paolo Bonzini
@ 2012-11-02 13:14 ` Paolo Bonzini
  2012-11-02 13:14 ` [Qemu-devel] [PATCH 4/5] vl: unify calls to init_timer_alarm Paolo Bonzini
  2012-11-02 13:14 ` [Qemu-devel] [PATCH 5/5] vl: delay thread initialization after daemonization Paolo Bonzini
  4 siblings, 0 replies; 11+ messages in thread
From: Paolo Bonzini @ 2012-11-02 13:14 UTC (permalink / raw)
  To: qemu-devel; +Cc: blauwirbel, jan.kiszka, peter.maydell

Timers are not inherited by the child of a fork(2), so just use
pthread_atfork to reinstate them after daemonize.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-timer.c | 15 ++++++++++++++-
 1 file modificato, 14 inserzioni(+). 1 rimozione(-)

diff --git a/qemu-timer.c b/qemu-timer.c
index f3426c9..1d87694 100644
--- a/qemu-timer.c
+++ b/qemu-timer.c
@@ -742,6 +742,17 @@ static void quit_timers(void)
     t->stop(t);
 }
 
+static void reinit_timers(void)
+{
+    struct qemu_alarm_timer *t = alarm_timer;
+    t->stop(t);
+    if (t->start(t)) {
+        fprintf(stderr, "Internal timer error: aborting\n");
+        exit(1);
+    }
+    qemu_rearm_alarm_timer(t);
+}
+
 int init_timer_alarm(void)
 {
     struct qemu_alarm_timer *t = NULL;
@@ -765,6 +776,9 @@ int init_timer_alarm(void)
     }
 
     atexit(quit_timers);
+#ifdef CONFIG_POSIX
+    pthread_atfork(NULL, NULL, reinit_timers);
+#endif
     alarm_timer = t;
     return 0;
 
-- 
1.7.12.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH 4/5] vl: unify calls to init_timer_alarm
  2012-11-02 13:14 [Qemu-devel] [PATCH 0/5] Fixes for thread pool patches Paolo Bonzini
                   ` (2 preceding siblings ...)
  2012-11-02 13:14 ` [Qemu-devel] [PATCH 3/5] qemu-timer: reinitialize timers after fork Paolo Bonzini
@ 2012-11-02 13:14 ` Paolo Bonzini
  2012-11-02 13:14 ` [Qemu-devel] [PATCH 5/5] vl: delay thread initialization after daemonization Paolo Bonzini
  4 siblings, 0 replies; 11+ messages in thread
From: Paolo Bonzini @ 2012-11-02 13:14 UTC (permalink / raw)
  To: qemu-devel; +Cc: blauwirbel, jan.kiszka, peter.maydell

init_timer_alarm was being called twice.  This is not needed.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 main-loop.c | 5 ++++-
 vl.c        | 5 -----
 2 file modificati, 4 inserzioni(+), 6 rimozioni(-)

diff --git a/main-loop.c b/main-loop.c
index e43c7c8..234a313 100644
--- a/main-loop.c
+++ b/main-loop.c
@@ -123,7 +123,10 @@ int qemu_init_main_loop(void)
     GSource *src;
 
     init_clocks();
-    init_timer_alarm();
+    if (init_timer_alarm() < 0) {
+        fprintf(stderr, "could not initialize alarm timer\n");
+        exit(1);
+    }
 
     qemu_mutex_lock_iothread();
     ret = qemu_signal_init();
diff --git a/vl.c b/vl.c
index 99681da..e2d5276 100644
--- a/vl.c
+++ b/vl.c
@@ -3616,11 +3616,6 @@ int main(int argc, char **argv, char **envp)
             add_device_config(DEV_VIRTCON, "vc:80Cx24C");
     }
 
-    if (init_timer_alarm() < 0) {
-        fprintf(stderr, "could not initialize alarm timer\n");
-        exit(1);
-    }
-
     socket_init();
 
     if (qemu_opts_foreach(qemu_find_opts("chardev"), chardev_init_func, NULL, 1) != 0)
-- 
1.7.12.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH 5/5] vl: delay thread initialization after daemonization
  2012-11-02 13:14 [Qemu-devel] [PATCH 0/5] Fixes for thread pool patches Paolo Bonzini
                   ` (3 preceding siblings ...)
  2012-11-02 13:14 ` [Qemu-devel] [PATCH 4/5] vl: unify calls to init_timer_alarm Paolo Bonzini
@ 2012-11-02 13:14 ` Paolo Bonzini
  4 siblings, 0 replies; 11+ messages in thread
From: Paolo Bonzini @ 2012-11-02 13:14 UTC (permalink / raw)
  To: qemu-devel; +Cc: blauwirbel, jan.kiszka, peter.maydell

Commit ac4119c (chardev: Use timer instead of bottom-half to postpone
open event, 2012-10-12) moved the alarm timer initialization to an earlier
point but failed to consider that it depends on qemu_init_main_loop.

Later, commit 1c53786 (vl: init main loop earlier, 2012-10-30) fixed
this, but left -daemonize in two different ways.  First, timers need to
be reinitialized after forking.  Second, the global mutex was being held
by the parent, and thus dropped after forking.

The first is now fixed using pthread_atfork.  For the second part,
make sure that the global mutex is not taken before daemonization,
and similarly delay qemu_thread_self.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 main-loop.c | 1 -
 vl.c        | 4 +++-
 2 file modificati, 3 inserzioni(+), 2 rimozioni(-)

diff --git a/main-loop.c b/main-loop.c
index 234a313..c87624e 100644
--- a/main-loop.c
+++ b/main-loop.c
@@ -128,7 +128,6 @@ int qemu_init_main_loop(void)
         exit(1);
     }
 
-    qemu_mutex_lock_iothread();
     ret = qemu_signal_init();
     if (ret) {
         return ret;
diff --git a/vl.c b/vl.c
index e2d5276..0f5b07b 100644
--- a/vl.c
+++ b/vl.c
@@ -3477,7 +3477,6 @@ int main(int argc, char **argv, char **envp)
     }
     loc_set_none();
 
-    qemu_init_cpu_loop();
     if (qemu_init_main_loop()) {
         fprintf(stderr, "qemu_init_main_loop failed\n");
         exit(1);
@@ -3677,6 +3676,9 @@ int main(int argc, char **argv, char **envp)
 
     os_set_line_buffering();
 
+    qemu_init_cpu_loop();
+    qemu_mutex_lock_iothread();
+
 #ifdef CONFIG_SPICE
     /* spice needs the timers to be initialized by this point */
     qemu_spice_init();
-- 
1.7.12.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 1/5] compiler: support Darwin weak references
  2012-11-02 13:14 ` [Qemu-devel] [PATCH 1/5] compiler: support Darwin weak references Paolo Bonzini
@ 2012-11-02 13:46   ` Peter Maydell
  0 siblings, 0 replies; 11+ messages in thread
From: Peter Maydell @ 2012-11-02 13:46 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: blauwirbel, jan.kiszka, qemu-devel

On 2 November 2012 14:14, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Weakrefs only tell you if the symbol was defined elsewhere, so you
> need a further check at runtime to pick the default definition
> when needed.
>
> This could be automated by the compiler, but it does not do it.

clang doesn't error out anymore, but this version still provokes
a warning:

  CC    osdep.o
osdep.c:85:1: warning: unused function
'weak_monitor_fdset_dup_fd_find' [-Wunused-function]
QEMU_WEAK_ALIAS(monitor_fdset_dup_fd_find, default_fdset_dup_fd_find);
^
./compiler.h:55:32: note: expanded from macro 'QEMU_WEAK_ALIAS'
        static typeof(oldname) weak_##newname __attribute__((weakref(#oldname)))
                               ^
<scratch space>:147:1: note: expanded from macro 'weak_'
weak_monitor_fdset_dup_fd_find
^
1 warning generated.

-- PMM

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 2/5] semaphore: implement fallback counting semaphores with mutex+condvar
  2012-11-02 13:14 ` [Qemu-devel] [PATCH 2/5] semaphore: implement fallback counting semaphores with mutex+condvar Paolo Bonzini
@ 2012-11-02 13:50   ` Peter Maydell
  2012-11-18  9:09   ` Brad Smith
  1 sibling, 0 replies; 11+ messages in thread
From: Peter Maydell @ 2012-11-02 13:50 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: blauwirbel, jan.kiszka, qemu-devel

On 2 November 2012 14:14, Paolo Bonzini <pbonzini@redhat.com> wrote:
> +#if defined(__OpenBSD__) || defined(__APPLE__) || defined(__NetBSD__)
> +    struct timespec ts;
> +    clock_gettime(CLOCK_REALTIME, &ts);


qemu-thread-posix.c:198:5: warning: implicit declaration of function
'clock_gettime' is invalid in C99
      [-Wimplicit-function-declaration]
    clock_gettime(CLOCK_REALTIME, &ts);
    ^
qemu-thread-posix.c:198:19: error: use of undeclared identifier 'CLOCK_REALTIME'
    clock_gettime(CLOCK_REALTIME, &ts);
                  ^
1 warning and 1 error generated.
make: *** [qemu-thread-posix.o] Error 1

MacOS doesn't implement clock_gettime()...

-- PMM

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 2/5] semaphore: implement fallback counting semaphores with mutex+condvar
  2012-11-02 13:14 ` [Qemu-devel] [PATCH 2/5] semaphore: implement fallback counting semaphores with mutex+condvar Paolo Bonzini
  2012-11-02 13:50   ` Peter Maydell
@ 2012-11-18  9:09   ` Brad Smith
  2012-11-18 16:06     ` Paolo Bonzini
  1 sibling, 1 reply; 11+ messages in thread
From: Brad Smith @ 2012-11-18  9:09 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: blauwirbel, jan.kiszka, qemu-devel, peter.maydell

On 11/02/12 09:14, Paolo Bonzini wrote:
> OpenBSD and Darwin do not have sem_timedwait.  Implement a fallback
> for them.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   qemu-thread-posix.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>   qemu-thread-posix.h |  6 +++++
>   2 file modificati, 80 inserzioni(+)
>
> diff --git a/qemu-thread-posix.c b/qemu-thread-posix.c
> index 6a3d3a1..048db8f 100644
> --- a/qemu-thread-posix.c
> +++ b/qemu-thread-posix.c
> @@ -122,36 +122,100 @@ void qemu_sem_init(QemuSemaphore *sem, int init)
>   {
>       int rc;
>
> +#if defined(__OpenBSD__) || defined(__APPLE__) || defined(__NetBSD__)

OpenBSD 5.2 & -current (libpthread) / NetBSD -current (librt) have 
supported sem_timedwait() for roughly 8 months now. Please change this 
to properly test for the presence of sem_timedwait() within the 
configure script.

-- 
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 2/5] semaphore: implement fallback counting semaphores with mutex+condvar
  2012-11-18  9:09   ` Brad Smith
@ 2012-11-18 16:06     ` Paolo Bonzini
  2012-11-27  2:56       ` Brad Smith
  0 siblings, 1 reply; 11+ messages in thread
From: Paolo Bonzini @ 2012-11-18 16:06 UTC (permalink / raw)
  To: Brad Smith; +Cc: blauwirbel, jan.kiszka, qemu-devel, peter.maydell

Il 18/11/2012 10:09, Brad Smith ha scritto:
> On 11/02/12 09:14, Paolo Bonzini wrote:
>> OpenBSD and Darwin do not have sem_timedwait.  Implement a fallback
>> for them.
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>   qemu-thread-posix.c | 74
>> +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   qemu-thread-posix.h |  6 +++++
>>   2 file modificati, 80 inserzioni(+)
>>
>> diff --git a/qemu-thread-posix.c b/qemu-thread-posix.c
>> index 6a3d3a1..048db8f 100644
>> --- a/qemu-thread-posix.c
>> +++ b/qemu-thread-posix.c
>> @@ -122,36 +122,100 @@ void qemu_sem_init(QemuSemaphore *sem, int init)
>>   {
>>       int rc;
>>
>> +#if defined(__OpenBSD__) || defined(__APPLE__) || defined(__NetBSD__)
> 
> OpenBSD 5.2 & -current (libpthread) / NetBSD -current (librt) have
> supported sem_timedwait() for roughly 8 months now. Please change this
> to properly test for the presence of sem_timedwait() within the
> configure script.

Please submit a patch.  The patched code works, and it's not even
suboptimal because *BSD use a mutex/condvar to implement semaphores.  We
end up executing the very same code.

Paolo

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 2/5] semaphore: implement fallback counting semaphores with mutex+condvar
  2012-11-18 16:06     ` Paolo Bonzini
@ 2012-11-27  2:56       ` Brad Smith
  0 siblings, 0 replies; 11+ messages in thread
From: Brad Smith @ 2012-11-27  2:56 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: blauwirbel, jan.kiszka, qemu-devel, peter.maydell

On Sun, Nov 18, 2012 at 05:06:40PM +0100, Paolo Bonzini wrote:
> Il 18/11/2012 10:09, Brad Smith ha scritto:
> > On 11/02/12 09:14, Paolo Bonzini wrote:
> >> OpenBSD and Darwin do not have sem_timedwait.  Implement a fallback
> >> for them.
> >>
> >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> >> ---
> >>   qemu-thread-posix.c | 74
> >> +++++++++++++++++++++++++++++++++++++++++++++++++++++
> >>   qemu-thread-posix.h |  6 +++++
> >>   2 file modificati, 80 inserzioni(+)
> >>
> >> diff --git a/qemu-thread-posix.c b/qemu-thread-posix.c
> >> index 6a3d3a1..048db8f 100644
> >> --- a/qemu-thread-posix.c
> >> +++ b/qemu-thread-posix.c
> >> @@ -122,36 +122,100 @@ void qemu_sem_init(QemuSemaphore *sem, int init)
> >>   {
> >>       int rc;
> >>
> >> +#if defined(__OpenBSD__) || defined(__APPLE__) || defined(__NetBSD__)
> > 
> > OpenBSD 5.2 & -current (libpthread) / NetBSD -current (librt) have
> > supported sem_timedwait() for roughly 8 months now. Please change this
> > to properly test for the presence of sem_timedwait() within the
> > configure script.
> 
> Please submit a patch.  The patched code works, and it's not even
> suboptimal because *BSD use a mutex/condvar to implement semaphores.  We
> end up executing the very same code.

I understand what you mean. It's more so out of principle to try and remove
workarounds wherever possible when it is possible to do so.

-- 
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2012-11-27  2:56 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-11-02 13:14 [Qemu-devel] [PATCH 0/5] Fixes for thread pool patches Paolo Bonzini
2012-11-02 13:14 ` [Qemu-devel] [PATCH 1/5] compiler: support Darwin weak references Paolo Bonzini
2012-11-02 13:46   ` Peter Maydell
2012-11-02 13:14 ` [Qemu-devel] [PATCH 2/5] semaphore: implement fallback counting semaphores with mutex+condvar Paolo Bonzini
2012-11-02 13:50   ` Peter Maydell
2012-11-18  9:09   ` Brad Smith
2012-11-18 16:06     ` Paolo Bonzini
2012-11-27  2:56       ` Brad Smith
2012-11-02 13:14 ` [Qemu-devel] [PATCH 3/5] qemu-timer: reinitialize timers after fork Paolo Bonzini
2012-11-02 13:14 ` [Qemu-devel] [PATCH 4/5] vl: unify calls to init_timer_alarm Paolo Bonzini
2012-11-02 13:14 ` [Qemu-devel] [PATCH 5/5] vl: delay thread initialization after daemonization Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).