Re: [Qemu-devel] [PATCH] Fix signal handling of SIG_IPI when io-thread is enabled

2011-07-16 Thread Blue Swirl
Thanks, applied.

On Sat, Jul 9, 2011 at 5:33 AM, Alexandre Raymond  wrote:
> ping?
>
> On Wed, Jun 15, 2011 at 10:11 AM, Alexandre Raymond  wrote:
>> Hi Jan,
>>
>>> Why? Ahh, because of qemu_cpu_kick_self: raise(SIG_IPI)! That should
>>> generate a per-process SIG_IPI. And that may not only affect Darwin.
>>> Looks good.
>>
>> Actually, with io-thread enabled, it goes through qemu_cpu_kick_self()
>> -> qemu_cpu_kick_thread() -> pthread_kill(..., SIG_IPI).
>>
>> I think the problem is with sigwait(). It doesn't state so in the
>> Linux or Darwin man pages, but on Solaris, it says : "All signals
>> identified by the set argument must be blocked on all threads,
>> including the calling thread; otherwise, sigwait() might not work
>> correctly", which might correspond to the issue I've been witnessing
>> (ie: sigwait() unblocking once in a while on a SIGUSR1 (SIG_IPI) in
>> the event thread).
>>
>> In any case, I don't think it should attempt to catch this signal at
>> all since the cpu thread is already catching it.
>>
>> Alexandre
>>
>
>



Re: [Qemu-devel] [PATCH] Fix signal handling of SIG_IPI when io-thread is enabled

2011-07-08 Thread Alexandre Raymond
ping?

On Wed, Jun 15, 2011 at 10:11 AM, Alexandre Raymond  wrote:
> Hi Jan,
>
>> Why? Ahh, because of qemu_cpu_kick_self: raise(SIG_IPI)! That should
>> generate a per-process SIG_IPI. And that may not only affect Darwin.
>> Looks good.
>
> Actually, with io-thread enabled, it goes through qemu_cpu_kick_self()
> -> qemu_cpu_kick_thread() -> pthread_kill(..., SIG_IPI).
>
> I think the problem is with sigwait(). It doesn't state so in the
> Linux or Darwin man pages, but on Solaris, it says : "All signals
> identified by the set argument must be blocked on all threads,
> including the calling thread; otherwise, sigwait() might not work
> correctly", which might correspond to the issue I've been witnessing
> (ie: sigwait() unblocking once in a while on a SIGUSR1 (SIG_IPI) in
> the event thread).
>
> In any case, I don't think it should attempt to catch this signal at
> all since the cpu thread is already catching it.
>
> Alexandre
>



Re: [Qemu-devel] [PATCH] Fix signal handling of SIG_IPI when io-thread is enabled

2011-06-15 Thread Alexandre Raymond
Hi Jan,

> Why? Ahh, because of qemu_cpu_kick_self: raise(SIG_IPI)! That should
> generate a per-process SIG_IPI. And that may not only affect Darwin.
> Looks good.

Actually, with io-thread enabled, it goes through qemu_cpu_kick_self()
-> qemu_cpu_kick_thread() -> pthread_kill(..., SIG_IPI).

I think the problem is with sigwait(). It doesn't state so in the
Linux or Darwin man pages, but on Solaris, it says : "All signals
identified by the set argument must be blocked on all threads,
including the calling thread; otherwise, sigwait() might not work
correctly", which might correspond to the issue I've been witnessing
(ie: sigwait() unblocking once in a while on a SIGUSR1 (SIG_IPI) in
the event thread).

In any case, I don't think it should attempt to catch this signal at
all since the cpu thread is already catching it.

Alexandre



Re: [Qemu-devel] [PATCH] Fix signal handling of SIG_IPI when io-thread is enabled

2011-06-15 Thread Jan Kiszka
On 2011-06-15 07:20, Alexandre Raymond wrote:
> Both the signal thread (via sigwait()) and the cpu thread (via
> a normal signal handler) were attempting to catch SIG_IPI.

Why? Ahh, because of qemu_cpu_kick_self: raise(SIG_IPI)! That should
generate a per-process SIG_IPI. And that may not only affect Darwin.
Looks good.

Acked-by: Jan Kiszka 

> 
> This resulted in random freezes under Darwin.
> 
> This patch separates SIG_IPI from the rest of the signals handled
> by the signal thread, because it is independently caught by the cpu
> thread.
> 
> Signed-off-by: Alexandre Raymond 
> ---
>  cpus.c |   10 +-
>  1 files changed, 9 insertions(+), 1 deletions(-)
> 
> diff --git a/cpus.c b/cpus.c
> index 18a1522..84ffd1c 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -394,10 +394,18 @@ static int qemu_signal_init(void)
>  sigaddset(&set, SIGUSR2);
>  pthread_sigmask(SIG_UNBLOCK, &set, NULL);
>  
> +/*
> + * 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);
> +
>  sigemptyset(&set);
>  sigaddset(&set, SIGIO);
>  sigaddset(&set, SIGALRM);
> -sigaddset(&set, SIG_IPI);
>  sigaddset(&set, SIGBUS);
>  #else
>  sigemptyset(&set);

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



[Qemu-devel] [PATCH] Fix signal handling of SIG_IPI when io-thread is enabled

2011-06-14 Thread Alexandre Raymond
Both the signal thread (via sigwait()) and the cpu thread (via
a normal signal handler) were attempting to catch SIG_IPI.

This resulted in random freezes under Darwin.

This patch separates SIG_IPI from the rest of the signals handled
by the signal thread, because it is independently caught by the cpu
thread.

Signed-off-by: Alexandre Raymond 
---
 cpus.c |   10 +-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/cpus.c b/cpus.c
index 18a1522..84ffd1c 100644
--- a/cpus.c
+++ b/cpus.c
@@ -394,10 +394,18 @@ static int qemu_signal_init(void)
 sigaddset(&set, SIGUSR2);
 pthread_sigmask(SIG_UNBLOCK, &set, NULL);
 
+/*
+ * 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);
+
 sigemptyset(&set);
 sigaddset(&set, SIGIO);
 sigaddset(&set, SIGALRM);
-sigaddset(&set, SIG_IPI);
 sigaddset(&set, SIGBUS);
 #else
 sigemptyset(&set);
-- 
1.7.5