linux-kernel - [PATCH] mm/migrate: fix BUG_ON in migrate_misplaced_folio() and compact

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-next>] [day] [month] [year] [list]

Date: Mon, 17 Jun 2024 10:39:25 -0400
From: Zi Yan <zi.yan@...t.com>
To: Andrew Morton <akpm@...ux-foundation.org>,
	Hugh Dickins <hughd@...gle.com>,
	"Huang, Ying" <ying.huang@...el.com>,
	linux-mm@...ck.org
Cc: Zi Yan <ziy@...dia.com>,
	oe-lkp@...ts.linux.dev,
	lkp@...el.com,
	kernel test robot <oliver.sang@...el.com>,
	"Matthew Wilcox (Oracle)" <willy@...radead.org>,
	Yang Shi <shy828301@...il.com>,
	David Hildenbrand <david@...hat.com>,
	Yin Fengwei <fengwei.yin@...el.com>,
	linux-kernel@...r.kernel.org
Subject: [PATCH] mm/migrate: fix BUG_ON in migrate_misplaced_folio() and compact_zone()

From: Zi Yan <ziy@...dia.com>

Both Hugh Dickins and kernel test bot hit an issue that migrate_pages()
reports success but still leaves unmigrated pages on from list. It
triggers BUG_ON in migrate_misplaced_folio() and compact_zone().

Our convention is that if migrate_pages() reports complete success (0),
then the migratepages list will be empty; but if it reports an error or
some pages remaining, then its caller must putback_movable_pages().

There's a new case in which migrate_pages() has been reporting complete
success, but returning with pages left on the migratepages list: when
migrate_pages_batch() successfully split a folio on the deferred list,
but then the "Failure isn't counted" call does not dispose of them all.

Since that block is expecting the large folio to have been counted as 1
failure already, and since the return code is later adjusted to success
whenever the returned list is found empty, the simple way to fix this
safely is to count splitting the deferred folio as "a failure".

This patch is based on Hugh's fix and suggestions from Huang, Ying.

[Hugh's version]
Link: https://lore.kernel.org/linux-mm/46c948b4-4dd8-6e03-4c7b-ce4e81cfa536@google.com/
[Additional stats change suggested by Huang, Ying]
Link: https://lore.kernel.org/linux-mm/87msnq7key.fsf@yhuang6-desk2.ccr.corp.intel.com/
Suggested-by: "Huang, Ying" <ying.huang@...el.com>
Fixes: 7262f208ca68 ("mm/migrate: split source folio if it is on deferred split list")
Reported-by: Hugh Dickins <hughd@...gle.com>
Closes: https://lore.kernel.org/linux-mm/46c948b4-4dd8-6e03-4c7b-ce4e81cfa536@google.com/
Reported-by: kernel test robot <oliver.sang@...el.com>
Closes: https://lore.kernel.org/oe-lkp/202406171436.a30c129-oliver.sang@intel.com
Signed-off-by: Zi Yan <ziy@...dia.com>
---
 mm/migrate.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/mm/migrate.c b/mm/migrate.c
index dd04f578c19c..30f9a61598ea 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -1654,7 +1654,16 @@ static int migrate_pages_batch(struct list_head *from,
 
 			/*
 			 * The rare folio on the deferred split list should
-			 * be split now. It should not count as a failure.
+			 * be split now. It should not count as a failure:
+			 * but increment nr_failed, because, without
+			 * doing so, migrate_pages() may report success with
+			 * (split but unmigrated) pages still on its fromlist;
+			 * whereas it always reports success when its fromlist
+			 * is empty. stats->nr_thp_failed should be increased
+			 * too, otherwise stats inconsistency will happen when
+			 * migrate_pages_batch is called via migrate_pages()
+			 * with MIGRATE_SYNC and MIGRATE_ASYNC.
+			 *
 			 * Only check it without removing it from the list.
 			 * Since the folio can be on deferred_split_scan()
 			 * local list and removing it can cause the local list
@@ -1669,6 +1678,8 @@ static int migrate_pages_batch(struct list_head *from,
 			if (nr_pages > 2 &&
 			   !list_empty(&folio->_deferred_list)) {
 				if (try_split_folio(folio, split_folios) == 0) {
+					nr_failed++;
+					stats->nr_thp_failed += is_thp;
 					stats->nr_thp_split += is_thp;
 					stats->nr_split++;
 					continue;

base-commit: 6ba59ff4227927d3a8530fc2973b80e94b54d58f
-- 
2.43.0