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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <56D6DBC1.2030701@suse.cz>
Date:	Wed, 2 Mar 2016 13:25:37 +0100
From:	Vlastimil Babka <vbabka@...e.cz>
To:	Joonsoo Kim <iamjoonsoo.kim@....com>,
	Andrew Morton <akpm@...ux-foundation.org>
Cc:	linux-mm@...ck.org, linux-kernel@...r.kernel.org,
	Andrea Arcangeli <aarcange@...hat.com>,
	"Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
	Rik van Riel <riel@...hat.com>,
	Mel Gorman <mgorman@...hsingularity.net>,
	David Rientjes <rientjes@...gle.com>,
	Michal Hocko <mhocko@...e.com>,
	Johannes Weiner <hannes@...xchg.org>
Subject: Re: [PATCH v2 2/5] mm, compaction: introduce kcompactd

>> +		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@...e.cz>
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@....com>
Signed-off-by: Vlastimil Babka <vbabka@...e.cz>
---
 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