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

[Matthew Wilcox <willy@infradead.org>]

On Fri, Mar 30, 2018 at 07:34:59PM +0900, Tetsuo Handa wrote:
> Maybe we can make "give up by default upon SIGKILL" and let callers
> explicitly say "do not give up upon SIGKILL".

I really strongly disapprove of this patch.  This GFP flag will be abused
like every other GFP flag.

> +++ b/mm/page_alloc.c
> @@ -4183,6 +4183,13 @@ bool gfp_pfmemalloc_allowed(gfp_t gfp_mask)
>  	if (current->flags & PF_MEMALLOC)
>  		goto nopage;
>  
> +	/* Can give up if caller is willing to give up upon fatal signals */
> +	if (fatal_signal_pending(current) &&
> +	    !(gfp_mask & (__GFP_UNKILLABLE | __GFP_NOFAIL))) {
> +		gfp_mask |= __GFP_NOWARN;
> +		goto nopage;
> +	}
> +
>  	/* Try direct reclaim and then allocating */

This part is superficially tempting, although without the UNKILLABLE.  ie:

+	if (fatal_signal_pending(current) && !(gfp_mask & __GFP_NOFAIL)) {
+		gfp_mask |= __GFP_NOWARN;
+		goto nopage;
+	}

It makes some sense to me to prevent tasks with a fatal signal pending
from being able to trigger reclaim.  But I'm worried about what memory
allocation failures it might trigger on paths that aren't accustomed to
seeing failures.