Re: [PATCH v2] fat: preallocate space before expanding in fat_cont_expand()

[OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>]

Matheus Alves de Almeida <matheus.aalmeida@inf.ufrgs.br> writes:

> On Fri, Oct 23, 2026 at 04:33:23PM +0900, OGAWA Hirofumi wrote:
>>Matheus Alves de Almeida <matheus.aalmeida@inf.ufrgs.br> writes:
>>
>>> The fat_cont_expand() implementation leaves allocation of new clusters
>>> to generic_cont_expand_simple(). This path does many small cluster
>>> allocations, harming performance.
>>>
>>> Preallocate cluster space in fat_cont_expand(). Move preallocation to
>>> a new fat_preallocate() helper used by both fat_cont_expand() and
>>> fat_fallocate(FALLOC_FL_KEEP_SIZE).
>>>
>>> Benchmarking was done under QEMU/KVM with a virtio-blk device using
>>> cache=none. Each result is the arithmetic mean of the listed samples.
>>>
>>> start = now_ns();
>>> for (unsigned i = 0; i < count; i++) {
>>>     if (truncate_mode)
>>>         ftruncate(fds[i], bytes);
>>>     else
>>>         fallocate(fds[i], 0, 0, bytes);
>>> }
>>> elapsed = now_ns() - start;
>>>
>>> 16 files, 512-byte clusters:
>>> length    mode      sync?    before    after    improvement    samples
>>> 256MiB    ftruncate no       9.576s    5.345s   1.79x          10
>>> 256MiB    fallocate no       9.634s    5.640s   1.71x          10
>>> 1MiB      ftruncate yes      58.66s    0.362s   162.15x        5
>>> 1MiB      fallocate yes      61.36s    0.390s   157.15x        5
>>>
>>> 128 files, 512-byte clusters:
>>> length    mode      sync?    before    after    improvement    samples
>>> 512KiB    ftruncate no       0.433s    0.352s   1.23x          10
>>> 512KiB    fallocate no       0.446s    0.393s   1.13x          10
>>> 64KiB     ftruncate yes      31.32s    0.429s   73.00x         10
>>> 64KiB     fallocate yes      29.59s    0.410s   72.42x         10
>>>
>>> 16 files, 4096-byte clusters:
>>> length    mode      sync?    before    after    improvement    samples
>>> 256MiB    ftruncate no       5.181s    4.417s   1.17x          10
>>> 256MiB    fallocate no       5.166s    4.442s   1.16x          10
>>> 1MiB      ftruncate yes      9.294s    0.111s   84.06x         10
>>> 1MiB      fallocate yes      8.710s    0.110s   79.36x         10
>>>
>>> The biggest improvement is on filesystems with a small cluster size and
>>> on synchronous mounts.
>>>
>>> Signed-off-by: Matheus Alves de Almeida <matheus.aalmeida@inf.ufrgs.br>
>>> ---
>>>
>>> Changes in v2:
>>> - fat_cont_expand(): Hold truncate_lock around fat_truncate_blocks().
>>>
>>> This patch depends on:
>>> https://lore.kernel.org/linux-fsdevel/20261012171106.146243-1-matheus.aalmeida@inf.ufrgs.br/T/#t
>>>
>>>  fs/fat/file.c | 56 ++++++++++++++++++++++++++++++++++-----------------
>>>  1 file changed, 37 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/fs/fat/file.c b/fs/fat/file.c
>>> index 3aa541c6b127..dd0ae795dc14 100644
>>> --- a/fs/fat/file.c
>>> +++ b/fs/fat/file.c
>>> @@ -217,15 +217,50 @@ const struct file_operations fat_file_operations = {
>>>  	.setlease	= generic_setlease,
>>>  };
>>>  
>>> +static int fat_preallocate(struct inode *inode, loff_t new_size)
>>> +{
>>> +	int nr_cluster; /* Number of clusters to be allocated */
>>> +	loff_t mm_bytes; /* Number of bytes to be allocated for file */
>>> +	loff_t ondisksize; /* block aligned on-disk size in bytes*/
>>> +	struct super_block *sb = inode->i_sb;
>>> +	struct msdos_sb_info *sbi = MSDOS_SB(sb);
>>> +
>>> +	ondisksize = inode->i_blocks << 9;
>>> +	if (new_size <= ondisksize)
>>> +		return 0;
>>> +
>>> +	/* First compute the number of clusters to be allocated */
>>> +	mm_bytes = new_size - ondisksize;
>>> +	nr_cluster = (mm_bytes + (sbi->cluster_size - 1)) >>
>>> +		     sbi->cluster_bits;
>>> +
>>> +	/* Allocate the clusters. We are not zeroing them out */
>>> +	return fat_add_clusters(inode, nr_cluster);
>>> +}
>>> +
>>>  static int fat_cont_expand(struct inode *inode, loff_t size)
>>>  {
>>>  	struct address_space *mapping = inode->i_mapping;
>>>  	loff_t start = inode->i_size, count = size - inode->i_size;
>>>  	int err;
>>>  
>>> -	err = generic_cont_expand_simple(inode, size);
>>> +	/*
>>> +	 * Preallocating space here is more efficient than during
>>> +	 * generic_cont_expand_simple() because of batching.
>>> +	 */
>>> +	err = inode_newsize_ok(inode, size);
>>>  	if (err)
>>>  		goto out;
>>> +	err = fat_preallocate(inode, size);
>>> +	if (err)
>>> +		goto out;
>>> +	err = generic_cont_expand_simple(inode, size);
>>> +	if (err) {
>>> +		down_write(&MSDOS_I(inode)->truncate_lock);
>>> +		fat_truncate_blocks(inode, inode->i_size);
>>> +		up_write(&MSDOS_I(inode)->truncate_lock);
>>
>>Is this fat_truncate_blocks() really required?
>>
> Yes. generic_cont_expand_simple() calls inode_newsize_ok() as well,
> which happens before fat_write_failed() can run and clean up. Another
> thread could lower RLIMIT_FSIZE between the FAT and generic checks and
> return without cleaning up.

If the reason is only for inode_newsize_ok(), we would be able to add
fat_cont_expand_simple() or such instead.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>