Re: [RFC PATCH v2 08/18] mm/compaction: free the migrated folios that wait for a grace period

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mm/compaction: compact_zone() misses pages captured in rcu_free_drain()
--

commit 0e470d35ade0bceb30271d7e57f7ca44bff99bf7
Author: Kiryl Shutsemau (Meta) <kas@kernel.org>

mm/compaction: free the migrated folios that wait for a grace period

This commit drains deferred RCU-freed folios in compact_zone() when non-async
compaction moves past a block it migrated from and the zone has pages waiting
for a grace period.

> diff --git a/mm/compaction.c b/mm/compaction.c
> index a049415512c67..2acb87a3446df 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -2760,6 +2760,14 @@ compact_zone(struct compact_control *cc, struct capture_control *capc)
>  				block_start_pfn(cc->migrate_pfn, cc->order);
>  
>  			if (last_migrated_pfn < current_block_start) {
> +				/*
> +				 * Folios migrated away from lockless readers
> +				 * are freed after a grace period.  Async
> +				 * compaction does not wait for it.
> +				 */
> +				if (cc->mode != MIGRATE_ASYNC &&
> +				    zone_rcu_free_waiting(cc->zone))
> +					rcu_free_drain();
>  				lru_add_drain_cpu_zone(cc->zone);
>  				/* No more flushing until we migrate again */
>  				last_migrated_pfn = 0;

[Severity: Medium]
Can compact_zone() miss a page that gets captured into capc->page during
rcu_free_drain() or lru_add_drain_cpu_zone()?

During direct compaction, current->capture_control points to capc. When a
migrated block contains a folio with PG_rcu_free set, migrate_folio_done() ->
folio_put() -> __free_pages_prepare() defers freeing it via rcu_free_defer().
Because the block has not coalesced yet, capc->page is still NULL when
compact_zone() checks it before the check_drain label:

mm/compaction.c:compact_zone() {
	...
		/* Stop if a page has been captured */
		if (capc && capc->page) {
			ret = COMPACT_SUCCESS;
			break;
		}
check_drain:
	...
}

When check_drain then runs rcu_free_drain() and lru_add_drain_cpu_zone(), the
deferred pages are freed in current's context through __free_one_page(). Once
the block coalesces to capc->order, compaction_capture() stores the page in
capc->page and __free_one_page() returns early without adding the page to
zone->free_area[order]:

mm/page_alloc.c:__free_one_page() {
	...
		if (compaction_capture(capc, page, order, migratetype)) {
			account_freepages(zone, -(1 << order), migratetype);
			return;
		}
	...
}

After check_drain finishes, the while loop condition calls
compact_finished() -> __compact_finished(), which only checks whether
cc->zone->free_area[order] is non-empty and does not check capc->page. Since
the captured page was not placed on the free list, compact_finished() returns
COMPACT_CONTINUE and compact_zone() proceeds to isolate and migrate another
block even though the allocation has already been captured.

Should compact_zone() recheck capc && capc->page after draining (or check it
in __compact_finished())?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104130118.2507513-1-kas@kernel.org?part=8