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

[Jan Kara]

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.

> +				folio_unlock(folio);
>  		}
>  		folio_batch_remove_exceptionals(&fbatch);
>  		folio_batch_release(&fbatch);

								Honza
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR