Re: [PATCH] mm/memory_hotplug: don't offline a hwpoisoned page still in the page cache

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] mm/memory_hotplug: do_migrate_range() isolates busy hwpoisoned folios from LRU
- [High] mm/memory_hotplug: scan_movable_pages() misses poisoned tail pages of folios
--

commit 90624953bdb2c5ae02a7473ca37edfda72128f6d
Author: Mani Tofigh <mt4022@columbia.edu>

mm/memory_hotplug: don't offline a hwpoisoned page still in the page cache

This commit prevents memory offlining from skipping hwpoisoned pages whose
folios still have a page-cache or swap-cache mapping in scan_movable_pages()
and __test_page_isolated_in_pageblock(). This avoids offlining memory while
files or swap caches still reference poisoned pages.

[ ... ]

> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index c9cc42e54f05d..3d36f7f2e04b6 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1820,6 +1820,8 @@ static int scan_movable_pages(unsigned long start, unsigned long end,
>  		struct folio *folio;
>  
>  		page = pfn_to_page(pfn);
> +		if (PageHWPoison(page) && !hwpoison_page_offlineable(page))
> +			return -EBUSY;
>  		if (PageLRU(page) || page_has_movable_ops(page))
>  			goto found;

[Severity: High]
Does do_migrate_range() still isolate a non-offlineable hwpoisoned folio from
the LRU and leak its reference when another movable page precedes it in the
range?

In offline_pages(), scan_movable_pages() returns 0 as soon as it finds the
first movable page, and offline_pages() then passes the entire remaining
range [pfn, end_pfn) to do_migrate_range():

mm/memory_hotplug.c:offline_pages() {
    ...
			ret = scan_movable_pages(pfn, end_pfn, &pfn);
			if (!ret) {
				/*
				 * TODO: fatal migration failures should bail
				 * out
				 */
				do_migrate_range(pfn, end_pfn);
			}
    ...
}

In addition, because memory_failure() does not hold mem_hotplug_lock,
TestSetPageHWPoison() can run after scan_movable_pages() has already scanned
past that PFN.

When do_migrate_range() reaches the hwpoisoned page-cache or swap-cache folio
while memory_failure() is waiting in get_hwpoison_page() on
pcp_batch_high_lock, it isolates the folio from the LRU without checking
hwpoison_page_offlineable():

mm/memory_hotplug.c:do_migrate_range() {
    ...
		if (folio_contain_hwpoisoned_page(folio)) {
			if (folio_test_large(folio) && !folio_test_hugetlb(folio))
				goto put_folio;
			if (folio_test_lru(folio) && !folio_isolate_lru(folio))
				goto put_folio;
			if (folio_mapped(folio)) {
				folio_lock(folio);
				unmap_poisoned_folio(folio, pfn, false);
				folio_unlock(folio);
			}

			goto put_folio;
		}
    ...
}

On the next pass of the loop in offline_pages(), scan_movable_pages() reaches
the poisoned page and returns -EBUSY. However, folio_isolate_lru() has
already cleared PG_lru and taken an extra reference on the folio that
put_folio does not drop.

When the waiting memory_failure() resumes in get_hwpoison_page() ->
get_any_page() -> __get_hwpoison_page(), HWPoisonHandlable() sees
!PageLRU(page) and fails with -EIO instead of truncating the folio, and the
leaked reference from folio_isolate_lru() keeps the poisoned folio pinned in
the page cache so subsequent offline_pages() attempts also fail with -EBUSY.

Should do_migrate_range() also skip isolating the folio when
!hwpoison_page_offlineable() is true?

[Severity: High]
What happens in scan_movable_pages() when a tail page of a large page-cache
or swap-cache folio is hwpoisoned?

PG_hwpoison is set per subpage whereas PG_lru is on the head page. Because
scan_movable_pages() scans sequentially from start, it visits the head page
of a large folio before any tail page. At the head page, PageHWPoison(page)
is false and PageLRU(page) is true, so scan_movable_pages() takes the
goto found branch at the head PFN without scanning the poisoned tail PFN.

In do_migrate_range(), if PG_has_hwpoisoned is already set on the large
folio (for instance, when memory_failure() could not split the large folio):

mm/memory_hotplug.c:do_migrate_range() {
    ...
		if (folio_test_large(folio))
			pfn = folio_pfn(folio) + folio_nr_pages(folio) - 1;

		if (folio_contain_hwpoisoned_page(folio)) {
			/*
			 * unmap_poisoned_folio() cannot handle large folios
			 * in all cases yet.
			 */
			if (folio_test_large(folio) && !folio_test_hugetlb(folio))
				goto put_folio;
    ...
}

do_migrate_range() skips the large folio without clearing PG_lru, and the
do-while (!ret) loop in offline_pages() calls scan_movable_pages() again at
the same head PFN, looping indefinitely while holding mem_hotplug_lock and
pcp_batch_high_lock.

Conversely, if memory_failure() on a tail page is blocked in
get_hwpoison_page() before setting PG_has_hwpoisoned, do_migrate_range()
advances pfn to the end of the large folio, sees
folio_contain_hwpoisoned_page(folio) as false, and migrates the folio
despite the poisoned tail page.

Can scan_movable_pages() and do_migrate_range() check for poisoned subpages
across the folio so that large folios with poisoned tail pages are handled
consistently?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261101-memhp-hwpoison-pagecache-v1-1-ed8785ca44eb@columbia.edu?part=1