Re: [Qemu-devel] [patch 1/2] qemu: mempath: prefault pages manually

[Paolo Bonzini]

Il 08/10/2013 23:51, Marcelo Tosatti ha scritto:
> On Tue, Oct 08, 2013 at 10:03:48AM +0200, Paolo Bonzini wrote:
>> Il 08/10/2013 02:41, Marcelo Tosatti ha scritto:
>>> +        /* unblock SIGBUS */
>>> +        pthread_sigmask(SIG_BLOCK, NULL, &oldset);
>>> +        sigemptyset(&set);
>>> +        sigaddset(&set, SIGBUS);
>>> +        pthread_sigmask(SIG_UNBLOCK, &set, NULL);
>>
>> Please instead modify qemu-thread-posix.c to unblock all per-thread
>> signals (SIGBUS, SIGSEGV, SIGILL, SIGFPE and SIGSYS).  There is no need
>> to keep those blocked.
> 
> main-loop.c handles SIGBUS via signalfd to emulate MCEs (associated
> commits). Therefore it must be blocked.

How was that tested?  For BUS_MCEERR_AO it can work, but BUS_MCEERR_AR
calls force_sig_info which does this:

        ignored = action->sa.sa_handler == SIG_IGN;
        blocked = sigismember(&t->blocked, sig);
        if (blocked || ignored) {
                action->sa.sa_handler = SIG_DFL;
                if (blocked) {
                        sigdelset(&t->blocked, sig);
                        recalc_sigpending_and_wake(t);
                }
        }
        if (action->sa.sa_handler == SIG_DFL)
                t->signal->flags &= ~SIGNAL_UNKILLABLE;

and kills the process (because that's the default action of SIG_DFL).

> Note that what this patch does it to maintain the signal handling state
> (it saves the previous state, modifies state, restores previous state) so 
> that its unchanged.

Yes, understood.  I was missing the part about MCE (I knew it used
SIGBUS, but forgot about signalfd).  So this patch is good, but the
above point about BUS_MCEERR_AR needs to be checked sooner or later.

Paolo