On Tue, Feb 28, 2012 at 10:02 AM, Oleg Nesterov <oleg@xxxxxxxxxx> wrote: > On 02/27, Will Drewry wrote: >> >> On Mon, Feb 27, 2012 at 11:22 AM, Oleg Nesterov <oleg@xxxxxxxxxx> wrote: >> > On 02/24, Will Drewry wrote: >> >> >> >> To ensure that SIGSYS delivery occurs on return from the triggering >> >> system call, SIGSYS is added to the SYNCHRONOUS_MASK macro. >> > >> > Hmm. Can't understand... please help. >> > >> >> #define SYNCHRONOUS_MASK \ >> >> (sigmask(SIGSEGV) | sigmask(SIGBUS) | sigmask(SIGILL) | \ >> >> - sigmask(SIGTRAP) | sigmask(SIGFPE)) >> >> + sigmask(SIGTRAP) | sigmask(SIGFPE) | sigmask(SIGSYS)) >> > >> > Why? >> > >> > SYNCHRONOUS_MASK just tells dequeue_signal() "pick them first". >> > This is needed to make sure that the handler for, say SIGSEGV, >> > can use ucontext->ip as a faulting addr. >> >> I think that Roland covered this. (Since the syscall_rollback was >> called it's nice to let our handler get first go.) > > OK, except I do not really understand the "our handler get first go". Err I meant "gets to go first". > Suppose SIGSYS "races" with the pending SIGHUP. With this change > the caller for SIGHUP will be called first. But yes, setup_frame() > will be called for SIGSYS first. Hopefully this is what you want. I believe it is. I just want ucontext_t to be properly populate since the registers were just rolled back for it. >> > But seccomp adds info->si_call_addr, this looks unneeded. >> >> True enough. I can drop it. > > Hmm. I meant, the change in SYNCHRONOUS_MASK looks unneeded. Please > keep ->si_call_addr, it is much more convenient than ucontext_t in > userspace. Sorry for the confusion >> It'd only be useful if the SIGSYS wasn't >> being forced and the signal was being handled without ucontext_t >> access. > > No, force_ doesn't make any difference in this sense... I guess I was thinking about users of signalfd wanting the call site but force_ avoids it being blockable so that seems largely irrelevant at present. > In short, the patch looks fine to me, but if you resend it may be > you can update the changelog. Thanks! I will try to clarify the changelog. will -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
