Re: [PATCH] mm/shmem: remove truncated folios from the page cache in batches

[Baolin Wang]

On 11/3/26 5:46 AM, Jan Kara wrote:
> On Thu 29-10-26 23:44:57, Qiliang Yuan wrote:
> > truncate_inode_pages_range() takes the folios it finds out of the page
> > cache with delete_from_page_cache_batch(), which takes the inode's
> > i_lock and the i_pages lock once for each batch of up to
> > FOLIO_BATCH_SIZE folios.
> >
> > shmem_undo_range() calls truncate_inode_folio() on each folio instead,
> > so filemap_remove_folio() takes and drops both locks once per folio.
> > Truncating, punching or freeing a tmpfs file or memfd made of small
> > folios, such as a large /dev/shm segment, pays for that on every page.
> >
> > Clean each folio up as truncate_inode_folio() does, collect the ones to
> > remove and take them out with delete_from_page_cache_batch(), which
> > needs truncate_cleanup_folio() outside mm/truncate.c. Swap entries, and
> > the folios kept when undoing a failed fallocate(), stay out of the
> > batch. Dropping an 8 GiB file of 4K folios, median of five runs,
> > averaged over two boots of each kernel, in a 32 vCPU VM:
> >
> >                          before    after
> >    ftruncate() to 0     1163 ms   913 ms
> >    hole punch           1155 ms   914 ms
> >    close() of a memfd   1161 ms   914 ms
> >    unlink()             1162 ms   916 ms
> >
> > Files of PMD-sized folios take 20-25 ms with or without this change.
> >
> > Signed-off-by: Qiliang Yuan <odys.yuan@xxxxxxxxx>
> > ---
> > To see it, fill and remove a large /dev/shm file:
> >
> >    fallocate -l 8G /dev/shm/f && time rm /dev/shm/f
>
> Looks sensible. Just one nit below. Otherwise feel free to add:
>
> Reviewed-by: Jan Kara <jack@xxxxxxx>
>
> > @@ -1352,6 +1352,9 @@ 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];
> >   
> > @@ -1363,9 +1366,16 @@ static void shmem_undo_range(struct inode *inode, loff_t lstart, uoff_t lend,
> >   				continue;
> >   			}
> >   
> > -			if (!unfalloc || !folio_test_uptodate(folio))
> > -				truncate_inode_folio(mapping, folio);
> > -			folio_unlock(folio);
> > +			if (!unfalloc || !folio_test_uptodate(folio)) {
> > +				truncate_cleanup_folio(folio);
> > +				folio_batch_add(&remove, folio);
> > +			}
> > +		}
> > +		delete_from_page_cache_batch(mapping, &remove);
> > +		for (i = 0; i < folio_batch_count(&fbatch); i++) {
> > +			folio = fbatch.folios[i];
> > +			if (!xa_is_value(folio))
>
> You can move this loop after folio_batch_remove_exceptionals() and drop
> this condition.

+1. With this addressed,
Reviewed-by: Baolin Wang <baolin.wang@xxxxxxxxxxxxxxxxx>