Hi, Olge: On 10/24/18 6:52 AM, Oleg Nesterov wrote: > On 10/23, Enke Chen wrote: >> >>>> + /* >>>> + * Send the pre-coredump signal to the parent if requested. >>>> + */ >>>> + read_lock(&tasklist_lock); >>>> + notify = do_notify_parent_predump(current); >>>> + read_unlock(&tasklist_lock); >>>> + if (notify) >>>> + cond_resched(); >>> >>> Hmm. I do not understand why do we need cond_resched(). And even if we need >>> it, >>> why we can't call it unconditionally? >> >> Remember the goal is to allow the parent (e.g., a process manager) to take >> early >> action. The "yield" before doing coredump will help. > > I don't see how can it actually help... > > cond_resched() is nop if CONFIG_PREEMPT or should_resched() == 0. > > and the coredumping thread will certainly need to sleep/wait anyway.
I am really surprised by this - cond_resched() is used in many places and it actually does not do anything w/o CONFIG_PREEMPT. Will remove. > >>> And once again, SIGCHLD/SIGUSR do not queue, this means that >>> PR_SET_PREDUMP_SIG >>> is pointless if you have 2 or more children. >> >> Hmm, could you point me to the code where SIGCHLD/SIGUSR is treated >> differently >> w.r.t. queuing? That does not sound right to me. > > see the legacy_queue() check. Any signal < SIGRTMIN do not queue. IOW, if > SIGCHLD > is already pending, then next SIGCHLD is simply ignored. Got it. This means that a distinct signal (in particular a RT signal) would be more preferred. This is what it is done in our application. You earlier suggestion about removing the signal limitation makes a lot sense to me now. Given that a distinct signal is more preferred, I am wondering if I should just remove CLD_PREDUMP from the patch. Thanks. -- Enke
