* [PATCH 00/44] Change a lot of min_t() that might mask high bits
@ 2025-11-19 22:40 david.laight.linux
2025-11-19 22:41 ` [PATCH 30/44] fs: use min() or umin() instead of min_t() david.laight.linux
` (5 more replies)
0 siblings, 6 replies; 19+ messages in thread
From: david.laight.linux @ 2025-11-19 22:40 UTC (permalink / raw)
To: linux-kernel
Cc: Alan Stern, Alexander Viro, Alexei Starovoitov, Andi Shyti,
Andreas Dilger, Andrew Lunn, Andrew Morton, Andrii Nakryiko,
Andy Shevchenko, Ard Biesheuvel, Arnaldo Carvalho de Melo,
Bjorn Helgaas, Borislav Petkov, Christian Brauner,
Christian König, Christoph Hellwig, Daniel Borkmann,
Dan Williams, Dave Hansen, Dave Jiang, David Ahern,
David Hildenbrand, Davidlohr Bueso, David S. Miller, Dennis Zhou,
Eric Dumazet, Greg Kroah-Hartman, Herbert Xu, Ingo Molnar,
Jakub Kicinski, Jakub Sitnicki, James E.J. Bottomley,
Jarkko Sakkinen, Jason A. Donenfeld, Jens Axboe, Jiri Slaby,
Johannes Weiner, John Allen, Jonathan Cameron, Juergen Gross,
Kees Cook, KP Singh, Linus Walleij, Martin K. Petersen,
Matthew Wilcox (Oracle),
Mika Westerberg, Mike Rapoport, Miklos Szeredi, Namhyung Kim,
Neal Cardwell, nic_swsd, OGAWA Hirofumi, Olivia Mackall,
Paolo Abeni, Paolo Bonzini, Peter Huewe, Peter Zijlstra,
Rafael J. Wysocki, Sean Christopherson, Srinivas Kandagatla,
Stefano Stabellini, Steven Rostedt, Tejun Heo, Theodore Ts'o,
Thomas Gleixner, Tom Lendacky, Willem de Bruijn, x86, Yury Norov,
amd-gfx, bpf, cgroups, dri-devel, io-uring, kvm, linux-acpi,
linux-block, linux-crypto, linux-cxl, linux-efi, linux-ext4,
linux-fsdevel, linux-gpio, linux-i2c, linux-integrity, linux-mm,
linux-nvme, linux-pci, linux-perf-users, linux-scsi,
linux-serial, linux-trace-kernel, linux-usb, mptcp, netdev,
usb-storage, David Laight
From: David Laight <david.laight.linux@gmail.com>
It in not uncommon for code to use min_t(uint, a, b) when one of a or b
is 64bit and can have a value that is larger than 2^32;
This is particularly prevelant with:
uint_var = min_t(uint, uint_var, uint64_expression);
Casts to u8 and u16 are very likely to discard significant bits.
These can be detected at compile time by changing min_t(), for example:
#define CHECK_SIZE(fn, type, val) \
BUILD_BUG_ON_MSG(sizeof (val) > sizeof (type) && \
!statically_true(((val) >> 8 * (sizeof (type) - 1)) < 256), \
fn "() significant bits of '" #val "' may be discarded")
#define min_t(type, x, y) ({ \
CHECK_SIZE("min_t", type, x); \
CHECK_SIZE("min_t", type, y); \
__cmp_once(min, type, x, y); })
(and similar changes to max_t() and clamp_t().)
This shows up some real bugs, some unlikely bugs and some false positives.
In most cases both arguments are unsigned type (just different ones)
and min_t() can just be replaced by min().
The patches are all independant and are most of the ones needed to
get the x86-64 kernel I build to compile.
I've not tried building an allyesconfig or allmodconfig kernel.
I've also not included the patch to minmax.h itself.
I've tried to put the patches that actually fix things first.
The last one is 0009.
I gave up on fixing sched/fair.c - it is too broken for a single patch!
The patch for net/ipv4/tcp.c is also absent because do_tcp_getsockopt()
needs multiple/larger changes to make it 'sane'.
I've had to trim the 124 maintainers/lists that get_maintainer.pl finds
from 124 to under 100 to be able to send the cover letter.
The individual patches only go to the addresses found for the associated files.
That reduces the number of emails to a less unsane number.
David Laight (44):
x86/asm/bitops: Change the return type of variable__ffs() to unsigned
int
ext4: Fix saturation of 64bit inode times for old filesystems
perf: Fix branch stack callchain limit
io_uring/net: Change some dubious min_t()
ipc/msg: Fix saturation of percpu counts in msgctl_info()
bpf: Verifier, remove some unusual uses of min_t() and max_t()
net/core/flow_dissector: Fix cap of __skb_flow_dissect() return value.
net: ethtool: Use min3() instead of nested min_t(u16,...)
ipv6: __ip6_append_data() don't abuse max_t() casts
x86/crypto: ctr_crypt() use min() instead of min_t()
arch/x96/kvm: use min() instead of min_t()
block: use min() instead of min_t()
drivers/acpi: use min() instead of min_t()
drivers/char/hw_random: use min3() instead of nested min_t()
drivers/char/tpm: use min() instead of min_t()
drivers/crypto/ccp: use min() instead of min_t()
drivers/cxl: use min() instead of min_t()
drivers/gpio: use min() instead of min_t()
drivers/gpu/drm/amd: use min() instead of min_t()
drivers/i2c/busses: use min() instead of min_t()
drivers/net/ethernet/realtek: use min() instead of min_t()
drivers/nvme: use min() instead of min_t()
arch/x86/mm: use min() instead of min_t()
drivers/nvmem: use min() instead of min_t()
drivers/pci: use min() instead of min_t()
drivers/scsi: use min() instead of min_t()
drivers/tty/vt: use umin() instead of min_t(u16, ...) for row/col
limits
drivers/usb/storage: use min() instead of min_t()
drivers/xen: use min() instead of min_t()
fs: use min() or umin() instead of min_t()
block: bvec.h: use min() instead of min_t()
nodemask: use min() instead of min_t()
ipc: use min() instead of min_t()
bpf: use min() instead of min_t()
bpf_trace: use min() instead of min_t()
lib/bucket_locks: use min() instead of min_t()
lib/crypto/mpi: use min() instead of min_t()
lib/dynamic_queue_limits: use max() instead of max_t()
mm: use min() instead of min_t()
net: Don't pass bitfields to max_t()
net/core: Change loop conditions so min() can be used
net: use min() instead of min_t()
net/netlink: Use umin() to avoid min_t(int, ...) discarding high bits
net/mptcp: Change some dubious min_t(int, ...) to min()
arch/x86/crypto/aesni-intel_glue.c | 3 +-
arch/x86/include/asm/bitops.h | 18 +++++-------
arch/x86/kvm/emulate.c | 3 +-
arch/x86/kvm/lapic.c | 2 +-
arch/x86/kvm/mmu/mmu.c | 2 +-
arch/x86/mm/pat/set_memory.c | 12 ++++----
block/blk-iocost.c | 6 ++--
block/blk-settings.c | 2 +-
block/partitions/efi.c | 3 +-
drivers/acpi/property.c | 2 +-
drivers/char/hw_random/core.c | 2 +-
drivers/char/tpm/tpm1-cmd.c | 2 +-
drivers/char/tpm/tpm_tis_core.c | 4 +--
drivers/crypto/ccp/ccp-dev.c | 2 +-
drivers/cxl/core/mbox.c | 2 +-
drivers/gpio/gpiolib-acpi-core.c | 2 +-
.../gpu/drm/amd/amdgpu/amdgpu_doorbell_mgr.c | 4 +--
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +-
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
drivers/i2c/busses/i2c-designware-master.c | 2 +-
drivers/net/ethernet/realtek/r8169_main.c | 3 +-
drivers/nvme/host/pci.c | 3 +-
drivers/nvme/host/zns.c | 3 +-
drivers/nvmem/core.c | 2 +-
drivers/pci/probe.c | 3 +-
drivers/scsi/hosts.c | 2 +-
drivers/tty/vt/selection.c | 9 +++---
drivers/usb/storage/protocol.c | 3 +-
drivers/xen/grant-table.c | 2 +-
fs/buffer.c | 2 +-
fs/exec.c | 2 +-
fs/ext4/ext4.h | 2 +-
fs/ext4/mballoc.c | 3 +-
fs/ext4/resize.c | 2 +-
fs/ext4/super.c | 2 +-
fs/fat/dir.c | 4 +--
fs/fat/file.c | 3 +-
fs/fuse/dev.c | 2 +-
fs/fuse/file.c | 8 ++---
fs/splice.c | 2 +-
include/linux/bvec.h | 3 +-
include/linux/nodemask.h | 9 +++---
include/linux/perf_event.h | 2 +-
include/net/tcp_ecn.h | 5 ++--
io_uring/net.c | 6 ++--
ipc/mqueue.c | 4 +--
ipc/msg.c | 6 ++--
kernel/bpf/core.c | 4 +--
kernel/bpf/log.c | 2 +-
kernel/bpf/verifier.c | 29 +++++++------------
kernel/trace/bpf_trace.c | 2 +-
lib/bucket_locks.c | 2 +-
lib/crypto/mpi/mpicoder.c | 2 +-
lib/dynamic_queue_limits.c | 2 +-
mm/gup.c | 4 +--
mm/memblock.c | 2 +-
mm/memory.c | 2 +-
mm/percpu.c | 2 +-
mm/truncate.c | 3 +-
mm/vmscan.c | 2 +-
net/core/datagram.c | 6 ++--
net/core/flow_dissector.c | 7 ++---
net/core/net-sysfs.c | 3 +-
net/core/skmsg.c | 4 +--
net/ethtool/cmis_cdb.c | 7 ++---
net/ipv4/fib_trie.c | 2 +-
net/ipv4/tcp_input.c | 4 +--
net/ipv4/tcp_output.c | 5 ++--
net/ipv4/tcp_timer.c | 4 +--
net/ipv6/addrconf.c | 8 ++---
net/ipv6/ip6_output.c | 7 +++--
net/ipv6/ndisc.c | 5 ++--
net/mptcp/protocol.c | 8 ++---
net/netlink/genetlink.c | 9 +++---
net/packet/af_packet.c | 2 +-
net/unix/af_unix.c | 4 +--
76 files changed, 141 insertions(+), 176 deletions(-)
--
2.39.5
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 30/44] fs: use min() or umin() instead of min_t()
2025-11-19 22:40 [PATCH 00/44] Change a lot of min_t() that might mask high bits david.laight.linux
@ 2025-11-19 22:41 ` david.laight.linux
2025-11-25 9:06 ` Christian Brauner
2025-11-19 22:41 ` [PATCH 39/44] mm: use min() " david.laight.linux
` (4 subsequent siblings)
5 siblings, 1 reply; 19+ messages in thread
From: david.laight.linux @ 2025-11-19 22:41 UTC (permalink / raw)
To: linux-kernel, linux-ext4, linux-fsdevel, linux-mm
Cc: Alexander Viro, Andreas Dilger, Christian Brauner, Kees Cook,
Miklos Szeredi, OGAWA Hirofumi, Theodore Ts'o, David Laight
From: David Laight <david.laight.linux@gmail.com>
min_t(unsigned int, a, b) casts an 'unsigned long' to 'unsigned int'.
Use min(a, b) instead as it promotes any 'unsigned int' to 'unsigned long'
and so cannot discard significant bits.
A couple of places need umin() because of loops like:
nfolios = DIV_ROUND_UP(ret + start, PAGE_SIZE);
for (i = 0; i < nfolios; i++) {
struct folio *folio = page_folio(pages[i]);
...
unsigned int len = umin(ret, PAGE_SIZE - start);
...
ret -= len;
...
}
where the compiler doesn't track things well enough to know that
'ret' is never negative.
The alternate loop:
for (i = 0; ret > 0; i++) {
struct folio *folio = page_folio(pages[i]);
...
unsigned int len = min(ret, PAGE_SIZE - start);
...
ret -= len;
...
}
would be equivalent and doesn't need 'nfolios'.
Most of the 'unsigned long' actually come from PAGE_SIZE.
Detected by an extra check added to min_t().
Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
fs/buffer.c | 2 +-
fs/exec.c | 2 +-
fs/ext4/mballoc.c | 3 +--
fs/ext4/resize.c | 2 +-
fs/ext4/super.c | 2 +-
fs/fat/dir.c | 4 ++--
fs/fat/file.c | 3 +--
fs/fuse/dev.c | 2 +-
fs/fuse/file.c | 8 +++-----
fs/splice.c | 2 +-
10 files changed, 13 insertions(+), 17 deletions(-)
diff --git a/fs/buffer.c b/fs/buffer.c
index 6a8752f7bbed..26c4c760b6c6 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -2354,7 +2354,7 @@ bool block_is_partially_uptodate(struct folio *folio, size_t from, size_t count)
if (!head)
return false;
blocksize = head->b_size;
- to = min_t(unsigned, folio_size(folio) - from, count);
+ to = min(folio_size(folio) - from, count);
to = from + to;
if (from < blocksize && to > folio_size(folio) - blocksize)
return false;
diff --git a/fs/exec.c b/fs/exec.c
index 4298e7e08d5d..6d699e48df82 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -555,7 +555,7 @@ int copy_string_kernel(const char *arg, struct linux_binprm *bprm)
return -E2BIG;
while (len > 0) {
- unsigned int bytes_to_copy = min_t(unsigned int, len,
+ unsigned int bytes_to_copy = min(len,
min_not_zero(offset_in_page(pos), PAGE_SIZE));
struct page *page;
diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 9087183602e4..cb68ea974de6 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -4254,8 +4254,7 @@ void ext4_mb_mark_bb(struct super_block *sb, ext4_fsblk_t block,
* get the corresponding group metadata to work with.
* For this we have goto again loop.
*/
- thisgrp_len = min_t(unsigned int, (unsigned int)len,
- EXT4_BLOCKS_PER_GROUP(sb) - EXT4_C2B(sbi, blkoff));
+ thisgrp_len = min(len, EXT4_BLOCKS_PER_GROUP(sb) - EXT4_C2B(sbi, blkoff));
clen = EXT4_NUM_B2C(sbi, thisgrp_len);
if (!ext4_sb_block_valid(sb, NULL, block, thisgrp_len)) {
diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
index 050f26168d97..76842f0957b5 100644
--- a/fs/ext4/resize.c
+++ b/fs/ext4/resize.c
@@ -1479,7 +1479,7 @@ static void ext4_update_super(struct super_block *sb,
/* Update the global fs size fields */
sbi->s_groups_count += flex_gd->count;
- sbi->s_blockfile_groups = min_t(ext4_group_t, sbi->s_groups_count,
+ sbi->s_blockfile_groups = min(sbi->s_groups_count,
(EXT4_MAX_BLOCK_FILE_PHYS / EXT4_BLOCKS_PER_GROUP(sb)));
/* Update the reserved block counts only once the new group is
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 33e7c08c9529..e116fe48ff43 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -4830,7 +4830,7 @@ static int ext4_check_geometry(struct super_block *sb,
return -EINVAL;
}
sbi->s_groups_count = blocks_count;
- sbi->s_blockfile_groups = min_t(ext4_group_t, sbi->s_groups_count,
+ sbi->s_blockfile_groups = min(sbi->s_groups_count,
(EXT4_MAX_BLOCK_FILE_PHYS / EXT4_BLOCKS_PER_GROUP(sb)));
if (((u64)sbi->s_groups_count * sbi->s_inodes_per_group) !=
le32_to_cpu(es->s_inodes_count)) {
diff --git a/fs/fat/dir.c b/fs/fat/dir.c
index 92b091783966..8375e7fbc1a5 100644
--- a/fs/fat/dir.c
+++ b/fs/fat/dir.c
@@ -1353,7 +1353,7 @@ int fat_add_entries(struct inode *dir, void *slots, int nr_slots,
/* Fill the long name slots. */
for (i = 0; i < long_bhs; i++) {
- int copy = min_t(int, sb->s_blocksize - offset, size);
+ int copy = umin(sb->s_blocksize - offset, size);
memcpy(bhs[i]->b_data + offset, slots, copy);
mark_buffer_dirty_inode(bhs[i], dir);
offset = 0;
@@ -1364,7 +1364,7 @@ int fat_add_entries(struct inode *dir, void *slots, int nr_slots,
err = fat_sync_bhs(bhs, long_bhs);
if (!err && i < nr_bhs) {
/* Fill the short name slot. */
- int copy = min_t(int, sb->s_blocksize - offset, size);
+ int copy = umin(sb->s_blocksize - offset, size);
memcpy(bhs[i]->b_data + offset, slots, copy);
mark_buffer_dirty_inode(bhs[i], dir);
if (IS_DIRSYNC(dir))
diff --git a/fs/fat/file.c b/fs/fat/file.c
index 4fc49a614fb8..f48435e586c7 100644
--- a/fs/fat/file.c
+++ b/fs/fat/file.c
@@ -140,8 +140,7 @@ static int fat_ioctl_fitrim(struct inode *inode, unsigned long arg)
if (copy_from_user(&range, user_range, sizeof(range)))
return -EFAULT;
- range.minlen = max_t(unsigned int, range.minlen,
- bdev_discard_granularity(sb->s_bdev));
+ range.minlen = max(range.minlen, bdev_discard_granularity(sb->s_bdev));
err = fat_trim_fs(inode, &range);
if (err < 0)
diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index 132f38619d70..0c9fb0db1de1 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -1813,7 +1813,7 @@ static int fuse_notify_store(struct fuse_conn *fc, unsigned int size,
goto out_iput;
folio_offset = ((index - folio->index) << PAGE_SHIFT) + offset;
- nr_bytes = min_t(unsigned, num, folio_size(folio) - folio_offset);
+ nr_bytes = min(num, folio_size(folio) - folio_offset);
nr_pages = (offset + nr_bytes + PAGE_SIZE - 1) >> PAGE_SHIFT;
err = fuse_copy_folio(cs, &folio, folio_offset, nr_bytes, 0);
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index f1ef77a0be05..f4ffa559ad26 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -1252,10 +1252,8 @@ static ssize_t fuse_fill_write_pages(struct fuse_io_args *ia,
static inline unsigned int fuse_wr_pages(loff_t pos, size_t len,
unsigned int max_pages)
{
- return min_t(unsigned int,
- ((pos + len - 1) >> PAGE_SHIFT) -
- (pos >> PAGE_SHIFT) + 1,
- max_pages);
+ return min(((pos + len - 1) >> PAGE_SHIFT) - (pos >> PAGE_SHIFT) + 1,
+ max_pages);
}
static ssize_t fuse_perform_write(struct kiocb *iocb, struct iov_iter *ii)
@@ -1550,7 +1548,7 @@ static int fuse_get_user_pages(struct fuse_args_pages *ap, struct iov_iter *ii,
struct folio *folio = page_folio(pages[i]);
unsigned int offset = start +
(folio_page_idx(folio, pages[i]) << PAGE_SHIFT);
- unsigned int len = min_t(unsigned int, ret, PAGE_SIZE - start);
+ unsigned int len = umin(ret, PAGE_SIZE - start);
ap->descs[ap->num_folios].offset = offset;
ap->descs[ap->num_folios].length = len;
diff --git a/fs/splice.c b/fs/splice.c
index f5094b6d00a0..41ce3a4ef74f 100644
--- a/fs/splice.c
+++ b/fs/splice.c
@@ -1467,7 +1467,7 @@ static ssize_t iter_to_pipe(struct iov_iter *from,
n = DIV_ROUND_UP(left + start, PAGE_SIZE);
for (i = 0; i < n; i++) {
- int size = min_t(int, left, PAGE_SIZE - start);
+ int size = umin(left, PAGE_SIZE - start);
buf.page = pages[i];
buf.offset = start;
--
2.39.5
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 39/44] mm: use min() instead of min_t()
2025-11-19 22:40 [PATCH 00/44] Change a lot of min_t() that might mask high bits david.laight.linux
2025-11-19 22:41 ` [PATCH 30/44] fs: use min() or umin() instead of min_t() david.laight.linux
@ 2025-11-19 22:41 ` david.laight.linux
2025-11-20 9:20 ` David Hildenbrand (Red Hat)
2025-11-20 10:36 ` Lorenzo Stoakes
2025-11-20 1:47 ` [PATCH 00/44] Change a lot of min_t() that might mask high bits Jakub Kicinski
` (3 subsequent siblings)
5 siblings, 2 replies; 19+ messages in thread
From: david.laight.linux @ 2025-11-19 22:41 UTC (permalink / raw)
To: linux-kernel, linux-fsdevel, linux-mm
Cc: Andrew Morton, Axel Rasmussen, Christoph Lameter,
David Hildenbrand, Dennis Zhou, Johannes Weiner,
Matthew Wilcox (Oracle),
Mike Rapoport, Tejun Heo, Yuanchu Xie, David Laight
From: David Laight <david.laight.linux@gmail.com>
min_t(unsigned int, a, b) casts an 'unsigned long' to 'unsigned int'.
Use min(a, b) instead as it promotes any 'unsigned int' to 'unsigned long'
and so cannot discard significant bits.
In this case the 'unsigned long' values are small enough that the result
is ok.
(Similarly for clamp_t().)
Detected by an extra check added to min_t().
Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
mm/gup.c | 4 ++--
mm/memblock.c | 2 +-
mm/memory.c | 2 +-
mm/percpu.c | 2 +-
mm/truncate.c | 3 +--
mm/vmscan.c | 2 +-
6 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/mm/gup.c b/mm/gup.c
index a8ba5112e4d0..55435b90dcc3 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -237,8 +237,8 @@ static inline struct folio *gup_folio_range_next(struct page *start,
unsigned int nr = 1;
if (folio_test_large(folio))
- nr = min_t(unsigned int, npages - i,
- folio_nr_pages(folio) - folio_page_idx(folio, next));
+ nr = min(npages - i,
+ folio_nr_pages(folio) - folio_page_idx(folio, next));
*ntails = nr;
return folio;
diff --git a/mm/memblock.c b/mm/memblock.c
index e23e16618e9b..19b491d39002 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -2208,7 +2208,7 @@ static void __init __free_pages_memory(unsigned long start, unsigned long end)
* the case.
*/
if (start)
- order = min_t(int, MAX_PAGE_ORDER, __ffs(start));
+ order = min(MAX_PAGE_ORDER, __ffs(start));
else
order = MAX_PAGE_ORDER;
diff --git a/mm/memory.c b/mm/memory.c
index 74b45e258323..72f7bd71d65f 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2375,7 +2375,7 @@ static int insert_pages(struct vm_area_struct *vma, unsigned long addr,
while (pages_to_write_in_pmd) {
int pte_idx = 0;
- const int batch_size = min_t(int, pages_to_write_in_pmd, 8);
+ const int batch_size = min(pages_to_write_in_pmd, 8);
start_pte = pte_offset_map_lock(mm, pmd, addr, &pte_lock);
if (!start_pte) {
diff --git a/mm/percpu.c b/mm/percpu.c
index 81462ce5866e..cad59221d298 100644
--- a/mm/percpu.c
+++ b/mm/percpu.c
@@ -1228,7 +1228,7 @@ static int pcpu_alloc_area(struct pcpu_chunk *chunk, int alloc_bits,
/*
* Search to find a fit.
*/
- end = min_t(int, start + alloc_bits + PCPU_BITMAP_BLOCK_BITS,
+ end = umin(start + alloc_bits + PCPU_BITMAP_BLOCK_BITS,
pcpu_chunk_map_bits(chunk));
bit_off = pcpu_find_zero_area(chunk->alloc_map, end, start, alloc_bits,
align_mask, &area_off, &area_bits);
diff --git a/mm/truncate.c b/mm/truncate.c
index 91eb92a5ce4f..7a56372d39a3 100644
--- a/mm/truncate.c
+++ b/mm/truncate.c
@@ -849,8 +849,7 @@ void pagecache_isize_extended(struct inode *inode, loff_t from, loff_t to)
unsigned int offset, end;
offset = from - folio_pos(folio);
- end = min_t(unsigned int, to - folio_pos(folio),
- folio_size(folio));
+ end = umin(to - folio_pos(folio), folio_size(folio));
folio_zero_segment(folio, offset, end);
}
diff --git a/mm/vmscan.c b/mm/vmscan.c
index b2fc8b626d3d..82cd99a5d843 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -3489,7 +3489,7 @@ static struct folio *get_pfn_folio(unsigned long pfn, struct mem_cgroup *memcg,
static bool suitable_to_scan(int total, int young)
{
- int n = clamp_t(int, cache_line_size() / sizeof(pte_t), 2, 8);
+ int n = clamp(cache_line_size() / sizeof(pte_t), 2, 8);
/* suitable if the average number of young PTEs per cacheline is >=1 */
return young * n >= total;
--
2.39.5
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 00/44] Change a lot of min_t() that might mask high bits
2025-11-19 22:40 [PATCH 00/44] Change a lot of min_t() that might mask high bits david.laight.linux
2025-11-19 22:41 ` [PATCH 30/44] fs: use min() or umin() instead of min_t() david.laight.linux
2025-11-19 22:41 ` [PATCH 39/44] mm: use min() " david.laight.linux
@ 2025-11-20 1:47 ` Jakub Kicinski
2025-11-20 9:38 ` Lorenzo Stoakes
` (2 subsequent siblings)
5 siblings, 0 replies; 19+ messages in thread
From: Jakub Kicinski @ 2025-11-20 1:47 UTC (permalink / raw)
To: david.laight.linux
Cc: linux-kernel, Alan Stern, Alexander Viro, Alexei Starovoitov,
Andi Shyti, Andreas Dilger, Andrew Lunn, Andrew Morton,
Andrii Nakryiko, Andy Shevchenko, Ard Biesheuvel,
Arnaldo Carvalho de Melo, Bjorn Helgaas, Borislav Petkov,
Christian Brauner, Christian König, Christoph Hellwig,
Daniel Borkmann, Dan Williams, Dave Hansen, Dave Jiang,
David Ahern, David Hildenbrand, Davidlohr Bueso, David S. Miller,
Dennis Zhou, Eric Dumazet, Greg Kroah-Hartman, Herbert Xu,
Ingo Molnar, Jakub Sitnicki, James E.J. Bottomley,
Jarkko Sakkinen, Jason A. Donenfeld, Jens Axboe, Jiri Slaby,
Johannes Weiner, John Allen, Jonathan Cameron, Juergen Gross,
Kees Cook, KP Singh, Linus Walleij, Martin K. Petersen,
Matthew Wilcox (Oracle),
Mika Westerberg, Mike Rapoport, Miklos Szeredi, Namhyung Kim,
Neal Cardwell, nic_swsd, OGAWA Hirofumi, Olivia Mackall,
Paolo Abeni, Paolo Bonzini, Peter Huewe, Peter Zijlstra,
Rafael J. Wysocki, Sean Christopherson, Srinivas Kandagatla,
Stefano Stabellini, Steven Rostedt, Tejun Heo, Theodore Ts'o,
Thomas Gleixner, Tom Lendacky, Willem de Bruijn, x86, Yury Norov,
amd-gfx, bpf, cgroups, dri-devel, io-uring, kvm, linux-acpi,
linux-block, linux-crypto, linux-cxl, linux-efi, linux-ext4,
linux-fsdevel, linux-gpio, linux-i2c, linux-integrity, linux-mm,
linux-nvme, linux-pci, linux-perf-users, linux-scsi,
linux-serial, linux-trace-kernel, linux-usb, mptcp, netdev,
usb-storage
On Wed, 19 Nov 2025 22:40:56 +0000 david.laight.linux@gmail.com wrote:
> I've had to trim the 124 maintainers/lists that get_maintainer.pl finds
> from 124 to under 100 to be able to send the cover letter.
> The individual patches only go to the addresses found for the associated files.
> That reduces the number of emails to a less unsane number.
Please split the networking (9?) patches out to a separate series.
It will help you with the CC list, and help us to get this applied..
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 39/44] mm: use min() instead of min_t()
2025-11-19 22:41 ` [PATCH 39/44] mm: use min() " david.laight.linux
@ 2025-11-20 9:20 ` David Hildenbrand (Red Hat)
2025-11-20 9:59 ` David Laight
2025-11-20 10:36 ` Lorenzo Stoakes
1 sibling, 1 reply; 19+ messages in thread
From: David Hildenbrand (Red Hat) @ 2025-11-20 9:20 UTC (permalink / raw)
To: david.laight.linux, linux-kernel, linux-fsdevel, linux-mm
Cc: Andrew Morton, Axel Rasmussen, Christoph Lameter, Dennis Zhou,
Johannes Weiner, Matthew Wilcox (Oracle),
Mike Rapoport, Tejun Heo, Yuanchu Xie
On 11/19/25 23:41, david.laight.linux@gmail.com wrote:
> From: David Laight <david.laight.linux@gmail.com>
>
> min_t(unsigned int, a, b) casts an 'unsigned long' to 'unsigned int'.
> Use min(a, b) instead as it promotes any 'unsigned int' to 'unsigned long'
> and so cannot discard significant bits.
I thought using min() was frowned upon and we were supposed to use
min_t() instead to make it clear which type we want to use.
Do I misremember or have things changed?
Wasn't there a checkpatch warning that states exactly that?
--
Cheers
David
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 00/44] Change a lot of min_t() that might mask high bits
2025-11-19 22:40 [PATCH 00/44] Change a lot of min_t() that might mask high bits david.laight.linux
` (2 preceding siblings ...)
2025-11-20 1:47 ` [PATCH 00/44] Change a lot of min_t() that might mask high bits Jakub Kicinski
@ 2025-11-20 9:38 ` Lorenzo Stoakes
2025-11-20 14:52 ` (subset) " Jens Axboe
2025-11-24 9:49 ` Herbert Xu
5 siblings, 0 replies; 19+ messages in thread
From: Lorenzo Stoakes @ 2025-11-20 9:38 UTC (permalink / raw)
To: david.laight.linux
Cc: linux-kernel, Alan Stern, Alexander Viro, Alexei Starovoitov,
Andi Shyti, Andreas Dilger, Andrew Lunn, Andrew Morton,
Andrii Nakryiko, Andy Shevchenko, Ard Biesheuvel,
Arnaldo Carvalho de Melo, Bjorn Helgaas, Borislav Petkov,
Christian Brauner, Christian König, Christoph Hellwig,
Daniel Borkmann, Dan Williams, Dave Hansen, Dave Jiang,
David Ahern, David Hildenbrand, Davidlohr Bueso, David S. Miller,
Dennis Zhou, Eric Dumazet, Greg Kroah-Hartman, Herbert Xu,
Ingo Molnar, Jakub Kicinski, Jakub Sitnicki,
James E.J. Bottomley, Jarkko Sakkinen, Jason A. Donenfeld,
Jens Axboe, Jiri Slaby, Johannes Weiner, John Allen,
Jonathan Cameron, Juergen Gross, Kees Cook, KP Singh,
Linus Walleij, Martin K. Petersen, Matthew Wilcox (Oracle),
Mika Westerberg, Mike Rapoport, Miklos Szeredi, Namhyung Kim,
Neal Cardwell, nic_swsd, OGAWA Hirofumi, Olivia Mackall,
Paolo Abeni, Paolo Bonzini, Peter Huewe, Peter Zijlstra,
Rafael J. Wysocki, Sean Christopherson, Srinivas Kandagatla,
Stefano Stabellini, Steven Rostedt, Tejun Heo, Theodore Ts'o,
Thomas Gleixner, Tom Lendacky, Willem de Bruijn, x86, Yury Norov,
amd-gfx, bpf, cgroups, dri-devel, io-uring, kvm, linux-acpi,
linux-block, linux-crypto, linux-cxl, linux-efi, linux-ext4,
linux-fsdevel, linux-gpio, linux-i2c, linux-integrity, linux-mm,
linux-nvme, linux-pci, linux-perf-users, linux-scsi,
linux-serial, linux-trace-kernel, linux-usb, mptcp, netdev,
usb-storage
On Wed, Nov 19, 2025 at 10:40:56PM +0000, david.laight.linux@gmail.com wrote:
> From: David Laight <david.laight.linux@gmail.com>
>
> It in not uncommon for code to use min_t(uint, a, b) when one of a or b
> is 64bit and can have a value that is larger than 2^32;
> This is particularly prevelant with:
> uint_var = min_t(uint, uint_var, uint64_expression);
>
> Casts to u8 and u16 are very likely to discard significant bits.
>
> These can be detected at compile time by changing min_t(), for example:
> #define CHECK_SIZE(fn, type, val) \
> BUILD_BUG_ON_MSG(sizeof (val) > sizeof (type) && \
> !statically_true(((val) >> 8 * (sizeof (type) - 1)) < 256), \
> fn "() significant bits of '" #val "' may be discarded")
>
> #define min_t(type, x, y) ({ \
> CHECK_SIZE("min_t", type, x); \
> CHECK_SIZE("min_t", type, y); \
> __cmp_once(min, type, x, y); })
>
> (and similar changes to max_t() and clamp_t().)
Have we made sure that the introduction of these don't cause a combinatorial
explosion like previous min()/max() changes did?
>
> This shows up some real bugs, some unlikely bugs and some false positives.
> In most cases both arguments are unsigned type (just different ones)
> and min_t() can just be replaced by min().
>
> The patches are all independant and are most of the ones needed to
> get the x86-64 kernel I build to compile.
> I've not tried building an allyesconfig or allmodconfig kernel.
Well I have a beefy box at my disposal so tried thiese for you :)
Both allyesconfig & allmodconfig works fine for x86-64 (I tried both for good
measure)
> I've also not included the patch to minmax.h itself.
>
> I've tried to put the patches that actually fix things first.
> The last one is 0009.
>
> I gave up on fixing sched/fair.c - it is too broken for a single patch!
> The patch for net/ipv4/tcp.c is also absent because do_tcp_getsockopt()
> needs multiple/larger changes to make it 'sane'.
I guess this isn't broken per se there just retain min_t()/max_t() right?
>
> I've had to trim the 124 maintainers/lists that get_maintainer.pl finds
> from 124 to under 100 to be able to send the cover letter.
> The individual patches only go to the addresses found for the associated files.
> That reduces the number of emails to a less unsane number.
>
> David Laight (44):
> x86/asm/bitops: Change the return type of variable__ffs() to unsigned
> int
> ext4: Fix saturation of 64bit inode times for old filesystems
> perf: Fix branch stack callchain limit
> io_uring/net: Change some dubious min_t()
> ipc/msg: Fix saturation of percpu counts in msgctl_info()
> bpf: Verifier, remove some unusual uses of min_t() and max_t()
> net/core/flow_dissector: Fix cap of __skb_flow_dissect() return value.
> net: ethtool: Use min3() instead of nested min_t(u16,...)
> ipv6: __ip6_append_data() don't abuse max_t() casts
> x86/crypto: ctr_crypt() use min() instead of min_t()
> arch/x96/kvm: use min() instead of min_t()
> block: use min() instead of min_t()
> drivers/acpi: use min() instead of min_t()
> drivers/char/hw_random: use min3() instead of nested min_t()
> drivers/char/tpm: use min() instead of min_t()
> drivers/crypto/ccp: use min() instead of min_t()
> drivers/cxl: use min() instead of min_t()
> drivers/gpio: use min() instead of min_t()
> drivers/gpu/drm/amd: use min() instead of min_t()
> drivers/i2c/busses: use min() instead of min_t()
> drivers/net/ethernet/realtek: use min() instead of min_t()
> drivers/nvme: use min() instead of min_t()
> arch/x86/mm: use min() instead of min_t()
> drivers/nvmem: use min() instead of min_t()
> drivers/pci: use min() instead of min_t()
> drivers/scsi: use min() instead of min_t()
> drivers/tty/vt: use umin() instead of min_t(u16, ...) for row/col
> limits
> drivers/usb/storage: use min() instead of min_t()
> drivers/xen: use min() instead of min_t()
> fs: use min() or umin() instead of min_t()
> block: bvec.h: use min() instead of min_t()
> nodemask: use min() instead of min_t()
> ipc: use min() instead of min_t()
> bpf: use min() instead of min_t()
> bpf_trace: use min() instead of min_t()
> lib/bucket_locks: use min() instead of min_t()
> lib/crypto/mpi: use min() instead of min_t()
> lib/dynamic_queue_limits: use max() instead of max_t()
> mm: use min() instead of min_t()
> net: Don't pass bitfields to max_t()
> net/core: Change loop conditions so min() can be used
> net: use min() instead of min_t()
> net/netlink: Use umin() to avoid min_t(int, ...) discarding high bits
> net/mptcp: Change some dubious min_t(int, ...) to min()
>
> arch/x86/crypto/aesni-intel_glue.c | 3 +-
> arch/x86/include/asm/bitops.h | 18 +++++-------
> arch/x86/kvm/emulate.c | 3 +-
> arch/x86/kvm/lapic.c | 2 +-
> arch/x86/kvm/mmu/mmu.c | 2 +-
> arch/x86/mm/pat/set_memory.c | 12 ++++----
> block/blk-iocost.c | 6 ++--
> block/blk-settings.c | 2 +-
> block/partitions/efi.c | 3 +-
> drivers/acpi/property.c | 2 +-
> drivers/char/hw_random/core.c | 2 +-
> drivers/char/tpm/tpm1-cmd.c | 2 +-
> drivers/char/tpm/tpm_tis_core.c | 4 +--
> drivers/crypto/ccp/ccp-dev.c | 2 +-
> drivers/cxl/core/mbox.c | 2 +-
> drivers/gpio/gpiolib-acpi-core.c | 2 +-
> .../gpu/drm/amd/amdgpu/amdgpu_doorbell_mgr.c | 4 +--
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +-
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
> drivers/i2c/busses/i2c-designware-master.c | 2 +-
> drivers/net/ethernet/realtek/r8169_main.c | 3 +-
> drivers/nvme/host/pci.c | 3 +-
> drivers/nvme/host/zns.c | 3 +-
> drivers/nvmem/core.c | 2 +-
> drivers/pci/probe.c | 3 +-
> drivers/scsi/hosts.c | 2 +-
> drivers/tty/vt/selection.c | 9 +++---
> drivers/usb/storage/protocol.c | 3 +-
> drivers/xen/grant-table.c | 2 +-
> fs/buffer.c | 2 +-
> fs/exec.c | 2 +-
> fs/ext4/ext4.h | 2 +-
> fs/ext4/mballoc.c | 3 +-
> fs/ext4/resize.c | 2 +-
> fs/ext4/super.c | 2 +-
> fs/fat/dir.c | 4 +--
> fs/fat/file.c | 3 +-
> fs/fuse/dev.c | 2 +-
> fs/fuse/file.c | 8 ++---
> fs/splice.c | 2 +-
> include/linux/bvec.h | 3 +-
> include/linux/nodemask.h | 9 +++---
> include/linux/perf_event.h | 2 +-
> include/net/tcp_ecn.h | 5 ++--
> io_uring/net.c | 6 ++--
> ipc/mqueue.c | 4 +--
> ipc/msg.c | 6 ++--
> kernel/bpf/core.c | 4 +--
> kernel/bpf/log.c | 2 +-
> kernel/bpf/verifier.c | 29 +++++++------------
> kernel/trace/bpf_trace.c | 2 +-
> lib/bucket_locks.c | 2 +-
> lib/crypto/mpi/mpicoder.c | 2 +-
> lib/dynamic_queue_limits.c | 2 +-
> mm/gup.c | 4 +--
> mm/memblock.c | 2 +-
> mm/memory.c | 2 +-
> mm/percpu.c | 2 +-
> mm/truncate.c | 3 +-
> mm/vmscan.c | 2 +-
> net/core/datagram.c | 6 ++--
> net/core/flow_dissector.c | 7 ++---
> net/core/net-sysfs.c | 3 +-
> net/core/skmsg.c | 4 +--
> net/ethtool/cmis_cdb.c | 7 ++---
> net/ipv4/fib_trie.c | 2 +-
> net/ipv4/tcp_input.c | 4 +--
> net/ipv4/tcp_output.c | 5 ++--
> net/ipv4/tcp_timer.c | 4 +--
> net/ipv6/addrconf.c | 8 ++---
> net/ipv6/ip6_output.c | 7 +++--
> net/ipv6/ndisc.c | 5 ++--
> net/mptcp/protocol.c | 8 ++---
> net/netlink/genetlink.c | 9 +++---
> net/packet/af_packet.c | 2 +-
> net/unix/af_unix.c | 4 +--
> 76 files changed, 141 insertions(+), 176 deletions(-)
>
> --
> 2.39.5
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 39/44] mm: use min() instead of min_t()
2025-11-20 9:20 ` David Hildenbrand (Red Hat)
@ 2025-11-20 9:59 ` David Laight
2025-11-20 23:45 ` Eric Biggers
0 siblings, 1 reply; 19+ messages in thread
From: David Laight @ 2025-11-20 9:59 UTC (permalink / raw)
To: David Hildenbrand (Red Hat)
Cc: linux-kernel, linux-fsdevel, linux-mm, Andrew Morton,
Axel Rasmussen, Christoph Lameter, Dennis Zhou, Johannes Weiner,
Matthew Wilcox (Oracle),
Mike Rapoport, Tejun Heo, Yuanchu Xie
On Thu, 20 Nov 2025 10:20:41 +0100
"David Hildenbrand (Red Hat)" <david@kernel.org> wrote:
> On 11/19/25 23:41, david.laight.linux@gmail.com wrote:
> > From: David Laight <david.laight.linux@gmail.com>
> >
> > min_t(unsigned int, a, b) casts an 'unsigned long' to 'unsigned int'.
> > Use min(a, b) instead as it promotes any 'unsigned int' to 'unsigned long'
> > and so cannot discard significant bits.
>
> I thought using min() was frowned upon and we were supposed to use
> min_t() instead to make it clear which type we want to use.
I'm not sure that was ever true.
min_t() is just an accident waiting to happen.
(and I found a few of them, the worst are in sched/fair.c)
Most of the min_t() are there because of the rather overzealous type
check that used to be in min().
But even then it would really be better to explicitly cast one of the
parameters to min(), so min_t(T, a, b) => min(a, (T)b).
Then it becomes rather more obvious that min_t(u8, x->m_u8, expr)
is going mask off the high bits of 'expr'.
> Do I misremember or have things changed?
>
> Wasn't there a checkpatch warning that states exactly that?
There is one that suggests min_t() - it ought to be nuked.
The real fix is to backtrack the types so there isn't an error.
min_t() ought to be a 'last resort' and a single cast is better.
With the relaxed checks in min() most of the min_t() can just
be replaced by min(), even this is ok:
int len = fun();
if (len < 0)
return;
count = min(len, sizeof(T));
I did look at the history of min() and min_t().
IIRC some of the networking code had a real function min() with
'unsigned int' arguments.
This was moved to a common header, changed to a #define and had
a type added - so min(T, a, b).
Pretty much immediately that was renamed min_t() and min() added
that accepted any type - but checked the types of 'a' and 'b'
exactly matched.
Code was then changed (over the years) to use min(), but in many
cases the types didn't quite match - so min_t() was used a lot.
I keep spotting new commits that pass too small a type to min_t().
So this is the start of a '5 year' campaign to nuke min_t() (et al).
David
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 39/44] mm: use min() instead of min_t()
2025-11-19 22:41 ` [PATCH 39/44] mm: use min() " david.laight.linux
2025-11-20 9:20 ` David Hildenbrand (Red Hat)
@ 2025-11-20 10:36 ` Lorenzo Stoakes
2025-11-20 12:09 ` Lorenzo Stoakes
2025-11-20 12:55 ` David Laight
1 sibling, 2 replies; 19+ messages in thread
From: Lorenzo Stoakes @ 2025-11-20 10:36 UTC (permalink / raw)
To: david.laight.linux
Cc: linux-kernel, linux-fsdevel, linux-mm, Andrew Morton,
Axel Rasmussen, Christoph Lameter, David Hildenbrand,
Dennis Zhou, Johannes Weiner, Matthew Wilcox (Oracle),
Mike Rapoport, Tejun Heo, Yuanchu Xie
I guess you decided to drop all reviewers for the series...?
I do wonder what the aversion to sending to more people is, email for review is
flawed but I don't think it's problematic to ensure that people signed up to
review everything for maintained files are cc'd...
On Wed, Nov 19, 2025 at 10:41:35PM +0000, david.laight.linux@gmail.com wrote:
> From: David Laight <david.laight.linux@gmail.com>
>
> min_t(unsigned int, a, b) casts an 'unsigned long' to 'unsigned int'.
> Use min(a, b) instead as it promotes any 'unsigned int' to 'unsigned long'
> and so cannot discard significant bits.
you're changing min_t(int, ...) too? This commit message seems incomplete as a
result.
None of the changes you make here seem to have any bearing on reality, so I
think the commit message should reflect that this is an entirely pedantic change
for the sake of satisfying a check you feel will reveal actual bugs in the
future or something?
Commit messages should include actual motivation rather than a theoretical one.
>
> In this case the 'unsigned long' values are small enough that the result
> is ok.
>
> (Similarly for clamp_t().)
>
> Detected by an extra check added to min_t().
In general I really question the value of the check when basically every use
here is pointless...?
I guess idea is in future it'll catch some real cases right?
Is this check implemented in this series at all? Because presumably with the
cover letter saying you couldn't fix the CFS code etc. you aren't? So it's just
laying the groundwork for this?
>
> Signed-off-by: David Laight <david.laight.linux@gmail.com>
> ---
> mm/gup.c | 4 ++--
> mm/memblock.c | 2 +-
> mm/memory.c | 2 +-
> mm/percpu.c | 2 +-
> mm/truncate.c | 3 +--
> mm/vmscan.c | 2 +-
> 6 files changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/mm/gup.c b/mm/gup.c
> index a8ba5112e4d0..55435b90dcc3 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -237,8 +237,8 @@ static inline struct folio *gup_folio_range_next(struct page *start,
> unsigned int nr = 1;
>
> if (folio_test_large(folio))
> - nr = min_t(unsigned int, npages - i,
> - folio_nr_pages(folio) - folio_page_idx(folio, next));
> + nr = min(npages - i,
> + folio_nr_pages(folio) - folio_page_idx(folio, next));
There's no cases where any of these would discard significant bits. But we
ultimately cast to unisnged int anyway (nr) so not sure this achieves anything.
But at the same time I guess no harm.
>
> *ntails = nr;
> return folio;
> diff --git a/mm/memblock.c b/mm/memblock.c
> index e23e16618e9b..19b491d39002 100644
> --- a/mm/memblock.c
> +++ b/mm/memblock.c
> @@ -2208,7 +2208,7 @@ static void __init __free_pages_memory(unsigned long start, unsigned long end)
> * the case.
> */
> if (start)
> - order = min_t(int, MAX_PAGE_ORDER, __ffs(start));
> + order = min(MAX_PAGE_ORDER, __ffs(start));
I guess this would already be defaulting to int anyway.
> else
> order = MAX_PAGE_ORDER;
>
> diff --git a/mm/memory.c b/mm/memory.c
> index 74b45e258323..72f7bd71d65f 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -2375,7 +2375,7 @@ static int insert_pages(struct vm_area_struct *vma, unsigned long addr,
>
> while (pages_to_write_in_pmd) {
> int pte_idx = 0;
> - const int batch_size = min_t(int, pages_to_write_in_pmd, 8);
> + const int batch_size = min(pages_to_write_in_pmd, 8);
Feels like there's just a mistake in pages_to_write_in_pmd being unsigned long?
Again I guess correct because we're not going to even come close to ulong64
issues with a count of pages to write.
>
> start_pte = pte_offset_map_lock(mm, pmd, addr, &pte_lock);
> if (!start_pte) {
> diff --git a/mm/percpu.c b/mm/percpu.c
> index 81462ce5866e..cad59221d298 100644
> --- a/mm/percpu.c
> +++ b/mm/percpu.c
> @@ -1228,7 +1228,7 @@ static int pcpu_alloc_area(struct pcpu_chunk *chunk, int alloc_bits,
> /*
> * Search to find a fit.
> */
> - end = min_t(int, start + alloc_bits + PCPU_BITMAP_BLOCK_BITS,
> + end = umin(start + alloc_bits + PCPU_BITMAP_BLOCK_BITS,
> pcpu_chunk_map_bits(chunk));
Is it really that useful to use umin() here? I mean in examples above all the
values would be positive too. Seems strange to use umin() when everything involves an int?
> bit_off = pcpu_find_zero_area(chunk->alloc_map, end, start, alloc_bits,
> align_mask, &area_off, &area_bits);
> diff --git a/mm/truncate.c b/mm/truncate.c
> index 91eb92a5ce4f..7a56372d39a3 100644
> --- a/mm/truncate.c
> +++ b/mm/truncate.c
> @@ -849,8 +849,7 @@ void pagecache_isize_extended(struct inode *inode, loff_t from, loff_t to)
> unsigned int offset, end;
>
> offset = from - folio_pos(folio);
> - end = min_t(unsigned int, to - folio_pos(folio),
> - folio_size(folio));
> + end = umin(to - folio_pos(folio), folio_size(folio));
Again confused about why we choose to use umin() here...
min(loff_t - loff_t, size_t)
so min(long long, unsigned long)
And I guess based on fact we don't expect delta between from and folio start to
be larger than a max folio size.
So probably fine.
> folio_zero_segment(folio, offset, end);
> }
>
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index b2fc8b626d3d..82cd99a5d843 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -3489,7 +3489,7 @@ static struct folio *get_pfn_folio(unsigned long pfn, struct mem_cgroup *memcg,
>
> static bool suitable_to_scan(int total, int young)
> {
> - int n = clamp_t(int, cache_line_size() / sizeof(pte_t), 2, 8);
> + int n = clamp(cache_line_size() / sizeof(pte_t), 2, 8);
int, size_t (but a size_t way < INT_MAX), int, int
So seems fine.
>
> /* suitable if the average number of young PTEs per cacheline is >=1 */
> return young * n >= total;
> --
> 2.39.5
>
Generally the changes look to be correct but pointless.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 39/44] mm: use min() instead of min_t()
2025-11-20 10:36 ` Lorenzo Stoakes
@ 2025-11-20 12:09 ` Lorenzo Stoakes
2025-11-20 12:55 ` David Laight
1 sibling, 0 replies; 19+ messages in thread
From: Lorenzo Stoakes @ 2025-11-20 12:09 UTC (permalink / raw)
To: david.laight.linux
Cc: linux-kernel, linux-fsdevel, linux-mm, Andrew Morton,
Axel Rasmussen, Christoph Lameter, David Hildenbrand,
Dennis Zhou, Johannes Weiner, Matthew Wilcox (Oracle),
Mike Rapoport, Tejun Heo, Yuanchu Xie
On Thu, Nov 20, 2025 at 10:36:16AM +0000, Lorenzo Stoakes wrote:
> I guess you decided to drop all reviewers for the series...?
>
> I do wonder what the aversion to sending to more people is, email for review is
> flawed but I don't think it's problematic to ensure that people signed up to
> review everything for maintained files are cc'd...
>
> On Wed, Nov 19, 2025 at 10:41:35PM +0000, david.laight.linux@gmail.com wrote:
> > From: David Laight <david.laight.linux@gmail.com>
> >
> > min_t(unsigned int, a, b) casts an 'unsigned long' to 'unsigned int'.
> > Use min(a, b) instead as it promotes any 'unsigned int' to 'unsigned long'
> > and so cannot discard significant bits.
>
> you're changing min_t(int, ...) too? This commit message seems incomplete as a
> result.
>
> None of the changes you make here seem to have any bearing on reality, so I
> think the commit message should reflect that this is an entirely pedantic change
> for the sake of satisfying a check you feel will reveal actual bugs in the
> future or something?
>
> Commit messages should include actual motivation rather than a theoretical one.
>
> >
> > In this case the 'unsigned long' values are small enough that the result
> > is ok.
> >
> > (Similarly for clamp_t().)
> >
> > Detected by an extra check added to min_t().
>
> In general I really question the value of the check when basically every use
> here is pointless...?
>
> I guess idea is in future it'll catch some real cases right?
>
> Is this check implemented in this series at all? Because presumably with the
> cover letter saying you couldn't fix the CFS code etc. you aren't? So it's just
> laying the groundwork for this?
>
> >
> > Signed-off-by: David Laight <david.laight.linux@gmail.com>
I mean I don't see anything wrong here, and on the basis that this will be
useful in adding this upcoming check, with the nit about commit msg above, this
LGTM so:
Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> > ---
> > mm/gup.c | 4 ++--
> > mm/memblock.c | 2 +-
> > mm/memory.c | 2 +-
> > mm/percpu.c | 2 +-
> > mm/truncate.c | 3 +--
> > mm/vmscan.c | 2 +-
> > 6 files changed, 7 insertions(+), 8 deletions(-)
> >
> > diff --git a/mm/gup.c b/mm/gup.c
> > index a8ba5112e4d0..55435b90dcc3 100644
> > --- a/mm/gup.c
> > +++ b/mm/gup.c
> > @@ -237,8 +237,8 @@ static inline struct folio *gup_folio_range_next(struct page *start,
> > unsigned int nr = 1;
> >
> > if (folio_test_large(folio))
> > - nr = min_t(unsigned int, npages - i,
> > - folio_nr_pages(folio) - folio_page_idx(folio, next));
> > + nr = min(npages - i,
> > + folio_nr_pages(folio) - folio_page_idx(folio, next));
>
> There's no cases where any of these would discard significant bits. But we
> ultimately cast to unisnged int anyway (nr) so not sure this achieves anything.
>
> But at the same time I guess no harm.
>
> >
> > *ntails = nr;
> > return folio;
> > diff --git a/mm/memblock.c b/mm/memblock.c
> > index e23e16618e9b..19b491d39002 100644
> > --- a/mm/memblock.c
> > +++ b/mm/memblock.c
> > @@ -2208,7 +2208,7 @@ static void __init __free_pages_memory(unsigned long start, unsigned long end)
> > * the case.
> > */
> > if (start)
> > - order = min_t(int, MAX_PAGE_ORDER, __ffs(start));
> > + order = min(MAX_PAGE_ORDER, __ffs(start));
>
> I guess this would already be defaulting to int anyway.
>
> > else
> > order = MAX_PAGE_ORDER;
> >
> > diff --git a/mm/memory.c b/mm/memory.c
> > index 74b45e258323..72f7bd71d65f 100644
> > --- a/mm/memory.c
> > +++ b/mm/memory.c
> > @@ -2375,7 +2375,7 @@ static int insert_pages(struct vm_area_struct *vma, unsigned long addr,
> >
> > while (pages_to_write_in_pmd) {
> > int pte_idx = 0;
> > - const int batch_size = min_t(int, pages_to_write_in_pmd, 8);
> > + const int batch_size = min(pages_to_write_in_pmd, 8);
>
> Feels like there's just a mistake in pages_to_write_in_pmd being unsigned long?
>
> Again I guess correct because we're not going to even come close to ulong64
> issues with a count of pages to write.
>
> >
> > start_pte = pte_offset_map_lock(mm, pmd, addr, &pte_lock);
> > if (!start_pte) {
> > diff --git a/mm/percpu.c b/mm/percpu.c
> > index 81462ce5866e..cad59221d298 100644
> > --- a/mm/percpu.c
> > +++ b/mm/percpu.c
> > @@ -1228,7 +1228,7 @@ static int pcpu_alloc_area(struct pcpu_chunk *chunk, int alloc_bits,
> > /*
> > * Search to find a fit.
> > */
> > - end = min_t(int, start + alloc_bits + PCPU_BITMAP_BLOCK_BITS,
> > + end = umin(start + alloc_bits + PCPU_BITMAP_BLOCK_BITS,
> > pcpu_chunk_map_bits(chunk));
>
> Is it really that useful to use umin() here? I mean in examples above all the
> values would be positive too. Seems strange to use umin() when everything involves an int?
>
> > bit_off = pcpu_find_zero_area(chunk->alloc_map, end, start, alloc_bits,
> > align_mask, &area_off, &area_bits);
> > diff --git a/mm/truncate.c b/mm/truncate.c
> > index 91eb92a5ce4f..7a56372d39a3 100644
> > --- a/mm/truncate.c
> > +++ b/mm/truncate.c
> > @@ -849,8 +849,7 @@ void pagecache_isize_extended(struct inode *inode, loff_t from, loff_t to)
> > unsigned int offset, end;
> >
> > offset = from - folio_pos(folio);
> > - end = min_t(unsigned int, to - folio_pos(folio),
> > - folio_size(folio));
> > + end = umin(to - folio_pos(folio), folio_size(folio));
>
> Again confused about why we choose to use umin() here...
>
> min(loff_t - loff_t, size_t)
>
> so min(long long, unsigned long)
>
> And I guess based on fact we don't expect delta between from and folio start to
> be larger than a max folio size.
>
> So probably fine.
>
> > folio_zero_segment(folio, offset, end);
> > }
> >
> > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > index b2fc8b626d3d..82cd99a5d843 100644
> > --- a/mm/vmscan.c
> > +++ b/mm/vmscan.c
> > @@ -3489,7 +3489,7 @@ static struct folio *get_pfn_folio(unsigned long pfn, struct mem_cgroup *memcg,
> >
> > static bool suitable_to_scan(int total, int young)
> > {
> > - int n = clamp_t(int, cache_line_size() / sizeof(pte_t), 2, 8);
> > + int n = clamp(cache_line_size() / sizeof(pte_t), 2, 8);
>
> int, size_t (but a size_t way < INT_MAX), int, int
>
> So seems fine.
>
> >
> > /* suitable if the average number of young PTEs per cacheline is >=1 */
> > return young * n >= total;
> > --
> > 2.39.5
> >
>
> Generally the changes look to be correct but pointless.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 39/44] mm: use min() instead of min_t()
2025-11-20 10:36 ` Lorenzo Stoakes
2025-11-20 12:09 ` Lorenzo Stoakes
@ 2025-11-20 12:55 ` David Laight
2025-11-20 13:42 ` David Hildenbrand (Red Hat)
1 sibling, 1 reply; 19+ messages in thread
From: David Laight @ 2025-11-20 12:55 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: linux-kernel, linux-fsdevel, linux-mm, Andrew Morton,
Axel Rasmussen, Christoph Lameter, David Hildenbrand,
Dennis Zhou, Johannes Weiner, Matthew Wilcox (Oracle),
Mike Rapoport, Tejun Heo, Yuanchu Xie
On Thu, 20 Nov 2025 10:36:16 +0000
Lorenzo Stoakes <lorenzo.stoakes@oracle.com> wrote:
> I guess you decided to drop all reviewers for the series...?
>
> I do wonder what the aversion to sending to more people is, email for review is
> flawed but I don't think it's problematic to ensure that people signed up to
> review everything for maintained files are cc'd...
Even sending all 44 patches to all the mailing lists was over 5000 emails.
Sending to all 124 maintainers and lists is some 50000 emails.
And that is just the maintainers, not the reviewers etc.
I don't have access to a mail server that will let me send more than
500 messages/day (the gmail limit is 100).
So each patch was send to the maintainers for the files it contained,
that reduced it to just under 400 emails.
>
> On Wed, Nov 19, 2025 at 10:41:35PM +0000, david.laight.linux@gmail.com wrote:
> > From: David Laight <david.laight.linux@gmail.com>
> >
> > min_t(unsigned int, a, b) casts an 'unsigned long' to 'unsigned int'.
> > Use min(a, b) instead as it promotes any 'unsigned int' to 'unsigned long'
> > and so cannot discard significant bits.
>
> you're changing min_t(int, ...) too? This commit message seems incomplete as a
> result.
Ok, I used the same commit message for most of the 44 patches.
The large majority are 'unsigned int' ones.
>
> None of the changes you make here seem to have any bearing on reality, so I
> think the commit message should reflect that this is an entirely pedantic change
> for the sake of satisfying a check you feel will reveal actual bugs in the
> future or something?
>
> Commit messages should include actual motivation rather than a theoretical one.
>
> >
> > In this case the 'unsigned long' values are small enough that the result
> > is ok.
> >
> > (Similarly for clamp_t().)
> >
> > Detected by an extra check added to min_t().
>
> In general I really question the value of the check when basically every use
> here is pointless...?
>
> I guess idea is in future it'll catch some real cases right?
>
> Is this check implemented in this series at all? Because presumably with the
> cover letter saying you couldn't fix the CFS code etc. you aren't? So it's just
> laying the groundwork for this?
I could fix the CFS code, but not with a trivial patch.
I also wanted to put the 'fixes' in the first few patches, I didn't realise
how bad that code was until I looked again.
(I've also not fixed all the drivers I don't build.)
>
> >
> > Signed-off-by: David Laight <david.laight.linux@gmail.com>
> > ---
> > mm/gup.c | 4 ++--
> > mm/memblock.c | 2 +-
> > mm/memory.c | 2 +-
> > mm/percpu.c | 2 +-
> > mm/truncate.c | 3 +--
> > mm/vmscan.c | 2 +-
> > 6 files changed, 7 insertions(+), 8 deletions(-)
> >
> > diff --git a/mm/gup.c b/mm/gup.c
> > index a8ba5112e4d0..55435b90dcc3 100644
> > --- a/mm/gup.c
> > +++ b/mm/gup.c
> > @@ -237,8 +237,8 @@ static inline struct folio *gup_folio_range_next(struct page *start,
> > unsigned int nr = 1;
> >
> > if (folio_test_large(folio))
> > - nr = min_t(unsigned int, npages - i,
> > - folio_nr_pages(folio) - folio_page_idx(folio, next));
> > + nr = min(npages - i,
> > + folio_nr_pages(folio) - folio_page_idx(folio, next));
>
> There's no cases where any of these would discard significant bits. But we
> ultimately cast to unisnged int anyway (nr) so not sure this achieves anything.
The (implicit) cast to unsigned int is irrelevant - that happens after the min().
The issue is that 'npages' is 'unsigned long' so can (in theory) be larger than 4G.
Ok that would be a 16TB buffer, but someone must have decided that npages might
not fit in 32 bits otherwise they wouldn't have used 'unsigned long'.
>
> But at the same time I guess no harm.
>
> >
> > *ntails = nr;
> > return folio;
> > diff --git a/mm/memblock.c b/mm/memblock.c
> > index e23e16618e9b..19b491d39002 100644
> > --- a/mm/memblock.c
> > +++ b/mm/memblock.c
> > @@ -2208,7 +2208,7 @@ static void __init __free_pages_memory(unsigned long start, unsigned long end)
> > * the case.
> > */
> > if (start)
> > - order = min_t(int, MAX_PAGE_ORDER, __ffs(start));
> > + order = min(MAX_PAGE_ORDER, __ffs(start));
>
> I guess this would already be defaulting to int anyway.
Actually that one is also fixed by patch 0001 - which changes the return
type of the x86-64 __ffs() to unsigned int.
Which will be why min_t() was used in the first place.
I probably did this edit first.
>
> > else
> > order = MAX_PAGE_ORDER;
> >
> > diff --git a/mm/memory.c b/mm/memory.c
> > index 74b45e258323..72f7bd71d65f 100644
> > --- a/mm/memory.c
> > +++ b/mm/memory.c
> > @@ -2375,7 +2375,7 @@ static int insert_pages(struct vm_area_struct *vma, unsigned long addr,
> >
> > while (pages_to_write_in_pmd) {
> > int pte_idx = 0;
> > - const int batch_size = min_t(int, pages_to_write_in_pmd, 8);
> > + const int batch_size = min(pages_to_write_in_pmd, 8);
>
> Feels like there's just a mistake in pages_to_write_in_pmd being unsigned long?
Changing that would be a different 'fix'.
> Again I guess correct because we're not going to even come close to ulong64
> issues with a count of pages to write.
That fact that the count of pages is small is why the existing code isn't wrong.
The patch can't make things worse.
>
> >
> > start_pte = pte_offset_map_lock(mm, pmd, addr, &pte_lock);
> > if (!start_pte) {
> > diff --git a/mm/percpu.c b/mm/percpu.c
> > index 81462ce5866e..cad59221d298 100644
> > --- a/mm/percpu.c
> > +++ b/mm/percpu.c
> > @@ -1228,7 +1228,7 @@ static int pcpu_alloc_area(struct pcpu_chunk *chunk, int alloc_bits,
> > /*
> > * Search to find a fit.
> > */
> > - end = min_t(int, start + alloc_bits + PCPU_BITMAP_BLOCK_BITS,
> > + end = umin(start + alloc_bits + PCPU_BITMAP_BLOCK_BITS,
> > pcpu_chunk_map_bits(chunk));
>
> Is it really that useful to use umin() here? I mean in examples above all the
> values would be positive too. Seems strange to use umin() when everything involves an int?
>
> > bit_off = pcpu_find_zero_area(chunk->alloc_map, end, start, alloc_bits,
> > align_mask, &area_off, &area_bits);
> > diff --git a/mm/truncate.c b/mm/truncate.c
> > index 91eb92a5ce4f..7a56372d39a3 100644
> > --- a/mm/truncate.c
> > +++ b/mm/truncate.c
> > @@ -849,8 +849,7 @@ void pagecache_isize_extended(struct inode *inode, loff_t from, loff_t to)
> > unsigned int offset, end;
> >
> > offset = from - folio_pos(folio);
> > - end = min_t(unsigned int, to - folio_pos(folio),
> > - folio_size(folio));
> > + end = umin(to - folio_pos(folio), folio_size(folio));
>
> Again confused about why we choose to use umin() here...
>
> min(loff_t - loff_t, size_t)
>
> so min(long long, unsigned long)
Which is a signedness error because both are 64bit.
min(s64, u32) also reports a signedness error even though u32 is promoted
to s64, allowing that would bloat min() somewhat (and it isn't common).
>
> And I guess based on fact we don't expect delta between from and folio start to
> be larger than a max folio size.
The problem arises if 'to - folio_pos(folio)' doesn't fit in 32 bits
(and its low 32bit are small).
I think that might be possible if truncating a large file.
So this might be a real bug.
>
> So probably fine.
>
> > folio_zero_segment(folio, offset, end);
> > }
> >
> > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > index b2fc8b626d3d..82cd99a5d843 100644
> > --- a/mm/vmscan.c
> > +++ b/mm/vmscan.c
> > @@ -3489,7 +3489,7 @@ static struct folio *get_pfn_folio(unsigned long pfn, struct mem_cgroup *memcg,
> >
> > static bool suitable_to_scan(int total, int young)
> > {
> > - int n = clamp_t(int, cache_line_size() / sizeof(pte_t), 2, 8);
> > + int n = clamp(cache_line_size() / sizeof(pte_t), 2, 8);
>
> int, size_t (but a size_t way < INT_MAX), int, int
Unfortunately even if cache_line_size() is u32, the division makes the result
size_t and gcc doesn't detect the value as being 'smaller that it used to be'.
David
>
> So seems fine.
>
> >
> > /* suitable if the average number of young PTEs per cacheline is >=1 */
> > return young * n >= total;
> > --
> > 2.39.5
> >
>
> Generally the changes look to be correct but pointless.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 39/44] mm: use min() instead of min_t()
2025-11-20 12:55 ` David Laight
@ 2025-11-20 13:42 ` David Hildenbrand (Red Hat)
2025-11-20 15:44 ` David Laight
0 siblings, 1 reply; 19+ messages in thread
From: David Hildenbrand (Red Hat) @ 2025-11-20 13:42 UTC (permalink / raw)
To: David Laight, Lorenzo Stoakes
Cc: linux-kernel, linux-fsdevel, linux-mm, Andrew Morton,
Axel Rasmussen, Christoph Lameter, Dennis Zhou, Johannes Weiner,
Matthew Wilcox (Oracle),
Mike Rapoport, Tejun Heo, Yuanchu Xie
>>
>>>
>>> Signed-off-by: David Laight <david.laight.linux@gmail.com>
>>> ---
>>> mm/gup.c | 4 ++--
>>> mm/memblock.c | 2 +-
>>> mm/memory.c | 2 +-
>>> mm/percpu.c | 2 +-
>>> mm/truncate.c | 3 +--
>>> mm/vmscan.c | 2 +-
>>> 6 files changed, 7 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/mm/gup.c b/mm/gup.c
>>> index a8ba5112e4d0..55435b90dcc3 100644
>>> --- a/mm/gup.c
>>> +++ b/mm/gup.c
>>> @@ -237,8 +237,8 @@ static inline struct folio *gup_folio_range_next(struct page *start,
>>> unsigned int nr = 1;
>>>
>>> if (folio_test_large(folio))
>>> - nr = min_t(unsigned int, npages - i,
>>> - folio_nr_pages(folio) - folio_page_idx(folio, next));
>>> + nr = min(npages - i,
>>> + folio_nr_pages(folio) - folio_page_idx(folio, next));
>>
>> There's no cases where any of these would discard significant bits. But we
>> ultimately cast to unisnged int anyway (nr) so not sure this achieves anything.
>
> The (implicit) cast to unsigned int is irrelevant - that happens after the min().
> The issue is that 'npages' is 'unsigned long' so can (in theory) be larger than 4G.
> Ok that would be a 16TB buffer, but someone must have decided that npages might
> not fit in 32 bits otherwise they wouldn't have used 'unsigned long'.
See commit fa17bcd5f65e ("mm: make folio page count functions return
unsigned") why that function used to return "long" instead of "unsigned
int" and how we changed it to "unsigned long".
Until that function actually returns something that large might take a
while, so no need to worry about that right now.
--
Cheers
David
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: (subset) [PATCH 00/44] Change a lot of min_t() that might mask high bits
2025-11-19 22:40 [PATCH 00/44] Change a lot of min_t() that might mask high bits david.laight.linux
` (3 preceding siblings ...)
2025-11-20 9:38 ` Lorenzo Stoakes
@ 2025-11-20 14:52 ` Jens Axboe
2025-11-24 9:49 ` Herbert Xu
5 siblings, 0 replies; 19+ messages in thread
From: Jens Axboe @ 2025-11-20 14:52 UTC (permalink / raw)
To: linux-kernel, david.laight.linux
Cc: Alan Stern, Alexander Viro, Alexei Starovoitov, Andi Shyti,
Andreas Dilger, Andrew Lunn, Andrew Morton, Andrii Nakryiko,
Andy Shevchenko, Ard Biesheuvel, Arnaldo Carvalho de Melo,
Bjorn Helgaas, Borislav Petkov, Christian Brauner,
Christian König, Christoph Hellwig, Daniel Borkmann,
Dan Williams, Dave Hansen, Dave Jiang, David Ahern,
Davidlohr Bueso, David S. Miller, Dennis Zhou, Eric Dumazet,
Greg Kroah-Hartman, Herbert Xu, Ingo Molnar, Jakub Kicinski,
Jakub Sitnicki, James E.J. Bottomley, Jarkko Sakkinen,
Jason A. Donenfeld, Jiri Slaby, Johannes Weiner, John Allen,
Jonathan Cameron, Juergen Gross, Kees Cook, KP Singh,
Linus Walleij, Martin K. Petersen, Matthew Wilcox (Oracle),
Mika Westerberg, Mike Rapoport, Miklos Szeredi, Namhyung Kim,
Neal Cardwell, nic_swsd, OGAWA Hirofumi, Olivia Mackall,
Paolo Abeni, Paolo Bonzini, Peter Huewe, Peter Zijlstra,
Rafael J. Wysocki, Sean Christopherson, Srinivas Kandagatla,
Stefano Stabellini, Steven Rostedt, Tejun Heo, Theodore Ts'o,
Thomas Gleixner, Tom Lendacky, Willem de Bruijn, x86, Yury Norov,
amd-gfx, bpf, cgroups, dri-devel, io-uring, kvm, linux-acpi,
linux-block, linux-crypto, linux-cxl, linux-efi, linux-ext4,
linux-fsdevel, linux-gpio, linux-i2c, linux-integrity, linux-mm,
linux-nvme, linux-pci, linux-perf-users, linux-scsi,
linux-serial, linux-trace-kernel, linux-usb, mptcp, netdev,
usb-storage, David Hildenbrand
On Wed, 19 Nov 2025 22:40:56 +0000, david.laight.linux@gmail.com wrote:
> It in not uncommon for code to use min_t(uint, a, b) when one of a or b
> is 64bit and can have a value that is larger than 2^32;
> This is particularly prevelant with:
> uint_var = min_t(uint, uint_var, uint64_expression);
>
> Casts to u8 and u16 are very likely to discard significant bits.
>
> [...]
Applied, thanks!
[12/44] block: use min() instead of min_t()
commit: 9420e720ad192c53c8d2803c5a2313b2d586adbd
Best regards,
--
Jens Axboe
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 39/44] mm: use min() instead of min_t()
2025-11-20 13:42 ` David Hildenbrand (Red Hat)
@ 2025-11-20 15:44 ` David Laight
2025-11-21 8:24 ` David Hildenbrand (Red Hat)
0 siblings, 1 reply; 19+ messages in thread
From: David Laight @ 2025-11-20 15:44 UTC (permalink / raw)
To: David Hildenbrand (Red Hat)
Cc: Lorenzo Stoakes, linux-kernel, linux-fsdevel, linux-mm,
Andrew Morton, Axel Rasmussen, Christoph Lameter, Dennis Zhou,
Johannes Weiner, Matthew Wilcox (Oracle),
Mike Rapoport, Tejun Heo, Yuanchu Xie
On Thu, 20 Nov 2025 14:42:24 +0100
"David Hildenbrand (Red Hat)" <david@kernel.org> wrote:
> >>
> >>>
> >>> Signed-off-by: David Laight <david.laight.linux@gmail.com>
> >>> ---
> >>> mm/gup.c | 4 ++--
> >>> mm/memblock.c | 2 +-
> >>> mm/memory.c | 2 +-
> >>> mm/percpu.c | 2 +-
> >>> mm/truncate.c | 3 +--
> >>> mm/vmscan.c | 2 +-
> >>> 6 files changed, 7 insertions(+), 8 deletions(-)
> >>>
> >>> diff --git a/mm/gup.c b/mm/gup.c
> >>> index a8ba5112e4d0..55435b90dcc3 100644
> >>> --- a/mm/gup.c
> >>> +++ b/mm/gup.c
> >>> @@ -237,8 +237,8 @@ static inline struct folio *gup_folio_range_next(struct page *start,
> >>> unsigned int nr = 1;
> >>>
> >>> if (folio_test_large(folio))
> >>> - nr = min_t(unsigned int, npages - i,
> >>> - folio_nr_pages(folio) - folio_page_idx(folio, next));
> >>> + nr = min(npages - i,
> >>> + folio_nr_pages(folio) - folio_page_idx(folio, next));
> >>
> >> There's no cases where any of these would discard significant bits. But we
> >> ultimately cast to unisnged int anyway (nr) so not sure this achieves anything.
> >
> > The (implicit) cast to unsigned int is irrelevant - that happens after the min().
> > The issue is that 'npages' is 'unsigned long' so can (in theory) be larger than 4G.
> > Ok that would be a 16TB buffer, but someone must have decided that npages might
> > not fit in 32 bits otherwise they wouldn't have used 'unsigned long'.
>
> See commit fa17bcd5f65e ("mm: make folio page count functions return
> unsigned") why that function used to return "long" instead of "unsigned
> int" and how we changed it to "unsigned long".
>
> Until that function actually returns something that large might take a
> while, so no need to worry about that right now.
Except that it gives a false positive on a compile-time test that finds a
few real bugs.
I've been (slowly) fixing 'allmodconfig' and found 'goodies' like:
min_t(u32, MAX_UINT, expr)
and
min_t(u8, expr, 255)
Pretty much all the min_t(unsigned xxx) that compile when changed to min()
are safe changes and might fix an obscure bug.
Probably 99% make no difference.
So I'd like to get rid of the ones that make no difference.
David
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 39/44] mm: use min() instead of min_t()
2025-11-20 9:59 ` David Laight
@ 2025-11-20 23:45 ` Eric Biggers
2025-11-21 8:27 ` David Hildenbrand (Red Hat)
2025-11-21 9:15 ` David Laight
0 siblings, 2 replies; 19+ messages in thread
From: Eric Biggers @ 2025-11-20 23:45 UTC (permalink / raw)
To: David Laight
Cc: David Hildenbrand (Red Hat),
linux-kernel, linux-fsdevel, linux-mm, Andrew Morton,
Axel Rasmussen, Christoph Lameter, Dennis Zhou, Johannes Weiner,
Matthew Wilcox (Oracle),
Mike Rapoport, Tejun Heo, Yuanchu Xie
On Thu, Nov 20, 2025 at 09:59:46AM +0000, David Laight wrote:
> On Thu, 20 Nov 2025 10:20:41 +0100
> "David Hildenbrand (Red Hat)" <david@kernel.org> wrote:
>
> > On 11/19/25 23:41, david.laight.linux@gmail.com wrote:
> > > From: David Laight <david.laight.linux@gmail.com>
> > >
> > > min_t(unsigned int, a, b) casts an 'unsigned long' to 'unsigned int'.
> > > Use min(a, b) instead as it promotes any 'unsigned int' to 'unsigned long'
> > > and so cannot discard significant bits.
> >
> > I thought using min() was frowned upon and we were supposed to use
> > min_t() instead to make it clear which type we want to use.
>
> I'm not sure that was ever true.
> min_t() is just an accident waiting to happen.
> (and I found a few of them, the worst are in sched/fair.c)
>
> Most of the min_t() are there because of the rather overzealous type
> check that used to be in min().
> But even then it would really be better to explicitly cast one of the
> parameters to min(), so min_t(T, a, b) => min(a, (T)b).
> Then it becomes rather more obvious that min_t(u8, x->m_u8, expr)
> is going mask off the high bits of 'expr'.
>
> > Do I misremember or have things changed?
> >
> > Wasn't there a checkpatch warning that states exactly that?
>
> There is one that suggests min_t() - it ought to be nuked.
> The real fix is to backtrack the types so there isn't an error.
> min_t() ought to be a 'last resort' and a single cast is better.
>
> With the relaxed checks in min() most of the min_t() can just
> be replaced by min(), even this is ok:
> int len = fun();
> if (len < 0)
> return;
> count = min(len, sizeof(T));
>
> I did look at the history of min() and min_t().
> IIRC some of the networking code had a real function min() with
> 'unsigned int' arguments.
> This was moved to a common header, changed to a #define and had
> a type added - so min(T, a, b).
> Pretty much immediately that was renamed min_t() and min() added
> that accepted any type - but checked the types of 'a' and 'b'
> exactly matched.
> Code was then changed (over the years) to use min(), but in many
> cases the types didn't quite match - so min_t() was used a lot.
>
> I keep spotting new commits that pass too small a type to min_t().
> So this is the start of a '5 year' campaign to nuke min_t() (et al).
Yes, checkpatch suggests min_t() or max_t() if you cast an argument to
min() or max(). Grep for "typecasts on min/max could be min_t/max_t" in
scripts/checkpatch.pl.
And historically you could not pass different types to min() and max(),
which is why people use min_t() and max_t(). It looks like you fixed
that a couple years ago in
https://lore.kernel.org/all/b97faef60ad24922b530241c5d7c933c@AcuMS.aculab.com/,
which is great! It just takes some time for the whole community to get
the message. Also, it seems that checkpatch is in need of an update.
Doing these conversions looks good to me, but unfortunately this is
probably the type of thing that shouldn't be a single kernel-wide patch
series. They should be sent out per-subsystem.
I suggest also putting a sentence in the commit message that mentions
that min() and max() have been updated to accept arguments with
different types. (Seeing as historically that wasn't true.) I suggest
also being extra clear about when each change is a cleanup vs a fix.
- Eric
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 39/44] mm: use min() instead of min_t()
2025-11-20 15:44 ` David Laight
@ 2025-11-21 8:24 ` David Hildenbrand (Red Hat)
0 siblings, 0 replies; 19+ messages in thread
From: David Hildenbrand (Red Hat) @ 2025-11-21 8:24 UTC (permalink / raw)
To: David Laight
Cc: Lorenzo Stoakes, linux-kernel, linux-fsdevel, linux-mm,
Andrew Morton, Axel Rasmussen, Christoph Lameter, Dennis Zhou,
Johannes Weiner, Matthew Wilcox (Oracle),
Mike Rapoport, Tejun Heo, Yuanchu Xie
On 11/20/25 16:44, David Laight wrote:
> On Thu, 20 Nov 2025 14:42:24 +0100
> "David Hildenbrand (Red Hat)" <david@kernel.org> wrote:
>
>>>>
>>>>>
>>>>> Signed-off-by: David Laight <david.laight.linux@gmail.com>
>>>>> ---
>>>>> mm/gup.c | 4 ++--
>>>>> mm/memblock.c | 2 +-
>>>>> mm/memory.c | 2 +-
>>>>> mm/percpu.c | 2 +-
>>>>> mm/truncate.c | 3 +--
>>>>> mm/vmscan.c | 2 +-
>>>>> 6 files changed, 7 insertions(+), 8 deletions(-)
>>>>>
>>>>> diff --git a/mm/gup.c b/mm/gup.c
>>>>> index a8ba5112e4d0..55435b90dcc3 100644
>>>>> --- a/mm/gup.c
>>>>> +++ b/mm/gup.c
>>>>> @@ -237,8 +237,8 @@ static inline struct folio *gup_folio_range_next(struct page *start,
>>>>> unsigned int nr = 1;
>>>>>
>>>>> if (folio_test_large(folio))
>>>>> - nr = min_t(unsigned int, npages - i,
>>>>> - folio_nr_pages(folio) - folio_page_idx(folio, next));
>>>>> + nr = min(npages - i,
>>>>> + folio_nr_pages(folio) - folio_page_idx(folio, next));
>>>>
>>>> There's no cases where any of these would discard significant bits. But we
>>>> ultimately cast to unisnged int anyway (nr) so not sure this achieves anything.
>>>
>>> The (implicit) cast to unsigned int is irrelevant - that happens after the min().
>>> The issue is that 'npages' is 'unsigned long' so can (in theory) be larger than 4G.
>>> Ok that would be a 16TB buffer, but someone must have decided that npages might
>>> not fit in 32 bits otherwise they wouldn't have used 'unsigned long'.
>>
>> See commit fa17bcd5f65e ("mm: make folio page count functions return
>> unsigned") why that function used to return "long" instead of "unsigned
>> int" and how we changed it to "unsigned long".
>>
>> Until that function actually returns something that large might take a
>> while, so no need to worry about that right now.
>
> Except that it gives a false positive on a compile-time test that finds a
> few real bugs.
>
> I've been (slowly) fixing 'allmodconfig' and found 'goodies' like:
> min_t(u32, MAX_UINT, expr)
> and
> min_t(u8, expr, 255)
>
:)
> Pretty much all the min_t(unsigned xxx) that compile when changed to min()
> are safe changes and might fix an obscure bug.
> Probably 99% make no difference.
>
> So I'd like to get rid of the ones that make no difference.
No objection from my side if using min() is the preferred way now and
introduces no observable changes.
--
Cheers
David
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 39/44] mm: use min() instead of min_t()
2025-11-20 23:45 ` Eric Biggers
@ 2025-11-21 8:27 ` David Hildenbrand (Red Hat)
2025-11-21 9:15 ` David Laight
1 sibling, 0 replies; 19+ messages in thread
From: David Hildenbrand (Red Hat) @ 2025-11-21 8:27 UTC (permalink / raw)
To: Eric Biggers, David Laight
Cc: linux-kernel, linux-fsdevel, linux-mm, Andrew Morton,
Axel Rasmussen, Christoph Lameter, Dennis Zhou, Johannes Weiner,
Matthew Wilcox (Oracle),
Mike Rapoport, Tejun Heo, Yuanchu Xie
On 11/21/25 00:45, Eric Biggers wrote:
> On Thu, Nov 20, 2025 at 09:59:46AM +0000, David Laight wrote:
>> On Thu, 20 Nov 2025 10:20:41 +0100
>> "David Hildenbrand (Red Hat)" <david@kernel.org> wrote:
>>
>>> On 11/19/25 23:41, david.laight.linux@gmail.com wrote:
>>>> From: David Laight <david.laight.linux@gmail.com>
>>>>
>>>> min_t(unsigned int, a, b) casts an 'unsigned long' to 'unsigned int'.
>>>> Use min(a, b) instead as it promotes any 'unsigned int' to 'unsigned long'
>>>> and so cannot discard significant bits.
>>>
>>> I thought using min() was frowned upon and we were supposed to use
>>> min_t() instead to make it clear which type we want to use.
>>
>> I'm not sure that was ever true.
>> min_t() is just an accident waiting to happen.
>> (and I found a few of them, the worst are in sched/fair.c)
>>
>> Most of the min_t() are there because of the rather overzealous type
>> check that used to be in min().
>> But even then it would really be better to explicitly cast one of the
>> parameters to min(), so min_t(T, a, b) => min(a, (T)b).
>> Then it becomes rather more obvious that min_t(u8, x->m_u8, expr)
>> is going mask off the high bits of 'expr'.
>>
>>> Do I misremember or have things changed?
>>>
>>> Wasn't there a checkpatch warning that states exactly that?
>>
>> There is one that suggests min_t() - it ought to be nuked.
>> The real fix is to backtrack the types so there isn't an error.
>> min_t() ought to be a 'last resort' and a single cast is better.
>>
>> With the relaxed checks in min() most of the min_t() can just
>> be replaced by min(), even this is ok:
>> int len = fun();
>> if (len < 0)
>> return;
>> count = min(len, sizeof(T));
>>
>> I did look at the history of min() and min_t().
>> IIRC some of the networking code had a real function min() with
>> 'unsigned int' arguments.
>> This was moved to a common header, changed to a #define and had
>> a type added - so min(T, a, b).
>> Pretty much immediately that was renamed min_t() and min() added
>> that accepted any type - but checked the types of 'a' and 'b'
>> exactly matched.
>> Code was then changed (over the years) to use min(), but in many
>> cases the types didn't quite match - so min_t() was used a lot.
>>
>> I keep spotting new commits that pass too small a type to min_t().
>> So this is the start of a '5 year' campaign to nuke min_t() (et al).
>
> Yes, checkpatch suggests min_t() or max_t() if you cast an argument to
> min() or max(). Grep for "typecasts on min/max could be min_t/max_t" in
> scripts/checkpatch.pl.
Right, that's the one I recalled.
>
> And historically you could not pass different types to min() and max(),
> which is why people use min_t() and max_t(). It looks like you fixed
> that a couple years ago in
> https://lore.kernel.org/all/b97faef60ad24922b530241c5d7c933c@AcuMS.aculab.com/,
> which is great! It just takes some time for the whole community to get
> the message. Also, it seems that checkpatch is in need of an update.
Exactly.
And whenever it comes to such things, I wonder if we want to clearly
spell them out somewhere (codying-style): especially, when to use
min/max and when to use min_t/max_t.
coding-style currently mentions:
"There are also min() and max() macros that do strict type checking ..."
is that also outdated or am I just confused at this point?
>
> Doing these conversions looks good to me, but unfortunately this is
> probably the type of thing that shouldn't be a single kernel-wide patch
> series. They should be sent out per-subsystem.
Agreed!
In particular as there is no need to rush and individual subsystems can
just pick it up separately.
>
> I suggest also putting a sentence in the commit message that mentions
> that min() and max() have been updated to accept arguments with
> different types. (Seeing as historically that wasn't true.) I suggest
> also being extra clear about when each change is a cleanup vs a fix.
+1
--
Cheers
David
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 39/44] mm: use min() instead of min_t()
2025-11-20 23:45 ` Eric Biggers
2025-11-21 8:27 ` David Hildenbrand (Red Hat)
@ 2025-11-21 9:15 ` David Laight
1 sibling, 0 replies; 19+ messages in thread
From: David Laight @ 2025-11-21 9:15 UTC (permalink / raw)
To: Eric Biggers
Cc: David Hildenbrand (Red Hat),
linux-kernel, linux-fsdevel, linux-mm, Andrew Morton,
Axel Rasmussen, Christoph Lameter, Dennis Zhou, Johannes Weiner,
Matthew Wilcox (Oracle),
Mike Rapoport, Tejun Heo, Yuanchu Xie
On Thu, 20 Nov 2025 23:45:22 +0000
Eric Biggers <ebiggers@kernel.org> wrote:
> On Thu, Nov 20, 2025 at 09:59:46AM +0000, David Laight wrote:
> > On Thu, 20 Nov 2025 10:20:41 +0100
> > "David Hildenbrand (Red Hat)" <david@kernel.org> wrote:
> >
> > > On 11/19/25 23:41, david.laight.linux@gmail.com wrote:
> > > > From: David Laight <david.laight.linux@gmail.com>
> > > >
> > > > min_t(unsigned int, a, b) casts an 'unsigned long' to 'unsigned int'.
> > > > Use min(a, b) instead as it promotes any 'unsigned int' to 'unsigned long'
> > > > and so cannot discard significant bits.
> > >
> > > I thought using min() was frowned upon and we were supposed to use
> > > min_t() instead to make it clear which type we want to use.
> >
> > I'm not sure that was ever true.
> > min_t() is just an accident waiting to happen.
> > (and I found a few of them, the worst are in sched/fair.c)
> >
> > Most of the min_t() are there because of the rather overzealous type
> > check that used to be in min().
> > But even then it would really be better to explicitly cast one of the
> > parameters to min(), so min_t(T, a, b) => min(a, (T)b).
> > Then it becomes rather more obvious that min_t(u8, x->m_u8, expr)
> > is going mask off the high bits of 'expr'.
> >
> > > Do I misremember or have things changed?
> > >
> > > Wasn't there a checkpatch warning that states exactly that?
> >
> > There is one that suggests min_t() - it ought to be nuked.
> > The real fix is to backtrack the types so there isn't an error.
> > min_t() ought to be a 'last resort' and a single cast is better.
> >
> > With the relaxed checks in min() most of the min_t() can just
> > be replaced by min(), even this is ok:
> > int len = fun();
> > if (len < 0)
> > return;
> > count = min(len, sizeof(T));
> >
> > I did look at the history of min() and min_t().
> > IIRC some of the networking code had a real function min() with
> > 'unsigned int' arguments.
> > This was moved to a common header, changed to a #define and had
> > a type added - so min(T, a, b).
> > Pretty much immediately that was renamed min_t() and min() added
> > that accepted any type - but checked the types of 'a' and 'b'
> > exactly matched.
> > Code was then changed (over the years) to use min(), but in many
> > cases the types didn't quite match - so min_t() was used a lot.
> >
> > I keep spotting new commits that pass too small a type to min_t().
> > So this is the start of a '5 year' campaign to nuke min_t() (et al).
>
> Yes, checkpatch suggests min_t() or max_t() if you cast an argument to
> min() or max(). Grep for "typecasts on min/max could be min_t/max_t" in
> scripts/checkpatch.pl.
IMHO that is a really bad suggestion (and always has been).
In reality min(a, (T)b) is less likely to be buggy than min_t(T, a, b).
Someone will notice that (u16)long_var is likely to be buggy but min_t()
is expected to 'do something magic'.
There are a log of examples of 'T_var = min_t(T, T_var, b)' which really
needed (typeof (b))T_var rather than (T)b
and T_var = min_t(T, a, b) which just doesn't need a cast at all.
>
> And historically you could not pass different types to min() and max(),
> which is why people use min_t() and max_t(). It looks like you fixed
> that a couple years ago in
> https://lore.kernel.org/all/b97faef60ad24922b530241c5d7c933c@AcuMS.aculab.com/,
> which is great!
I wrote that, and then Linus redid it to avoid some very long lines
from nested expansion (with some tree-wide patches that only he could do).
> It just takes some time for the whole community to get
> the message. Also, it seems that checkpatch is in need of an update.
>
> Doing these conversions looks good to me, but unfortunately this is
> probably the type of thing that shouldn't be a single kernel-wide patch
> series. They should be sent out per-subsystem.
In effect it is a list of separate patches, one per subsystem.
They just have a common 0/n wrapper.
I wanted to link them together, I guess I could have put a bit more
text in the common commit message I pasted into all the commits.
I didn't post the change to minmax.h (apart from a summary in 0/44)
because I hadn't even tried to build a 32bit kernel nevery mind
an allmodconfig or allyesconfig one.
I spent all yesterday trying to build allyesconfig...
David
>
> I suggest also putting a sentence in the commit message that mentions
> that min() and max() have been updated to accept arguments with
> different types. (Seeing as historically that wasn't true.) I suggest
> also being extra clear about when each change is a cleanup vs a fix.
>
> - Eric
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 00/44] Change a lot of min_t() that might mask high bits
2025-11-19 22:40 [PATCH 00/44] Change a lot of min_t() that might mask high bits david.laight.linux
` (4 preceding siblings ...)
2025-11-20 14:52 ` (subset) " Jens Axboe
@ 2025-11-24 9:49 ` Herbert Xu
5 siblings, 0 replies; 19+ messages in thread
From: Herbert Xu @ 2025-11-24 9:49 UTC (permalink / raw)
To: david.laight.linux
Cc: linux-kernel, Alan Stern, Alexander Viro, Alexei Starovoitov,
Andi Shyti, Andreas Dilger, Andrew Lunn, Andrew Morton,
Andrii Nakryiko, Andy Shevchenko, Ard Biesheuvel,
Arnaldo Carvalho de Melo, Bjorn Helgaas, Borislav Petkov,
Christian Brauner, Christian König, Christoph Hellwig,
Daniel Borkmann, Dan Williams, Dave Hansen, Dave Jiang,
David Ahern, David Hildenbrand, Davidlohr Bueso, David S. Miller,
Dennis Zhou, Eric Dumazet, Greg Kroah-Hartman, Ingo Molnar,
Jakub Kicinski, Jakub Sitnicki, James E.J. Bottomley,
Jarkko Sakkinen, Jason A. Donenfeld, Jens Axboe, Jiri Slaby,
Johannes Weiner, John Allen, Jonathan Cameron, Juergen Gross,
Kees Cook, KP Singh, Linus Walleij, Martin K. Petersen,
Matthew Wilcox (Oracle),
Mika Westerberg, Mike Rapoport, Miklos Szeredi, Namhyung Kim,
Neal Cardwell, nic_swsd, OGAWA Hirofumi, Olivia Mackall,
Paolo Abeni, Paolo Bonzini, Peter Huewe, Peter Zijlstra,
Rafael J. Wysocki, Sean Christopherson, Srinivas Kandagatla,
Stefano Stabellini, Steven Rostedt, Tejun Heo, Theodore Ts'o,
Thomas Gleixner, Tom Lendacky, Willem de Bruijn, x86, Yury Norov,
amd-gfx, bpf, cgroups, dri-devel, io-uring, kvm, linux-acpi,
linux-block, linux-crypto, linux-cxl, linux-efi, linux-ext4,
linux-fsdevel, linux-gpio, linux-i2c, linux-integrity, linux-mm,
linux-nvme, linux-pci, linux-perf-users, linux-scsi,
linux-serial, linux-trace-kernel, linux-usb, mptcp, netdev,
usb-storage
On Wed, Nov 19, 2025 at 10:40:56PM +0000, david.laight.linux@gmail.com wrote:
> From: David Laight <david.laight.linux@gmail.com>
>
> It in not uncommon for code to use min_t(uint, a, b) when one of a or b
> is 64bit and can have a value that is larger than 2^32;
> This is particularly prevelant with:
> uint_var = min_t(uint, uint_var, uint64_expression);
>
> Casts to u8 and u16 are very likely to discard significant bits.
>
> These can be detected at compile time by changing min_t(), for example:
> #define CHECK_SIZE(fn, type, val) \
> BUILD_BUG_ON_MSG(sizeof (val) > sizeof (type) && \
> !statically_true(((val) >> 8 * (sizeof (type) - 1)) < 256), \
> fn "() significant bits of '" #val "' may be discarded")
>
> #define min_t(type, x, y) ({ \
> CHECK_SIZE("min_t", type, x); \
> CHECK_SIZE("min_t", type, y); \
> __cmp_once(min, type, x, y); })
>
> (and similar changes to max_t() and clamp_t().)
>
> This shows up some real bugs, some unlikely bugs and some false positives.
> In most cases both arguments are unsigned type (just different ones)
> and min_t() can just be replaced by min().
>
> The patches are all independant and are most of the ones needed to
> get the x86-64 kernel I build to compile.
> I've not tried building an allyesconfig or allmodconfig kernel.
> I've also not included the patch to minmax.h itself.
>
> I've tried to put the patches that actually fix things first.
> The last one is 0009.
>
> I gave up on fixing sched/fair.c - it is too broken for a single patch!
> The patch for net/ipv4/tcp.c is also absent because do_tcp_getsockopt()
> needs multiple/larger changes to make it 'sane'.
>
> I've had to trim the 124 maintainers/lists that get_maintainer.pl finds
> from 124 to under 100 to be able to send the cover letter.
> The individual patches only go to the addresses found for the associated files.
> That reduces the number of emails to a less unsane number.
>
> David Laight (44):
> x86/asm/bitops: Change the return type of variable__ffs() to unsigned
> int
> ext4: Fix saturation of 64bit inode times for old filesystems
> perf: Fix branch stack callchain limit
> io_uring/net: Change some dubious min_t()
> ipc/msg: Fix saturation of percpu counts in msgctl_info()
> bpf: Verifier, remove some unusual uses of min_t() and max_t()
> net/core/flow_dissector: Fix cap of __skb_flow_dissect() return value.
> net: ethtool: Use min3() instead of nested min_t(u16,...)
> ipv6: __ip6_append_data() don't abuse max_t() casts
> x86/crypto: ctr_crypt() use min() instead of min_t()
> arch/x96/kvm: use min() instead of min_t()
> block: use min() instead of min_t()
> drivers/acpi: use min() instead of min_t()
> drivers/char/hw_random: use min3() instead of nested min_t()
> drivers/char/tpm: use min() instead of min_t()
> drivers/crypto/ccp: use min() instead of min_t()
> drivers/cxl: use min() instead of min_t()
> drivers/gpio: use min() instead of min_t()
> drivers/gpu/drm/amd: use min() instead of min_t()
> drivers/i2c/busses: use min() instead of min_t()
> drivers/net/ethernet/realtek: use min() instead of min_t()
> drivers/nvme: use min() instead of min_t()
> arch/x86/mm: use min() instead of min_t()
> drivers/nvmem: use min() instead of min_t()
> drivers/pci: use min() instead of min_t()
> drivers/scsi: use min() instead of min_t()
> drivers/tty/vt: use umin() instead of min_t(u16, ...) for row/col
> limits
> drivers/usb/storage: use min() instead of min_t()
> drivers/xen: use min() instead of min_t()
> fs: use min() or umin() instead of min_t()
> block: bvec.h: use min() instead of min_t()
> nodemask: use min() instead of min_t()
> ipc: use min() instead of min_t()
> bpf: use min() instead of min_t()
> bpf_trace: use min() instead of min_t()
> lib/bucket_locks: use min() instead of min_t()
> lib/crypto/mpi: use min() instead of min_t()
> lib/dynamic_queue_limits: use max() instead of max_t()
> mm: use min() instead of min_t()
> net: Don't pass bitfields to max_t()
> net/core: Change loop conditions so min() can be used
> net: use min() instead of min_t()
> net/netlink: Use umin() to avoid min_t(int, ...) discarding high bits
> net/mptcp: Change some dubious min_t(int, ...) to min()
>
> arch/x86/crypto/aesni-intel_glue.c | 3 +-
> arch/x86/include/asm/bitops.h | 18 +++++-------
> arch/x86/kvm/emulate.c | 3 +-
> arch/x86/kvm/lapic.c | 2 +-
> arch/x86/kvm/mmu/mmu.c | 2 +-
> arch/x86/mm/pat/set_memory.c | 12 ++++----
> block/blk-iocost.c | 6 ++--
> block/blk-settings.c | 2 +-
> block/partitions/efi.c | 3 +-
> drivers/acpi/property.c | 2 +-
> drivers/char/hw_random/core.c | 2 +-
> drivers/char/tpm/tpm1-cmd.c | 2 +-
> drivers/char/tpm/tpm_tis_core.c | 4 +--
> drivers/crypto/ccp/ccp-dev.c | 2 +-
> drivers/cxl/core/mbox.c | 2 +-
> drivers/gpio/gpiolib-acpi-core.c | 2 +-
> .../gpu/drm/amd/amdgpu/amdgpu_doorbell_mgr.c | 4 +--
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +-
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
> drivers/i2c/busses/i2c-designware-master.c | 2 +-
> drivers/net/ethernet/realtek/r8169_main.c | 3 +-
> drivers/nvme/host/pci.c | 3 +-
> drivers/nvme/host/zns.c | 3 +-
> drivers/nvmem/core.c | 2 +-
> drivers/pci/probe.c | 3 +-
> drivers/scsi/hosts.c | 2 +-
> drivers/tty/vt/selection.c | 9 +++---
> drivers/usb/storage/protocol.c | 3 +-
> drivers/xen/grant-table.c | 2 +-
> fs/buffer.c | 2 +-
> fs/exec.c | 2 +-
> fs/ext4/ext4.h | 2 +-
> fs/ext4/mballoc.c | 3 +-
> fs/ext4/resize.c | 2 +-
> fs/ext4/super.c | 2 +-
> fs/fat/dir.c | 4 +--
> fs/fat/file.c | 3 +-
> fs/fuse/dev.c | 2 +-
> fs/fuse/file.c | 8 ++---
> fs/splice.c | 2 +-
> include/linux/bvec.h | 3 +-
> include/linux/nodemask.h | 9 +++---
> include/linux/perf_event.h | 2 +-
> include/net/tcp_ecn.h | 5 ++--
> io_uring/net.c | 6 ++--
> ipc/mqueue.c | 4 +--
> ipc/msg.c | 6 ++--
> kernel/bpf/core.c | 4 +--
> kernel/bpf/log.c | 2 +-
> kernel/bpf/verifier.c | 29 +++++++------------
> kernel/trace/bpf_trace.c | 2 +-
> lib/bucket_locks.c | 2 +-
> lib/crypto/mpi/mpicoder.c | 2 +-
> lib/dynamic_queue_limits.c | 2 +-
> mm/gup.c | 4 +--
> mm/memblock.c | 2 +-
> mm/memory.c | 2 +-
> mm/percpu.c | 2 +-
> mm/truncate.c | 3 +-
> mm/vmscan.c | 2 +-
> net/core/datagram.c | 6 ++--
> net/core/flow_dissector.c | 7 ++---
> net/core/net-sysfs.c | 3 +-
> net/core/skmsg.c | 4 +--
> net/ethtool/cmis_cdb.c | 7 ++---
> net/ipv4/fib_trie.c | 2 +-
> net/ipv4/tcp_input.c | 4 +--
> net/ipv4/tcp_output.c | 5 ++--
> net/ipv4/tcp_timer.c | 4 +--
> net/ipv6/addrconf.c | 8 ++---
> net/ipv6/ip6_output.c | 7 +++--
> net/ipv6/ndisc.c | 5 ++--
> net/mptcp/protocol.c | 8 ++---
> net/netlink/genetlink.c | 9 +++---
> net/packet/af_packet.c | 2 +-
> net/unix/af_unix.c | 4 +--
> 76 files changed, 141 insertions(+), 176 deletions(-)
Patches 10,14,16,37 applied. Thanks.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 30/44] fs: use min() or umin() instead of min_t()
2025-11-19 22:41 ` [PATCH 30/44] fs: use min() or umin() instead of min_t() david.laight.linux
@ 2025-11-25 9:06 ` Christian Brauner
0 siblings, 0 replies; 19+ messages in thread
From: Christian Brauner @ 2025-11-25 9:06 UTC (permalink / raw)
To: david.laight.linux
Cc: linux-kernel, linux-ext4, linux-fsdevel, linux-mm,
Alexander Viro, Andreas Dilger, Kees Cook, Miklos Szeredi,
OGAWA Hirofumi, Theodore Ts'o
On Wed, Nov 19, 2025 at 10:41:26PM +0000, david.laight.linux@gmail.com wrote:
> From: David Laight <david.laight.linux@gmail.com>
>
> min_t(unsigned int, a, b) casts an 'unsigned long' to 'unsigned int'.
> Use min(a, b) instead as it promotes any 'unsigned int' to 'unsigned long'
> and so cannot discard significant bits.
>
> A couple of places need umin() because of loops like:
> nfolios = DIV_ROUND_UP(ret + start, PAGE_SIZE);
>
> for (i = 0; i < nfolios; i++) {
> struct folio *folio = page_folio(pages[i]);
> ...
> unsigned int len = umin(ret, PAGE_SIZE - start);
> ...
> ret -= len;
> ...
> }
> where the compiler doesn't track things well enough to know that
> 'ret' is never negative.
>
> The alternate loop:
> for (i = 0; ret > 0; i++) {
> struct folio *folio = page_folio(pages[i]);
> ...
> unsigned int len = min(ret, PAGE_SIZE - start);
> ...
> ret -= len;
> ...
> }
> would be equivalent and doesn't need 'nfolios'.
>
> Most of the 'unsigned long' actually come from PAGE_SIZE.
>
> Detected by an extra check added to min_t().
>
> Signed-off-by: David Laight <david.laight.linux@gmail.com>
> ---
Too late for this cycle but we will pick this up next cycle!
> fs/buffer.c | 2 +-
> fs/exec.c | 2 +-
> fs/ext4/mballoc.c | 3 +--
> fs/ext4/resize.c | 2 +-
> fs/ext4/super.c | 2 +-
> fs/fat/dir.c | 4 ++--
> fs/fat/file.c | 3 +--
> fs/fuse/dev.c | 2 +-
> fs/fuse/file.c | 8 +++-----
> fs/splice.c | 2 +-
> 10 files changed, 13 insertions(+), 17 deletions(-)
>
> diff --git a/fs/buffer.c b/fs/buffer.c
> index 6a8752f7bbed..26c4c760b6c6 100644
> --- a/fs/buffer.c
> +++ b/fs/buffer.c
> @@ -2354,7 +2354,7 @@ bool block_is_partially_uptodate(struct folio *folio, size_t from, size_t count)
> if (!head)
> return false;
> blocksize = head->b_size;
> - to = min_t(unsigned, folio_size(folio) - from, count);
> + to = min(folio_size(folio) - from, count);
> to = from + to;
> if (from < blocksize && to > folio_size(folio) - blocksize)
> return false;
> diff --git a/fs/exec.c b/fs/exec.c
> index 4298e7e08d5d..6d699e48df82 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -555,7 +555,7 @@ int copy_string_kernel(const char *arg, struct linux_binprm *bprm)
> return -E2BIG;
>
> while (len > 0) {
> - unsigned int bytes_to_copy = min_t(unsigned int, len,
> + unsigned int bytes_to_copy = min(len,
> min_not_zero(offset_in_page(pos), PAGE_SIZE));
> struct page *page;
>
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index 9087183602e4..cb68ea974de6 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -4254,8 +4254,7 @@ void ext4_mb_mark_bb(struct super_block *sb, ext4_fsblk_t block,
> * get the corresponding group metadata to work with.
> * For this we have goto again loop.
> */
> - thisgrp_len = min_t(unsigned int, (unsigned int)len,
> - EXT4_BLOCKS_PER_GROUP(sb) - EXT4_C2B(sbi, blkoff));
> + thisgrp_len = min(len, EXT4_BLOCKS_PER_GROUP(sb) - EXT4_C2B(sbi, blkoff));
> clen = EXT4_NUM_B2C(sbi, thisgrp_len);
>
> if (!ext4_sb_block_valid(sb, NULL, block, thisgrp_len)) {
> diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
> index 050f26168d97..76842f0957b5 100644
> --- a/fs/ext4/resize.c
> +++ b/fs/ext4/resize.c
> @@ -1479,7 +1479,7 @@ static void ext4_update_super(struct super_block *sb,
>
> /* Update the global fs size fields */
> sbi->s_groups_count += flex_gd->count;
> - sbi->s_blockfile_groups = min_t(ext4_group_t, sbi->s_groups_count,
> + sbi->s_blockfile_groups = min(sbi->s_groups_count,
> (EXT4_MAX_BLOCK_FILE_PHYS / EXT4_BLOCKS_PER_GROUP(sb)));
>
> /* Update the reserved block counts only once the new group is
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 33e7c08c9529..e116fe48ff43 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -4830,7 +4830,7 @@ static int ext4_check_geometry(struct super_block *sb,
> return -EINVAL;
> }
> sbi->s_groups_count = blocks_count;
> - sbi->s_blockfile_groups = min_t(ext4_group_t, sbi->s_groups_count,
> + sbi->s_blockfile_groups = min(sbi->s_groups_count,
> (EXT4_MAX_BLOCK_FILE_PHYS / EXT4_BLOCKS_PER_GROUP(sb)));
> if (((u64)sbi->s_groups_count * sbi->s_inodes_per_group) !=
> le32_to_cpu(es->s_inodes_count)) {
> diff --git a/fs/fat/dir.c b/fs/fat/dir.c
> index 92b091783966..8375e7fbc1a5 100644
> --- a/fs/fat/dir.c
> +++ b/fs/fat/dir.c
> @@ -1353,7 +1353,7 @@ int fat_add_entries(struct inode *dir, void *slots, int nr_slots,
>
> /* Fill the long name slots. */
> for (i = 0; i < long_bhs; i++) {
> - int copy = min_t(int, sb->s_blocksize - offset, size);
> + int copy = umin(sb->s_blocksize - offset, size);
> memcpy(bhs[i]->b_data + offset, slots, copy);
> mark_buffer_dirty_inode(bhs[i], dir);
> offset = 0;
> @@ -1364,7 +1364,7 @@ int fat_add_entries(struct inode *dir, void *slots, int nr_slots,
> err = fat_sync_bhs(bhs, long_bhs);
> if (!err && i < nr_bhs) {
> /* Fill the short name slot. */
> - int copy = min_t(int, sb->s_blocksize - offset, size);
> + int copy = umin(sb->s_blocksize - offset, size);
> memcpy(bhs[i]->b_data + offset, slots, copy);
> mark_buffer_dirty_inode(bhs[i], dir);
> if (IS_DIRSYNC(dir))
> diff --git a/fs/fat/file.c b/fs/fat/file.c
> index 4fc49a614fb8..f48435e586c7 100644
> --- a/fs/fat/file.c
> +++ b/fs/fat/file.c
> @@ -140,8 +140,7 @@ static int fat_ioctl_fitrim(struct inode *inode, unsigned long arg)
> if (copy_from_user(&range, user_range, sizeof(range)))
> return -EFAULT;
>
> - range.minlen = max_t(unsigned int, range.minlen,
> - bdev_discard_granularity(sb->s_bdev));
> + range.minlen = max(range.minlen, bdev_discard_granularity(sb->s_bdev));
>
> err = fat_trim_fs(inode, &range);
> if (err < 0)
> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> index 132f38619d70..0c9fb0db1de1 100644
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -1813,7 +1813,7 @@ static int fuse_notify_store(struct fuse_conn *fc, unsigned int size,
> goto out_iput;
>
> folio_offset = ((index - folio->index) << PAGE_SHIFT) + offset;
> - nr_bytes = min_t(unsigned, num, folio_size(folio) - folio_offset);
> + nr_bytes = min(num, folio_size(folio) - folio_offset);
> nr_pages = (offset + nr_bytes + PAGE_SIZE - 1) >> PAGE_SHIFT;
>
> err = fuse_copy_folio(cs, &folio, folio_offset, nr_bytes, 0);
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index f1ef77a0be05..f4ffa559ad26 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -1252,10 +1252,8 @@ static ssize_t fuse_fill_write_pages(struct fuse_io_args *ia,
> static inline unsigned int fuse_wr_pages(loff_t pos, size_t len,
> unsigned int max_pages)
> {
> - return min_t(unsigned int,
> - ((pos + len - 1) >> PAGE_SHIFT) -
> - (pos >> PAGE_SHIFT) + 1,
> - max_pages);
> + return min(((pos + len - 1) >> PAGE_SHIFT) - (pos >> PAGE_SHIFT) + 1,
> + max_pages);
> }
>
> static ssize_t fuse_perform_write(struct kiocb *iocb, struct iov_iter *ii)
> @@ -1550,7 +1548,7 @@ static int fuse_get_user_pages(struct fuse_args_pages *ap, struct iov_iter *ii,
> struct folio *folio = page_folio(pages[i]);
> unsigned int offset = start +
> (folio_page_idx(folio, pages[i]) << PAGE_SHIFT);
> - unsigned int len = min_t(unsigned int, ret, PAGE_SIZE - start);
> + unsigned int len = umin(ret, PAGE_SIZE - start);
>
> ap->descs[ap->num_folios].offset = offset;
> ap->descs[ap->num_folios].length = len;
> diff --git a/fs/splice.c b/fs/splice.c
> index f5094b6d00a0..41ce3a4ef74f 100644
> --- a/fs/splice.c
> +++ b/fs/splice.c
> @@ -1467,7 +1467,7 @@ static ssize_t iter_to_pipe(struct iov_iter *from,
>
> n = DIV_ROUND_UP(left + start, PAGE_SIZE);
> for (i = 0; i < n; i++) {
> - int size = min_t(int, left, PAGE_SIZE - start);
> + int size = umin(left, PAGE_SIZE - start);
>
> buf.page = pages[i];
> buf.offset = start;
> --
> 2.39.5
>
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2025-11-25 9:06 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-11-19 22:40 [PATCH 00/44] Change a lot of min_t() that might mask high bits david.laight.linux
2025-11-19 22:41 ` [PATCH 30/44] fs: use min() or umin() instead of min_t() david.laight.linux
2025-11-25 9:06 ` Christian Brauner
2025-11-19 22:41 ` [PATCH 39/44] mm: use min() " david.laight.linux
2025-11-20 9:20 ` David Hildenbrand (Red Hat)
2025-11-20 9:59 ` David Laight
2025-11-20 23:45 ` Eric Biggers
2025-11-21 8:27 ` David Hildenbrand (Red Hat)
2025-11-21 9:15 ` David Laight
2025-11-20 10:36 ` Lorenzo Stoakes
2025-11-20 12:09 ` Lorenzo Stoakes
2025-11-20 12:55 ` David Laight
2025-11-20 13:42 ` David Hildenbrand (Red Hat)
2025-11-20 15:44 ` David Laight
2025-11-21 8:24 ` David Hildenbrand (Red Hat)
2025-11-20 1:47 ` [PATCH 00/44] Change a lot of min_t() that might mask high bits Jakub Kicinski
2025-11-20 9:38 ` Lorenzo Stoakes
2025-11-20 14:52 ` (subset) " Jens Axboe
2025-11-24 9:49 ` Herbert Xu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox