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

[David Hildenbrand (Arm)]

On 11/2/26 22:46, 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.
> 

@Quiliang, can you resend with that?

-- 
Cheers,

David