* [PATCH] erofs: use wrapper i_blocksize() in erofs_file_read_iter()
@ 2023-03-06 7:55 Yue Hu
2023-03-06 14:46 ` Gao Xiang
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Yue Hu @ 2023-03-06 7:55 UTC (permalink / raw)
To: xiang, chao, jefflexu, linux-erofs; +Cc: linux-kernel, huyue2, zhangwen
From: Yue Hu <huyue2@coolpad.com>
linux/fs.h has a wrapper for this operation.
Signed-off-by: Yue Hu <huyue2@coolpad.com>
---
fs/erofs/data.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/erofs/data.c b/fs/erofs/data.c
index 5bd0c956a142..7e8baf56faa5 100644
--- a/fs/erofs/data.c
+++ b/fs/erofs/data.c
@@ -380,7 +380,7 @@ static ssize_t erofs_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
if (bdev)
blksize_mask = bdev_logical_block_size(bdev) - 1;
else
- blksize_mask = (1 << inode->i_blkbits) - 1;
+ blksize_mask = i_blocksize(inode) - 1;
if ((iocb->ki_pos | iov_iter_count(to) |
iov_iter_alignment(to)) & blksize_mask)
--
2.17.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH] erofs: use wrapper i_blocksize() in erofs_file_read_iter()
2023-03-06 7:55 [PATCH] erofs: use wrapper i_blocksize() in erofs_file_read_iter() Yue Hu
@ 2023-03-06 14:46 ` Gao Xiang
2023-03-09 7:15 ` Yangtao Li
2023-03-09 14:33 ` [PATCH] " Chao Yu
2 siblings, 0 replies; 9+ messages in thread
From: Gao Xiang @ 2023-03-06 14:46 UTC (permalink / raw)
To: Yue Hu, xiang, chao, jefflexu, linux-erofs; +Cc: linux-kernel, huyue2, zhangwen
On 2023/3/6 15:55, Yue Hu wrote:
> From: Yue Hu <huyue2@coolpad.com>
>
> linux/fs.h has a wrapper for this operation.
>
> Signed-off-by: Yue Hu <huyue2@coolpad.com>
Reviewed-by: Gao Xiang <hsiangkao@linux.alibaba.com>
Thanks,
Gao Xiang
> ---
> fs/erofs/data.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/erofs/data.c b/fs/erofs/data.c
> index 5bd0c956a142..7e8baf56faa5 100644
> --- a/fs/erofs/data.c
> +++ b/fs/erofs/data.c
> @@ -380,7 +380,7 @@ static ssize_t erofs_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
> if (bdev)
> blksize_mask = bdev_logical_block_size(bdev) - 1;
> else
> - blksize_mask = (1 << inode->i_blkbits) - 1;
> + blksize_mask = i_blocksize(inode) - 1;
>
> if ((iocb->ki_pos | iov_iter_count(to) |
> iov_iter_alignment(to)) & blksize_mask)
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: erofs: use wrapper i_blocksize() in erofs_file_read_iter()
2023-03-06 7:55 [PATCH] erofs: use wrapper i_blocksize() in erofs_file_read_iter() Yue Hu
2023-03-06 14:46 ` Gao Xiang
@ 2023-03-09 7:15 ` Yangtao Li
2023-03-09 7:36 ` Jingbo Xu
2023-03-09 7:37 ` Yue Hu
2023-03-09 14:33 ` [PATCH] " Chao Yu
2 siblings, 2 replies; 9+ messages in thread
From: Yangtao Li @ 2023-03-09 7:15 UTC (permalink / raw)
To: zbestahu; +Cc: chao, huyue2, jefflexu, linux-erofs, linux-kernel, xiang,
zhangwen
> @@ -380,7 +380,7 @@ static ssize_t erofs_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
> if (bdev)
> blksize_mask = bdev_logical_block_size(bdev) - 1;
> else
> - blksize_mask = (1 << inode->i_blkbits) - 1;
> + blksize_mask = i_blocksize(inode) - 1;
Since the mask is to be obtained here, is it more appropriate to use GENMASK(inode->i_blkbits - 1, 0)?
Thx,
Yangtao
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: erofs: use wrapper i_blocksize() in erofs_file_read_iter()
2023-03-09 7:15 ` Yangtao Li
@ 2023-03-09 7:36 ` Jingbo Xu
2023-03-09 7:42 ` Yangtao Li
2023-03-09 7:37 ` Yue Hu
1 sibling, 1 reply; 9+ messages in thread
From: Jingbo Xu @ 2023-03-09 7:36 UTC (permalink / raw)
To: Yangtao Li, zbestahu
Cc: chao, huyue2, linux-erofs, linux-kernel, xiang, zhangwen
On 3/9/23 3:15 PM, Yangtao Li wrote:
>> @@ -380,7 +380,7 @@ static ssize_t erofs_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
>> if (bdev)
>> blksize_mask = bdev_logical_block_size(bdev) - 1;
>> else
>> - blksize_mask = (1 << inode->i_blkbits) - 1;
>> + blksize_mask = i_blocksize(inode) - 1;
>
> Since the mask is to be obtained here, is it more appropriate to use GENMASK(inode->i_blkbits - 1, 0)?
FYI it seems that GENMASK macro is widely used in driver and arch code
base, while it's rarely used in fs, except for f2fs.
--
Thanks,
Jingbo
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: erofs: use wrapper i_blocksize() in erofs_file_read_iter()
2023-03-09 7:36 ` Jingbo Xu
@ 2023-03-09 7:42 ` Yangtao Li
2023-03-09 8:09 ` Gao Xiang
0 siblings, 1 reply; 9+ messages in thread
From: Yangtao Li @ 2023-03-09 7:42 UTC (permalink / raw)
To: zbestahu; +Cc: chao, huyue2, jefflexu, linux-erofs, linux-kernel, xiang,
zhangwen
> FYI it seems that GENMASK macro is widely used in driver and arch code base, while it's rarely used in fs, except for f2fs.
I think the following usage can be changed to bitmap api, just like in f2fs?
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=c4ca1f7164734a1baf40d4ff1552172a07d4fc4d
fs/erofs/fscache.c:135: unsigned long flags = 1 << NETFS_SREQ_ONDEMAND;
fs/erofs/internal.h:250:#define SECTORS_PER_BLOCK (1 << SECTORS_PER_BLOCK)
fs/erofs/internal.h:252:#define EROFS_BLKSIZ (1 << LOG_BLOCK_SIZE)
fs/erofs/internal.h:354: return (value >> bit) & ((1 << bits) - 1);
fs/erofs/zmap.c:66: ((1 << Z_EROFS_VLE_DI_CLUSTER_TYPE_BITS) - 1);
fs/erofs/zmap.c:69: m->clusterofs = 1 << vi->z_logical_clusterbits;
fs/erofs/zmap.c:114: const unsigned int lomask = (1 << lclusterbits) - 1;
fs/erofs/zmap.c:141: const unsigned int lomask = (1 << lclusterbits) - 1;
fs/erofs/zmap.c:147: if (1 << amortizedshift == 4)
fs/erofs/zmap.c:149: else if (1 << amortizedshift == 2 && lclusterbits == 12)
fs/erofs/zmap.c:169: m->clusterofs = 1 << lclusterbits;
fs/erofs/zmap.c:291: pos += lcn * (1 << amortizedshift);
fs/erofs/zmap.c:409: m->compressedblks = 1 << (lclusterbits - LOG_BLOCK_SIZE);
fs/erofs/zmap.c:457: m->clusterofs != 1 << lclusterbits);
fs/erofs/zmap.c:497: endoff = ofs & ((1 << lclusterbits) - 1);
fs/erofs/erofs_fs.h:120: ((1 << (EROFS_I_DATALAYOUT_BIT + EROFS_I_DATALAYOUT_BITS)) - 1)
fs/erofs/erofs_fs.h:279:#define Z_EROFS_ALL_COMPR_ALGS ((1 << Z_EROFS_COMPRESSION_MAX) - 1)
fs/erofs/erofs_fs.h:377:#define Z_EROFS_VLE_DI_PARTIAL_REF (1 << 15)
fs/erofs/erofs_fs.h:384:#define Z_EROFS_VLE_DI_D0_CBLKCNT (1 << 11)
fs/erofs/erofs_fs.h:427: .h_clusterbits = 1 << Z_EROFS_FRAGMENT_INODE_BIT
fs/erofs/data.c:379: blksize_mask = (1 << inode->i_blkbits) - 1;
fs/erofs/zdata.c:133:#define Z_EROFS_PAGE_EIO (1 << 30)
Thx,
Yangtao
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: erofs: use wrapper i_blocksize() in erofs_file_read_iter()
2023-03-09 7:42 ` Yangtao Li
@ 2023-03-09 8:09 ` Gao Xiang
0 siblings, 0 replies; 9+ messages in thread
From: Gao Xiang @ 2023-03-09 8:09 UTC (permalink / raw)
To: Yangtao Li, zbestahu
Cc: chao, huyue2, jefflexu, linux-erofs, linux-kernel, xiang,
zhangwen
On 2023/3/9 15:42, Yangtao Li wrote:
>> FYI it seems that GENMASK macro is widely used in driver and arch code base, while it's rarely used in fs, except for f2fs.
>
> I think the following usage can be changed to bitmap api, just like in f2fs?
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=c4ca1f7164734a1baf40d4ff1552172a07d4fc4d
>
> fs/erofs/fscache.c:135: unsigned long flags = 1 << NETFS_SREQ_ONDEMAND;
> fs/erofs/internal.h:250:#define SECTORS_PER_BLOCK (1 << SECTORS_PER_BLOCK)
> fs/erofs/internal.h:252:#define EROFS_BLKSIZ (1 << LOG_BLOCK_SIZE)
> fs/erofs/internal.h:354: return (value >> bit) & ((1 << bits) - 1);
> fs/erofs/zmap.c:66: ((1 << Z_EROFS_VLE_DI_CLUSTER_TYPE_BITS) - 1);
> fs/erofs/zmap.c:69: m->clusterofs = 1 << vi->z_logical_clusterbits;
> fs/erofs/zmap.c:114: const unsigned int lomask = (1 << lclusterbits) - 1;
> fs/erofs/zmap.c:141: const unsigned int lomask = (1 << lclusterbits) - 1;
> fs/erofs/zmap.c:147: if (1 << amortizedshift == 4)
> fs/erofs/zmap.c:149: else if (1 << amortizedshift == 2 && lclusterbits == 12)
> fs/erofs/zmap.c:169: m->clusterofs = 1 << lclusterbits;
> fs/erofs/zmap.c:291: pos += lcn * (1 << amortizedshift);
> fs/erofs/zmap.c:409: m->compressedblks = 1 << (lclusterbits - LOG_BLOCK_SIZE);
> fs/erofs/zmap.c:457: m->clusterofs != 1 << lclusterbits);
> fs/erofs/zmap.c:497: endoff = ofs & ((1 << lclusterbits) - 1);
> fs/erofs/erofs_fs.h:120: ((1 << (EROFS_I_DATALAYOUT_BIT + EROFS_I_DATALAYOUT_BITS)) - 1)
> fs/erofs/erofs_fs.h:279:#define Z_EROFS_ALL_COMPR_ALGS ((1 << Z_EROFS_COMPRESSION_MAX) - 1)
> fs/erofs/erofs_fs.h:377:#define Z_EROFS_VLE_DI_PARTIAL_REF (1 << 15)
> fs/erofs/erofs_fs.h:384:#define Z_EROFS_VLE_DI_D0_CBLKCNT (1 << 11)
> fs/erofs/erofs_fs.h:427: .h_clusterbits = 1 << Z_EROFS_FRAGMENT_INODE_BIT
> fs/erofs/data.c:379: blksize_mask = (1 << inode->i_blkbits) - 1;
> fs/erofs/zdata.c:133:#define Z_EROFS_PAGE_EIO (1 << 30)
>
Is there some benefit to use these? BIT(1) vs 1 << 1? also almost all
filesystems rarely use such APIs honestly.
Thanks,
Gao Xiang
> Thx,
> Yangtao
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: erofs: use wrapper i_blocksize() in erofs_file_read_iter()
2023-03-09 7:15 ` Yangtao Li
2023-03-09 7:36 ` Jingbo Xu
@ 2023-03-09 7:37 ` Yue Hu
2023-03-09 8:07 ` Gao Xiang
1 sibling, 1 reply; 9+ messages in thread
From: Yue Hu @ 2023-03-09 7:37 UTC (permalink / raw)
To: Yangtao Li
Cc: chao, huyue2, jefflexu, linux-erofs, linux-kernel, xiang,
zhangwen
On Thu, 9 Mar 2023 15:15:15 +0800
Yangtao Li <frank.li@vivo.com> wrote:
> > @@ -380,7 +380,7 @@ static ssize_t erofs_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
> > if (bdev)
> > blksize_mask = bdev_logical_block_size(bdev) - 1;
> > else
> > - blksize_mask = (1 << inode->i_blkbits) - 1;
> > + blksize_mask = i_blocksize(inode) - 1;
>
> Since the mask is to be obtained here, is it more appropriate to use GENMASK(inode->i_blkbits - 1, 0)?
It should be another change independently to this patch. rt?
>
> Thx,
> Yangtao
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: erofs: use wrapper i_blocksize() in erofs_file_read_iter()
2023-03-09 7:37 ` Yue Hu
@ 2023-03-09 8:07 ` Gao Xiang
0 siblings, 0 replies; 9+ messages in thread
From: Gao Xiang @ 2023-03-09 8:07 UTC (permalink / raw)
To: Yue Hu, Yangtao Li
Cc: chao, huyue2, jefflexu, linux-erofs, linux-kernel, xiang,
zhangwen
On 2023/3/9 15:37, Yue Hu wrote:
> On Thu, 9 Mar 2023 15:15:15 +0800
> Yangtao Li <frank.li@vivo.com> wrote:
>
>>> @@ -380,7 +380,7 @@ static ssize_t erofs_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
>>> if (bdev)
>>> blksize_mask = bdev_logical_block_size(bdev) - 1;
>>> else
>>> - blksize_mask = (1 << inode->i_blkbits) - 1;
>>> + blksize_mask = i_blocksize(inode) - 1;
>>
>> Since the mask is to be obtained here, is it more appropriate to use GENMASK(inode->i_blkbits - 1, 0)?
>
> It should be another change independently to this patch. rt?
I'd suggest that keep to use (i_blocksize(inode) - 1) here, for example:
https://git.kernel.org/pub/scm/linux/kernel/git/xiang/erofs.git/tree/fs/gfs2/bmap.c#n963
Thanks,
Gao Xiang
>
>>
>> Thx,
>> Yangtao
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] erofs: use wrapper i_blocksize() in erofs_file_read_iter()
2023-03-06 7:55 [PATCH] erofs: use wrapper i_blocksize() in erofs_file_read_iter() Yue Hu
2023-03-06 14:46 ` Gao Xiang
2023-03-09 7:15 ` Yangtao Li
@ 2023-03-09 14:33 ` Chao Yu
2 siblings, 0 replies; 9+ messages in thread
From: Chao Yu @ 2023-03-09 14:33 UTC (permalink / raw)
To: Yue Hu, xiang, jefflexu, linux-erofs; +Cc: linux-kernel, huyue2, zhangwen
On 2023/3/6 15:55, Yue Hu wrote:
> From: Yue Hu <huyue2@coolpad.com>
>
> linux/fs.h has a wrapper for this operation.
>
> Signed-off-by: Yue Hu <huyue2@coolpad.com>
Reviewed-by: Chao Yu <chao@kernel.org>
Thanks,
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-03-09 14:33 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-03-06 7:55 [PATCH] erofs: use wrapper i_blocksize() in erofs_file_read_iter() Yue Hu
2023-03-06 14:46 ` Gao Xiang
2023-03-09 7:15 ` Yangtao Li
2023-03-09 7:36 ` Jingbo Xu
2023-03-09 7:42 ` Yangtao Li
2023-03-09 8:09 ` Gao Xiang
2023-03-09 7:37 ` Yue Hu
2023-03-09 8:07 ` Gao Xiang
2023-03-09 14:33 ` [PATCH] " Chao Yu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox