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:

> 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?

> +	if (ret > 0)
> +		ret = generic_write_sync(iocb, ret);

Maybe we want to do this outside inode_unlock()?

> +out:
> +	inode_unlock(inode);
> +	return ret;
> +}
> +

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