Re: [PATCH] mm/shmem: drop the second folio batch from shmem_undo_range()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Critical] mm/shmem: skipped boundary zeroing on swapin failure in
  shmem_undo_range() [1]
--

commit 785bd3384006f8bb06c3c8f9b1b5d455eba3d449
Author: Qiliang Yuan <qiliang.yuan@linux.dev>

mm/shmem: drop the second folio batch from shmem_undo_range()

This commit removes the second folio_batch from shmem_undo_range() by
passing fbatch directly to delete_from_page_cache_batch() after removing
swap entries, and using truncate_inode_folio() when undoing a failed
fallocate().

> diff --git a/mm/shmem.c b/mm/shmem.c
> index 156469423a866..5bcbdd6078a73 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -1352,9 +1352,6 @@ static void shmem_undo_range(struct inode *inode, loff_t lstart, uoff_t lend,
>  	index = start;
>  	while (index < end && find_lock_entries(mapping, &index, end - 1,
>  			&fbatch, indices)) {
> -		struct folio_batch remove;
> -
> -		folio_batch_init(&remove);
>  		for (i = 0; i < folio_batch_count(&fbatch); i++) {
>  			folio = fbatch.folios[i];
>  
> @@ -1366,13 +1363,14 @@ static void shmem_undo_range(struct inode *inode, loff_t lstart, uoff_t lend,
>  				continue;
>  			}
>  
> -			if (!unfalloc || !folio_test_uptodate(folio)) {
> +			if (!unfalloc)
>  				truncate_cleanup_folio(folio);
> -				folio_batch_add(&remove, folio);
> -			}
> +			else if (!folio_test_uptodate(folio))
> +				truncate_inode_folio(mapping, folio);
>  		}
> -		delete_from_page_cache_batch(mapping, &remove);
>  		folio_batch_remove_exceptionals(&fbatch);
> +		if (!unfalloc)
> +			delete_from_page_cache_batch(mapping, &fbatch);
>  		for (i = 0; i < folio_batch_count(&fbatch); i++)
>  			folio_unlock(fbatch.folios[i]);
>  		folio_batch_release(&fbatch);

[Severity: Critical]
This problem wasn't introduced by this patch, but can shmem_undo_range()
leave partial boundary ranges unzeroed when swapping in a boundary folio
fails [1]?

Later in shmem_undo_range(), shmem_get_partial_folio() is called to fault
in boundary folios at lstart >> PAGE_SHIFT and lend >> PAGE_SHIFT before
passing them to truncate_inode_partial_folio():

mm/shmem.c:shmem_get_partial_folio() {
	...
	folio = NULL;
	shmem_get_folio(inode, index, 0, &folio, SGP_READ);
	return folio;
}

mm/shmem.c:shmem_undo_range() {
	...
	folio = shmem_get_partial_folio(inode, lstart >> PAGE_SHIFT);
	if (folio) {
		same_folio = lend < folio_next_pos(folio);
		folio_mark_dirty(folio);
		truncate_inode_partial_folio(folio, lstart, lend, &start,
					     same_folio ? &end : NULL);
	...
}

If the boundary falls inside a swapped-out entry and shmem_swapin_folio()
fails with -ENOMEM under memory pressure, shmem_get_partial_folio() ignores
the error from shmem_get_folio() and returns NULL.  shmem_undo_range() then
skips truncate_inode_partial_folio() for that boundary while the subsequent
whole_folios loop skips any swap entry crossing start or end, leaving stale
non-zero data in the swap entry that becomes visible on later reads or file
extensions.

[1] https://sashiko.dev/#/bug/linux-c8b370f0-7820-4bcf-bfc3-a27db7a7ba48

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111-bug-mm-shmem-undo-frame-v1-1-e50a95b8f9b2@linux.dev?part=1