[Qemu-devel] [PATCH] sigfd: use pthread_sigmask

[Qemu-devel] [PATCH] sigfd: use pthread_sigmask

[Alexander Graf]

Qemu uses signalfd to figure out, if a signal occured without the need
to actually receive the signal. Instead, it can read from the fd to receive
its news.

Now, we obviously don't always have signalfd around. Especially not on
non-Linux systems. So what we do there is that we create a new thread,
block that thread on all signals and simply call sigwait to wait for a
signal we're interested in to occur.

This all sounds great, but what we're really doing is:

    sigset_t all;

    sigfillset(&all);
    sigprocmask(SIG_BLOCK, &all, NULL);

which - on Darwin - blocks all signals on the current _process_, not only
on the current thread. To block signals on the thread, we can use
pthread_sigmask().

This patch does that, assuming that my above analysis is correct, and thus
renders Qemu useable on Darwin again.

Reported-by: Andreas Färber <andreas.faerber@web.de>
CC: Paolo Bonzini <pbonzini@redhat.com>
CC: Jan Kiszka <jan.kiszka@siemens.com>
CC: Anthony Liguori <anthony@codemonkey.ws>
Signed-off-by: Alexander Graf <agraf@suse.de>
---
 compatfd.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/compatfd.c b/compatfd.c
index bd377c4..41586ce 100644
--- a/compatfd.c
+++ b/compatfd.c
@@ -29,7 +29,7 @@ static void *sigwait_compat(void *opaque)
     sigset_t all;
 
     sigfillset(&all);
-    sigprocmask(SIG_BLOCK, &all, NULL);
+    pthread_sigmask(SIG_BLOCK, &all, NULL);
 
     while (1) {
         int sig;
-- 
1.7.1

Re: [Qemu-devel] [PATCH] sigfd: use pthread_sigmask

[Alexandre Raymond]

On Wed, Jun 8, 2011 at 6:55 PM, Alexander Graf <agraf@suse.de> wrote:
> Qemu uses signalfd to figure out, if a signal occured without the need
> to actually receive the signal. Instead, it can read from the fd to receive
> its news.
>
> Now, we obviously don't always have signalfd around. Especially not on
> non-Linux systems. So what we do there is that we create a new thread,
> block that thread on all signals and simply call sigwait to wait for a
> signal we're interested in to occur.
>
> This all sounds great, but what we're really doing is:
>
>    sigset_t all;
>
>    sigfillset(&all);
>    sigprocmask(SIG_BLOCK, &all, NULL);
>
> which - on Darwin - blocks all signals on the current _process_, not only
> on the current thread. To block signals on the thread, we can use
> pthread_sigmask().
>
> This patch does that, assuming that my above analysis is correct, and thus
> renders Qemu useable on Darwin again.
I confirm that Qemu works much better on Darwin with this patch :)

Thanks!
Alexandre

Re: [Qemu-devel] [PATCH] sigfd: use pthread_sigmask

[Paolo Bonzini]

On 06/09/2011 12:55 AM, Alexander Graf wrote:
> Qemu uses signalfd to figure out, if a signal occured without the need
> to actually receive the signal. Instead, it can read from the fd to receive
> its news.
>
> Now, we obviously don't always have signalfd around. Especially not on
> non-Linux systems. So what we do there is that we create a new thread,
> block that thread on all signals and simply call sigwait to wait for a
> signal we're interested in to occur.
>
> This all sounds great, but what we're really doing is:
>
>      sigset_t all;
>
>      sigfillset(&all);
>      sigprocmask(SIG_BLOCK,&all, NULL);
>
> which - on Darwin - blocks all signals on the current_process_, not only
> on the current thread. To block signals on the thread, we can use
> pthread_sigmask().
>
> This patch does that, assuming that my above analysis is correct, and thus
> renders Qemu useable on Darwin again.
>
> Reported-by: Andreas Färber<andreas.faerber@web.de>
> CC: Paolo Bonzini<pbonzini@redhat.com>
> CC: Jan Kiszka<jan.kiszka@siemens.com>
> CC: Anthony Liguori<anthony@codemonkey.ws>
> Signed-off-by: Alexander Graf<agraf@suse.de>

Acked-by: Paolo Bonizni <pbonzini@redhat.com>

Paolo

Re: [Qemu-devel] [PATCH] sigfd: use pthread_sigmask

[Jan Kiszka]

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

On 2011-06-09 00:55, Alexander Graf wrote:
> Qemu uses signalfd to figure out, if a signal occured without the need
> to actually receive the signal. Instead, it can read from the fd to receive
> its news.
> 
> Now, we obviously don't always have signalfd around. Especially not on
> non-Linux systems. So what we do there is that we create a new thread,
> block that thread on all signals and simply call sigwait to wait for a
> signal we're interested in to occur.
> 
> This all sounds great, but what we're really doing is:
> 
>     sigset_t all;
> 
>     sigfillset(&all);
>     sigprocmask(SIG_BLOCK, &all, NULL);
> 
> which - on Darwin - blocks all signals on the current _process_, not only
> on the current thread. To block signals on the thread, we can use
> pthread_sigmask().
> 
> This patch does that, assuming that my above analysis is correct, and thus
> renders Qemu useable on Darwin again.
> 
> Reported-by: Andreas Färber <andreas.faerber@web.de>
> CC: Paolo Bonzini <pbonzini@redhat.com>
> CC: Jan Kiszka <jan.kiszka@siemens.com>
> CC: Anthony Liguori <anthony@codemonkey.ws>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  compatfd.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/compatfd.c b/compatfd.c
> index bd377c4..41586ce 100644
> --- a/compatfd.c
> +++ b/compatfd.c
> @@ -29,7 +29,7 @@ static void *sigwait_compat(void *opaque)
>      sigset_t all;
>  
>      sigfillset(&all);
> -    sigprocmask(SIG_BLOCK, &all, NULL);
> +    pthread_sigmask(SIG_BLOCK, &all, NULL);
>  
>      while (1) {
>          int sig;

Makes a lot of sense. And it also effects pre-signalfd Linux (<2.6.27).

Acked-by: Jan Kiszka <jan.kiszka@siemens.com>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

Re: [Qemu-devel] [PATCH] sigfd: use pthread_sigmask

[Andreas Färber]

Am 09.06.2011 um 00:55 schrieb Alexander Graf:

> Qemu uses signalfd to figure out, if a signal occured without the need
> to actually receive the signal. Instead, it can read from the fd to  
> receive
> its news.
>
> Now, we obviously don't always have signalfd around. Especially not on
> non-Linux systems. So what we do there is that we create a new thread,
> block that thread on all signals and simply call sigwait to wait for a
> signal we're interested in to occur.
>
> This all sounds great, but what we're really doing is:
>
>    sigset_t all;
>
>    sigfillset(&all);
>    sigprocmask(SIG_BLOCK, &all, NULL);
>
> which - on Darwin - blocks all signals on the current _process_, not  
> only
> on the current thread. To block signals on the thread, we can use
> pthread_sigmask().
>
> This patch does that, assuming that my above analysis is correct,  
> and thus
> renders Qemu useable on Darwin again.
>
> Reported-by: Andreas Färber <andreas.faerber@web.de>
> CC: Paolo Bonzini <pbonzini@redhat.com>
> CC: Jan Kiszka <jan.kiszka@siemens.com>
> CC: Anthony Liguori <anthony@codemonkey.ws>
> Signed-off-by: Alexander Graf <agraf@suse.de>

According to POSIX:2008, the use of sigprocmask() is only well-defined  
for a single-threaded process.

This patch fixed the default configuration (without --enable-io- 
thread) for me.

Thanks,
Andreas

> ---
> compatfd.c |    2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/compatfd.c b/compatfd.c
> index bd377c4..41586ce 100644
> --- a/compatfd.c
> +++ b/compatfd.c
> @@ -29,7 +29,7 @@ static void *sigwait_compat(void *opaque)
>     sigset_t all;
>
>     sigfillset(&all);
> -    sigprocmask(SIG_BLOCK, &all, NULL);
> +    pthread_sigmask(SIG_BLOCK, &all, NULL);
>
>     while (1) {
>         int sig;
> -- 
> 1.7.1

Re: [Qemu-devel] [PATCH] sigfd: use pthread_sigmask

[Andreas Färber]

Am 09.06.2011 um 14:36 schrieb Andreas Färber:

> Am 09.06.2011 um 00:55 schrieb Alexander Graf:
>
>> Qemu uses signalfd to figure out, if a signal occured without the  
>> need
>> to actually receive the signal. Instead, it can read from the fd to  
>> receive
>> its news.
>>
>> Now, we obviously don't always have signalfd around. Especially not  
>> on
>> non-Linux systems. So what we do there is that we create a new  
>> thread,
>> block that thread on all signals and simply call sigwait to wait  
>> for a
>> signal we're interested in to occur.
>>
>> This all sounds great, but what we're really doing is:
>>
>>   sigset_t all;
>>
>>   sigfillset(&all);
>>   sigprocmask(SIG_BLOCK, &all, NULL);
>>
>> which - on Darwin - blocks all signals on the current _process_,  
>> not only
>> on the current thread. To block signals on the thread, we can use
>> pthread_sigmask().
>>
>> This patch does that, assuming that my above analysis is correct,  
>> and thus
>> renders Qemu useable on Darwin again.
>>
>> Reported-by: Andreas Färber <andreas.faerber@web.de>
>> CC: Paolo Bonzini <pbonzini@redhat.com>
>> CC: Jan Kiszka <jan.kiszka@siemens.com>
>> CC: Anthony Liguori <anthony@codemonkey.ws>
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>
> According to POSIX:2008, the use of sigprocmask() is only well- 
> defined for a single-threaded process.

And of course I forgot:

Acked-by: Andreas Färber <andreas.faerber@web.de>

> This patch fixed the default configuration (without --enable-io- 
> thread) for me.

> Thanks,
> Andreas
>
>> ---
>> compatfd.c |    2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/compatfd.c b/compatfd.c
>> index bd377c4..41586ce 100644
>> --- a/compatfd.c
>> +++ b/compatfd.c
>> @@ -29,7 +29,7 @@ static void *sigwait_compat(void *opaque)
>>    sigset_t all;
>>
>>    sigfillset(&all);
>> -    sigprocmask(SIG_BLOCK, &all, NULL);
>> +    pthread_sigmask(SIG_BLOCK, &all, NULL);
>>
>>    while (1) {
>>        int sig;
>> -- 
>> 1.7.1
>
>

Re: [Qemu-devel] [PATCH] sigfd: use pthread_sigmask

[Edgar E. Iglesias]

On Thu, Jun 09, 2011 at 12:55:37AM +0200, Alexander Graf wrote:
> Qemu uses signalfd to figure out, if a signal occured without the need
> to actually receive the signal. Instead, it can read from the fd to receive
> its news.
> 
> Now, we obviously don't always have signalfd around. Especially not on
> non-Linux systems. So what we do there is that we create a new thread,
> block that thread on all signals and simply call sigwait to wait for a
> signal we're interested in to occur.
> 
> This all sounds great, but what we're really doing is:
> 
>     sigset_t all;
> 
>     sigfillset(&all);
>     sigprocmask(SIG_BLOCK, &all, NULL);
> 
> which - on Darwin - blocks all signals on the current _process_, not only
> on the current thread. To block signals on the thread, we can use
> pthread_sigmask().
> 
> This patch does that, assuming that my above analysis is correct, and thus
> renders Qemu useable on Darwin again.


Applied, thanks all.

Cheers


> 
> Reported-by: Andreas Färber <andreas.faerber@web.de>
> CC: Paolo Bonzini <pbonzini@redhat.com>
> CC: Jan Kiszka <jan.kiszka@siemens.com>
> CC: Anthony Liguori <anthony@codemonkey.ws>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  compatfd.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/compatfd.c b/compatfd.c
> index bd377c4..41586ce 100644
> --- a/compatfd.c
> +++ b/compatfd.c
> @@ -29,7 +29,7 @@ static void *sigwait_compat(void *opaque)
>      sigset_t all;
>  
>      sigfillset(&all);
> -    sigprocmask(SIG_BLOCK, &all, NULL);
> +    pthread_sigmask(SIG_BLOCK, &all, NULL);
>  
>      while (1) {
>          int sig;
> -- 
> 1.7.1