ext4: simplify 32bit calculation of lblk
PATCH 1/1 ext4: simplify 32bit calculation of lblk
commit <ad4fb9cafe100a> (ext4: fix 32bit overflow in ext4_ext_find_goal()) uses value compare to fix 32bit overflow. Try to simplify that.
--- fs/ext4/extents.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)
fs/ext4/extents.c +1 -4
@@ -237,10 +237,7 @@ static ext4_fsblk_t ext4_ext_find_goal(struct inode *inode,ext4_fsblk_t ext_pblk =ext4_ext_pblock (ex);ext4_lblk_t ext_block =le32_to_cpu (ex->ee_block); -if (block > ext_block) -return ext_pblk + (block - ext_block); -else -return ext_pblk - (ext_block - block); +return ext_pblk + ((signed long long )block - (signed long long )ext_block); }/* it looks like index is empty;
Signature · 1 line
2.20.1
fs/ext4/extents.c static ext4_fsblk_t ext4_ext_find_goal(struct inode *inode, · line 240
Quoted 5 lines · wuchi
fs/ext4/extents.c +1
-4-if (block > ext_block) -return ext_pblk + (block - ext_block); -else -return ext_pblk - (ext_block - block); +return ext_pblk + ((signed long long )block - (signed long long )ext_block);
And what exactly is the value add here, except for turning an easy to parse statement into a complex expression using casts?
Quoted 11 lines · Christoph Hellwig <[email protected]> 于2023年4月5日周三 13:40写道：
fs/ext4/extents.c static ext4_fsblk_t ext4_ext_find_goal(struct inode *inode, · line 240
Quoted 5 lines · wuchi
fs/ext4/extents.c +1
-4-if (block > ext_block) -return ext_pblk + (block - ext_block); -else -return ext_pblk - (ext_block - block); +return ext_pblk + ((signed long long )block - (signed long long )ext_block);And what exactly is the value add here, except for turning an easy to parse statement into a complex expression using casts?
Yes，it will be more complex. the original intention is to reduce the conditional branch.
=======
From: chi wu
Quoted 11 lines
Sent: 05 April 2023 09:48
Quoted 8 lines · Christoph Hellwig <[email protected]> 于2023年4月5日周三 13:40写道：
Quoted 5 lines · wuchi
fs/ext4/extents.c +1
-4-if (block > ext_block) -return ext_pblk + (block - ext_block); -else -return ext_pblk - (ext_block - block); +return ext_pblk + ((signed long long )block - (signed long long )ext_block);
Quoted 5 lines
Quoted 3 lines
And what exactly is the value add here, except for turning an easy to parse statement into a complex expression using casts?
Yes，it will be more complex. the original intention is to reduce the conditional branch.
What is wrong with just:
return ext_pblk + block - ext_block;
(64bit + 32bit - 32bit)
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
Quoted 16 lines · David Laight <[email protected]> 于2023年4月5日周三 19:43写道：
From: chi wu
Quoted 14 lines
Sent: 05 April 2023 09:48
Quoted 11 lines · Christoph Hellwig <[email protected]> 于2023年4月5日周三 13:40写道：
fs/ext4/extents.c static ext4_fsblk_t ext4_ext_find_goal(struct inode *inode, · line 240
Quoted 5 lines · wuchi
fs/ext4/extents.c +1
-4-if (block > ext_block) -return ext_pblk + (block - ext_block); -else -return ext_pblk - (ext_block - block); +return ext_pblk + ((signed long long )block - (signed long long )ext_block);And what exactly is the value add here, except for turning an easy to parse statement into a complex expression using casts?
Quoted 7 lines
Quoted 2 lines
Yes，it will be more complex. the original intention is to reduce the conditional branch.
What is wrong with just:
return ext_pblk + block - ext_block;(64bit + 32bit - 32bit)
oh, It's my fault. I am trapped in that ext_pblk + block may overflow, but it is ok here. thanks.
Quoted 5 lines
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
From: chi wu
Quoted 23 lines
Sent: 05 April 2023 13:48
Quoted 12 lines · David Laight <[email protected]> 于2023年4月5日周三 19:43写道：
From: chi wu
Quoted 10 lines
Sent: 05 April 2023 09:48
Quoted 7 lines · Christoph Hellwig <[email protected]> 于2023年4月5日周三 13:40写道：
Quoted 5 lines · wuchi
fs/ext4/extents.c +1
-4-if (block > ext_block) -return ext_pblk + (block - ext_block); -else -return ext_pblk - (ext_block - block); + return ext_pblk + ((signed long long)block - (signed longlong)ext_block);
Quoted 7 lines
Quoted 6 lines
Quoted 4 lines
And what exactly is the value add here, except for turning an easy to parse statement into a complex expression using casts?
Yes，it will be more complex. the original intention is to reduce the conditional branch.
Quoted 7 lines
Quoted 4 lines
What is wrong with just:
return ext_pblk + block - ext_block;(64bit + 32bit - 32bit)
oh, It's my fault. I am trapped in that
ext_pblk+ block may overflow, but it is ok here. thanks.
That doesn't matter, it will 'un-overflow' when ext_block is subtracted. You do need the '+' to happen before the '-', ok because +/- group left to right in C.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)