| Message ID | 20250225194416.3076650-1-arnd@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] btrfs: use min_t() for mismatched type comparison | expand |
在 2025/2/26 06:14, Arnd Bergmann 写道: > From: Arnd Bergmann <arnd@arndb.de> > > loff_t is a signed type, so using min() to compare it against a u64 > causes a compiler warning: > > fs/btrfs/extent_io.c:2497:13: error: call to '__compiletime_assert_728' declared with 'error' attribute: min(folio_pos(folio) + folio_size(folio) - 1, end) signedness error > 2497 | cur_end = min(folio_pos(folio) + folio_size(folio) - 1, end); > | ^ > > Use min_t() instead. > > Reported-by: kernel test robot <lkp@intel.com> > Closes: https://lore.kernel.org/oe-kbuild-all/202502211908.aCcQQyEY-lkp@intel.com/ > Fixes: aba063bf9336 ("btrfs: prepare extent_io.c for future larger folio support") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> Thanks a lot, those fixes will be merged into the next version. For now the series is only for test purposes as there is a backlog of subpage block size related patches pending. Thanks, Qu > --- > fs/btrfs/extent_io.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c > index f0a1da40d641..7dc996e7e249 100644 > --- a/fs/btrfs/extent_io.c > +++ b/fs/btrfs/extent_io.c > @@ -2485,7 +2485,7 @@ void extent_write_locked_range(struct inode *inode, const struct folio *locked_f > * code is just in case, but shouldn't actually be run. > */ > if (IS_ERR(folio)) { > - cur_end = min(round_down(cur, PAGE_SIZE) + PAGE_SIZE - 1, end); > + cur_end = min_t(u64, round_down(cur, PAGE_SIZE) + PAGE_SIZE - 1, end); > cur_len = cur_end + 1 - cur; > btrfs_mark_ordered_io_finished(BTRFS_I(inode), NULL, > cur, cur_len, false); > @@ -2494,7 +2494,7 @@ void extent_write_locked_range(struct inode *inode, const struct folio *locked_f > continue; > } > > - cur_end = min(folio_pos(folio) + folio_size(folio) - 1, end); > + cur_end = min_t(u64, folio_pos(folio) + folio_size(folio) - 1, end); > cur_len = cur_end + 1 - cur; > > ASSERT(folio_test_locked(folio));
On Tue, Feb 25, 2025, at 22:22, Qu Wenruo wrote: > 在 2025/2/26 06:14, Arnd Bergmann 写道: >> From: Arnd Bergmann <arnd@arndb.de> >> >> loff_t is a signed type, so using min() to compare it against a u64 >> causes a compiler warning: >> >> fs/btrfs/extent_io.c:2497:13: error: call to '__compiletime_assert_728' declared with 'error' attribute: min(folio_pos(folio) + folio_size(folio) - 1, end) signedness error >> 2497 | cur_end = min(folio_pos(folio) + folio_size(folio) - 1, end); >> | ^ >> >> Use min_t() instead. >> >> Reported-by: kernel test robot <lkp@intel.com> >> Closes: https://lore.kernel.org/oe-kbuild-all/202502211908.aCcQQyEY-lkp@intel.com/ >> Fixes: aba063bf9336 ("btrfs: prepare extent_io.c for future larger folio support") >> Signed-off-by: Arnd Bergmann <arnd@arndb.de> > > Thanks a lot, those fixes will be merged into the next version. > > For now the series is only for test purposes as there is a backlog of > subpage block size related patches pending. Ok, thanks! Please double-check that the calculation in patch 2/2 is actually correct though, as I wasn't entirely sure about that part. Arnd
On Tue, 25 Feb 2025 20:44:10 +0100 Arnd Bergmann <arnd@kernel.org> wrote: > From: Arnd Bergmann <arnd@arndb.de> > > loff_t is a signed type, so using min() to compare it against a u64 > causes a compiler warning: > > fs/btrfs/extent_io.c:2497:13: error: call to '__compiletime_assert_728' declared with 'error' attribute: min(folio_pos(folio) + folio_size(folio) - 1, end) signedness error > 2497 | cur_end = min(folio_pos(folio) + folio_size(folio) - 1, end); Isn't the actual problem that folio_pos() has the wrong return type. I can't remember what loff_t is supposed to be for, but here you want something that reduces to 'unsigned long'. > Use min_t() instead. If a signed variable is known to contain a non-negative value then min_unsigned() is better. In particular it will never discard upper bits. Enough min_t() cause bugs (usually due to high bits being discarded when the type of the destination (eg u8) is used) that is is tempting to start a 'duck shoot' season against them. > > Reported-by: kernel test robot <lkp@intel.com> > Closes: https://lore.kernel.org/oe-kbuild-all/202502211908.aCcQQyEY-lkp@intel.com/ > Fixes: aba063bf9336 ("btrfs: prepare extent_io.c for future larger folio support") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > fs/btrfs/extent_io.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c > index f0a1da40d641..7dc996e7e249 100644 > --- a/fs/btrfs/extent_io.c > +++ b/fs/btrfs/extent_io.c > @@ -2485,7 +2485,7 @@ void extent_write_locked_range(struct inode *inode, const struct folio *locked_f > * code is just in case, but shouldn't actually be run. > */ > if (IS_ERR(folio)) { > - cur_end = min(round_down(cur, PAGE_SIZE) + PAGE_SIZE - 1, end); > + cur_end = min_t(u64, round_down(cur, PAGE_SIZE) + PAGE_SIZE - 1, end); That one is fine and doesn't need changing. > cur_len = cur_end + 1 - cur; > btrfs_mark_ordered_io_finished(BTRFS_I(inode), NULL, > cur, cur_len, false); > @@ -2494,7 +2494,7 @@ void extent_write_locked_range(struct inode *inode, const struct folio *locked_f > continue; > } > > - cur_end = min(folio_pos(folio) + folio_size(folio) - 1, end); > + cur_end = min_t(u64, folio_pos(folio) + folio_size(folio) - 1, end); A subtle alternative to min_unsigned() is to change the 1 to 1ull. David > cur_len = cur_end + 1 - cur; > > ASSERT(folio_test_locked(folio));
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c index f0a1da40d641..7dc996e7e249 100644 --- a/fs/btrfs/extent_io.c +++ b/fs/btrfs/extent_io.c @@ -2485,7 +2485,7 @@ void extent_write_locked_range(struct inode *inode, const struct folio *locked_f * code is just in case, but shouldn't actually be run. */ if (IS_ERR(folio)) { - cur_end = min(round_down(cur, PAGE_SIZE) + PAGE_SIZE - 1, end); + cur_end = min_t(u64, round_down(cur, PAGE_SIZE) + PAGE_SIZE - 1, end); cur_len = cur_end + 1 - cur; btrfs_mark_ordered_io_finished(BTRFS_I(inode), NULL, cur, cur_len, false); @@ -2494,7 +2494,7 @@ void extent_write_locked_range(struct inode *inode, const struct folio *locked_f continue; } - cur_end = min(folio_pos(folio) + folio_size(folio) - 1, end); + cur_end = min_t(u64, folio_pos(folio) + folio_size(folio) - 1, end); cur_len = cur_end + 1 - cur; ASSERT(folio_test_locked(folio));