| Message ID | 20230309152127.41427-4-frank.li@vivo.com (mailing list archive) |
|---|---|
| State | Mainlined, archived |
| Headers | show |
| Series | [v3,1/6] fs: add i_blockmask() | expand |
On Thu, Mar 09, 2023 at 11:21:25PM +0800, Yangtao Li wrote: > Use i_blockmask() to simplify code. > > Signed-off-by: Yangtao Li <frank.li@vivo.com> > --- > 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...
On Fri, Mar 10, 2023 at 03:19:40AM +0000, 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 <frank.li@vivo.com> > > --- > > 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.
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))
Use i_blockmask() to simplify code. Signed-off-by: Yangtao Li <frank.li@vivo.com> --- v3: -none v2: -convert to i_blockmask() fs/ext4/inode.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)