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

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

[Qiliang Yuan]

shmem_undo_range() keeps a folio_batch and an array of FOLIO_BATCH_SIZE
indices on its stack. Removing truncated folios from the page cache in
batches added a second folio_batch to collect the folios to delete.

That grows the stack frame by about 200 bytes, past the 1024 byte limit
of mips eyeq6_defconfig built with clang.

Pass fbatch itself to delete_from_page_cache_batch() once the swap
entries are gone, as truncate_inode_pages_range() does. Undoing a failed
fallocate() only removes the folios that are not uptodate, so remove
those one at a time with truncate_inode_folio() as before.

Stack frame of shmem_undo_range() built with clang 18:

  config                before (bytes)  after (bytes)  change
  mips eyeq6_defconfig             928            720    -22%
  x86_64                           920            712    -23%

Fixes: b97fbd541ecba ("mm/shmem: remove truncated folios from the page cache in batches")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202611112032.TMyPU6Rm-lkp@intel.com/
Signed-off-by: Qiliang Yuan <qiliang.yuan@linux.dev>
---
David, this applies on top of b97fbd541ecba in shmem-7.5.misc; please
fold it in if you prefer.
---
 mm/shmem.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

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);

---
base-commit: b97fbd541ecba5d6640077618e9dfec6f5e71132
change-id: 20261111-bug-mm-shmem-undo-frame-66828e197ad5

Best regards,
-- 
Qiliang Yuan <qiliang.yuan@linux.dev>

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

[Baolin Wang]

On 11/12/26 11:39 AM, Qiliang Yuan wrote:
> shmem_undo_range() keeps a folio_batch and an array of FOLIO_BATCH_SIZE
> indices on its stack. Removing truncated folios from the page cache in
> batches added a second folio_batch to collect the folios to delete.
> 
> That grows the stack frame by about 200 bytes, past the 1024 byte limit
> of mips eyeq6_defconfig built with clang.
> 
> Pass fbatch itself to delete_from_page_cache_batch() once the swap
> entries are gone, as truncate_inode_pages_range() does. Undoing a failed
> fallocate() only removes the folios that are not uptodate, so remove
> those one at a time with truncate_inode_folio() as before.
> 
> Stack frame of shmem_undo_range() built with clang 18:
> 
>    config                before (bytes)  after (bytes)  change
>    mips eyeq6_defconfig             928            720    -22%
>    x86_64                           920            712    -23%
> 
> Fixes: b97fbd541ecba ("mm/shmem: remove truncated folios from the page cache in batches")
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202611112032.TMyPU6Rm-lkp@intel.com/
> Signed-off-by: Qiliang Yuan <qiliang.yuan@linux.dev>
> ---
> David, this applies on top of b97fbd541ecba in shmem-7.5.misc; please
> fold it in if you prefer.
> ---

I think you should send out a complete v2, and David could queue your 
new version.

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

[David Hildenbrand (Arm)]

On 11/12/26 09:05, Baolin Wang wrote:
> 
> 
> On 11/12/26 11:39 AM, Qiliang Yuan wrote:
>> shmem_undo_range() keeps a folio_batch and an array of FOLIO_BATCH_SIZE
>> indices on its stack. Removing truncated folios from the page cache in
>> batches added a second folio_batch to collect the folios to delete.
>>
>> That grows the stack frame by about 200 bytes, past the 1024 byte limit
>> of mips eyeq6_defconfig built with clang.
>>
>> Pass fbatch itself to delete_from_page_cache_batch() once the swap
>> entries are gone, as truncate_inode_pages_range() does. Undoing a failed
>> fallocate() only removes the folios that are not uptodate, so remove
>> those one at a time with truncate_inode_folio() as before.
>>
>> Stack frame of shmem_undo_range() built with clang 18:
>>
>>    config                before (bytes)  after (bytes)  change
>>    mips eyeq6_defconfig             928            720    -22%
>>    x86_64                           920            712    -23%
>>
>> Fixes: b97fbd541ecba ("mm/shmem: remove truncated folios from the page cache
>> in batches")
>> Reported-by: kernel test robot <lkp@intel.com>
>> Closes: https://lore.kernel.org/oe-kbuild-all/202611112032.TMyPU6Rm-
>> lkp@intel.com/
>> Signed-off-by: Qiliang Yuan <qiliang.yuan@linux.dev>
>> ---
>> David, this applies on top of b97fbd541ecba in shmem-7.5.misc; please
>> fold it in if you prefer.
>> ---
> 
> I think you should send out a complete v2, and David could queue your new version.

Yes please. I'll drop the old patch from core.git. Should be gone later today.

-- 
Cheers,

David

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

[Qiliang Yuan]

On Thu, Nov 12, 2026 at 09:33:00AM +0100, David Hildenbrand (Arm) wrote:
> On 11/12/26 09:05, Baolin Wang wrote:
>> I think you should send out a complete v2, and David could queue your new version.
>
> Yes please. I'll drop the old patch from core.git. Should be gone later today.

Thanks. This patch is superseded by v3, the complete patch rebuilt from
the v2 in shmem-7.5.misc with this fix folded in. It fixes the stack
frame warning reported at
https://lore.kernel.org/oe-kbuild-all/202611112032.TMyPU6Rm-lkp@intel.com/

v3: https://lore.kernel.org/r/20261112-bug-mm-shmem-undo-batch-v3-1-212db8c24058@linux.dev

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

[sashiko-bot]

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

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

[Baolin Wang]

On 11/13/26 11:39 AM, sashiko-bot@kernel.org wrote:
> 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.

This is unrelated to the current patch. David already raised this 
concern before [1], and I'll think about whether there's a reliable fix 
for such edge cases.

[1] 
https://lore.kernel.org/all/20261026-fixes_shmem-v1-0-703206d5fa0c@kernel.org/