On Wed, 23 Aug 2017, Vlastimil Babka wrote:

quoted

diff --git a/mm/compaction.c b/mm/compaction.c
--- a/mm/compaction.c
+++ b/mm/compaction.c

@@ -217,6 +217,20 @@ static void reset_cached_positions(struct zone *zone)
 				pageblock_start_pfn(zone_end_pfn(zone) - 1);
 }
 
+/*
+ * Hugetlbfs pages should consistenly be skipped until updated by the hugetlb
+ * subsystem.  It is always pointless to compact pages of pageblock_order and
+ * the free scanner can reconsider when no longer huge.
+ */
+static bool pageblock_skip_persistent(struct page *page, unsigned int order)
+{
+	if (!PageHuge(page))
+		return false;
+	if (order != pageblock_order)
+		return false;
+	return true;

Why just HugeTLBfs? There's also no point in migrating/finding free
pages in THPs. Actually, any compound page of pageblock order?

Yes, any page where compound_order(page) == pageblock_order would probably 
benefit from the same treatment.  I haven't encountered such an issue, 
however, so I thought it was best to restrict it only to hugetlb: hugetlb 
memory usually sits in the hugetlb free pool and seldom gets freed under 
normal conditions even when unmapped whereas thp is much more likely to be 
unmapped and split.  I wasn't sure that it was worth the pageblock skip.

quoted

+}
+
 /*
  * This function is called to clear all cached information on pageblocks that
  * should be skipped for page isolation when the migrate and free page scanner

@@ -241,6 +255,8 @@ static void __reset_isolation_suitable(struct zone *zone)
 			continue;
 		if (zone != page_zone(page))
 			continue;
+		if (pageblock_skip_persistent(page, compound_order(page)))
+			continue;

I like the idea of how persistency is achieved by rechecking in the reset.

quoted

 
 		clear_pageblock_skip(page);
 	}

@@ -448,13 +464,15 @@ static unsigned long isolate_freepages_block(struct compact_control *cc,
 		 * and the only danger is skipping too much.
 		 */
 		if (PageCompound(page)) {
-			unsigned int comp_order = compound_order(page);
-
-			if (likely(comp_order < MAX_ORDER)) {
-				blockpfn += (1UL << comp_order) - 1;
-				cursor += (1UL << comp_order) - 1;
+			const unsigned int order = compound_order(page);
+
+			if (pageblock_skip_persistent(page, order)) {
+				set_pageblock_skip(page);
+				blockpfn = end_pfn;
+			} else if (likely(order < MAX_ORDER)) {
+				blockpfn += (1UL << order) - 1;
+				cursor += (1UL << order) - 1;
 			}

Is this new code (and below) really necessary? The existing code should
already lead to skip bit being set via update_pageblock_skip()?

I wanted to set the persistent pageblock skip regardless of 
cc->ignore_skip_hint without a local change to update_pageblock_skip().

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>