On 09/22/2016 04:52 PM, Michal Hocko wrote:
> On Thu 22-09-16 16:08:21, Michal Hocko wrote:
>> On Thu 22-09-16 14:51:48, Vlastimil Babka wrote:
>> > >From 465e1bd61b7a6d6901a44f09b1a76514dbc220fa Mon Sep 17 00:00:00 2001
>> > From: Vlastimil Babka <vba...@suse.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
> did that should be
>
>> the compaction retry logic. What do you think?
Makes sense.
-----8<-----
>From f775ec4be05a21c78a718a382c13132dedd5e2a4 Mon Sep 17 00:00:00 2001
From: Vlastimil Babka <vba...@suse.cz>
Date: Thu, 22 Sep 2016 13:54:32 +0200
Subject: [PATCH] mm, compaction: more reliably increase direct compaction
priority-fix-v2
When increasing the compaction priority, also reset retries. Otherwise we can
consume all retries on the lower priorities. Also pull the retries increment
into should_compact_retry() so it counts only the rounds where we actually
rely on it.
Suggested-by: Michal Hocko <mho...@suse.com>
Signed-off-by: Vlastimil Babka <vba...@suse.cz>
Acked-by: Michal Hocko <mho...@suse.com>
---
mm/page_alloc.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index f8bed910e3cf..582820080601 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -3162,13 +3162,16 @@ 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;
if (!order)
return false;
+ if (compaction_made_progress(compact_result))
+ (*compaction_retries)++;
+
/*
* compaction considers all the zone as desperately out of memory
* so it doesn't really make much sense to retry except when the
@@ -3196,16 +3199,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 +3228,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;
@@ -3626,9 +3630,6 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
if (page)
goto got_pg;
- if (order && compaction_made_progress(compact_result))
- compaction_retries++;
-
/* Do not loop if specifically requested */
if (gfp_mask & __GFP_NORETRY)
goto nopage;
@@ -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
