| Message ID | 1308115231-33690-1-git-send-email-cerbere@gmail.com |
|---|---|
| State | New |
| Headers | show |
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 <jan.kiszka@siemens.com> > > 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 <cerbere@gmail.com> > --- > 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);
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
ping? On Wed, Jun 15, 2011 at 10:11 AM, Alexandre Raymond <cerbere@gmail.com> 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 >
Thanks, applied. On Sat, Jul 9, 2011 at 5:33 AM, Alexandre Raymond <cerbere@gmail.com> wrote: > ping? > > On Wed, Jun 15, 2011 at 10:11 AM, Alexandre Raymond <cerbere@gmail.com> 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 >> > >
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);
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 <cerbere@gmail.com> --- cpus.c | 10 +++++++++- 1 files changed, 9 insertions(+), 1 deletions(-)