Re: [PATCH v6 5/5] mm/vmscan: batch dirty TLB flushes before pageout

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mm/vmscan: split shmem tail folios left unprocessed in shrink_folio_list()
--

commit cb4d38468a11c1912d88e753c3510180ab891a8b
Author: Zhang Peng <zippermonkey@icloud.com>

mm/vmscan: batch dirty TLB flushes before pageout

This commit batches dirty swap-backed folios during shrink_folio_list() into
a folio_batch so that pending writable TLB entries can be flushed once per
batch with try_to_unmap_flush_dirty() before paging the folios out. Folios
are unlocked while queued in the batch and re-locked and revalidated in
folio_batch_pageout().

> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 061414ff45bb8..1150361e7d525 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1334,6 +1334,92 @@ static enum folio_pageout_result folio_try_pageout(struct folio *folio,
>  	unreachable();
>  }
>  
> +static void folio_batch_pageout(struct folio_batch *batch,
> +		struct reclaim_folio_state *state, struct scan_control *sc,
> +		struct reclaim_stat *stat, struct swap_io_ctx *ctx,
> +		struct list_head *folio_list)
> +{

[ ... ]

> +	for (i = 0; i < nr; i++) {
> +		struct folio *folio = batch->folios[i];
> +
> +		switch (folio_try_pageout(folio, sc, ctx, folio_list)) {

[ ... ]

> @@ -1721,6 +1804,10 @@ static unsigned int shrink_folio_list(struct list_head *folio_list,
>  		VM_BUG_ON_FOLIO(folio_test_lru(folio) ||
>  				folio_test_unevictable(folio), folio);
>  	}
> +	if (folio_batch_count(&pageout_folios)) {
> +		folio_batch_pageout(&pageout_folios, &state, sc, stat,
> +				    &ctx, folio_list);
> +	}
>  	/* 'folio_list' is always empty here */

[Severity: Medium]
What happens if a dirty large shmem folio is split during this final
folio_batch_pageout() call after the while (!list_empty(folio_list)) loop
has already exited?

When folio_batch_pageout() calls folio_try_pageout() -> pageout() ->
shmem_writeout(), shmem_writeout() can split a large shmem folio via
split_folio_to_list(folio, folio_list) (for example when CONFIG_THP_SWAP is
disabled, the folio crosses i_size, or folio_alloc_swap() fails with
-E2BIG):

mm/shmem.c:shmem_writeout() {
	...
		if (split_folio_to_list(folio, folio_list))
			goto redirty;
	...
}

Then folio_try_pageout() adjusts sc->nr_scanned expecting the newly split
tail folios on folio_list to be scanned by the main loop in
shrink_folio_list():

mm/vmscan.c:folio_try_pageout() {
	...
	case PAGE_ACTIVATE:
		/*
		 * If shmem folio is split when writeback to swap, the
		 * tail pages will make their own pass through this
		 * function and be accounted then.  There is no need to
		 * clamp nr_pages here: folio_activate_locked() derives
		 * the count from folio_nr_pages() itself.
		 */
		if (nr_pages > 1 && !folio_test_large(folio))
			sc->nr_scanned -= (nr_pages - 1);
		return FOLIO_PAGEOUT_ACTIVATE;
	case PAGE_KEEP:
		return FOLIO_PAGEOUT_KEEP_LOCKED;
	case PAGE_SUCCESS:
		if (nr_pages > 1 && !folio_test_large(folio))
			sc->nr_scanned -= (nr_pages - 1);
	...
}

Because this final folio_batch_pageout() runs after the
while (!list_empty(folio_list)) loop has finished, won't those split tail
folios remain unprocessed on folio_list (violating the "'folio_list' is
always empty here" invariant) and leave sc->nr_scanned undercounted by
nr_pages - 1?

Should shrink_folio_list() loop back to process folio_list again if
folio_batch_pageout() adds split tail folios back onto folio_list?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261031-batch-tlb-flush-v6-0-190539839eec@tencent.com?part=5