On 04/05/2016 01:25 PM, Michal Hocko wrote:
From: Michal Hocko <mhocko@xxxxxxxx>
Compaction can provide a wild variation of feedback to the caller. Many
of them are implementation specific and the caller of the compaction
(especially the page allocator) shouldn't be bound to specifics of the
current implementation.
This patch abstracts the feedback into three basic types:
- compaction_made_progress - compaction was active and made some
progress.
- compaction_failed - compaction failed and further attempts to
invoke it would most probably fail and therefore it is not
worth retrying
- compaction_withdrawn - compaction wasn't invoked for an
implementation specific reasons. In the current implementation
it means that the compaction was deferred, contended or the
page scanners met too early without any progress. Retrying is
still worthwhile.
Signed-off-by: Michal Hocko <mhocko@xxxxxxxx>
---
include/linux/compaction.h | 74 ++++++++++++++++++++++++++++++++++++++++++++++
mm/page_alloc.c | 25 ++++------------
2 files changed, 80 insertions(+), 19 deletions(-)
diff --git a/include/linux/compaction.h b/include/linux/compaction.h
index a7b9091ff349..512db9c3f0ed 100644
--- a/include/linux/compaction.h
+++ b/include/linux/compaction.h
@@ -78,6 +78,70 @@ extern void compaction_defer_reset(struct zone *zone, int order,
bool alloc_success);
extern bool compaction_restarting(struct zone *zone, int order);
+/* Compaction has made some progress and retrying makes sense */
+static inline bool compaction_made_progress(enum compact_result result)
+{
+ /*
+ * Even though this might sound confusing this in fact tells us
+ * that the compaction successfully isolated and migrated some
+ * pageblocks.
+ */
+ if (result == COMPACT_PARTIAL)
+ return true;
+
+ return false;
+}
+
+/* Compaction has failed and it doesn't make much sense to keep retrying. */
+static inline bool compaction_failed(enum compact_result result)
+{
+ /* All zones where scanned completely and still not result. */
Hmm given that try_to_compact_pages() uses a max() on results, then in
fact it takes only one zone to get this. Others could have been also
SKIPPED or DEFERRED. Is that what you want?
+ if (result == COMPACT_COMPLETE)
+ return true;
+
+ 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 backed off due to watermark checks for order-0
+ * so the regular reclaim has to try harder and reclaim something.
+ */
+ if (result == COMPACT_SKIPPED)
+ return true;
+
+ /*
+ * If compaction is deferred for high-order allocations, it is
+ * because sync compaction recently failed. If this is the case
+ * and the caller requested a THP allocation, we do not want
+ * to heavily disrupt the system, so we fail the allocation
+ * instead of entering direct reclaim.
+ */
+ if (result == COMPACT_DEFERRED)
+ return true;
+
+ /*
+ * If compaction in async mode encounters contention or blocks higher
+ * priority task we back off early rather than cause stalls.
+ */
+ if (result == COMPACT_CONTENDED)
+ return true;
+
+ /*
+ * Page scanners have met but we haven't scanned full zones so this
+ * is a back off in fact.
+ */
+ if (result == COMPACT_PARTIAL_SKIPPED)
+ return true;
+
+ return false;
+}
+
[...]
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index c37e6d1ad643..c05de84c8157 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -3362,25 +3362,12 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
if (page)
goto got_pg;
- /* Checks for THP-specific high-order allocations */
- if (is_thp_gfp_mask(gfp_mask)) {
- /*
- * If compaction is deferred for high-order allocations, it is
- * because sync compaction recently failed. If this is the case
- * and the caller requested a THP allocation, we do not want
- * to heavily disrupt the system, so we fail the allocation
- * instead of entering direct reclaim.
- */
- if (compact_result == COMPACT_DEFERRED)
- goto nopage;
-
- /*
- * Compaction is contended so rather back off than cause
- * excessive stalls.
- */
- if(compact_result == COMPACT_CONTENDED)
- goto nopage;
- }
+ /*
+ * Checks for THP-specific high-order allocations and back off
+ * if the the compaction backed off
+ */
+ if (is_thp_gfp_mask(gfp_mask) && compaction_withdrawn(compact_result))
+ goto nopage;
The change of semantics for THP is not trivial here and should at least
be discussed in changelog. CONTENDED and DEFERRED is only subset of
compaction_withdrawn() as seen above. Why is it useful to back off due
to COMPACT_PARTIAL_SKIPPED (we were just unlucky in our starting
position), but not due to COMPACT_COMPLETE (we have seen the whole zone
but failed anyway)? Why back off due to COMPACT_SKIPPED (not enough
order-0 pages) without trying reclaim at least once, and then another
async compaction, like before?
/*
* It can become very expensive to allocate transparent hugepages at
--
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>