Re: [PATCH] mm: Check for SIGKILL inside dup_mmap() loop.

[Matthew Wilcox <willy@infradead.org>]

On Thu, Mar 29, 2018 at 02:30:03PM -0700, Andrew Morton wrote:
> > @@ -440,6 +440,10 @@ static __latent_entropy int dup_mmap(struct mm_struct *mm,
> >  			continue;
> >  		}
> >  		charge = 0;
> > +		if (fatal_signal_pending(current)) {
> > +			retval = -EINTR;
> > +			goto out;
> > +		}
> >  		if (mpnt->vm_flags & VM_ACCOUNT) {
> >  			unsigned long len = vma_pages(mpnt);
> 
> I think a comment explaining why we're doing this would help.
> 
> Better would be to add a new function "current_is_oom_killed()" or
> such, which becomes self-documenting.  Because there are other reasons
> why a task may have a fatal signal pending.

I disagree that we need a comment here, or to create an alias.  Someone
who knows nothing of the oom-killer (like, er, me) reading that code sees
"Oh, we're checking for fatal signals here.  I guess it doesn't make sense
to continue forking a process if it's already received a fatal signal."

One might speculate about the causes of the fatal signal having been
received and settle on reasons which make sense even without thinking
of the OOM case.  Because it's why it was introduced, I always think
about a task blocked on a dead NFS mount.  If it's multithreaded and
one of the threads called fork() while another thread was blocked on a
page fault and the dup_mmap() had to wait for the page fault to finish
... that would make some kind of sense.