From: Vlastimil Babka <vbabka@xxxxxxx> Subject: mm, compaction: raise compaction priority after it withdrawns Mike Kravetz reports that "hugetlb allocations could stall for minutes or hours when should_compact_retry() would return true more often then it should. Specifically, this was in the case where compact_result was COMPACT_DEFERRED and COMPACT_PARTIAL_SKIPPED and no progress was being made." The problem is that the compaction_withdrawn() test in should_compact_retry() includes compaction outcomes that are only possible on low compaction priority, and results in a retry without increasing the priority. This may result in furter reclaim, and more incomplete compaction attempts. With this patch, compaction priority is raised when possible, or should_compact_retry() returns false. The COMPACT_SKIPPED result doesn't really fit together with the other outcomes in compaction_withdrawn(), as that's a result caused by insufficient order-0 pages, not due to low compaction priority. With this patch, it is moved to a new compaction_needs_reclaim() function, and for that outcome we keep the current logic of retrying if it looks like reclaim will be able to help. Link: http://lkml.kernel.org/r/20190806014744.15446-4-mike.kravetz@xxxxxxxxxx Reported-by: Mike Kravetz <mike.kravetz@xxxxxxxxxx> Signed-off-by: Vlastimil Babka <vbabka@xxxxxxx> Signed-off-by: Mike Kravetz <mike.kravetz@xxxxxxxxxx> Tested-by: Mike Kravetz <mike.kravetz@xxxxxxxxxx> Cc: Hillf Danton <hdanton@xxxxxxxx> Cc: Johannes Weiner <hannes@xxxxxxxxxxx> Cc: Mel Gorman <mgorman@xxxxxxx> Cc: Michal Hocko <mhocko@xxxxxxxxxx> Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> --- include/linux/compaction.h | 22 +++++++++++++++++----- mm/page_alloc.c | 16 ++++++++++++---- 2 files changed, 29 insertions(+), 9 deletions(-) --- a/include/linux/compaction.h~mm-compaction-raise-compaction-priority-after-it-withdrawns +++ a/include/linux/compaction.h @@ -129,11 +129,8 @@ static inline bool compaction_failed(enu return false; } -/* - * Compaction has backed off for some reason. It might be throttling or - * lock contention. Retrying is still worthwhile. - */ -static inline bool compaction_withdrawn(enum compact_result result) +/* Compaction needs reclaim to be performed first, so it can continue. */ +static inline bool compaction_needs_reclaim(enum compact_result result) { /* * Compaction backed off due to watermark checks for order-0 @@ -142,6 +139,16 @@ static inline bool compaction_withdrawn( if (result == COMPACT_SKIPPED) return true; + return false; +} + +/* + * Compaction has backed off for some reason after doing some work or none + * at all. It might be throttling or lock contention. Retrying might be still + * worthwhile, but with a higher priority if allowed. + */ +static inline bool compaction_withdrawn(enum compact_result result) +{ /* * If compaction is deferred for high-order allocations, it is * because sync compaction recently failed. If this is the case @@ -206,6 +213,11 @@ static inline bool compaction_failed(enu { return false; } + +static inline bool compaction_needs_reclaim(enum compact_result result) +{ + return false; +} static inline bool compaction_withdrawn(enum compact_result result) { --- a/mm/page_alloc.c~mm-compaction-raise-compaction-priority-after-it-withdrawns +++ a/mm/page_alloc.c @@ -3955,14 +3955,22 @@ should_compact_retry(struct alloc_contex goto check_priority; /* + * compaction was skipped because there are not enough order-0 pages + * to work with, so we retry only if it looks like reclaim can help. + */ + if (compaction_needs_reclaim(compact_result)) { + ret = compaction_zonelist_suitable(ac, order, alloc_flags); + goto out; + } + + /* * make sure the compaction wasn't deferred or didn't bail out early * due to locks contention before we declare that we should give up. - * But do not retry if the given zonelist is not suitable for - * compaction. + * But the next retry should use a higher priority if allowed, so + * we don't just keep bailing out endlessly. */ if (compaction_withdrawn(compact_result)) { - ret = compaction_zonelist_suitable(ac, order, alloc_flags); - goto out; + goto check_priority; } /* _
