[Qemu-devel] [RFC PATCH 0/2] Signal fixes for OSX

[Qemu-devel] [RFC PATCH 0/2] Signal fixes for OSX

[Alexandre Raymond]

This series fixes a race condition that occurs under OS X.

It also reworks the signal initialization to make it simpler for later
maintenance/additions.

Note that although it _appears_ to fix this race condition, I have not been
able to pinpoint exactly how it is triggered.

Does anyone know if there is a specific reason why SIGUSR2 is not currently
handled via the signal thread?

Alexandre Raymond (2):
  Signals: fix race condition with aio-compat
  Signals: rework initial signal setup

 cpus.c |   43 +++++++++++++++++++++++--------------------
 1 files changed, 23 insertions(+), 20 deletions(-)

-- 
1.7.5

[Qemu-devel] [RFC PATCH 1/2] Signals: fix race condition with aio-compat

[Alexandre Raymond]

There appears to be a race condition when SIGUSR2 is not handled synchronously
by the signalfd thread. This caused random freezes/segfaults under OS X.

This fix also appears to fix most of the I/O errors that occur when the io-thread
is enabled on OS X.

Signed-off-by: Alexandre Raymond <cerbere@gmail.com>
---
 cpus.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/cpus.c b/cpus.c
index 3035314..f466d95 100644
--- a/cpus.c
+++ b/cpus.c
@@ -391,10 +391,6 @@ static int qemu_signal_init(void)
     sigset_t set;
 
 #ifdef CONFIG_IOTHREAD
-    /* SIGUSR2 used by posix-aio-compat.c */
-    sigemptyset(&set);
-    sigaddset(&set, SIGUSR2);
-    pthread_sigmask(SIG_UNBLOCK, &set, NULL);
 
     /*
      * SIG_IPI must be blocked in the main thread and must not be caught
@@ -406,11 +402,13 @@ static int qemu_signal_init(void)
     pthread_sigmask(SIG_BLOCK, &set, NULL);
 
     sigemptyset(&set);
+    sigaddset(&set, SIGUSR2);
     sigaddset(&set, SIGIO);
     sigaddset(&set, SIGALRM);
     sigaddset(&set, SIGBUS);
 #else
     sigemptyset(&set);
+    sigaddset(&set, SIGUSR2);
     sigaddset(&set, SIGBUS);
     if (kvm_enabled()) {
         /*
-- 
1.7.5

[Qemu-devel] [RFC PATCH 2/2] Signals: rework initial signal setup

[Alexandre Raymond]

-Restructure the signal setup by creating two groups:
  * blocked_set, which contains signals that are ignored by QEMU or caught
                 directly by a specific thread (e.g.: SIG_IPI).
  * handled_set, which contains signals handled synchronously via signalfd.

Signed-off-by: Alexandre Raymond <cerbere@gmail.com>
---
 cpus.c |   41 +++++++++++++++++++++++------------------
 1 files changed, 23 insertions(+), 18 deletions(-)

diff --git a/cpus.c b/cpus.c
index f466d95..565676a 100644
--- a/cpus.c
+++ b/cpus.c
@@ -388,40 +388,45 @@ static void sigfd_handler(void *opaque)
 static int qemu_signal_init(void)
 {
     int sigfd;
-    sigset_t set;
+    /*
+     * - blocked_set contains signals that are handled directly by a specific
+     *   thread such as SIG_IPI, which is caught directly by the cpu thread.
+     * - handled_set contains signals that are handled synchronously via the
+     *   signal thread.
+     */
+    sigset_t blocked_set;
+    sigset_t handled_set;
 
-#ifdef CONFIG_IOTHREAD
+    sigemptyset(&blocked_set);
+    sigemptyset(&handled_set);
 
+    /* SIGUSR2 used by posix-aio-compat.c */
+    sigaddset(&handled_set, SIGUSR2);
+    sigaddset(&handled_set, SIGBUS);
+#ifdef CONFIG_IOTHREAD
     /*
      * SIG_IPI must be blocked in the main thread and must not be caught
      * by sigwait() in the signal thread. Otherwise, the cpu thread will
      * not catch it reliably.
      */
-    sigemptyset(&set);
-    sigaddset(&set, SIG_IPI);
-    pthread_sigmask(SIG_BLOCK, &set, NULL);
+    sigaddset(&blocked_set, SIG_IPI);
 
-    sigemptyset(&set);
-    sigaddset(&set, SIGUSR2);
-    sigaddset(&set, SIGIO);
-    sigaddset(&set, SIGALRM);
-    sigaddset(&set, SIGBUS);
+    sigaddset(&handled_set, SIGIO);
+    sigaddset(&handled_set, SIGALRM);
 #else
-    sigemptyset(&set);
-    sigaddset(&set, SIGUSR2);
-    sigaddset(&set, SIGBUS);
     if (kvm_enabled()) {
         /*
          * We need to process timer signals synchronously to avoid a race
          * between exit_request check and KVM vcpu entry.
          */
-        sigaddset(&set, SIGIO);
-        sigaddset(&set, SIGALRM);
+        sigaddset(&handled_set, SIGIO);
+        sigaddset(&handled_set, SIGALRM);
     }
 #endif
-    pthread_sigmask(SIG_BLOCK, &set, NULL);
+    pthread_sigmask(SIG_BLOCK, &blocked_set, NULL);
+    pthread_sigmask(SIG_BLOCK, &handled_set, NULL);
 
-    sigfd = qemu_signalfd(&set);
+    sigfd = qemu_signalfd(&handled_set);
     if (sigfd == -1) {
         fprintf(stderr, "failed to create signalfd\n");
         return -errno;
-- 
1.7.5

Re: [Qemu-devel] [RFC PATCH 0/2] Signal fixes for OSX

[Alexandre Raymond]

Please ignore this patch series. It is most definitely wrong.

Alexandre

On Wed, Jul 20, 2011 at 1:01 AM, Alexandre Raymond <cerbere@gmail.com> wrote:
> This series fixes a race condition that occurs under OS X.
>
> It also reworks the signal initialization to make it simpler for later
> maintenance/additions.
>
> Note that although it _appears_ to fix this race condition, I have not been
> able to pinpoint exactly how it is triggered.
>
> Does anyone know if there is a specific reason why SIGUSR2 is not currently
> handled via the signal thread?
>
> Alexandre Raymond (2):
>  Signals: fix race condition with aio-compat
>  Signals: rework initial signal setup
>
>  cpus.c |   43 +++++++++++++++++++++++--------------------
>  1 files changed, 23 insertions(+), 20 deletions(-)
>
> --
> 1.7.5
>
>