Re: [PATCH v2 2/5] mm, compaction: introduce kcompactd

[Vlastimil Babka]

>> +		if (zone_watermark_ok(zone, cc.order, low_wmark_pages(zone),
>> +						cc.classzone_idx, 0)) {
>> +			success = true;
>> +			compaction_defer_reset(zone, cc.order, false);
>> +		} else if (cc.mode != MIGRATE_ASYNC &&
>> +						status == COMPACT_COMPLETE) {
>> +			defer_compaction(zone, cc.order);
>> +		}
> 
> We alerady set mode to MIGRATE_SYNC_LIGHT so this cc.mode check looks weird.
> It would be better to change it and add some comment that we can
> safely call defer_compaction() here.

Right.
 
>> +
>> +		VM_BUG_ON(!list_empty(&cc.freepages));
>> +		VM_BUG_ON(!list_empty(&cc.migratepages));
>> +	}
>> +
>> +	/*
>> +	 * Regardless of success, we are done until woken up next. But remember
>> +	 * the requested order/classzone_idx in case it was higher/tighter than
>> +	 * our current ones
>> +	 */
>> +	if (pgdat->kcompactd_max_order <= cc.order)
>> +		pgdat->kcompactd_max_order = 0;
>> +	if (pgdat->classzone_idx >= cc.classzone_idx)
>> +		pgdat->classzone_idx = pgdat->nr_zones - 1;
>> +}
> 
> Maybe, you intend to update kcompactd_classzone_idx.

Oops, true. Thanks for the review!

Here's a fixlet.

----8<----
From: Vlastimil Babka <vbabka@xxxxxxx>
Date: Wed, 2 Mar 2016 10:15:22 +0100
Subject: mm-compaction-introduce-kcompactd-fix-3

Remove extraneous check for sync compaction before deferring.
Correctly adjust kcompactd's classzone_idx instead of kswapd's.

Reported-by: Joonsoo Kim <iamjoonsoo.kim@xxxxxxx>
Signed-off-by: Vlastimil Babka <vbabka@xxxxxxx>
---
 mm/compaction.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/mm/compaction.c b/mm/compaction.c
index c03715ba65c7..9a605c3d4177 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -1811,8 +1811,11 @@ static void kcompactd_do_work(pg_data_t *pgdat)
 						cc.classzone_idx, 0)) {
 			success = true;
 			compaction_defer_reset(zone, cc.order, false);
-		} else if (cc.mode != MIGRATE_ASYNC &&
-						status == COMPACT_COMPLETE) {
+		} else if (status == COMPACT_COMPLETE) {
+			/*
+			 * We use sync migration mode here, so we defer like
+			 * sync direct compaction does.
+			 */
 			defer_compaction(zone, cc.order);
 		}
 
@@ -1827,8 +1830,8 @@ static void kcompactd_do_work(pg_data_t *pgdat)
 	 */
 	if (pgdat->kcompactd_max_order <= cc.order)
 		pgdat->kcompactd_max_order = 0;
-	if (pgdat->classzone_idx >= cc.classzone_idx)
-		pgdat->classzone_idx = pgdat->nr_zones - 1;
+	if (pgdat->kcompactd_classzone_idx >= cc.classzone_idx)
+		pgdat->kcompactd_classzone_idx = pgdat->nr_zones - 1;
 }
 
 void wakeup_kcompactd(pg_data_t *pgdat, int order, int classzone_idx)
-- 
2.7.2