| Message ID | 20230403135304.19858-1-wuchi.zero@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ext4: simplify 32bit calculation of lblk | expand |
On Mon, Apr 03, 2023 at 09:53:04PM +0800, wuchi wrote: > - 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?
Christoph Hellwig <hch@infradead.org> 于2023年4月5日周三 13:40写道： > > On Mon, Apr 03, 2023 at 09:53:04PM +0800, wuchi wrote: > > - 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 > Sent: 05 April 2023 09:48 > > Christoph Hellwig <hch@infradead.org> 于2023年4月5日周三 13:40写道： > > > > On Mon, Apr 03, 2023 at 09:53:04PM +0800, wuchi wrote: > > > - 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. 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)
David Laight <David.Laight@aculab.com> 于2023年4月5日周三 19:43写道： > > From: chi wu > > Sent: 05 April 2023 09:48 > > > > Christoph Hellwig <hch@infradead.org> 于2023年4月5日周三 13:40写道： > > > > > > On Mon, Apr 03, 2023 at 09:53:04PM +0800, wuchi wrote: > > > > - 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. > > 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. > David > > - > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK > Registration No: 1397386 (Wales)
From: chi wu > Sent: 05 April 2023 13:48 > > David Laight <David.Laight@aculab.com> 于2023年4月5日周三 19:43写道： > > > > From: chi wu > > > Sent: 05 April 2023 09:48 > > > > > > Christoph Hellwig <hch@infradead.org> 于2023年4月5日周三 13:40写道： > > > > > > > > On Mon, Apr 03, 2023 at 09:53:04PM +0800, wuchi wrote: > > > > > - 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. > > > > 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)
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c index 3559ea6b0781..324b7d1386e0 100644 --- a/fs/ext4/extents.c +++ b/fs/ext4/extents.c @@ -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;
commit <ad4fb9cafe100a> (ext4: fix 32bit overflow in ext4_ext_find_goal()) uses value compare to fix 32bit overflow. Try to simplify that. Signed-off-by: wuchi <wuchi.zero@gmail.com> --- fs/ext4/extents.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)