Re: [RFC PATCH 6/8] fat: add iomap buffered I/O support

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

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

> On Mon, Nov 09, 2026 at 12:03:00AM +0900, OGAWA Hirofumi wrote:
>>Matheus Alves de Almeida <matheus.aalmeida@inf.ufrgs.br> writes:
>>
>>> Use iomap for buffered I/O, replacing buffer_heads. Remove
>>> fat_get_block(), fat_write_begin(), fat_write_failed() as the iomap path
>>> makes them unnecessary.
>>>
>>> Signed-off-by: Matheus Alves de Almeida <matheus.aalmeida@inf.ufrgs.br>
>>> ---
>>>  fs/fat/fat.h   |   4 +-
>>>  fs/fat/file.c  |  72 +++++++++++++++++++----
>>>  fs/fat/inode.c | 157 +++++--------------------------------------------
>>>  fs/fat/iomap.c |  84 ++++++++++++++++++++++++++
>>>  4 files changed, 164 insertions(+), 153 deletions(-)
>>>
>>
>>[...]
>>
>>> diff --git a/fs/fat/file.c b/fs/fat/file.c
>>> index 8689cbbdad13..723b671d5b4b 100644
>>> --- a/fs/fat/file.c
>>> +++ b/fs/fat/file.c
>>
>>[...]
>>
>>> +static int fat_cont_expand(struct inode *inode, loff_t new_size);
>>> +
>>> +static ssize_t fat_file_write_iter_buffered(struct kiocb *iocb,
>>> +					    struct iov_iter *iter)
>>> +{
>>> +	struct inode *inode = file_inode(iocb->ki_filp);
>>> +	struct msdos_sb_info *sbi = MSDOS_SB(inode->i_sb);
>>> +	loff_t offset, old_size, old_alloc, keep_alloc;
>>> +	ssize_t ret;
>>> +
>>> +	inode_lock(inode);
>>> +	ret = generic_write_checks(iocb, iter);
>>> +	if (ret <= 0)
>>> +		goto out;
>>> +
>>> +	ret = file_modified(iocb->ki_filp);
>>> +	if (ret)
>>> +		goto out;
>>> +
>>> +	offset = iocb->ki_pos;
>>> +	old_size = i_size_read(inode);
>>> +	if (offset > old_size) {
>>> +		/* We need to fill this hole */
>>> +		ret = fat_cont_expand(inode, offset);
>>> +		if (ret)
>>> +			goto out;
>>> +	}
>>> +
>>> +	inode_dio_wait(inode);
>>> +	old_alloc = inode_get_bytes(inode);
>>> +	ret = iomap_file_buffered_write(iocb, iter, &fat_iomap_write_ops,
>>> +					NULL, NULL);
>>> +	keep_alloc = max(old_alloc, round_up(i_size_read(inode),
>>> +					     sbi->cluster_size));
>>> +	if (inode_get_bytes(inode) > keep_alloc)
>>> +		fat_truncate_blocks(inode, keep_alloc);
>>
>>What is doing by this fat_truncate_blocks?
>>
>
> It's trimming the clusters past EOF and keeping the fallocate KEEP_SIZE
> preallocation. I can add a comment stating this.

Hm, I wonder mostly why this truncate without checking "ret"...

Thanks.

>>> +	if (ret > 0)
>>> +		ret = generic_write_sync(iocb, ret);
>>
>>Maybe we want to do this outside inode_unlock()?
>>
>
> True, good catch.
>
>>> +out:
>>> +	inode_unlock(inode);
>>> +	return ret;
>>> +}
>>> +
>
> Thanks,
> Matheus

-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>