Re: [Cluster-devel] [PATCH 7/7] mm: return an ERR_PTR from __filemap_get_folio
On Fri, Mar 10, 2023 at 01:31:37PM +0900, Naoya Horiguchi wrote: > On Sat, Jan 21, 2023 at 07:57:55AM +0100, Christoph Hellwig wrote: > > Instead of returning NULL for all errors, distinguish between: > > > > - no entry found and not asked to allocated (-ENOENT) > > - failed to allocate memory (-ENOMEM) > > - would block (-EAGAIN) > > > > so that callers don't have to guess the error based on the passed > > in flags. > > > > Also pass through the error through the direct callers: > > filemap_get_folio, filemap_lock_folio filemap_grab_folio > > and filemap_get_incore_folio. > > > > Signed-off-by: Christoph Hellwig > > Hello, > > I found a NULL pointer dereference issue related to this patch, > so let me share it. > > Here is the bug message (I used akpm/mm-unstable on Mar 9): > > [ 2871.648659] BUG: kernel NULL pointer dereference, address: > [ 2871.651286] #PF: supervisor read access in kernel mode > [ 2871.653231] #PF: error_code(0x) - not-present page > [ 2871.655170] PGD 8001517dd067 P4D 8001517dd067 PUD 1491d1067 PMD 0 > [ 2871.657739] Oops: [#1] PREEMPT SMP PTI > [ 2871.659329] CPU: 4 PID: 1599 Comm: page-types Tainted: GEN > 6.3.0-rc1-v6.3-rc1-230309-1629-189-ga71a7+ #36 > [ 2871.663362] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS > 1.16.1-2.fc37 04/01/2014 > [ 2871.666507] RIP: 0010:mincore_page+0x19/0x90 > [ 2871.668086] Code: cc 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 0f 1f > 44 00 00 41 54 55 53 e8 92 2b 03 00 48 3d 00 f0 ff ff 77 54 48 89 c3 <48> 8b > 00 48 c1 e8 02 89 c5 83 e5 01 75 21 8b 43 34 85 c0 74 47 f0 > [ 2871.678313] RSP: 0018:be57c203fd00 EFLAGS: 00010207 > [ 2871.681422] RAX: RBX: RCX: > > [ 2871.685609] RDX: RSI: 9f59ca1506d8 RDI: > 9f59ce7c2880 > [ 2871.689599] RBP: R08: 7f9f1420 R09: > 9f59c9078508 > [ 2871.693295] R10: 7f9ed440 R11: R12: > 0200 > [ 2871.695969] R13: 0001 R14: 9f59c9ef4450 R15: > 9f59c4ac9000 > [ 2871.699927] FS: 7f9ed47ee740() GS:9f5abbc0() > knlGS: > [ 2871.703969] CS: 0010 DS: ES: CR0: 80050033 > [ 2871.706689] CR2: CR3: 000149ffe006 CR4: > 00170ee0 > [ 2871.709923] DR0: 91531760 DR1: 91531761 DR2: > 91531762 > [ 2871.713424] DR3: 91531763 DR6: 0ff0 DR7: > 0600 > [ 2871.716758] Call Trace: > [ 2871.717998] > [ 2871.719008] __mincore_unmapped_range+0x6e/0xd0 > [ 2871.721220] mincore_unmapped_range+0x16/0x30 > [ 2871.723288] walk_pgd_range+0x485/0x9e0 > [ 2871.725128] __walk_page_range+0x195/0x1b0 > [ 2871.727224] walk_page_range+0x151/0x180 > [ 2871.728883] __do_sys_mincore+0xec/0x2b0 > [ 2871.730707] do_syscall_64+0x3a/0x90 > [ 2871.732179] entry_SYSCALL_64_after_hwframe+0x72/0xdc > [ 2871.734148] RIP: 0033:0x7f9ed443f4ab > [ 2871.735548] Code: 73 01 c3 48 8b 0d 75 99 1b 00 f7 d8 64 89 01 48 83 c8 ff > c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa b8 1b 00 00 00 0f 05 <48> 3d > 01 f0 ff ff 73 01 c3 48 8b 0d 45 99 1b 00 f7 d8 64 89 01 48 > [ 2871.742194] RSP: 002b:7ffe924d72b8 EFLAGS: 0206 ORIG_RAX: > 001b > [ 2871.744787] RAX: ffda RBX: RCX: > 7f9ed443f4ab > [ 2871.747186] RDX: 7ffe92557300 RSI: 0020 RDI: > 7f9ed420 > [ 2871.749404] RBP: 7ffe92567330 R08: 0005 R09: > > [ 2871.751683] R10: 7f9ed4405d68 R11: 0206 R12: > 7ffe925674b8 > [ 2871.753925] R13: 00404af1 R14: 0040ad78 R15: > 7f9ed4833000 > [ 2871.756493] > > The precedure to reproduce this is (1) punch hole some page in a shmem > file, then (2) call mincore() over the punch-holed address range. > > I confirmed that filemap_get_incore_folio() (actually filemap_get_entry() > inside it) returns NULL in that case, so we unexpectedly enter the following > if-block for the "not found" case. Yes. Please try this fix: diff --git a/mm/swap_state.c b/mm/swap_state.c index c7160070b9daa9..b76a65ac28b319 100644 --- a/mm/swap_state.c +++ b/mm/swap_state.c @@ -390,6 +390,8 @@ struct folio *filemap_get_incore_folio(struct address_space *mapping, struct swap_info_struct *si; struct folio *folio = filemap_get_entry(mapping, index); + if (!folio) + return ERR_PTR(-ENOENT); if (!xa_is_value(folio)) return folio; if (!shmem_mapping(mapping))
Re: [Cluster-devel] erofs: convert to use i_blockmask()
> I can pick the stuff in the areas that don't have active development.
Could you please consider helping to pick this
patch("ecryptfs: make splice write available again")?
ecryptfs seems to be unmaintained.
https://lore.kernel.org/lkml/20220831033505.23178-1-frank...@vivo.com/
Thx,
Yangtao
Re: [Cluster-devel] [PATCH v4 2/5] erofs: convert to use i_blockmask()
On 2023/3/10 14:15, Yangtao Li wrote: Hi Gao Xiang, Please help drop this one since we'd like to use it until i_blockmask() lands to upstream. I'm OK. Not sure if I need to resend v5? Thanks, your patch looks fine to me. The main reasons are that 1) active cross tree development on cleanup stuffs; 2) we'd like to add subpage block support for the next cycle, and they seem somewhat convolved... So I will apply your patch when i_blockmask() is landed upstream then. Thanks, Gao Xiang Thx, Yangtao
Re: [Cluster-devel] [PATCH v4 2/5] erofs: convert to use i_blockmask()
Hi Gao Xiang, > Please help drop this one since we'd like to use it until i_blockmask() lands > to upstream. I'm OK. Not sure if I need to resend v5? Thx, Yangtao
Re: [Cluster-devel] [PATCH v4 2/5] erofs: convert to use i_blockmask()
Hi Yangtao, On 2023/3/10 13:48, Yangtao Li wrote: Use i_blockmask() to simplify code. Signed-off-by: Yangtao Li Please help drop this one since we'd like to use it until i_blockmask() lands to upstream. 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 e16545849ea7..d394102ef9de 100644 --- a/fs/erofs/data.c +++ b/fs/erofs/data.c @@ -376,7 +376,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_blockmask(inode); if ((iocb->ki_pos | iov_iter_count(to) | iov_iter_alignment(to)) & blksize_mask)
[Cluster-devel] [PATCH v4 1/5] fs: add i_blockmask()
Introduce i_blockmask() to simplify code, which replace
(i_blocksize(node) - 1). Like done in commit
93407472a21b("fs: add i_blocksize()").
Signed-off-by: Yangtao Li
---
v4:
-drop ext4 patch
-erofs patch based on mainline
-a bit change in ocfs2 patch
include/linux/fs.h | 5 +
1 file changed, 5 insertions(+)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index c85916e9f7db..17387d465b8b 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -711,6 +711,11 @@ static inline unsigned int i_blocksize(const struct inode
*node)
return (1 << node->i_blkbits);
}
+static inline unsigned int i_blockmask(const struct inode *node)
+{
+ return i_blocksize(node) - 1;
+}
+
static inline int inode_unhashed(struct inode *inode)
{
return hlist_unhashed(>i_hash);
--
2.25.1
[Cluster-devel] [PATCH v4 5/5] fs/remap_range: convert to use i_blockmask()
Use i_blockmask() to simplify code.
Signed-off-by: Yangtao Li
---
fs/remap_range.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/remap_range.c b/fs/remap_range.c
index 1331a890f2f2..7a524b620e7d 100644
--- a/fs/remap_range.c
+++ b/fs/remap_range.c
@@ -127,7 +127,7 @@ static int generic_remap_check_len(struct inode *inode_in,
loff_t *len,
unsigned int remap_flags)
{
- u64 blkmask = i_blocksize(inode_in) - 1;
+ u64 blkmask = i_blockmask(inode_in);
loff_t new_len = *len;
if ((*len & blkmask) == 0)
--
2.25.1
[Cluster-devel] [PATCH v4 4/5] ocfs2: convert to use i_blockmask()
Use i_blockmask() to simplify code. BTW convert ocfs2_is_io_unaligned
to return bool type and the fact that the value will be the same
(i.e. that ->i_blkbits is never changed by ocfs2).
Signed-off-by: Yangtao Li
---
fs/ocfs2/file.c | 9 ++---
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index efb09de4343d..7fd06a4d27d4 100644
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c
@@ -2159,14 +2159,9 @@ int ocfs2_check_range_for_refcount(struct inode *inode,
loff_t pos,
return ret;
}
-static int ocfs2_is_io_unaligned(struct inode *inode, size_t count, loff_t pos)
+static bool ocfs2_is_io_unaligned(struct inode *inode, size_t count, loff_t
pos)
{
- int blockmask = inode->i_sb->s_blocksize - 1;
- loff_t final_size = pos + count;
-
- if ((pos & blockmask) || (final_size & blockmask))
- return 1;
- return 0;
+ return ((pos | count) & i_blockmask(inode)) != 0;
}
static int ocfs2_inode_lock_for_extent_tree(struct inode *inode,
--
2.25.1
[Cluster-devel] [PATCH v4 3/5] gfs2: convert to use i_blockmask()
Use i_blockmask() to simplify code.
Signed-off-by: Yangtao Li
---
fs/gfs2/bmap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index eedf6926c652..1c6874b3851a 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -960,7 +960,7 @@ static struct folio *
gfs2_iomap_get_folio(struct iomap_iter *iter, loff_t pos, unsigned len)
{
struct inode *inode = iter->inode;
- unsigned int blockmask = i_blocksize(inode) - 1;
+ unsigned int blockmask = i_blockmask(inode);
struct gfs2_sbd *sdp = GFS2_SB(inode);
unsigned int blocks;
struct folio *folio;
--
2.25.1
[Cluster-devel] [PATCH v4 2/5] erofs: convert to use i_blockmask()
Use i_blockmask() to simplify code. Signed-off-by: Yangtao Li --- 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 e16545849ea7..d394102ef9de 100644 --- a/fs/erofs/data.c +++ b/fs/erofs/data.c @@ -376,7 +376,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_blockmask(inode); if ((iocb->ki_pos | iov_iter_count(to) | iov_iter_alignment(to)) & blksize_mask) -- 2.25.1
Re: [Cluster-devel] erofs: convert to use i_blockmask()
On Fri, Mar 10, 2023 at 11:51:21AM +0800, Yangtao Li wrote: > Hi AI, > > > Umm... What's the branchpoint for that series? > > Not the mainline - there we have i_blocksize() open-coded... > > Sorry, I'm based on the latest branch of the erofs repository. > > https://git.kernel.org/pub/scm/linux/kernel/git/xiang/erofs.git/log/?h=dev-test > > I think I can resend based on mainline. > > > Umm... That actually asks for DIV_ROUND_UP(i_size_read_inode(), > > i_blocksize(inode)) > > - compiler should bloody well be able to figure out that division by (1 << > > n) > > is shift down by n and it's easier to follow that way... > > So it seems better to change to DIV_ROUND_UP(i_size_read_inode(), > i_blocksize(inode))? > > > And the fact that the value will be the same (i.e. that ->i_blkbits is > > never changed by ocfs2) > > is worth mentioning in commit message... > > How about the following msg? > > Use i_blockmask() to simplify code. BTW convert ocfs2_is_io_unaligned > to return bool type and the fact that the value will be the same > (i.e. that ->i_blkbits is never changed by ocfs2). > > > > A small question, whether this series of changes will be merged > into each fs branch or all merged into vfs? Depends. The thing to avoid is conflicts between the trees and convoluted commit graph. In cases like that the usual approach is * put the helper into never-rebased branch - in vfs tree, in this case; I've no real objections against the helper in question. * let other trees convert to the helper at leisure - merging that never-rebased branch from vfs.git before they use the helper, of course. Or wait until the next cycle, for that matter... I can pick the stuff in the areas that don't have active development, but doing that for e.g. ext4 won't help anybody - it would only cause headache for everyone involved down the road. And I'd expect the gfs2 to be in the same situation...
Re: [Cluster-devel] erofs: convert to use i_blockmask()
Hi AI, > Umm... What's the branchpoint for that series? > Not the mainline - there we have i_blocksize() open-coded... Sorry, I'm based on the latest branch of the erofs repository. https://git.kernel.org/pub/scm/linux/kernel/git/xiang/erofs.git/log/?h=dev-test I think I can resend based on mainline. > Umm... That actually asks for DIV_ROUND_UP(i_size_read_inode(), > i_blocksize(inode)) > - compiler should bloody well be able to figure out that division by (1 << n) > is shift down by n and it's easier to follow that way... So it seems better to change to DIV_ROUND_UP(i_size_read_inode(), i_blocksize(inode))? > And the fact that the value will be the same (i.e. that ->i_blkbits is never > changed by ocfs2) > is worth mentioning in commit message... How about the following msg? Use i_blockmask() to simplify code. BTW convert ocfs2_is_io_unaligned to return bool type and the fact that the value will be the same (i.e. that ->i_blkbits is never changed by ocfs2). A small question, whether this series of changes will be merged into each fs branch or all merged into vfs? Thx, Yangtao
Re: [Cluster-devel] [PATCH v3 2/6] erofs: convert to use i_blockmask()
Hi Al, On 2023/3/10 11:15, Al Viro wrote: On Thu, Mar 09, 2023 at 11:21:23PM +0800, Yangtao Li wrote: Use i_blockmask() to simplify code. Umm... What's the branchpoint for that series? Not the mainline - there we have i_blocksize() open-coded... Actually Yue Hu sent out a clean-up patch and I applied to -next for almost a week and will be upstreamed for 6.3-rc2: https://lore.kernel.org/r/a238dca1-256f-ae2f-4a33-e54861fe4...@kernel.org/T/#t And then Yangtao would like to wrap this as a new VFS helper, I'm not sure why it's necessary since it doesn't save a lot but anyway, I'm open to it if VFS could have such new helper. Thanks, Gao Xiang Signed-off-by: Yangtao Li --- v3: -none v2: -convert to i_blockmask() 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 7e8baf56faa5..e9d1869cd4b3 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 = i_blocksize(inode) - 1; + blksize_mask = i_blockmask(inode); if ((iocb->ki_pos | iov_iter_count(to) | iov_iter_alignment(to)) & blksize_mask) -- 2.25.1
Re: [Cluster-devel] [PATCH v3 2/6] erofs: convert to use i_blockmask()
On 2023/3/10 11:42, Gao Xiang wrote: Hi Al, On 2023/3/10 11:15, Al Viro wrote: On Thu, Mar 09, 2023 at 11:21:23PM +0800, Yangtao Li wrote: Use i_blockmask() to simplify code. Umm... What's the branchpoint for that series? Not the mainline - there we have i_blocksize() open-coded... Actually Yue Hu sent out a clean-up patch and I applied to -next for almost a week and will be upstreamed for 6.3-rc2: https://lore.kernel.org/r/a238dca1-256f-ae2f-4a33-e54861fe4...@kernel.org/T/#t Sorry this link: https://lore.kernel.org/r/0261de31-e98b-85cd-80de-96af5a76e...@linux.alibaba.com Yangtao's suggestion was to use GENMASK, and I'm not sure it's a good way since (i_blocksize(inode) - 1) is simple enough, and then it becomes like this. Thanks, Gao Xiang And then Yangtao would like to wrap this as a new VFS helper, I'm not sure why it's necessary since it doesn't save a lot but anyway, I'm open to it if VFS could have such new helper. Thanks, Gao Xiang Signed-off-by: Yangtao Li --- v3: -none v2: -convert to i_blockmask() 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 7e8baf56faa5..e9d1869cd4b3 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 = i_blocksize(inode) - 1; + blksize_mask = i_blockmask(inode); if ((iocb->ki_pos | iov_iter_count(to) | iov_iter_alignment(to)) & blksize_mask) -- 2.25.1
Re: [Cluster-devel] [PATCH v3 4/6] ext4: convert to use i_blockmask()
On Fri, Mar 10, 2023 at 03:19:40AM +, Al Viro wrote:
> On Thu, Mar 09, 2023 at 11:21:25PM +0800, Yangtao Li wrote:
> > Use i_blockmask() to simplify code.
> >
> > Signed-off-by: Yangtao Li
> > ---
> > v3:
> > -none
> > v2:
> > -convert to i_blockmask()
> > fs/ext4/inode.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> > index d251d705c276..eec36520e5e9 100644
> > --- a/fs/ext4/inode.c
> > +++ b/fs/ext4/inode.c
> > @@ -2218,7 +2218,7 @@ static int mpage_process_page_bufs(struct
> > mpage_da_data *mpd,
> > {
> > struct inode *inode = mpd->inode;
> > int err;
> > - ext4_lblk_t blocks = (i_size_read(inode) + i_blocksize(inode) - 1)
> > + ext4_lblk_t blocks = (i_size_read(inode) + i_blockmask(inode))
> > >> inode->i_blkbits;
>
> Umm... That actually asks for DIV_ROUND_UP(i_size_read_inode(),
> i_blocksize(inode)) -
> compiler should bloody well be able to figure out that division by (1 << n) is
> shift down by n and it's easier to follow that way...
BTW, here the fact that you are adding the mask is misleading - it's true,
but we are not using it as a mask - it's straight arithmetics.
One can do an equivalent code where it would've been used as a mask, but that
would be harder to read -
(((i_size_read(inode) - 1) | i_blockmask(inode)) + 1) >>
inode->i_blkbits
and I doubt anyone wants that kind of puzzles to stumble upon.
Re: [Cluster-devel] [PATCH v3 5/6] ocfs2: convert to use i_blockmask()
On Thu, Mar 09, 2023 at 11:21:26PM +0800, Yangtao Li wrote:
> Use i_blockmask() to simplify code. BTW convert ocfs2_is_io_unaligned
> to return bool type.
>
> Signed-off-by: Yangtao Li
> ---
> v3:
> -none
> fs/ocfs2/file.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
> index efb09de4343d..baefab3b12c9 100644
> --- a/fs/ocfs2/file.c
> +++ b/fs/ocfs2/file.c
> @@ -2159,14 +2159,14 @@ int ocfs2_check_range_for_refcount(struct inode
> *inode, loff_t pos,
> return ret;
> }
>
> -static int ocfs2_is_io_unaligned(struct inode *inode, size_t count, loff_t
> pos)
> +static bool ocfs2_is_io_unaligned(struct inode *inode, size_t count, loff_t
> pos)
> {
> - int blockmask = inode->i_sb->s_blocksize - 1;
> + int blockmask = i_blockmask(inode);
> loff_t final_size = pos + count;
>
> if ((pos & blockmask) || (final_size & blockmask))
> - return 1;
> - return 0;
> + return true;
> + return false;
> }
Ugh...
return (pos | count) & blockmask;
surely? Conversion to bool will take care of the rest. Or you could make
that
return ((pos | count) & blockmask) != 0;
And the fact that the value will be the same (i.e. that ->i_blkbits is never
changed by ocfs2) is worth mentioning in commit message...
Re: [Cluster-devel] [PATCH v3 2/6] erofs: convert to use i_blockmask()
On Thu, Mar 09, 2023 at 11:21:23PM +0800, Yangtao Li wrote: > Use i_blockmask() to simplify code. Umm... What's the branchpoint for that series? Not the mainline - there we have i_blocksize() open-coded... > Signed-off-by: Yangtao Li > --- > v3: > -none > v2: > -convert to i_blockmask() > 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 7e8baf56faa5..e9d1869cd4b3 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 = i_blocksize(inode) - 1; > + blksize_mask = i_blockmask(inode); > > if ((iocb->ki_pos | iov_iter_count(to) | >iov_iter_alignment(to)) & blksize_mask) > -- > 2.25.1 >
Re: [Cluster-devel] [PATCH v3 4/6] ext4: convert to use i_blockmask()
On Thu, Mar 09, 2023 at 11:21:25PM +0800, Yangtao Li wrote:
> Use i_blockmask() to simplify code.
>
> Signed-off-by: Yangtao Li
> ---
> v3:
> -none
> v2:
> -convert to i_blockmask()
> fs/ext4/inode.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index d251d705c276..eec36520e5e9 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -2218,7 +2218,7 @@ static int mpage_process_page_bufs(struct mpage_da_data
> *mpd,
> {
> struct inode *inode = mpd->inode;
> int err;
> - ext4_lblk_t blocks = (i_size_read(inode) + i_blocksize(inode) - 1)
> + ext4_lblk_t blocks = (i_size_read(inode) + i_blockmask(inode))
> >> inode->i_blkbits;
Umm... That actually asks for DIV_ROUND_UP(i_size_read_inode(),
i_blocksize(inode)) -
compiler should bloody well be able to figure out that division by (1 << n) is
shift down by n and it's easier to follow that way...
[Cluster-devel] [PATCH v3 3/6] gfs2: convert to use i_blockmask()
Use i_blockmask() to simplify code.
Signed-off-by: Yangtao Li
---
v3:
-none
v2:
-convert to i_blockmask()
fs/gfs2/bmap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index eedf6926c652..1c6874b3851a 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -960,7 +960,7 @@ static struct folio *
gfs2_iomap_get_folio(struct iomap_iter *iter, loff_t pos, unsigned len)
{
struct inode *inode = iter->inode;
- unsigned int blockmask = i_blocksize(inode) - 1;
+ unsigned int blockmask = i_blockmask(inode);
struct gfs2_sbd *sdp = GFS2_SB(inode);
unsigned int blocks;
struct folio *folio;
--
2.25.1
[Cluster-devel] [PATCH v3 1/6] fs: add i_blockmask()
Introduce i_blockmask() to simplify code, which replace
(i_blocksize(node) - 1). Like done in commit
93407472a21b("fs: add i_blocksize()").
Signed-off-by: Yangtao Li
---
v3:
-none
v2:
-convert to i_blockmask()
include/linux/fs.h | 5 +
1 file changed, 5 insertions(+)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index c85916e9f7db..17387d465b8b 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -711,6 +711,11 @@ static inline unsigned int i_blocksize(const struct inode
*node)
return (1 << node->i_blkbits);
}
+static inline unsigned int i_blockmask(const struct inode *node)
+{
+ return i_blocksize(node) - 1;
+}
+
static inline int inode_unhashed(struct inode *inode)
{
return hlist_unhashed(>i_hash);
--
2.25.1
[Cluster-devel] [PATCH v3 6/6] fs/remap_range: convert to use i_blockmask()
Use i_blockmask() to simplify code.
Signed-off-by: Yangtao Li
---
fs/remap_range.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/remap_range.c b/fs/remap_range.c
index 1331a890f2f2..7a524b620e7d 100644
--- a/fs/remap_range.c
+++ b/fs/remap_range.c
@@ -127,7 +127,7 @@ static int generic_remap_check_len(struct inode *inode_in,
loff_t *len,
unsigned int remap_flags)
{
- u64 blkmask = i_blocksize(inode_in) - 1;
+ u64 blkmask = i_blockmask(inode_in);
loff_t new_len = *len;
if ((*len & blkmask) == 0)
--
2.25.1
[Cluster-devel] [PATCH v3 4/6] ext4: convert to use i_blockmask()
Use i_blockmask() to simplify code.
Signed-off-by: Yangtao Li
---
v3:
-none
v2:
-convert to i_blockmask()
fs/ext4/inode.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index d251d705c276..eec36520e5e9 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -2218,7 +2218,7 @@ static int mpage_process_page_bufs(struct mpage_da_data
*mpd,
{
struct inode *inode = mpd->inode;
int err;
- ext4_lblk_t blocks = (i_size_read(inode) + i_blocksize(inode) - 1)
+ ext4_lblk_t blocks = (i_size_read(inode) + i_blockmask(inode))
>> inode->i_blkbits;
if (ext4_verity_in_progress(inode))
--
2.25.1
[Cluster-devel] [PATCH v3 2/6] erofs: convert to use i_blockmask()
Use i_blockmask() to simplify code. Signed-off-by: Yangtao Li --- v3: -none v2: -convert to i_blockmask() 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 7e8baf56faa5..e9d1869cd4b3 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 = i_blocksize(inode) - 1; + blksize_mask = i_blockmask(inode); if ((iocb->ki_pos | iov_iter_count(to) | iov_iter_alignment(to)) & blksize_mask) -- 2.25.1
[Cluster-devel] [PATCH v3 5/6] ocfs2: convert to use i_blockmask()
Use i_blockmask() to simplify code. BTW convert ocfs2_is_io_unaligned
to return bool type.
Signed-off-by: Yangtao Li
---
v3:
-none
fs/ocfs2/file.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index efb09de4343d..baefab3b12c9 100644
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c
@@ -2159,14 +2159,14 @@ int ocfs2_check_range_for_refcount(struct inode *inode,
loff_t pos,
return ret;
}
-static int ocfs2_is_io_unaligned(struct inode *inode, size_t count, loff_t pos)
+static bool ocfs2_is_io_unaligned(struct inode *inode, size_t count, loff_t
pos)
{
- int blockmask = inode->i_sb->s_blocksize - 1;
+ int blockmask = i_blockmask(inode);
loff_t final_size = pos + count;
if ((pos & blockmask) || (final_size & blockmask))
- return 1;
- return 0;
+ return true;
+ return false;
}
static int ocfs2_inode_lock_for_extent_tree(struct inode *inode,
--
2.25.1
Re: [Cluster-devel] [PATCH v2 1/5] fs: add i_blockmask()
On Thu, Mar 09, 2023 at 08:40:31PM +0800, Yangtao Li wrote:
> Introduce i_blockmask() to simplify code, which replace
> (i_blocksize(node) - 1). Like done in commit
> 93407472a21b("fs: add i_blocksize()").
>
> Signed-off-by: Yangtao Li
> ---
Looks good but did you forget to convert fs/remap_range.c by any chance?
static int generic_remap_check_len(struct inode *inode_in,
struct inode *inode_out,
loff_t pos_out,
loff_t *len,
unsigned int remap_flags)
{
u64 blkmask = i_blocksize(inode_in) - 1;
[Cluster-devel] [PATCH v2 5/5] ocfs2: convert to use i_blockmask()
Use i_blockmask() to simplify code. BTW convert ocfs2_is_io_unaligned
to return bool type.
Signed-off-by: Yangtao Li
---
fs/ocfs2/file.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index efb09de4343d..baefab3b12c9 100644
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c
@@ -2159,14 +2159,14 @@ int ocfs2_check_range_for_refcount(struct inode *inode,
loff_t pos,
return ret;
}
-static int ocfs2_is_io_unaligned(struct inode *inode, size_t count, loff_t pos)
+static bool ocfs2_is_io_unaligned(struct inode *inode, size_t count, loff_t
pos)
{
- int blockmask = inode->i_sb->s_blocksize - 1;
+ int blockmask = i_blockmask(inode);
loff_t final_size = pos + count;
if ((pos & blockmask) || (final_size & blockmask))
- return 1;
- return 0;
+ return true;
+ return false;
}
static int ocfs2_inode_lock_for_extent_tree(struct inode *inode,
--
2.25.1
[Cluster-devel] [PATCH v2 4/5] ext4: convert to use i_blockmask()
Use i_blockmask() to simplify code.
Signed-off-by: Yangtao Li
---
v2:
-convert to i_blockmask()
fs/ext4/inode.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index d251d705c276..eec36520e5e9 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -2218,7 +2218,7 @@ static int mpage_process_page_bufs(struct mpage_da_data
*mpd,
{
struct inode *inode = mpd->inode;
int err;
- ext4_lblk_t blocks = (i_size_read(inode) + i_blocksize(inode) - 1)
+ ext4_lblk_t blocks = (i_size_read(inode) + i_blockmask(inode))
>> inode->i_blkbits;
if (ext4_verity_in_progress(inode))
--
2.25.1
[Cluster-devel] [PATCH v2 3/5] gfs2: convert to use i_blockmask()
Use i_blockmask() to simplify code.
Signed-off-by: Yangtao Li
---
v2:
-convert to i_blockmask()
fs/gfs2/bmap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index eedf6926c652..1c6874b3851a 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -960,7 +960,7 @@ static struct folio *
gfs2_iomap_get_folio(struct iomap_iter *iter, loff_t pos, unsigned len)
{
struct inode *inode = iter->inode;
- unsigned int blockmask = i_blocksize(inode) - 1;
+ unsigned int blockmask = i_blockmask(inode);
struct gfs2_sbd *sdp = GFS2_SB(inode);
unsigned int blocks;
struct folio *folio;
--
2.25.1
[Cluster-devel] [PATCH v2 1/5] fs: add i_blockmask()
Introduce i_blockmask() to simplify code, which replace
(i_blocksize(node) - 1). Like done in commit
93407472a21b("fs: add i_blocksize()").
Signed-off-by: Yangtao Li
---
v2:
-convert to i_blockmask()
include/linux/fs.h | 5 +
1 file changed, 5 insertions(+)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index c85916e9f7db..17387d465b8b 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -711,6 +711,11 @@ static inline unsigned int i_blocksize(const struct inode
*node)
return (1 << node->i_blkbits);
}
+static inline unsigned int i_blockmask(const struct inode *node)
+{
+ return i_blocksize(node) - 1;
+}
+
static inline int inode_unhashed(struct inode *inode)
{
return hlist_unhashed(>i_hash);
--
2.25.1
[Cluster-devel] [PATCH v2 2/5] erofs: convert to use i_blockmask()
Use i_blockmask() to simplify code. Signed-off-by: Yangtao Li --- v2: -convert to i_blockmask() 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 7e8baf56faa5..e9d1869cd4b3 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 = i_blocksize(inode) - 1; + blksize_mask = i_blockmask(inode); if ((iocb->ki_pos | iov_iter_count(to) | iov_iter_alignment(to)) & blksize_mask) -- 2.25.1
Re: [Cluster-devel] [PATCH 1/4] fs: add i_blocksize_mask()
On Thu, Mar 9, 2023 at 10:43 AM Yangtao Li wrote:
> Introduce i_blocksize_mask() to simplify code, which replace
> (i_blocksize(node) - 1). Like done in commit
> 93407472a21b("fs: add i_blocksize()").
I would call this i_blockmask().
Note that it could be used in ocfs2_is_io_unaligned() as well.
>
> Signed-off-by: Yangtao Li
> ---
> include/linux/fs.h | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index c85916e9f7db..db335bd9c256 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -711,6 +711,11 @@ static inline unsigned int i_blocksize(const struct
> inode *node)
> return (1 << node->i_blkbits);
> }
>
> +static inline unsigned int i_blocksize_mask(const struct inode *node)
> +{
> + return i_blocksize(node) - 1;
> +}
> +
> static inline int inode_unhashed(struct inode *inode)
> {
> return hlist_unhashed(>i_hash);
> --
> 2.25.1
>
Thanks,
Andreas
[Cluster-devel] [PATCH 1/4] fs: add i_blocksize_mask()
Introduce i_blocksize_mask() to simplify code, which replace
(i_blocksize(node) - 1). Like done in commit
93407472a21b("fs: add i_blocksize()").
Signed-off-by: Yangtao Li
---
include/linux/fs.h | 5 +
1 file changed, 5 insertions(+)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index c85916e9f7db..db335bd9c256 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -711,6 +711,11 @@ static inline unsigned int i_blocksize(const struct inode
*node)
return (1 << node->i_blkbits);
}
+static inline unsigned int i_blocksize_mask(const struct inode *node)
+{
+ return i_blocksize(node) - 1;
+}
+
static inline int inode_unhashed(struct inode *inode)
{
return hlist_unhashed(>i_hash);
--
2.25.1
[Cluster-devel] [PATCH 3/4] gfs2: convert to use i_blocksize_mask()
Use i_blocksize_mask() to simplify code.
Signed-off-by: Yangtao Li
---
fs/gfs2/bmap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index eedf6926c652..d59f9b3f0b85 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -960,7 +960,7 @@ static struct folio *
gfs2_iomap_get_folio(struct iomap_iter *iter, loff_t pos, unsigned len)
{
struct inode *inode = iter->inode;
- unsigned int blockmask = i_blocksize(inode) - 1;
+ unsigned int blockmask = i_blocksize_mask(inode);
struct gfs2_sbd *sdp = GFS2_SB(inode);
unsigned int blocks;
struct folio *folio;
--
2.25.1
[Cluster-devel] [PATCH 4/4] ext4: convert to use i_blocksize_mask()
Use i_blocksize_mask() to simplify code.
Signed-off-by: Yangtao Li
---
fs/ext4/inode.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index d251d705c276..c33f91f3b749 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -2218,7 +2218,7 @@ static int mpage_process_page_bufs(struct mpage_da_data
*mpd,
{
struct inode *inode = mpd->inode;
int err;
- ext4_lblk_t blocks = (i_size_read(inode) + i_blocksize(inode) - 1)
+ ext4_lblk_t blocks = (i_size_read(inode) + i_blocksize_mask(inode))
>> inode->i_blkbits;
if (ext4_verity_in_progress(inode))
--
2.25.1
[Cluster-devel] [PATCH 2/4] erofs: convert to use i_blocksize_mask()
Use i_blocksize_mask() to simplify code. Signed-off-by: Yangtao Li --- 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 7e8baf56faa5..234ca4dd5053 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 = i_blocksize(inode) - 1; + blksize_mask = i_blocksize_mask(inode); if ((iocb->ki_pos | iov_iter_count(to) | iov_iter_alignment(to)) & blksize_mask) -- 2.25.1
