Re: [PATCH v2] erofs: get rid of erofs_prepare_dio() helper

[JeffleXu]

On 7/20/22 4:22 PM, Gao Xiang wrote:
> Fold in erofs_prepare_dio() in order to simplify the code.
> 
> Signed-off-by: Gao Xiang <hsiangkao@xxxxxxxxxxxxxxxxx>
> ---
> v1: https://lore.kernel.org/r/20220506194612.117120-1-hsiangkao@xxxxxxxxxxxxxxxxx
> 
>  fs/erofs/data.c | 39 +++++++++++++++------------------------
>  1 file changed, 15 insertions(+), 24 deletions(-)
> 
> diff --git a/fs/erofs/data.c b/fs/erofs/data.c
> index fbb037ba326e..fe8ac0e163f7 100644
> --- a/fs/erofs/data.c
> +++ b/fs/erofs/data.c
> @@ -366,42 +366,33 @@ static sector_t erofs_bmap(struct address_space *mapping, sector_t block)
>  	return iomap_bmap(mapping, block, &erofs_iomap_ops);
>  }
>  
> -static int erofs_prepare_dio(struct kiocb *iocb, struct iov_iter *to)
> +static ssize_t erofs_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
>  {
>  	struct inode *inode = file_inode(iocb->ki_filp);
> -	loff_t align = iocb->ki_pos | iov_iter_count(to) |
> -		iov_iter_alignment(to);
> -	struct block_device *bdev = inode->i_sb->s_bdev;
> -	unsigned int blksize_mask;
> -
> -	if (bdev)
> -		blksize_mask = (1 << ilog2(bdev_logical_block_size(bdev))) - 1;
> -	else
> -		blksize_mask = (1 << inode->i_blkbits) - 1;
>  
> -	if (align & blksize_mask)
> -		return -EINVAL;
> -	return 0;
> -}
> -
> -static ssize_t erofs_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
> -{
>  	/* no need taking (shared) inode lock since it's a ro filesystem */
>  	if (!iov_iter_count(to))
>  		return 0;
>  
>  #ifdef CONFIG_FS_DAX
> -	if (IS_DAX(iocb->ki_filp->f_mapping->host))
> +	if (IS_DAX(inode))
>  		return dax_iomap_rw(iocb, to, &erofs_iomap_ops);
>  #endif
>  	if (iocb->ki_flags & IOCB_DIRECT) {
> -		int err = erofs_prepare_dio(iocb, to);
> +		struct block_device *bdev = inode->i_sb->s_bdev;
> +		unsigned int blksize_mask;
> +
> +		if (bdev)
> +			blksize_mask = bdev_logical_block_size(bdev) - 1;
> +		else
> +			blksize_mask = (1 << inode->i_blkbits) - 1;
> +
> +		if ((iocb->ki_pos | iov_iter_count(to) |
> +		     iov_iter_alignment(to)) & blksize_mask)
> +			return -EINVAL;
>  
> -		if (!err)
> -			return iomap_dio_rw(iocb, to, &erofs_iomap_ops,
> -					    NULL, 0, NULL, 0);
> -		if (err < 0)
> -			return err;
> +		return iomap_dio_rw(iocb, to, &erofs_iomap_ops,
> +				    NULL, 0, NULL, 0);
>  	}
>  	return filemap_read(iocb, to, 0);
>  }

LGTM.

Reviewed-by: Jeffle Xu <jefflexu@xxxxxxxxxxxxxxxxx>

-- 
Thanks,
Jeffle