linux-kernel - Re: [PATCH 2/4] mm, compaction: more reliably increase direct compaction priority

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160922140821.GG11875@dhcp22.suse.cz>
Date:   Thu, 22 Sep 2016 16:08:21 +0200
From:   Michal Hocko <mhocko@...nel.org>
To:     Vlastimil Babka <vbabka@...e.cz>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Arkadiusz Miskiewicz <a.miskiewicz@...il.com>,
        Ralf-Peter Rohbeck <Ralf-Peter.Rohbeck@...ntum.com>,
        Olaf Hering <olaf@...fle.de>, linux-kernel@...r.kernel.org,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        linux-mm@...ck.org, Mel Gorman <mgorman@...hsingularity.net>,
        Joonsoo Kim <iamjoonsoo.kim@....com>,
        David Rientjes <rientjes@...gle.com>,
        Rik van Riel <riel@...hat.com>
Subject: Re: [PATCH 2/4] mm, compaction: more reliably increase direct
 compaction priority

On Thu 22-09-16 14:51:48, Vlastimil Babka wrote:
> >From 465e1bd61b7a6d6901a44f09b1a76514dbc220fa Mon Sep 17 00:00:00 2001
> From: Vlastimil Babka <vbabka@...e.cz>
> Date: Thu, 22 Sep 2016 13:54:32 +0200
> Subject: [PATCH] mm, compaction: more reliably increase direct compaction
>  priority-fix
> 
> When increasing the compaction priority, also reset retries. Otherwise we can
> consume all retries on the lower priorities.

OK, this is an improvement. I am just thinking that we might want to
pull
	if (order && compaction_made_progress(compact_result))
		compaction_retries++;

into should_compact_retry as well. I've had it there originally because
it was in line with no_progress_loops but now that we have compaction
priorities it would fit into retry logic better. As a plus it would
count only those compaction rounds where we we didn't have to rely on
the compaction retry logic. What do you think?

> Suggested-by: Michal Hocko <mhocko@...e.com>
> Signed-off-by: Vlastimil Babka <vbabka@...e.cz>

Anyway
Acked-by: Michal Hocko <mhocko@...e.com>

> ---
>  mm/page_alloc.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index f8bed910e3cf..82fdb690ac62 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -3162,7 +3162,7 @@ static inline bool
>  should_compact_retry(struct alloc_context *ac, int order, int alloc_flags,
>  		     enum compact_result compact_result,
>  		     enum compact_priority *compact_priority,
> -		     int compaction_retries)
> +		     int *compaction_retries)
>  {
>  	int max_retries = MAX_COMPACT_RETRIES;
>  
> @@ -3196,16 +3196,17 @@ should_compact_retry(struct alloc_context *ac, int order, int alloc_flags,
>  	 */
>  	if (order > PAGE_ALLOC_COSTLY_ORDER)
>  		max_retries /= 4;
> -	if (compaction_retries <= max_retries)
> +	if (*compaction_retries <= max_retries)
>  		return true;
>  
>  	/*
> -	 * Make sure there is at least one attempt at the highest priority
> -	 * if we exhausted all retries at the lower priorities
> +	 * Make sure there are attempts at the highest priority if we exhausted
> +	 * all retries or failed at the lower priorities.
>  	 */
>  check_priority:
>  	if (*compact_priority > MIN_COMPACT_PRIORITY) {
>  		(*compact_priority)--;
> +		*compaction_retries = 0;
>  		return true;
>  	}
>  	return false;
> @@ -3224,7 +3225,7 @@ static inline bool
>  should_compact_retry(struct alloc_context *ac, unsigned int order, int alloc_flags,
>  		     enum compact_result compact_result,
>  		     enum compact_priority *compact_priority,
> -		     int compaction_retries)
> +		     int *compaction_retries)
>  {
>  	struct zone *zone;
>  	struct zoneref *z;
> @@ -3663,7 +3664,7 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
>  	if (did_some_progress > 0 &&
>  			should_compact_retry(ac, order, alloc_flags,
>  				compact_result, &compact_priority,
> -				compaction_retries))
> +				&compaction_retries))
>  		goto retry;
>  
>  	/* Reclaim has failed us, start killing things */
> -- 
> 2.10.0
> 

-- 
Michal Hocko
SUSE Labs