Re: [BISECTED] first bad commit is c203c6d5b3f0597 ("migrate_pages: batch _unmap and _move")

["Huang, Ying" <ying.huang@intel.com>]

Hyeonggon Yoo <42.hyeyoo@gmail.com> writes:

> On Fri, Feb 03, 2023 at 07:17:14AM +0800, Huang, Ying wrote:
>> "Huang, Ying" <ying.huang@intel.com> writes:
>> 
>> > Hi, Hyeonggon,
>> >
>> > Hyeonggon Yoo <42.hyeyoo@gmail.com> writes:
>> >
>> >> On Wed, Feb 01, 2023 at 01:09:10AM +0900, Hyeonggon Yoo wrote:
>> >>> I've observed random list_del corruption on mm-unstable,
>> >>> where HEAD is commit d69862e693c069f4
>> >>> ("mm/migrate: convert putback_movable_pages() to use folios").
>> >>> 
>> >>> The issue can be easily reproduced by stressing MM multiple times:
>> >>> 	stress-ng --bigheap 0 --timeout 300
>> >>> 
>> >>> The compiler is gcc 12.2.1 and config, dmesg are included as attachment.
>> >>> I will try to bisect but can't promise quick resolution :)
>> >>
>> >>
>> >> The first bad commits appears to be:
>> >> c203c6d5b3f0597 ("migrate_pages: batch _unmap and _move")
>> >>
>> >> the first bad commit _probably_ be earlier, but this is quite
>> >> easy to reproduce so at this point I think above is the real bad commit.
>> >
>> > Thank you very much for reporting the bug.  I'm in travel now but I will
>> > try to find some time to reproduce and debug it.
>> 
>> Still haven't reproduced the issue.  But after reviewing the code, I
>> found a bug in the code, which may cause list corruption.  Can you try
>> the debug patch below?
>
> Unfortunately my home server seems to be broken again :(
> That means I only have access to VMs and not a real machine now.
>
> FYI it was not reproduced on KVM but reproduced on real machine.
> Could you try checking on your machine with the config I attached? [1]

Thank you very much for testing!

> Sorry to bother your travel!

Never mind.  Your report helps me very much!

> [1] https://marc.info/?l=linux-mm&m=167518135116956

I have reproduced the bug successfully!  And I can reproduce the bug
with the previous debug patch too, although the reproduction rate isn't
high.

And in my test, the following patch can fix the bug.

It appears that zswap code will touch dst->lru during moving page.

Best Regards,
Huang, Ying

-------------------------8<----------------------------------
From b2e3f4aab16d8af0033286fde669b46e7467c7ec Mon Sep 17 00:00:00 2001
From: Huang Ying <ying.huang@intel.com>
Date: Fri, 3 Feb 2023 22:03:24 +0800
Subject: [PATCH] dbg,migrate_pages: restore destination folio state before
 move

---
 mm/migrate.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/mm/migrate.c b/mm/migrate.c
index 143d96775b4d..fa7212330cb6 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -1225,13 +1225,19 @@ static int __migrate_folio_move(struct folio *src, struct folio *dst,
 	int page_was_mapped = 0;
 	struct anon_vma *anon_vma = NULL;
 	bool is_lru = !__PageMovable(&src->page);
+	struct list_head *prev;
 
 	__migrate_folio_extract(dst, &page_was_mapped, &anon_vma);
+	prev = dst->lru.prev;
+	list_del(&dst->lru);
 
 	rc = move_to_new_folio(dst, src, mode);
 
-	if (rc != -EAGAIN)
-		list_del(&dst->lru);
+	if (rc == -EAGAIN) {
+		list_add(&dst->lru, prev);
+		__migrate_folio_record(dst, page_was_mapped, anon_vma);
+		return rc;
+	}
 
 	if (unlikely(!is_lru))
 		goto out_unlock_both;
@@ -1251,11 +1257,6 @@ static int __migrate_folio_move(struct folio *src, struct folio *dst,
 			lru_add_drain();
 	}
 
-	if (rc == -EAGAIN) {
-		__migrate_folio_record(dst, page_was_mapped, anon_vma);
-		return rc;
-	}
-
 	if (page_was_mapped)
 		remove_migration_ptes(src,
 			rc == MIGRATEPAGE_SUCCESS ? dst : src, false);
-- 
2.35.1