diff mbox

[12/12] mm: page_alloc: do not lock up low-order allocations upon OOM

Message ID 1427264236-17249-13-git-send-email-hannes@cmpxchg.org (mailing list archive)
State New, archived
Headers show

Commit Message

Johannes Weiner March 25, 2015, 6:17 a.m. UTC
When both page reclaim and the OOM killer fail to free memory, there
are no more options for the allocator to make progress on its own.

Don't risk hanging these allocations.  Leave it to the allocation site
to implement the fallback policy for failing allocations.

Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
---
 mm/page_alloc.c | 19 ++++++-------------
 1 file changed, 6 insertions(+), 13 deletions(-)

Comments

Michal Hocko March 26, 2015, 3:32 p.m. UTC | #1
On Wed 25-03-15 02:17:16, Johannes Weiner wrote:
> When both page reclaim and the OOM killer fail to free memory, there
> are no more options for the allocator to make progress on its own.
> 
> Don't risk hanging these allocations.  Leave it to the allocation site
> to implement the fallback policy for failing allocations.

The changelog communicates the impact of this patch _very_ poorly. The
potential regression space is quite large. Every syscall which is not
allowed to return ENOMEM and it relies on an allocation would have to be
audited or a common mechanism to catch them deployed.

I really believe this is a good thing _longterm_ but I still do not
think it is the upstream material anytime soon without extensive testing
which is even not mentioned here.

> Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
> ---
>  mm/page_alloc.c | 19 ++++++-------------
>  1 file changed, 6 insertions(+), 13 deletions(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 9e45e97aa934..f2b1a17416c4 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -2331,12 +2331,10 @@ void warn_alloc_failed(gfp_t gfp_mask, int order, const char *fmt, ...)
>  
>  static inline struct page *
>  __alloc_pages_may_oom(gfp_t gfp_mask, unsigned int order, int alloc_flags,
> -	const struct alloc_context *ac, unsigned long *did_some_progress)
> +		      const struct alloc_context *ac)
>  {
>  	struct page *page = NULL;
>  
> -	*did_some_progress = 0;
> -
>  	/*
>  	 * This allocating task can become the OOM victim itself at
>  	 * any point before acquiring the lock.  In that case, exit
> @@ -2376,13 +2374,9 @@ __alloc_pages_may_oom(gfp_t gfp_mask, unsigned int order, int alloc_flags,
>  			goto out;
>  	}
>  
> -	if (out_of_memory(ac->zonelist, gfp_mask, order, ac->nodemask, false)) {
> -		*did_some_progress = 1;
> -	} else {
> +	if (!out_of_memory(ac->zonelist, gfp_mask, order, ac->nodemask, false))
>  		/* Oops, these shouldn't happen with the OOM killer disabled */
> -		if (WARN_ON_ONCE(gfp_mask & __GFP_NOFAIL))
> -			*did_some_progress = 1;
> -	}
> +		WARN_ON_ONCE(gfp_mask & __GFP_NOFAIL);
>  
>  	/*
>  	 * Allocate from the OOM killer reserves.
> @@ -2799,13 +2793,12 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
>  	}
>  
>  	/* Reclaim has failed us, start killing things */
> -	page = __alloc_pages_may_oom(gfp_mask, order, alloc_flags, ac,
> -				     &did_some_progress);
> +	page = __alloc_pages_may_oom(gfp_mask, order, alloc_flags, ac);
>  	if (page)
>  		goto got_pg;
>  
> -	/* Retry as long as the OOM killer is making progress */
> -	if (did_some_progress)
> +	/* Wait for user to order more dimms, cuz these are done */
> +	if (gfp_mask & __GFP_NOFAIL)
>  		goto retry;
>  
>  noretry:
> -- 
> 2.3.3
>
diff mbox

Patch

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 9e45e97aa934..f2b1a17416c4 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -2331,12 +2331,10 @@  void warn_alloc_failed(gfp_t gfp_mask, int order, const char *fmt, ...)
 
 static inline struct page *
 __alloc_pages_may_oom(gfp_t gfp_mask, unsigned int order, int alloc_flags,
-	const struct alloc_context *ac, unsigned long *did_some_progress)
+		      const struct alloc_context *ac)
 {
 	struct page *page = NULL;
 
-	*did_some_progress = 0;
-
 	/*
 	 * This allocating task can become the OOM victim itself at
 	 * any point before acquiring the lock.  In that case, exit
@@ -2376,13 +2374,9 @@  __alloc_pages_may_oom(gfp_t gfp_mask, unsigned int order, int alloc_flags,
 			goto out;
 	}
 
-	if (out_of_memory(ac->zonelist, gfp_mask, order, ac->nodemask, false)) {
-		*did_some_progress = 1;
-	} else {
+	if (!out_of_memory(ac->zonelist, gfp_mask, order, ac->nodemask, false))
 		/* Oops, these shouldn't happen with the OOM killer disabled */
-		if (WARN_ON_ONCE(gfp_mask & __GFP_NOFAIL))
-			*did_some_progress = 1;
-	}
+		WARN_ON_ONCE(gfp_mask & __GFP_NOFAIL);
 
 	/*
 	 * Allocate from the OOM killer reserves.
@@ -2799,13 +2793,12 @@  __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
 	}
 
 	/* Reclaim has failed us, start killing things */
-	page = __alloc_pages_may_oom(gfp_mask, order, alloc_flags, ac,
-				     &did_some_progress);
+	page = __alloc_pages_may_oom(gfp_mask, order, alloc_flags, ac);
 	if (page)
 		goto got_pg;
 
-	/* Retry as long as the OOM killer is making progress */
-	if (did_some_progress)
+	/* Wait for user to order more dimms, cuz these are done */
+	if (gfp_mask & __GFP_NOFAIL)
 		goto retry;
 
 noretry: