On Thu 22-09-16 14:51:48, Vlastimil Babka wrote:
> >From 465e1bd61b7a6d6901a44f09b1a76514dbc220fa Mon Sep 17 00:00:00 2001
> From: Vlastimil Babka <vbabka@xxxxxxx>
> 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@xxxxxxxx>
> Signed-off-by: Vlastimil Babka <vbabka@xxxxxxx>
Anyway
Acked-by: Michal Hocko <mhocko@xxxxxxxx>
> ---
> 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
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>