mm: compaction: fix the page state calculation in too_many_isolated

Commit "3611badc1baa" (mm: vmscan: fix the page state calculation in

too_many_isolated) fixed an issue where a number of tasks were blocked in reclaim path for seconds, because of vmstat_diff not being synced in time. A similar problem can happen in isolate_migratepages_block, similar calculation is performed. This patch fixes that.

Signed-off-by Vinayak Menon <[email protected]>

---
 mm/compaction.c | 32 +++++++++++++++++++++++++++-----
 1 file changed, 27 insertions(+), 5 deletions(-)

mm/compaction.c +1 -3

index 546e571..2d9730d 100644
@@ -537,21 +537,43 @@ static void acct_isolated(struct zone *zone, struct compact_control *cc)
 	mod_zone_page_state(zone, NR_ISOLATED_FILE, count[1]);
 }
 
-/* Similar to reclaim, but different enough that they don't share logic */
-static bool too_many_isolated(struct zone *zone)
+static bool __too_many_isolated(struct zone *zone, int safe)
 {
 	unsigned long active, inactive, isolated;
 
-	inactive = zone_page_state(zone, NR_INACTIVE_FILE) +

mm/compaction.c +10 -2

-	active = zone_page_state(zone, NR_ACTIVE_FILE) +
+		active = zone_page_state(zone, NR_ACTIVE_FILE) +
 					zone_page_state(zone, NR_ACTIVE_ANON);
-	isolated = zone_page_state(zone, NR_ISOLATED_FILE) +
+		isolated = zone_page_state(zone, NR_ISOLATED_FILE) +
 					zone_page_state(zone, NR_ISOLATED_ANON);
+	}
 
 	return isolated > (inactive + active) / 2;
 }
 
+/* Similar to reclaim, but different enough that they don't share logic */
+static bool too_many_isolated(struct zone *zone)
+{
+	/*
+	 * __too_many_isolated(safe=0) is fast but inaccurate, because it
+	 * doesn't account for the vm_stat_diff[] counters.  So if it looks
+	 * like too_many_isolated() is about to return true, fall back to the

Quoted 1 line · Vinayak Menon

Commit "3611badc1baa" (mm: vmscan: fix the page state calculation in

That appears to be a -next commit ID, which won't be the same in Linus' tree, so it shouldn't be in commit message, AFAIK.

Quoted 4 lines

too_many_isolated) fixed an issue where a number of tasks were blocked in reclaim path for seconds, because of vmstat_diff not being synced in time. A similar problem can happen in isolate_migratepages_block, similar calculation is performed. This patch fixes that.

I guess it's not possible to fix the stats instantly and once in the safe versions, so that future readings will be correct without safe, right? So until it gets fixed, each reading will have to be safe and thus expensive?

I think in case of async compaction, we could skip the safe stuff and just terminate it - it's already done when too_many_isolated returns true, and there's no congestion waiting in that case.

So you could extend the too_many_isolated() with "safe" parameter (as you did for vmscan) and pass it "cc->mode != MIGRATE_ASYNC" value from isolate_migrate_block().

Quoted 8 lines

I think in case of async compaction, we could skip the safe stuff and just terminate it - it's already done when too_many_isolated returns true, and there's no congestion waiting in that case.

So you could extend the too_many_isolated() with "safe" parameter (as you did for vmscan) and pass it "cc->mode != MIGRATE_ASYNC" value from isolate_migrate_block().

Or just pass it struct compact_control *cc and use both cc->zone and cc->mode inside this compaction-only function.