2022-07-20 08:41:05

by Gao Xiang

[permalink] [raw]
Subject: [PATCH v2] erofs: get rid of erofs_prepare_dio() helper

Fold in erofs_prepare_dio() in order to simplify the code.

Signed-off-by: Gao Xiang <[email protected]>
---
v1: https://lore.kernel.org/r/[email protected]

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);
}
--
2.24.4


2022-07-22 09:48:27

by Jingbo Xu

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



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 <[email protected]>
> ---
> v1: https://lore.kernel.org/r/[email protected]
>
> 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 <[email protected]>

--
Thanks,
Jeffle