* [PATCH 04/15 6.6.y] minmax: make generic MIN() and MAX() macros available everywhere
From: Eliav Farber @ 2025-09-22 10:32 UTC (permalink / raw)
To: richard, anton.ivanov, johannes, dave.hansen, luto, peterz, tglx,
mingo, bp, x86, hpa, tony.luck, qiuxu.zhuo, james.morse, mchehab,
rric, harry.wentland, sunpeng.li, Rodrigo.Siqueira,
alexander.deucher, christian.koenig, Xinhui.Pan, airlied, daniel,
evan.quan, maarten.lankhorst, mripard, tzimmermann, jdelvare,
linux, linus.walleij, dmitry.torokhov, wens, jernej.skrabec,
samuel, agk, snitzer, dm-devel, mailhol.vincent, wg, mkl, davem,
edumazet, kuba, pabeni, alexandre.torgue, joabreu,
mcoquelin.stm32, krzysztof.kozlowski, malattia, hdegoede,
ilpo.jarvinen, markgross, artur.paszkiewicz, jejb,
martin.petersen, sakari.ailus, gregkh, clm, josef, dsterba,
luc.vanoostenryck, rostedt, mhiramat, pmladek, andriy.shevchenko,
linux, senozhatsky, minchan, akpm, dsahern, shuah, keescook, wad,
farbere, David.Laight, arnd, linux-um, linux-kernel, linux-edac,
amd-gfx, dri-devel, linux-hwmon, linux-input, linux-arm-kernel,
linux-sunxi, linux-media, linux-can, netdev, linux-stm32,
platform-driver-x86, linux-scsi, linux-staging, linux-btrfs,
linux-sparse, linux-trace-kernel, linux-mm, linux-kselftest, bpf,
stable
Cc: Linus Torvalds, David Laight, Lorenzo Stoakes
In-Reply-To: <20250922103241.16213-1-farbere@amazon.com>
From: Linus Torvalds <torvalds@linux-foundation.org>
[ Upstream commit 1a251f52cfdc417c84411a056bc142cbd77baef4 ]
This just standardizes the use of MIN() and MAX() macros, with the very
traditional semantics. The goal is to use these for C constant
expressions and for top-level / static initializers, and so be able to
simplify the min()/max() macros.
These macro names were used by various kernel code - they are very
traditional, after all - and all such users have been fixed up, with a
few different approaches:
- trivial duplicated macro definitions have been removed
Note that 'trivial' here means that it's obviously kernel code that
already included all the major kernel headers, and thus gets the new
generic MIN/MAX macros automatically.
- non-trivial duplicated macro definitions are guarded with #ifndef
This is the "yes, they define their own versions, but no, the include
situation is not entirely obvious, and maybe they don't get the
generic version automatically" case.
- strange use case #1
A couple of drivers decided that the way they want to describe their
versioning is with
#define MAJ 1
#define MIN 2
#define DRV_VERSION __stringify(MAJ) "." __stringify(MIN)
which adds zero value and I just did my Alexander the Great
impersonation, and rewrote that pointless Gordian knot as
#define DRV_VERSION "1.2"
instead.
- strange use case #2
A couple of drivers thought that it's a good idea to have a random
'MIN' or 'MAX' define for a value or index into a table, rather than
the traditional macro that takes arguments.
These values were re-written as C enum's instead. The new
function-line macros only expand when followed by an open
parenthesis, and thus don't clash with enum use.
Happily, there weren't really all that many of these cases, and a lot of
users already had the pattern of using '#ifndef' guarding (or in one
case just using '#undef MIN') before defining their own private version
that does the same thing. I left such cases alone.
Cc: David Laight <David.Laight@aculab.com>
Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Eliav Farber <farbere@amazon.com>
---
arch/um/drivers/mconsole_user.c | 2 ++
drivers/edac/skx_common.h | 1 -
.../drm/amd/display/modules/hdcp/hdcp_ddc.c | 2 ++
.../drm/amd/pm/powerplay/hwmgr/ppevvmath.h | 14 +++++++----
drivers/gpu/drm/radeon/evergreen_cs.c | 2 ++
drivers/hwmon/adt7475.c | 24 +++++++++----------
drivers/media/dvb-frontends/stv0367_priv.h | 3 +++
drivers/net/fjes/fjes_main.c | 4 +---
drivers/nfc/pn544/i2c.c | 2 --
drivers/platform/x86/sony-laptop.c | 1 -
drivers/scsi/isci/init.c | 6 +----
.../pci/hive_isp_css_include/math_support.h | 5 ----
include/linux/minmax.h | 2 ++
kernel/trace/preemptirq_delay_test.c | 2 --
lib/btree.c | 1 -
lib/decompress_unlzma.c | 2 ++
mm/zsmalloc.c | 2 --
tools/testing/selftests/mm/mremap_test.c | 2 ++
tools/testing/selftests/seccomp/seccomp_bpf.c | 2 ++
19 files changed, 41 insertions(+), 38 deletions(-)
diff --git a/arch/um/drivers/mconsole_user.c b/arch/um/drivers/mconsole_user.c
index e24298a734be..a04cd13c6315 100644
--- a/arch/um/drivers/mconsole_user.c
+++ b/arch/um/drivers/mconsole_user.c
@@ -71,7 +71,9 @@ static struct mconsole_command *mconsole_parse(struct mc_request *req)
return NULL;
}
+#ifndef MIN
#define MIN(a,b) ((a)<(b) ? (a):(b))
+#endif
#define STRINGX(x) #x
#define STRING(x) STRINGX(x)
diff --git a/drivers/edac/skx_common.h b/drivers/edac/skx_common.h
index 2ea4d1d1fbef..1fa31a6fdfcd 100644
--- a/drivers/edac/skx_common.h
+++ b/drivers/edac/skx_common.h
@@ -45,7 +45,6 @@
#define I10NM_NUM_CHANNELS MAX(I10NM_NUM_DDR_CHANNELS, I10NM_NUM_HBM_CHANNELS)
#define I10NM_NUM_DIMMS MAX(I10NM_NUM_DDR_DIMMS, I10NM_NUM_HBM_DIMMS)
-#define MAX(a, b) ((a) > (b) ? (a) : (b))
#define NUM_IMC MAX(SKX_NUM_IMC, I10NM_NUM_IMC)
#define NUM_CHANNELS MAX(SKX_NUM_CHANNELS, I10NM_NUM_CHANNELS)
#define NUM_DIMMS MAX(SKX_NUM_DIMMS, I10NM_NUM_DIMMS)
diff --git a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_ddc.c b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_ddc.c
index 1b2df97226a3..40286e8dd4e1 100644
--- a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_ddc.c
+++ b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_ddc.c
@@ -25,7 +25,9 @@
#include "hdcp.h"
+#ifndef MIN
#define MIN(a, b) ((a) < (b) ? (a) : (b))
+#endif
#define HDCP_I2C_ADDR 0x3a /* 0x74 >> 1*/
#define KSV_READ_SIZE 0xf /* 0x6803b - 0x6802c */
#define HDCP_MAX_AUX_TRANSACTION_SIZE 16
diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/ppevvmath.h b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/ppevvmath.h
index 6f54c410c2f9..409aeec6baa9 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/ppevvmath.h
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/ppevvmath.h
@@ -22,12 +22,18 @@
*/
#include <asm/div64.h>
-#define SHIFT_AMOUNT 16 /* We multiply all original integers with 2^SHIFT_AMOUNT to get the fInt representation */
+enum ppevvmath_constants {
+ /* We multiply all original integers with 2^SHIFT_AMOUNT to get the fInt representation */
+ SHIFT_AMOUNT = 16,
-#define PRECISION 5 /* Change this value to change the number of decimal places in the final output - 5 is a good default */
+ /* Change this value to change the number of decimal places in the final output - 5 is a good default */
+ PRECISION = 5,
-#define SHIFTED_2 (2 << SHIFT_AMOUNT)
-#define MAX (1 << (SHIFT_AMOUNT - 1)) - 1 /* 32767 - Might change in the future */
+ SHIFTED_2 = (2 << SHIFT_AMOUNT),
+
+ /* 32767 - Might change in the future */
+ MAX = (1 << (SHIFT_AMOUNT - 1)) - 1,
+};
/* -------------------------------------------------------------------------------
* NEW TYPE - fINT
diff --git a/drivers/gpu/drm/radeon/evergreen_cs.c b/drivers/gpu/drm/radeon/evergreen_cs.c
index 820c2c3641d3..1311f10fad66 100644
--- a/drivers/gpu/drm/radeon/evergreen_cs.c
+++ b/drivers/gpu/drm/radeon/evergreen_cs.c
@@ -33,8 +33,10 @@
#include "evergreen_reg_safe.h"
#include "cayman_reg_safe.h"
+#ifndef MIN
#define MAX(a,b) (((a)>(b))?(a):(b))
#define MIN(a,b) (((a)<(b))?(a):(b))
+#endif
#define REG_SAFE_BM_SIZE ARRAY_SIZE(evergreen_reg_safe_bm)
diff --git a/drivers/hwmon/adt7475.c b/drivers/hwmon/adt7475.c
index 14b2547adae8..ce0ab4e4e41a 100644
--- a/drivers/hwmon/adt7475.c
+++ b/drivers/hwmon/adt7475.c
@@ -22,23 +22,23 @@
#include <linux/util_macros.h>
/* Indexes for the sysfs hooks */
-
-#define INPUT 0
-#define MIN 1
-#define MAX 2
-#define CONTROL 3
-#define OFFSET 3
-#define AUTOMIN 4
-#define THERM 5
-#define HYSTERSIS 6
-
+enum adt_sysfs_id {
+ INPUT = 0,
+ MIN = 1,
+ MAX = 2,
+ CONTROL = 3,
+ OFFSET = 3, // Dup
+ AUTOMIN = 4,
+ THERM = 5,
+ HYSTERSIS = 6,
/*
* These are unique identifiers for the sysfs functions - unlike the
* numbers above, these are not also indexes into an array
*/
+ ALARM = 9,
+ FAULT = 10,
+};
-#define ALARM 9
-#define FAULT 10
/* 7475 Common Registers */
diff --git a/drivers/media/dvb-frontends/stv0367_priv.h b/drivers/media/dvb-frontends/stv0367_priv.h
index 617f605947b2..7f056d1cce82 100644
--- a/drivers/media/dvb-frontends/stv0367_priv.h
+++ b/drivers/media/dvb-frontends/stv0367_priv.h
@@ -25,8 +25,11 @@
#endif
/* MACRO definitions */
+#ifndef MIN
#define MAX(X, Y) ((X) >= (Y) ? (X) : (Y))
#define MIN(X, Y) ((X) <= (Y) ? (X) : (Y))
+#endif
+
#define INRANGE(X, Y, Z) \
((((X) <= (Y)) && ((Y) <= (Z))) || \
(((Z) <= (Y)) && ((Y) <= (X))) ? 1 : 0)
diff --git a/drivers/net/fjes/fjes_main.c b/drivers/net/fjes/fjes_main.c
index 2513be6d4e11..cfcf0f7d9d90 100644
--- a/drivers/net/fjes/fjes_main.c
+++ b/drivers/net/fjes/fjes_main.c
@@ -14,9 +14,7 @@
#include "fjes.h"
#include "fjes_trace.h"
-#define MAJ 1
-#define MIN 2
-#define DRV_VERSION __stringify(MAJ) "." __stringify(MIN)
+#define DRV_VERSION "1.2"
#define DRV_NAME "fjes"
char fjes_driver_name[] = DRV_NAME;
char fjes_driver_version[] = DRV_VERSION;
diff --git a/drivers/nfc/pn544/i2c.c b/drivers/nfc/pn544/i2c.c
index 3f6d74832bac..be566ca7bae9 100644
--- a/drivers/nfc/pn544/i2c.c
+++ b/drivers/nfc/pn544/i2c.c
@@ -126,8 +126,6 @@ struct pn544_i2c_fw_secure_blob {
#define PN544_FW_CMD_RESULT_COMMAND_REJECTED 0xE0
#define PN544_FW_CMD_RESULT_CHUNK_ERROR 0xE6
-#define MIN(X, Y) ((X) < (Y) ? (X) : (Y))
-
#define PN544_FW_WRITE_BUFFER_MAX_LEN 0x9f7
#define PN544_FW_I2C_MAX_PAYLOAD PN544_HCI_I2C_LLC_MAX_SIZE
#define PN544_FW_I2C_WRITE_FRAME_HEADER_LEN 8
diff --git a/drivers/platform/x86/sony-laptop.c b/drivers/platform/x86/sony-laptop.c
index 9569f11dec8c..bd7323cf9a97 100644
--- a/drivers/platform/x86/sony-laptop.c
+++ b/drivers/platform/x86/sony-laptop.c
@@ -757,7 +757,6 @@ static union acpi_object *__call_snc_method(acpi_handle handle, char *method,
return result;
}
-#define MIN(a, b) (a > b ? b : a)
static int sony_nc_buffer_call(acpi_handle handle, char *name, u64 *value,
void *buffer, size_t buflen)
{
diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
index 6277162a028b..6eefa7e5f4cf 100644
--- a/drivers/scsi/isci/init.c
+++ b/drivers/scsi/isci/init.c
@@ -65,11 +65,7 @@
#include "task.h"
#include "probe_roms.h"
-#define MAJ 1
-#define MIN 2
-#define BUILD 0
-#define DRV_VERSION __stringify(MAJ) "." __stringify(MIN) "." \
- __stringify(BUILD)
+#define DRV_VERSION "1.2.0"
MODULE_VERSION(DRV_VERSION);
diff --git a/drivers/staging/media/atomisp/pci/hive_isp_css_include/math_support.h b/drivers/staging/media/atomisp/pci/hive_isp_css_include/math_support.h
index a444ec14ff9d..1c17a87a8572 100644
--- a/drivers/staging/media/atomisp/pci/hive_isp_css_include/math_support.h
+++ b/drivers/staging/media/atomisp/pci/hive_isp_css_include/math_support.h
@@ -31,11 +31,6 @@
/* A => B */
#define IMPLIES(a, b) (!(a) || (b))
-/* for preprocessor and array sizing use MIN and MAX
- otherwise use min and max */
-#define MAX(a, b) (((a) > (b)) ? (a) : (b))
-#define MIN(a, b) (((a) < (b)) ? (a) : (b))
-
#define ROUND_DIV(a, b) (((b) != 0) ? ((a) + ((b) >> 1)) / (b) : 0)
#define CEIL_DIV(a, b) (((b) != 0) ? ((a) + (b) - 1) / (b) : 0)
#define CEIL_MUL(a, b) (CEIL_DIV(a, b) * (b))
diff --git a/include/linux/minmax.h b/include/linux/minmax.h
index 9c2848abc804..fc384714da45 100644
--- a/include/linux/minmax.h
+++ b/include/linux/minmax.h
@@ -277,6 +277,8 @@ static inline bool in_range32(u32 val, u32 start, u32 len)
* Use these carefully: no type checking, and uses the arguments
* multiple times. Use for obvious constants only.
*/
+#define MIN(a,b) __cmp(min,a,b)
+#define MAX(a,b) __cmp(max,a,b)
#define MIN_T(type,a,b) __cmp(min,(type)(a),(type)(b))
#define MAX_T(type,a,b) __cmp(max,(type)(a),(type)(b))
diff --git a/kernel/trace/preemptirq_delay_test.c b/kernel/trace/preemptirq_delay_test.c
index 8af92dbe98f0..acb0c971a408 100644
--- a/kernel/trace/preemptirq_delay_test.c
+++ b/kernel/trace/preemptirq_delay_test.c
@@ -34,8 +34,6 @@ MODULE_PARM_DESC(cpu_affinity, "Cpu num test is running on");
static struct completion done;
-#define MIN(x, y) ((x) < (y) ? (x) : (y))
-
static void busy_wait(ulong time)
{
u64 start, end;
diff --git a/lib/btree.c b/lib/btree.c
index 49420cae3a83..bb81d3393ac5 100644
--- a/lib/btree.c
+++ b/lib/btree.c
@@ -43,7 +43,6 @@
#include <linux/slab.h>
#include <linux/module.h>
-#define MAX(a, b) ((a) > (b) ? (a) : (b))
#define NODESIZE MAX(L1_CACHE_BYTES, 128)
struct btree_geo {
diff --git a/lib/decompress_unlzma.c b/lib/decompress_unlzma.c
index 20a858031f12..9d34d35908da 100644
--- a/lib/decompress_unlzma.c
+++ b/lib/decompress_unlzma.c
@@ -37,7 +37,9 @@
#include <linux/decompress/mm.h>
+#ifndef MIN
#define MIN(a, b) (((a) < (b)) ? (a) : (b))
+#endif
static long long INIT read_int(unsigned char *ptr, int size)
{
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index c82070167d8a..14327fc34aa7 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -119,8 +119,6 @@
#define ISOLATED_BITS 5
#define MAGIC_VAL_BITS 8
-#define MAX(a, b) ((a) >= (b) ? (a) : (b))
-
#define ZS_MAX_PAGES_PER_ZSPAGE (_AC(CONFIG_ZSMALLOC_CHAIN_SIZE, UL))
/* ZS_MIN_ALLOC_SIZE must be multiple of ZS_ALIGN */
diff --git a/tools/testing/selftests/mm/mremap_test.c b/tools/testing/selftests/mm/mremap_test.c
index 5c3773de9f0f..944645f9d2b3 100644
--- a/tools/testing/selftests/mm/mremap_test.c
+++ b/tools/testing/selftests/mm/mremap_test.c
@@ -22,7 +22,9 @@
#define VALIDATION_DEFAULT_THRESHOLD 4 /* 4MB */
#define VALIDATION_NO_THRESHOLD 0 /* Verify the entire region */
+#ifndef MIN
#define MIN(X, Y) ((X) < (Y) ? (X) : (Y))
+#endif
struct config {
unsigned long long src_alignment;
diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c
index 15325ca35f1e..4390965c84c4 100644
--- a/tools/testing/selftests/seccomp/seccomp_bpf.c
+++ b/tools/testing/selftests/seccomp/seccomp_bpf.c
@@ -60,7 +60,9 @@
#define SKIP(s, ...) XFAIL(s, ##__VA_ARGS__)
#endif
+#ifndef MIN
#define MIN(X, Y) ((X) < (Y) ? (X) : (Y))
+#endif
#ifndef PR_SET_PTRACER
# define PR_SET_PTRACER 0x59616d61
--
2.47.3
^ permalink raw reply related
* [PATCH 05/15 6.6.y] minmax: simplify min()/max()/clamp() implementation
From: Eliav Farber @ 2025-09-22 10:32 UTC (permalink / raw)
To: richard, anton.ivanov, johannes, dave.hansen, luto, peterz, tglx,
mingo, bp, x86, hpa, tony.luck, qiuxu.zhuo, james.morse, mchehab,
rric, harry.wentland, sunpeng.li, Rodrigo.Siqueira,
alexander.deucher, christian.koenig, Xinhui.Pan, airlied, daniel,
evan.quan, maarten.lankhorst, mripard, tzimmermann, jdelvare,
linux, linus.walleij, dmitry.torokhov, wens, jernej.skrabec,
samuel, agk, snitzer, dm-devel, mailhol.vincent, wg, mkl, davem,
edumazet, kuba, pabeni, alexandre.torgue, joabreu,
mcoquelin.stm32, krzysztof.kozlowski, malattia, hdegoede,
ilpo.jarvinen, markgross, artur.paszkiewicz, jejb,
martin.petersen, sakari.ailus, gregkh, clm, josef, dsterba,
luc.vanoostenryck, rostedt, mhiramat, pmladek, andriy.shevchenko,
linux, senozhatsky, minchan, akpm, dsahern, shuah, keescook, wad,
farbere, David.Laight, arnd, linux-um, linux-kernel, linux-edac,
amd-gfx, dri-devel, linux-hwmon, linux-input, linux-arm-kernel,
linux-sunxi, linux-media, linux-can, netdev, linux-stm32,
platform-driver-x86, linux-scsi, linux-staging, linux-btrfs,
linux-sparse, linux-trace-kernel, linux-mm, linux-kselftest, bpf,
stable
Cc: Linus Torvalds, David Laight, Lorenzo Stoakes
In-Reply-To: <20250922103241.16213-1-farbere@amazon.com>
From: Linus Torvalds <torvalds@linux-foundation.org>
[ Upstream commit dc1c8034e31b14a2e5e212104ec508aec44ce1b9 ]
Now that we no longer have any C constant expression contexts (ie array
size declarations or static initializers) that use min() or max(), we
can simpify the implementation by not having to worry about the result
staying as a C constant expression.
So now we can unconditionally just use temporary variables of the right
type, and get rid of the excessive expansion that used to come from the
use of
__builtin_choose_expr(__is_constexpr(...), ..
to pick the specialized code for constant expressions.
Another expansion simplification is to pass the temporary variables (in
addition to the original expression) to our __types_ok() macro. That
may superficially look like it complicates the macro, but when we only
want the type of the expression, expanding the temporary variable names
is much simpler and smaller than expanding the potentially complicated
original expression.
As a result, on my machine, doing a
$ time make drivers/staging/media/atomisp/pci/isp/kernels/ynr/ynr_1.0/ia_css_ynr.host.i
goes from
real 0m16.621s
user 0m15.360s
sys 0m1.221s
to
real 0m2.532s
user 0m2.091s
sys 0m0.452s
because the token expansion goes down dramatically.
In particular, the longest line expansion (which was line 71 of that
'ia_css_ynr.host.c' file) shrinks from 23,338kB (yes, 23MB for one
single line) to "just" 1,444kB (now "only" 1.4MB).
And yes, that line is still the line from hell, because it's doing
multiple levels of "min()/max()" expansion thanks to some of them being
hidden inside the uDIGIT_FITTING() macro.
Lorenzo has a nice cleanup patch that makes that driver use inline
functions instead of macros for sDIGIT_FITTING() and uDIGIT_FITTING(),
which will fix that line once and for all, but the 16-fold reduction in
this case does show why we need to simplify these helpers.
Cc: David Laight <David.Laight@aculab.com>
Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Eliav Farber <farbere@amazon.com>
---
include/linux/minmax.h | 43 ++++++++++++++++++++----------------------
1 file changed, 20 insertions(+), 23 deletions(-)
diff --git a/include/linux/minmax.h b/include/linux/minmax.h
index fc384714da45..e3e4353df983 100644
--- a/include/linux/minmax.h
+++ b/include/linux/minmax.h
@@ -35,10 +35,10 @@
#define __is_noneg_int(x) \
(__builtin_choose_expr(__is_constexpr(x) && __is_signed(x), x, -1) >= 0)
-#define __types_ok(x, y) \
- (__is_signed(x) == __is_signed(y) || \
- __is_signed((x) + 0) == __is_signed((y) + 0) || \
- __is_noneg_int(x) || __is_noneg_int(y))
+#define __types_ok(x, y, ux, uy) \
+ (__is_signed(ux) == __is_signed(uy) || \
+ __is_signed((ux) + 0) == __is_signed((uy) + 0) || \
+ __is_noneg_int(x) || __is_noneg_int(y))
#define __cmp_op_min <
#define __cmp_op_max >
@@ -51,34 +51,31 @@
#define __cmp_once(op, type, x, y) \
__cmp_once_unique(op, type, x, y, __UNIQUE_ID(x_), __UNIQUE_ID(y_))
-#define __careful_cmp_once(op, x, y) ({ \
- static_assert(__types_ok(x, y), \
+#define __careful_cmp_once(op, x, y, ux, uy) ({ \
+ __auto_type ux = (x); __auto_type uy = (y); \
+ static_assert(__types_ok(x, y, ux, uy), \
#op "(" #x ", " #y ") signedness error, fix types or consider u" #op "() before " #op "_t()"); \
- __cmp_once(op, __auto_type, x, y); })
+ __cmp(op, ux, uy); })
-#define __careful_cmp(op, x, y) \
- __builtin_choose_expr(__is_constexpr((x) - (y)), \
- __cmp(op, x, y), __careful_cmp_once(op, x, y))
+#define __careful_cmp(op, x, y) \
+ __careful_cmp_once(op, x, y, __UNIQUE_ID(x_), __UNIQUE_ID(y_))
#define __clamp(val, lo, hi) \
((val) >= (hi) ? (hi) : ((val) <= (lo) ? (lo) : (val)))
-#define __clamp_once(val, lo, hi, unique_val, unique_lo, unique_hi) ({ \
- typeof(val) unique_val = (val); \
- typeof(lo) unique_lo = (lo); \
- typeof(hi) unique_hi = (hi); \
+#define __clamp_once(val, lo, hi, uval, ulo, uhi) ({ \
+ __auto_type uval = (val); \
+ __auto_type ulo = (lo); \
+ __auto_type uhi = (hi); \
static_assert(__builtin_choose_expr(__is_constexpr((lo) > (hi)), \
(lo) <= (hi), true), \
"clamp() low limit " #lo " greater than high limit " #hi); \
- static_assert(__types_ok(val, lo), "clamp() 'lo' signedness error"); \
- static_assert(__types_ok(val, hi), "clamp() 'hi' signedness error"); \
- __clamp(unique_val, unique_lo, unique_hi); })
-
-#define __careful_clamp(val, lo, hi) ({ \
- __builtin_choose_expr(__is_constexpr((val) - (lo) + (hi)), \
- __clamp(val, lo, hi), \
- __clamp_once(val, lo, hi, __UNIQUE_ID(__val), \
- __UNIQUE_ID(__lo), __UNIQUE_ID(__hi))); })
+ static_assert(__types_ok(uval, lo, uval, ulo), "clamp() 'lo' signedness error"); \
+ static_assert(__types_ok(uval, hi, uval, uhi), "clamp() 'hi' signedness error"); \
+ __clamp(uval, ulo, uhi); })
+
+#define __careful_clamp(val, lo, hi) \
+ __clamp_once(val, lo, hi, __UNIQUE_ID(v_), __UNIQUE_ID(l_), __UNIQUE_ID(h_))
/**
* min - return minimum of two values of the same or compatible types
--
2.47.3
^ permalink raw reply related
* [PATCH 06/15 6.6.y] minmax: don't use max() in situations that want a C constant expression
From: Eliav Farber @ 2025-09-22 10:32 UTC (permalink / raw)
To: richard, anton.ivanov, johannes, dave.hansen, luto, peterz, tglx,
mingo, bp, x86, hpa, tony.luck, qiuxu.zhuo, james.morse, mchehab,
rric, harry.wentland, sunpeng.li, Rodrigo.Siqueira,
alexander.deucher, christian.koenig, Xinhui.Pan, airlied, daniel,
evan.quan, maarten.lankhorst, mripard, tzimmermann, jdelvare,
linux, linus.walleij, dmitry.torokhov, wens, jernej.skrabec,
samuel, agk, snitzer, dm-devel, mailhol.vincent, wg, mkl, davem,
edumazet, kuba, pabeni, alexandre.torgue, joabreu,
mcoquelin.stm32, krzysztof.kozlowski, malattia, hdegoede,
ilpo.jarvinen, markgross, artur.paszkiewicz, jejb,
martin.petersen, sakari.ailus, gregkh, clm, josef, dsterba,
luc.vanoostenryck, rostedt, mhiramat, pmladek, andriy.shevchenko,
linux, senozhatsky, minchan, akpm, dsahern, shuah, keescook, wad,
farbere, David.Laight, arnd, linux-um, linux-kernel, linux-edac,
amd-gfx, dri-devel, linux-hwmon, linux-input, linux-arm-kernel,
linux-sunxi, linux-media, linux-can, netdev, linux-stm32,
platform-driver-x86, linux-scsi, linux-staging, linux-btrfs,
linux-sparse, linux-trace-kernel, linux-mm, linux-kselftest, bpf,
stable
Cc: Linus Torvalds, David Laight, Lorenzo Stoakes
In-Reply-To: <20250922103241.16213-1-farbere@amazon.com>
From: Linus Torvalds <torvalds@linux-foundation.org>
[ Upstream commit cb04e8b1d2f24c4c2c92f7b7529031fc35a16fed ]
We only had a couple of array[] declarations, and changing them to just
use 'MAX()' instead of 'max()' fixes the issue.
This will allow us to simplify our min/max macros enormously, since they
can now unconditionally use temporary variables to avoid using the
argument values multiple times.
Cc: David Laight <David.Laight@aculab.com>
Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Eliav Farber <farbere@amazon.com>
---
drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c | 2 +-
drivers/input/touchscreen/cyttsp4_core.c | 2 +-
drivers/irqchip/irq-sun6i-r.c | 2 +-
drivers/net/can/usb/etas_es58x/es58x_devlink.c | 2 +-
fs/btrfs/tree-checker.c | 2 +-
lib/vsprintf.c | 2 +-
6 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
index 12618a583e97..c1962f1974c6 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
@@ -708,7 +708,7 @@ static const char *smu_get_feature_name(struct smu_context *smu,
size_t smu_cmn_get_pp_feature_mask(struct smu_context *smu,
char *buf)
{
- int8_t sort_feature[max(SMU_FEATURE_COUNT, SMU_FEATURE_MAX)];
+ int8_t sort_feature[MAX(SMU_FEATURE_COUNT, SMU_FEATURE_MAX)];
uint64_t feature_mask;
int i, feature_index;
uint32_t count = 0;
diff --git a/drivers/input/touchscreen/cyttsp4_core.c b/drivers/input/touchscreen/cyttsp4_core.c
index 7cb26929dc73..9dc25eb2be44 100644
--- a/drivers/input/touchscreen/cyttsp4_core.c
+++ b/drivers/input/touchscreen/cyttsp4_core.c
@@ -871,7 +871,7 @@ static void cyttsp4_get_mt_touches(struct cyttsp4_mt_data *md, int num_cur_tch)
struct cyttsp4_touch tch;
int sig;
int i, j, t = 0;
- int ids[max(CY_TMA1036_MAX_TCH, CY_TMA4XX_MAX_TCH)];
+ int ids[MAX(CY_TMA1036_MAX_TCH, CY_TMA4XX_MAX_TCH)];
memset(ids, 0, si->si_ofs.tch_abs[CY_TCH_T].max * sizeof(int));
for (i = 0; i < num_cur_tch; i++) {
diff --git a/drivers/irqchip/irq-sun6i-r.c b/drivers/irqchip/irq-sun6i-r.c
index a01e44049415..99958d470d62 100644
--- a/drivers/irqchip/irq-sun6i-r.c
+++ b/drivers/irqchip/irq-sun6i-r.c
@@ -270,7 +270,7 @@ static const struct irq_domain_ops sun6i_r_intc_domain_ops = {
static int sun6i_r_intc_suspend(void)
{
- u32 buf[BITS_TO_U32(max(SUN6I_NR_TOP_LEVEL_IRQS, SUN6I_NR_MUX_BITS))];
+ u32 buf[BITS_TO_U32(MAX(SUN6I_NR_TOP_LEVEL_IRQS, SUN6I_NR_MUX_BITS))];
int i;
/* Wake IRQs are enabled during system sleep and shutdown. */
diff --git a/drivers/net/can/usb/etas_es58x/es58x_devlink.c b/drivers/net/can/usb/etas_es58x/es58x_devlink.c
index e763a9904bed..0d155eb1b9e9 100644
--- a/drivers/net/can/usb/etas_es58x/es58x_devlink.c
+++ b/drivers/net/can/usb/etas_es58x/es58x_devlink.c
@@ -215,7 +215,7 @@ static int es58x_devlink_info_get(struct devlink *devlink,
struct es58x_sw_version *fw_ver = &es58x_dev->firmware_version;
struct es58x_sw_version *bl_ver = &es58x_dev->bootloader_version;
struct es58x_hw_revision *hw_rev = &es58x_dev->hardware_revision;
- char buf[max(sizeof("xx.xx.xx"), sizeof("axxx/xxx"))];
+ char buf[MAX(sizeof("xx.xx.xx"), sizeof("axxx/xxx"))];
int ret = 0;
if (es58x_sw_version_is_valid(fw_ver)) {
diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
index 6d16506bbdc0..6f4c42e66519 100644
--- a/fs/btrfs/tree-checker.c
+++ b/fs/btrfs/tree-checker.c
@@ -614,7 +614,7 @@ static int check_dir_item(struct extent_buffer *leaf,
*/
if (key->type == BTRFS_DIR_ITEM_KEY ||
key->type == BTRFS_XATTR_ITEM_KEY) {
- char namebuf[max(BTRFS_NAME_LEN, XATTR_NAME_MAX)];
+ char namebuf[MAX(BTRFS_NAME_LEN, XATTR_NAME_MAX)];
read_extent_buffer(leaf, namebuf,
(unsigned long)(di + 1), name_len);
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 2aa408441cd3..f4ab2750cfc1 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1079,7 +1079,7 @@ char *resource_string(char *buf, char *end, struct resource *res,
#define FLAG_BUF_SIZE (2 * sizeof(res->flags))
#define DECODED_BUF_SIZE sizeof("[mem - 64bit pref window disabled]")
#define RAW_BUF_SIZE sizeof("[mem - flags 0x]")
- char sym[max(2*RSRC_BUF_SIZE + DECODED_BUF_SIZE,
+ char sym[MAX(2*RSRC_BUF_SIZE + DECODED_BUF_SIZE,
2*RSRC_BUF_SIZE + FLAG_BUF_SIZE + RAW_BUF_SIZE)];
char *p = sym, *pend = sym + sizeof(sym);
--
2.47.3
^ permalink raw reply related
* [PATCH 07/15 6.6.y] minmax: improve macro expansion and type checking
From: Eliav Farber @ 2025-09-22 10:32 UTC (permalink / raw)
To: richard, anton.ivanov, johannes, dave.hansen, luto, peterz, tglx,
mingo, bp, x86, hpa, tony.luck, qiuxu.zhuo, james.morse, mchehab,
rric, harry.wentland, sunpeng.li, Rodrigo.Siqueira,
alexander.deucher, christian.koenig, Xinhui.Pan, airlied, daniel,
evan.quan, maarten.lankhorst, mripard, tzimmermann, jdelvare,
linux, linus.walleij, dmitry.torokhov, wens, jernej.skrabec,
samuel, agk, snitzer, dm-devel, mailhol.vincent, wg, mkl, davem,
edumazet, kuba, pabeni, alexandre.torgue, joabreu,
mcoquelin.stm32, krzysztof.kozlowski, malattia, hdegoede,
ilpo.jarvinen, markgross, artur.paszkiewicz, jejb,
martin.petersen, sakari.ailus, gregkh, clm, josef, dsterba,
luc.vanoostenryck, rostedt, mhiramat, pmladek, andriy.shevchenko,
linux, senozhatsky, minchan, akpm, dsahern, shuah, keescook, wad,
farbere, David.Laight, arnd, linux-um, linux-kernel, linux-edac,
amd-gfx, dri-devel, linux-hwmon, linux-input, linux-arm-kernel,
linux-sunxi, linux-media, linux-can, netdev, linux-stm32,
platform-driver-x86, linux-scsi, linux-staging, linux-btrfs,
linux-sparse, linux-trace-kernel, linux-mm, linux-kselftest, bpf,
stable
Cc: Linus Torvalds, David Laight, Lorenzo Stoakes
In-Reply-To: <20250922103241.16213-1-farbere@amazon.com>
From: Linus Torvalds <torvalds@linux-foundation.org>
[ Upstream commit 22f5468731491e53356ba7c028f0fdea20b18e2c ]
This clarifies the rules for min()/max()/clamp() type checking and makes
them a much more efficient macro expansion.
In particular, we now look at the type and range of the inputs to see
whether they work together, generating a mask of acceptable comparisons,
and then just verifying that the inputs have a shared case:
- an expression with a signed type can be used for
(1) signed comparisons
(2) unsigned comparisons if it is statically known to have a
non-negative value
- an expression with an unsigned type can be used for
(3) unsigned comparison
(4) signed comparisons if the type is smaller than 'int' and thus
the C integer promotion rules will make it signed anyway
Here rule (1) and (3) are obvious, and rule (2) is important in order to
allow obvious trivial constants to be used together with unsigned
values.
Rule (4) is not necessarily a good idea, but matches what we used to do,
and we have extant cases of this situation in the kernel. Notably with
bcachefs having an expression like
min(bch2_bucket_sectors_dirty(a), ca->mi.bucket_size)
where bch2_bucket_sectors_dirty() returns an 's64', and
'ca->mi.bucket_size' is of type 'u16'.
Technically that bcachefs comparison is clearly sensible on a C type
level, because the 'u16' will go through the normal C integer promotion,
and become 'int', and then we're comparing two signed values and
everything looks sane.
However, it's not entirely clear that a 'min(s64,u16)' operation makes a
lot of conceptual sense, and it's possible that we will remove rule (4).
After all, the _reason_ we have these complicated type checks is exactly
that the C type promotion rules are not very intuitive.
But at least for now the rule is in place for backwards compatibility.
Also note that rule (2) existed before, but is hugely relaxed by this
commit. It used to be true only for the simplest compile-time
non-negative integer constants. The new macro model will allow cases
where the compiler can trivially see that an expression is non-negative
even if it isn't necessarily a constant.
For example, the amdgpu driver does
min_t(size_t, sizeof(fru_info->serial), pia[addr] & 0x3F));
because our old 'min()' macro would see that 'pia[addr] & 0x3F' is of
type 'int' and clearly not a C constant expression, so doing a 'min()'
with a 'size_t' is a signedness violation.
Our new 'min()' macro still sees that 'pia[addr] & 0x3F' is of type
'int', but is smart enough to also see that it is clearly non-negative,
and thus would allow that case without any complaints.
Cc: Arnd Bergmann <arnd@kernel.org>
Cc: David Laight <David.Laight@aculab.com>
Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Eliav Farber <farbere@amazon.com>
---
include/linux/compiler.h | 9 +++++
include/linux/minmax.h | 74 ++++++++++++++++++++++++++++++++--------
2 files changed, 68 insertions(+), 15 deletions(-)
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index e84ed3a43f1f..1d4f23ea3105 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -244,6 +244,15 @@ static inline void *offset_to_ptr(const int *off)
#define is_signed_type(type) (((type)(-1)) < (__force type)1)
#define is_unsigned_type(type) (!is_signed_type(type))
+/*
+ * Useful shorthand for "is this condition known at compile-time?"
+ *
+ * Note that the condition may involve non-constant values,
+ * but the compiler may know enough about the details of the
+ * values to determine that the condition is statically true.
+ */
+#define statically_true(x) (__builtin_constant_p(x) && (x))
+
/*
* This is needed in functions which generate the stack canary, see
* arch/x86/kernel/smpboot.c::start_secondary() for an example.
diff --git a/include/linux/minmax.h b/include/linux/minmax.h
index e3e4353df983..41da6f85a407 100644
--- a/include/linux/minmax.h
+++ b/include/linux/minmax.h
@@ -26,19 +26,63 @@
#define __typecheck(x, y) \
(!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
-/* is_signed_type() isn't a constexpr for pointer types */
-#define __is_signed(x) \
- __builtin_choose_expr(__is_constexpr(is_signed_type(typeof(x))), \
- is_signed_type(typeof(x)), 0)
+/*
+ * __sign_use for integer expressions:
+ * bit #0 set if ok for unsigned comparisons
+ * bit #1 set if ok for signed comparisons
+ *
+ * In particular, statically non-negative signed integer
+ * expressions are ok for both.
+ *
+ * NOTE! Unsigned types smaller than 'int' are implicitly
+ * converted to 'int' in expressions, and are accepted for
+ * signed conversions for now. This is debatable.
+ *
+ * Note that 'x' is the original expression, and 'ux' is
+ * the unique variable that contains the value.
+ *
+ * We use 'ux' for pure type checking, and 'x' for when
+ * we need to look at the value (but without evaluating
+ * it for side effects! Careful to only ever evaluate it
+ * with sizeof() or __builtin_constant_p() etc).
+ *
+ * Pointers end up being checked by the normal C type
+ * rules at the actual comparison, and these expressions
+ * only need to be careful to not cause warnings for
+ * pointer use.
+ */
+#define __signed_type_use(x,ux) (2+__is_nonneg(x,ux))
+#define __unsigned_type_use(x,ux) (1+2*(sizeof(ux)<4))
+#define __sign_use(x,ux) (is_signed_type(typeof(ux))? \
+ __signed_type_use(x,ux):__unsigned_type_use(x,ux))
+
+/*
+ * To avoid warnings about casting pointers to integers
+ * of different sizes, we need that special sign type.
+ *
+ * On 64-bit we can just always use 'long', since any
+ * integer or pointer type can just be cast to that.
+ *
+ * This does not work for 128-bit signed integers since
+ * the cast would truncate them, but we do not use s128
+ * types in the kernel (we do use 'u128', but they will
+ * be handled by the !is_signed_type() case).
+ *
+ * NOTE! The cast is there only to avoid any warnings
+ * from when values that aren't signed integer types.
+ */
+#ifdef CONFIG_64BIT
+ #define __signed_type(ux) long
+#else
+ #define __signed_type(ux) typeof(__builtin_choose_expr(sizeof(ux)>4,1LL,1L))
+#endif
+#define __is_nonneg(x,ux) statically_true((__signed_type(ux))(x)>=0)
-/* True for a non-negative signed int constant */
-#define __is_noneg_int(x) \
- (__builtin_choose_expr(__is_constexpr(x) && __is_signed(x), x, -1) >= 0)
+#define __types_ok(x,y,ux,uy) \
+ (__sign_use(x,ux) & __sign_use(y,uy))
-#define __types_ok(x, y, ux, uy) \
- (__is_signed(ux) == __is_signed(uy) || \
- __is_signed((ux) + 0) == __is_signed((uy) + 0) || \
- __is_noneg_int(x) || __is_noneg_int(y))
+#define __types_ok3(x,y,z,ux,uy,uz) \
+ (__sign_use(x,ux) & __sign_use(y,uy) & __sign_use(z,uz))
#define __cmp_op_min <
#define __cmp_op_max >
@@ -53,8 +97,8 @@
#define __careful_cmp_once(op, x, y, ux, uy) ({ \
__auto_type ux = (x); __auto_type uy = (y); \
- static_assert(__types_ok(x, y, ux, uy), \
- #op "(" #x ", " #y ") signedness error, fix types or consider u" #op "() before " #op "_t()"); \
+ BUILD_BUG_ON_MSG(!__types_ok(x,y,ux,uy), \
+ #op"("#x", "#y") signedness error"); \
__cmp(op, ux, uy); })
#define __careful_cmp(op, x, y) \
@@ -70,8 +114,8 @@
static_assert(__builtin_choose_expr(__is_constexpr((lo) > (hi)), \
(lo) <= (hi), true), \
"clamp() low limit " #lo " greater than high limit " #hi); \
- static_assert(__types_ok(uval, lo, uval, ulo), "clamp() 'lo' signedness error"); \
- static_assert(__types_ok(uval, hi, uval, uhi), "clamp() 'hi' signedness error"); \
+ BUILD_BUG_ON_MSG(!__types_ok3(val,lo,hi,uval,ulo,uhi), \
+ "clamp("#val", "#lo", "#hi") signedness error"); \
__clamp(uval, ulo, uhi); })
#define __careful_clamp(val, lo, hi) \
--
2.47.3
^ permalink raw reply related
* [PATCH 08/15 6.6.y] minmax: fix up min3() and max3() too
From: Eliav Farber @ 2025-09-22 10:32 UTC (permalink / raw)
To: richard, anton.ivanov, johannes, dave.hansen, luto, peterz, tglx,
mingo, bp, x86, hpa, tony.luck, qiuxu.zhuo, james.morse, mchehab,
rric, harry.wentland, sunpeng.li, Rodrigo.Siqueira,
alexander.deucher, christian.koenig, Xinhui.Pan, airlied, daniel,
evan.quan, maarten.lankhorst, mripard, tzimmermann, jdelvare,
linux, linus.walleij, dmitry.torokhov, wens, jernej.skrabec,
samuel, agk, snitzer, dm-devel, mailhol.vincent, wg, mkl, davem,
edumazet, kuba, pabeni, alexandre.torgue, joabreu,
mcoquelin.stm32, krzysztof.kozlowski, malattia, hdegoede,
ilpo.jarvinen, markgross, artur.paszkiewicz, jejb,
martin.petersen, sakari.ailus, gregkh, clm, josef, dsterba,
luc.vanoostenryck, rostedt, mhiramat, pmladek, andriy.shevchenko,
linux, senozhatsky, minchan, akpm, dsahern, shuah, keescook, wad,
farbere, David.Laight, arnd, linux-um, linux-kernel, linux-edac,
amd-gfx, dri-devel, linux-hwmon, linux-input, linux-arm-kernel,
linux-sunxi, linux-media, linux-can, netdev, linux-stm32,
platform-driver-x86, linux-scsi, linux-staging, linux-btrfs,
linux-sparse, linux-trace-kernel, linux-mm, linux-kselftest, bpf,
stable
Cc: Linus Torvalds, David Laight
In-Reply-To: <20250922103241.16213-1-farbere@amazon.com>
From: Linus Torvalds <torvalds@linux-foundation.org>
[ Upstream commit 21b136cc63d2a9ddd60d4699552b69c214b32964 ]
David Laight pointed out that we should deal with the min3() and max3()
mess too, which still does excessive expansion.
And our current macros are actually rather broken.
In particular, the macros did this:
#define min3(x, y, z) min((typeof(x))min(x, y), z)
#define max3(x, y, z) max((typeof(x))max(x, y), z)
and that not only is a nested expansion of possibly very complex
arguments with all that involves, the typing with that "typeof()" cast
is completely wrong.
For example, imagine what happens in max3() if 'x' happens to be a
'unsigned char', but 'y' and 'z' are 'unsigned long'. The types are
compatible, and there's no warning - but the result is just random
garbage.
No, I don't think we've ever hit that issue in practice, but since we
now have sane infrastructure for doing this right, let's just use it.
It fixes any excessive expansion, and also avoids these kinds of broken
type issues.
Requested-by: David Laight <David.Laight@aculab.com>
Acked-by: Arnd Bergmann <arnd@kernel.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Eliav Farber <farbere@amazon.com>
---
include/linux/minmax.h | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/include/linux/minmax.h b/include/linux/minmax.h
index 41da6f85a407..98008dd92153 100644
--- a/include/linux/minmax.h
+++ b/include/linux/minmax.h
@@ -152,13 +152,20 @@
#define umax(x, y) \
__careful_cmp(max, (x) + 0u + 0ul + 0ull, (y) + 0u + 0ul + 0ull)
+#define __careful_op3(op, x, y, z, ux, uy, uz) ({ \
+ __auto_type ux = (x); __auto_type uy = (y);__auto_type uz = (z);\
+ BUILD_BUG_ON_MSG(!__types_ok3(x,y,z,ux,uy,uz), \
+ #op"3("#x", "#y", "#z") signedness error"); \
+ __cmp(op, ux, __cmp(op, uy, uz)); })
+
/**
* min3 - return minimum of three values
* @x: first value
* @y: second value
* @z: third value
*/
-#define min3(x, y, z) min((typeof(x))min(x, y), z)
+#define min3(x, y, z) \
+ __careful_op3(min, x, y, z, __UNIQUE_ID(x_), __UNIQUE_ID(y_), __UNIQUE_ID(z_))
/**
* max3 - return maximum of three values
@@ -166,7 +173,8 @@
* @y: second value
* @z: third value
*/
-#define max3(x, y, z) max((typeof(x))max(x, y), z)
+#define max3(x, y, z) \
+ __careful_op3(max, x, y, z, __UNIQUE_ID(x_), __UNIQUE_ID(y_), __UNIQUE_ID(z_))
/**
* min_not_zero - return the minimum that is _not_ zero, unless both are zero
--
2.47.3
^ permalink raw reply related
* [PATCH 09/15 6.6.y] minmax.h: add whitespace around operators and after commas
From: Eliav Farber @ 2025-09-22 10:32 UTC (permalink / raw)
To: richard, anton.ivanov, johannes, dave.hansen, luto, peterz, tglx,
mingo, bp, x86, hpa, tony.luck, qiuxu.zhuo, james.morse, mchehab,
rric, harry.wentland, sunpeng.li, Rodrigo.Siqueira,
alexander.deucher, christian.koenig, Xinhui.Pan, airlied, daniel,
evan.quan, maarten.lankhorst, mripard, tzimmermann, jdelvare,
linux, linus.walleij, dmitry.torokhov, wens, jernej.skrabec,
samuel, agk, snitzer, dm-devel, mailhol.vincent, wg, mkl, davem,
edumazet, kuba, pabeni, alexandre.torgue, joabreu,
mcoquelin.stm32, krzysztof.kozlowski, malattia, hdegoede,
ilpo.jarvinen, markgross, artur.paszkiewicz, jejb,
martin.petersen, sakari.ailus, gregkh, clm, josef, dsterba,
luc.vanoostenryck, rostedt, mhiramat, pmladek, andriy.shevchenko,
linux, senozhatsky, minchan, akpm, dsahern, shuah, keescook, wad,
farbere, David.Laight, arnd, linux-um, linux-kernel, linux-edac,
amd-gfx, dri-devel, linux-hwmon, linux-input, linux-arm-kernel,
linux-sunxi, linux-media, linux-can, netdev, linux-stm32,
platform-driver-x86, linux-scsi, linux-staging, linux-btrfs,
linux-sparse, linux-trace-kernel, linux-mm, linux-kselftest, bpf,
stable
Cc: Christoph Hellwig, Dan Carpenter, Jason A. Donenfeld, Jens Axboe,
Lorenzo Stoakes, Mateusz Guzik, Matthew Wilcox, Pedro Falcato
In-Reply-To: <20250922103241.16213-1-farbere@amazon.com>
From: David Laight <David.Laight@ACULAB.COM>
[ Upstream commit 71ee9b16251ea4bf7c1fe222517c82bdb3220acc ]
Patch series "minmax.h: Cleanups and minor optimisations".
Some tidyups and minor changes to minmax.h.
This patch (of 7):
Link: https://lkml.kernel.org/r/c50365d214e04f9ba256d417c8bebbc0@AcuMS.aculab.com
Link: https://lkml.kernel.org/r/f04b2e1310244f62826267346fde0553@AcuMS.aculab.com
Signed-off-by: David Laight <david.laight@aculab.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Arnd Bergmann <arnd@kernel.org>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Dan Carpenter <dan.carpenter@linaro.org>
Cc: Jason A. Donenfeld <Jason@zx2c4.com>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Cc: Mateusz Guzik <mjguzik@gmail.com>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Pedro Falcato <pedro.falcato@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Eliav Farber <farbere@amazon.com>
---
include/linux/minmax.h | 34 +++++++++++++++++-----------------
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/include/linux/minmax.h b/include/linux/minmax.h
index 98008dd92153..51b0d988e322 100644
--- a/include/linux/minmax.h
+++ b/include/linux/minmax.h
@@ -51,10 +51,10 @@
* only need to be careful to not cause warnings for
* pointer use.
*/
-#define __signed_type_use(x,ux) (2+__is_nonneg(x,ux))
-#define __unsigned_type_use(x,ux) (1+2*(sizeof(ux)<4))
-#define __sign_use(x,ux) (is_signed_type(typeof(ux))? \
- __signed_type_use(x,ux):__unsigned_type_use(x,ux))
+#define __signed_type_use(x, ux) (2 + __is_nonneg(x, ux))
+#define __unsigned_type_use(x, ux) (1 + 2 * (sizeof(ux) < 4))
+#define __sign_use(x, ux) (is_signed_type(typeof(ux)) ? \
+ __signed_type_use(x, ux) : __unsigned_type_use(x, ux))
/*
* To avoid warnings about casting pointers to integers
@@ -74,15 +74,15 @@
#ifdef CONFIG_64BIT
#define __signed_type(ux) long
#else
- #define __signed_type(ux) typeof(__builtin_choose_expr(sizeof(ux)>4,1LL,1L))
+ #define __signed_type(ux) typeof(__builtin_choose_expr(sizeof(ux) > 4, 1LL, 1L))
#endif
-#define __is_nonneg(x,ux) statically_true((__signed_type(ux))(x)>=0)
+#define __is_nonneg(x, ux) statically_true((__signed_type(ux))(x) >= 0)
-#define __types_ok(x,y,ux,uy) \
- (__sign_use(x,ux) & __sign_use(y,uy))
+#define __types_ok(x, y, ux, uy) \
+ (__sign_use(x, ux) & __sign_use(y, uy))
-#define __types_ok3(x,y,z,ux,uy,uz) \
- (__sign_use(x,ux) & __sign_use(y,uy) & __sign_use(z,uz))
+#define __types_ok3(x, y, z, ux, uy, uz) \
+ (__sign_use(x, ux) & __sign_use(y, uy) & __sign_use(z, uz))
#define __cmp_op_min <
#define __cmp_op_max >
@@ -97,7 +97,7 @@
#define __careful_cmp_once(op, x, y, ux, uy) ({ \
__auto_type ux = (x); __auto_type uy = (y); \
- BUILD_BUG_ON_MSG(!__types_ok(x,y,ux,uy), \
+ BUILD_BUG_ON_MSG(!__types_ok(x, y, ux, uy), \
#op"("#x", "#y") signedness error"); \
__cmp(op, ux, uy); })
@@ -114,7 +114,7 @@
static_assert(__builtin_choose_expr(__is_constexpr((lo) > (hi)), \
(lo) <= (hi), true), \
"clamp() low limit " #lo " greater than high limit " #hi); \
- BUILD_BUG_ON_MSG(!__types_ok3(val,lo,hi,uval,ulo,uhi), \
+ BUILD_BUG_ON_MSG(!__types_ok3(val, lo, hi, uval, ulo, uhi), \
"clamp("#val", "#lo", "#hi") signedness error"); \
__clamp(uval, ulo, uhi); })
@@ -154,7 +154,7 @@
#define __careful_op3(op, x, y, z, ux, uy, uz) ({ \
__auto_type ux = (x); __auto_type uy = (y);__auto_type uz = (z);\
- BUILD_BUG_ON_MSG(!__types_ok3(x,y,z,ux,uy,uz), \
+ BUILD_BUG_ON_MSG(!__types_ok3(x, y, z, ux, uy, uz), \
#op"3("#x", "#y", "#z") signedness error"); \
__cmp(op, ux, __cmp(op, uy, uz)); })
@@ -326,9 +326,9 @@ static inline bool in_range32(u32 val, u32 start, u32 len)
* Use these carefully: no type checking, and uses the arguments
* multiple times. Use for obvious constants only.
*/
-#define MIN(a,b) __cmp(min,a,b)
-#define MAX(a,b) __cmp(max,a,b)
-#define MIN_T(type,a,b) __cmp(min,(type)(a),(type)(b))
-#define MAX_T(type,a,b) __cmp(max,(type)(a),(type)(b))
+#define MIN(a, b) __cmp(min, a, b)
+#define MAX(a, b) __cmp(max, a, b)
+#define MIN_T(type, a, b) __cmp(min, (type)(a), (type)(b))
+#define MAX_T(type, a, b) __cmp(max, (type)(a), (type)(b))
#endif /* _LINUX_MINMAX_H */
--
2.47.3
^ permalink raw reply related
* [PATCH 10/15 6.6.y] minmax.h: update some comments
From: Eliav Farber @ 2025-09-22 10:32 UTC (permalink / raw)
To: richard, anton.ivanov, johannes, dave.hansen, luto, peterz, tglx,
mingo, bp, x86, hpa, tony.luck, qiuxu.zhuo, james.morse, mchehab,
rric, harry.wentland, sunpeng.li, Rodrigo.Siqueira,
alexander.deucher, christian.koenig, Xinhui.Pan, airlied, daniel,
evan.quan, maarten.lankhorst, mripard, tzimmermann, jdelvare,
linux, linus.walleij, dmitry.torokhov, wens, jernej.skrabec,
samuel, agk, snitzer, dm-devel, mailhol.vincent, wg, mkl, davem,
edumazet, kuba, pabeni, alexandre.torgue, joabreu,
mcoquelin.stm32, krzysztof.kozlowski, malattia, hdegoede,
ilpo.jarvinen, markgross, artur.paszkiewicz, jejb,
martin.petersen, sakari.ailus, gregkh, clm, josef, dsterba,
luc.vanoostenryck, rostedt, mhiramat, pmladek, andriy.shevchenko,
linux, senozhatsky, minchan, akpm, dsahern, shuah, keescook, wad,
farbere, David.Laight, arnd, linux-um, linux-kernel, linux-edac,
amd-gfx, dri-devel, linux-hwmon, linux-input, linux-arm-kernel,
linux-sunxi, linux-media, linux-can, netdev, linux-stm32,
platform-driver-x86, linux-scsi, linux-staging, linux-btrfs,
linux-sparse, linux-trace-kernel, linux-mm, linux-kselftest, bpf,
stable
Cc: Christoph Hellwig, Dan Carpenter, Jason A. Donenfeld, Jens Axboe,
Lorenzo Stoakes, Mateusz Guzik, Matthew Wilcox, Pedro Falcato
In-Reply-To: <20250922103241.16213-1-farbere@amazon.com>
From: David Laight <David.Laight@ACULAB.COM>
[ Upstream commit 10666e99204818ef45c702469488353b5bb09ec7 ]
- Change three to several.
- Remove the comment about retaining constant expressions, no longer true.
- Realign to nearer 80 columns and break on major punctiation.
- Add a leading comment to the block before __signed_type() and __is_nonneg()
Otherwise the block explaining the cast is a bit 'floating'.
Reword the rest of that comment to improve readability.
Link: https://lkml.kernel.org/r/85b050c81c1d4076aeb91a6cded45fee@AcuMS.aculab.com
Signed-off-by: David Laight <david.laight@aculab.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Arnd Bergmann <arnd@kernel.org>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Dan Carpenter <dan.carpenter@linaro.org>
Cc: Jason A. Donenfeld <Jason@zx2c4.com>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Cc: Mateusz Guzik <mjguzik@gmail.com>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Pedro Falcato <pedro.falcato@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Eliav Farber <farbere@amazon.com>
---
include/linux/minmax.h | 53 +++++++++++++++++++-----------------------
1 file changed, 24 insertions(+), 29 deletions(-)
diff --git a/include/linux/minmax.h b/include/linux/minmax.h
index 51b0d988e322..24e4b372649a 100644
--- a/include/linux/minmax.h
+++ b/include/linux/minmax.h
@@ -8,13 +8,10 @@
#include <linux/types.h>
/*
- * min()/max()/clamp() macros must accomplish three things:
+ * min()/max()/clamp() macros must accomplish several things:
*
* - Avoid multiple evaluations of the arguments (so side-effects like
* "x++" happen only once) when non-constant.
- * - Retain result as a constant expressions when called with only
- * constant expressions (to avoid tripping VLA warnings in stack
- * allocation usage).
* - Perform signed v unsigned type-checking (to generate compile
* errors instead of nasty runtime surprises).
* - Unsigned char/short are always promoted to signed int and can be
@@ -31,25 +28,23 @@
* bit #0 set if ok for unsigned comparisons
* bit #1 set if ok for signed comparisons
*
- * In particular, statically non-negative signed integer
- * expressions are ok for both.
+ * In particular, statically non-negative signed integer expressions
+ * are ok for both.
*
- * NOTE! Unsigned types smaller than 'int' are implicitly
- * converted to 'int' in expressions, and are accepted for
- * signed conversions for now. This is debatable.
+ * NOTE! Unsigned types smaller than 'int' are implicitly converted to 'int'
+ * in expressions, and are accepted for signed conversions for now.
+ * This is debatable.
*
- * Note that 'x' is the original expression, and 'ux' is
- * the unique variable that contains the value.
+ * Note that 'x' is the original expression, and 'ux' is the unique variable
+ * that contains the value.
*
- * We use 'ux' for pure type checking, and 'x' for when
- * we need to look at the value (but without evaluating
- * it for side effects! Careful to only ever evaluate it
- * with sizeof() or __builtin_constant_p() etc).
+ * We use 'ux' for pure type checking, and 'x' for when we need to look at the
+ * value (but without evaluating it for side effects!
+ * Careful to only ever evaluate it with sizeof() or __builtin_constant_p() etc).
*
- * Pointers end up being checked by the normal C type
- * rules at the actual comparison, and these expressions
- * only need to be careful to not cause warnings for
- * pointer use.
+ * Pointers end up being checked by the normal C type rules at the actual
+ * comparison, and these expressions only need to be careful to not cause
+ * warnings for pointer use.
*/
#define __signed_type_use(x, ux) (2 + __is_nonneg(x, ux))
#define __unsigned_type_use(x, ux) (1 + 2 * (sizeof(ux) < 4))
@@ -57,19 +52,19 @@
__signed_type_use(x, ux) : __unsigned_type_use(x, ux))
/*
- * To avoid warnings about casting pointers to integers
- * of different sizes, we need that special sign type.
+ * Check whether a signed value is always non-negative.
*
- * On 64-bit we can just always use 'long', since any
- * integer or pointer type can just be cast to that.
+ * A cast is needed to avoid any warnings from values that aren't signed
+ * integer types (in which case the result doesn't matter).
*
- * This does not work for 128-bit signed integers since
- * the cast would truncate them, but we do not use s128
- * types in the kernel (we do use 'u128', but they will
- * be handled by the !is_signed_type() case).
+ * On 64-bit any integer or pointer type can safely be cast to 'long'.
+ * But on 32-bit we need to avoid warnings about casting pointers to integers
+ * of different sizes without truncating 64-bit values so 'long' or 'long long'
+ * must be used depending on the size of the value.
*
- * NOTE! The cast is there only to avoid any warnings
- * from when values that aren't signed integer types.
+ * This does not work for 128-bit signed integers since the cast would truncate
+ * them, but we do not use s128 types in the kernel (we do use 'u128',
+ * but they are handled by the !is_signed_type() case).
*/
#ifdef CONFIG_64BIT
#define __signed_type(ux) long
--
2.47.3
^ permalink raw reply related
* [PATCH 11/15 6.6.y] minmax.h: reduce the #define expansion of min(), max() and clamp()
From: Eliav Farber @ 2025-09-22 10:32 UTC (permalink / raw)
To: richard, anton.ivanov, johannes, dave.hansen, luto, peterz, tglx,
mingo, bp, x86, hpa, tony.luck, qiuxu.zhuo, james.morse, mchehab,
rric, harry.wentland, sunpeng.li, Rodrigo.Siqueira,
alexander.deucher, christian.koenig, Xinhui.Pan, airlied, daniel,
evan.quan, maarten.lankhorst, mripard, tzimmermann, jdelvare,
linux, linus.walleij, dmitry.torokhov, wens, jernej.skrabec,
samuel, agk, snitzer, dm-devel, mailhol.vincent, wg, mkl, davem,
edumazet, kuba, pabeni, alexandre.torgue, joabreu,
mcoquelin.stm32, krzysztof.kozlowski, malattia, hdegoede,
ilpo.jarvinen, markgross, artur.paszkiewicz, jejb,
martin.petersen, sakari.ailus, gregkh, clm, josef, dsterba,
luc.vanoostenryck, rostedt, mhiramat, pmladek, andriy.shevchenko,
linux, senozhatsky, minchan, akpm, dsahern, shuah, keescook, wad,
farbere, David.Laight, arnd, linux-um, linux-kernel, linux-edac,
amd-gfx, dri-devel, linux-hwmon, linux-input, linux-arm-kernel,
linux-sunxi, linux-media, linux-can, netdev, linux-stm32,
platform-driver-x86, linux-scsi, linux-staging, linux-btrfs,
linux-sparse, linux-trace-kernel, linux-mm, linux-kselftest, bpf,
stable
Cc: Christoph Hellwig, Dan Carpenter, Jason A. Donenfeld, Jens Axboe,
Lorenzo Stoakes, Mateusz Guzik, Matthew Wilcox, Pedro Falcato
In-Reply-To: <20250922103241.16213-1-farbere@amazon.com>
From: David Laight <David.Laight@ACULAB.COM>
[ Upstream commit b280bb27a9f7c91ddab730e1ad91a9c18a051f41 ]
Since the test for signed values being non-negative only relies on
__builtion_constant_p() (not is_constexpr()) it can use the 'ux' variable
instead of the caller supplied expression. This means that the #define
parameters are only expanded twice. Once in the code and once quoted in
the error message.
Link: https://lkml.kernel.org/r/051afc171806425da991908ed8688a98@AcuMS.aculab.com
Signed-off-by: David Laight <david.laight@aculab.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Arnd Bergmann <arnd@kernel.org>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Dan Carpenter <dan.carpenter@linaro.org>
Cc: Jason A. Donenfeld <Jason@zx2c4.com>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Cc: Mateusz Guzik <mjguzik@gmail.com>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Pedro Falcato <pedro.falcato@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Eliav Farber <farbere@amazon.com>
---
include/linux/minmax.h | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/include/linux/minmax.h b/include/linux/minmax.h
index 24e4b372649a..6f7ea669d305 100644
--- a/include/linux/minmax.h
+++ b/include/linux/minmax.h
@@ -46,10 +46,10 @@
* comparison, and these expressions only need to be careful to not cause
* warnings for pointer use.
*/
-#define __signed_type_use(x, ux) (2 + __is_nonneg(x, ux))
-#define __unsigned_type_use(x, ux) (1 + 2 * (sizeof(ux) < 4))
-#define __sign_use(x, ux) (is_signed_type(typeof(ux)) ? \
- __signed_type_use(x, ux) : __unsigned_type_use(x, ux))
+#define __signed_type_use(ux) (2 + __is_nonneg(ux))
+#define __unsigned_type_use(ux) (1 + 2 * (sizeof(ux) < 4))
+#define __sign_use(ux) (is_signed_type(typeof(ux)) ? \
+ __signed_type_use(ux) : __unsigned_type_use(ux))
/*
* Check whether a signed value is always non-negative.
@@ -71,13 +71,13 @@
#else
#define __signed_type(ux) typeof(__builtin_choose_expr(sizeof(ux) > 4, 1LL, 1L))
#endif
-#define __is_nonneg(x, ux) statically_true((__signed_type(ux))(x) >= 0)
+#define __is_nonneg(ux) statically_true((__signed_type(ux))(ux) >= 0)
-#define __types_ok(x, y, ux, uy) \
- (__sign_use(x, ux) & __sign_use(y, uy))
+#define __types_ok(ux, uy) \
+ (__sign_use(ux) & __sign_use(uy))
-#define __types_ok3(x, y, z, ux, uy, uz) \
- (__sign_use(x, ux) & __sign_use(y, uy) & __sign_use(z, uz))
+#define __types_ok3(ux, uy, uz) \
+ (__sign_use(ux) & __sign_use(uy) & __sign_use(uz))
#define __cmp_op_min <
#define __cmp_op_max >
@@ -92,7 +92,7 @@
#define __careful_cmp_once(op, x, y, ux, uy) ({ \
__auto_type ux = (x); __auto_type uy = (y); \
- BUILD_BUG_ON_MSG(!__types_ok(x, y, ux, uy), \
+ BUILD_BUG_ON_MSG(!__types_ok(ux, uy), \
#op"("#x", "#y") signedness error"); \
__cmp(op, ux, uy); })
@@ -109,7 +109,7 @@
static_assert(__builtin_choose_expr(__is_constexpr((lo) > (hi)), \
(lo) <= (hi), true), \
"clamp() low limit " #lo " greater than high limit " #hi); \
- BUILD_BUG_ON_MSG(!__types_ok3(val, lo, hi, uval, ulo, uhi), \
+ BUILD_BUG_ON_MSG(!__types_ok3(uval, ulo, uhi), \
"clamp("#val", "#lo", "#hi") signedness error"); \
__clamp(uval, ulo, uhi); })
@@ -149,7 +149,7 @@
#define __careful_op3(op, x, y, z, ux, uy, uz) ({ \
__auto_type ux = (x); __auto_type uy = (y);__auto_type uz = (z);\
- BUILD_BUG_ON_MSG(!__types_ok3(x, y, z, ux, uy, uz), \
+ BUILD_BUG_ON_MSG(!__types_ok3(ux, uy, uz), \
#op"3("#x", "#y", "#z") signedness error"); \
__cmp(op, ux, __cmp(op, uy, uz)); })
--
2.47.3
^ permalink raw reply related
* [PATCH 12/15 6.6.y] minmax.h: use BUILD_BUG_ON_MSG() for the lo < hi test in clamp()
From: Eliav Farber @ 2025-09-22 10:32 UTC (permalink / raw)
To: richard, anton.ivanov, johannes, dave.hansen, luto, peterz, tglx,
mingo, bp, x86, hpa, tony.luck, qiuxu.zhuo, james.morse, mchehab,
rric, harry.wentland, sunpeng.li, Rodrigo.Siqueira,
alexander.deucher, christian.koenig, Xinhui.Pan, airlied, daniel,
evan.quan, maarten.lankhorst, mripard, tzimmermann, jdelvare,
linux, linus.walleij, dmitry.torokhov, wens, jernej.skrabec,
samuel, agk, snitzer, dm-devel, mailhol.vincent, wg, mkl, davem,
edumazet, kuba, pabeni, alexandre.torgue, joabreu,
mcoquelin.stm32, krzysztof.kozlowski, malattia, hdegoede,
ilpo.jarvinen, markgross, artur.paszkiewicz, jejb,
martin.petersen, sakari.ailus, gregkh, clm, josef, dsterba,
luc.vanoostenryck, rostedt, mhiramat, pmladek, andriy.shevchenko,
linux, senozhatsky, minchan, akpm, dsahern, shuah, keescook, wad,
farbere, David.Laight, arnd, linux-um, linux-kernel, linux-edac,
amd-gfx, dri-devel, linux-hwmon, linux-input, linux-arm-kernel,
linux-sunxi, linux-media, linux-can, netdev, linux-stm32,
platform-driver-x86, linux-scsi, linux-staging, linux-btrfs,
linux-sparse, linux-trace-kernel, linux-mm, linux-kselftest, bpf,
stable
Cc: Christoph Hellwig, Dan Carpenter, Jason A. Donenfeld, Jens Axboe,
Lorenzo Stoakes, Mateusz Guzik, Matthew Wilcox, Pedro Falcato
In-Reply-To: <20250922103241.16213-1-farbere@amazon.com>
From: David Laight <David.Laight@ACULAB.COM>
[ Upstream commit a5743f32baec4728711bbc01d6ac2b33d4c67040 ]
Use BUILD_BUG_ON_MSG(statically_true(ulo > uhi), ...) for the sanity check
of the bounds in clamp(). Gives better error coverage and one less
expansion of the arguments.
Link: https://lkml.kernel.org/r/34d53778977747f19cce2abb287bb3e6@AcuMS.aculab.com
Signed-off-by: David Laight <david.laight@aculab.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Arnd Bergmann <arnd@kernel.org>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Dan Carpenter <dan.carpenter@linaro.org>
Cc: Jason A. Donenfeld <Jason@zx2c4.com>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Cc: Mateusz Guzik <mjguzik@gmail.com>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Pedro Falcato <pedro.falcato@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Eliav Farber <farbere@amazon.com>
---
include/linux/minmax.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/include/linux/minmax.h b/include/linux/minmax.h
index 6f7ea669d305..91aa1b90c1bb 100644
--- a/include/linux/minmax.h
+++ b/include/linux/minmax.h
@@ -106,8 +106,7 @@
__auto_type uval = (val); \
__auto_type ulo = (lo); \
__auto_type uhi = (hi); \
- static_assert(__builtin_choose_expr(__is_constexpr((lo) > (hi)), \
- (lo) <= (hi), true), \
+ BUILD_BUG_ON_MSG(statically_true(ulo > uhi), \
"clamp() low limit " #lo " greater than high limit " #hi); \
BUILD_BUG_ON_MSG(!__types_ok3(uval, ulo, uhi), \
"clamp("#val", "#lo", "#hi") signedness error"); \
--
2.47.3
^ permalink raw reply related
* [PATCH 13/15 6.6.y] minmax.h: move all the clamp() definitions after the min/max() ones
From: Eliav Farber @ 2025-09-22 10:32 UTC (permalink / raw)
To: richard, anton.ivanov, johannes, dave.hansen, luto, peterz, tglx,
mingo, bp, x86, hpa, tony.luck, qiuxu.zhuo, james.morse, mchehab,
rric, harry.wentland, sunpeng.li, Rodrigo.Siqueira,
alexander.deucher, christian.koenig, Xinhui.Pan, airlied, daniel,
evan.quan, maarten.lankhorst, mripard, tzimmermann, jdelvare,
linux, linus.walleij, dmitry.torokhov, wens, jernej.skrabec,
samuel, agk, snitzer, dm-devel, mailhol.vincent, wg, mkl, davem,
edumazet, kuba, pabeni, alexandre.torgue, joabreu,
mcoquelin.stm32, krzysztof.kozlowski, malattia, hdegoede,
ilpo.jarvinen, markgross, artur.paszkiewicz, jejb,
martin.petersen, sakari.ailus, gregkh, clm, josef, dsterba,
luc.vanoostenryck, rostedt, mhiramat, pmladek, andriy.shevchenko,
linux, senozhatsky, minchan, akpm, dsahern, shuah, keescook, wad,
farbere, David.Laight, arnd, linux-um, linux-kernel, linux-edac,
amd-gfx, dri-devel, linux-hwmon, linux-input, linux-arm-kernel,
linux-sunxi, linux-media, linux-can, netdev, linux-stm32,
platform-driver-x86, linux-scsi, linux-staging, linux-btrfs,
linux-sparse, linux-trace-kernel, linux-mm, linux-kselftest, bpf,
stable
Cc: Christoph Hellwig, Dan Carpenter, Jason A. Donenfeld, Jens Axboe,
Lorenzo Stoakes, Mateusz Guzik, Matthew Wilcox, Pedro Falcato
In-Reply-To: <20250922103241.16213-1-farbere@amazon.com>
From: David Laight <David.Laight@ACULAB.COM>
[ Upstream commit c3939872ee4a6b8bdcd0e813c66823b31e6e26f7 ]
At some point the definitions for clamp() got added in the middle of the
ones for min() and max(). Re-order the definitions so they are more
sensibly grouped.
Link: https://lkml.kernel.org/r/8bb285818e4846469121c8abc3dfb6e2@AcuMS.aculab.com
Signed-off-by: David Laight <david.laight@aculab.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Arnd Bergmann <arnd@kernel.org>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Dan Carpenter <dan.carpenter@linaro.org>
Cc: Jason A. Donenfeld <Jason@zx2c4.com>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Cc: Mateusz Guzik <mjguzik@gmail.com>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Pedro Falcato <pedro.falcato@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Eliav Farber <farbere@amazon.com>
---
include/linux/minmax.h | 109 +++++++++++++++++++----------------------
1 file changed, 51 insertions(+), 58 deletions(-)
diff --git a/include/linux/minmax.h b/include/linux/minmax.h
index 91aa1b90c1bb..75fb7a6ad4c6 100644
--- a/include/linux/minmax.h
+++ b/include/linux/minmax.h
@@ -99,22 +99,6 @@
#define __careful_cmp(op, x, y) \
__careful_cmp_once(op, x, y, __UNIQUE_ID(x_), __UNIQUE_ID(y_))
-#define __clamp(val, lo, hi) \
- ((val) >= (hi) ? (hi) : ((val) <= (lo) ? (lo) : (val)))
-
-#define __clamp_once(val, lo, hi, uval, ulo, uhi) ({ \
- __auto_type uval = (val); \
- __auto_type ulo = (lo); \
- __auto_type uhi = (hi); \
- BUILD_BUG_ON_MSG(statically_true(ulo > uhi), \
- "clamp() low limit " #lo " greater than high limit " #hi); \
- BUILD_BUG_ON_MSG(!__types_ok3(uval, ulo, uhi), \
- "clamp("#val", "#lo", "#hi") signedness error"); \
- __clamp(uval, ulo, uhi); })
-
-#define __careful_clamp(val, lo, hi) \
- __clamp_once(val, lo, hi, __UNIQUE_ID(v_), __UNIQUE_ID(l_), __UNIQUE_ID(h_))
-
/**
* min - return minimum of two values of the same or compatible types
* @x: first value
@@ -170,6 +154,22 @@
#define max3(x, y, z) \
__careful_op3(max, x, y, z, __UNIQUE_ID(x_), __UNIQUE_ID(y_), __UNIQUE_ID(z_))
+/**
+ * min_t - return minimum of two values, using the specified type
+ * @type: data type to use
+ * @x: first value
+ * @y: second value
+ */
+#define min_t(type, x, y) __cmp_once(min, type, x, y)
+
+/**
+ * max_t - return maximum of two values, using the specified type
+ * @type: data type to use
+ * @x: first value
+ * @y: second value
+ */
+#define max_t(type, x, y) __cmp_once(max, type, x, y)
+
/**
* min_not_zero - return the minimum that is _not_ zero, unless both are zero
* @x: value1
@@ -180,6 +180,22 @@
typeof(y) __y = (y); \
__x == 0 ? __y : ((__y == 0) ? __x : min(__x, __y)); })
+#define __clamp(val, lo, hi) \
+ ((val) >= (hi) ? (hi) : ((val) <= (lo) ? (lo) : (val)))
+
+#define __clamp_once(val, lo, hi, uval, ulo, uhi) ({ \
+ __auto_type uval = (val); \
+ __auto_type ulo = (lo); \
+ __auto_type uhi = (hi); \
+ BUILD_BUG_ON_MSG(statically_true(ulo > uhi), \
+ "clamp() low limit " #lo " greater than high limit " #hi); \
+ BUILD_BUG_ON_MSG(!__types_ok3(uval, ulo, uhi), \
+ "clamp("#val", "#lo", "#hi") signedness error"); \
+ __clamp(uval, ulo, uhi); })
+
+#define __careful_clamp(val, lo, hi) \
+ __clamp_once(val, lo, hi, __UNIQUE_ID(v_), __UNIQUE_ID(l_), __UNIQUE_ID(h_))
+
/**
* clamp - return a value clamped to a given range with strict typechecking
* @val: current value
@@ -191,28 +207,30 @@
*/
#define clamp(val, lo, hi) __careful_clamp(val, lo, hi)
-/*
- * ..and if you can't take the strict
- * types, you can specify one yourself.
- *
- * Or not use min/max/clamp at all, of course.
- */
-
/**
- * min_t - return minimum of two values, using the specified type
- * @type: data type to use
- * @x: first value
- * @y: second value
+ * clamp_t - return a value clamped to a given range using a given type
+ * @type: the type of variable to use
+ * @val: current value
+ * @lo: minimum allowable value
+ * @hi: maximum allowable value
+ *
+ * This macro does no typechecking and uses temporary variables of type
+ * @type to make all the comparisons.
*/
-#define min_t(type, x, y) __cmp_once(min, type, x, y)
+#define clamp_t(type, val, lo, hi) __careful_clamp((type)(val), (type)(lo), (type)(hi))
/**
- * max_t - return maximum of two values, using the specified type
- * @type: data type to use
- * @x: first value
- * @y: second value
+ * clamp_val - return a value clamped to a given range using val's type
+ * @val: current value
+ * @lo: minimum allowable value
+ * @hi: maximum allowable value
+ *
+ * This macro does no typechecking and uses temporary variables of whatever
+ * type the input argument @val is. This is useful when @val is an unsigned
+ * type and @lo and @hi are literals that will otherwise be assigned a signed
+ * integer type.
*/
-#define max_t(type, x, y) __cmp_once(max, type, x, y)
+#define clamp_val(val, lo, hi) clamp_t(typeof(val), val, lo, hi)
/*
* Do not check the array parameter using __must_be_array().
@@ -257,31 +275,6 @@
*/
#define max_array(array, len) __minmax_array(max, array, len)
-/**
- * clamp_t - return a value clamped to a given range using a given type
- * @type: the type of variable to use
- * @val: current value
- * @lo: minimum allowable value
- * @hi: maximum allowable value
- *
- * This macro does no typechecking and uses temporary variables of type
- * @type to make all the comparisons.
- */
-#define clamp_t(type, val, lo, hi) __careful_clamp((type)(val), (type)(lo), (type)(hi))
-
-/**
- * clamp_val - return a value clamped to a given range using val's type
- * @val: current value
- * @lo: minimum allowable value
- * @hi: maximum allowable value
- *
- * This macro does no typechecking and uses temporary variables of whatever
- * type the input argument @val is. This is useful when @val is an unsigned
- * type and @lo and @hi are literals that will otherwise be assigned a signed
- * integer type.
- */
-#define clamp_val(val, lo, hi) clamp_t(typeof(val), val, lo, hi)
-
static inline bool in_range64(u64 val, u64 start, u64 len)
{
return (val - start) < len;
--
2.47.3
^ permalink raw reply related
* [PATCH 14/15 6.6.y] minmax.h: simplify the variants of clamp()
From: Eliav Farber @ 2025-09-22 10:32 UTC (permalink / raw)
To: richard, anton.ivanov, johannes, dave.hansen, luto, peterz, tglx,
mingo, bp, x86, hpa, tony.luck, qiuxu.zhuo, james.morse, mchehab,
rric, harry.wentland, sunpeng.li, Rodrigo.Siqueira,
alexander.deucher, christian.koenig, Xinhui.Pan, airlied, daniel,
evan.quan, maarten.lankhorst, mripard, tzimmermann, jdelvare,
linux, linus.walleij, dmitry.torokhov, wens, jernej.skrabec,
samuel, agk, snitzer, dm-devel, mailhol.vincent, wg, mkl, davem,
edumazet, kuba, pabeni, alexandre.torgue, joabreu,
mcoquelin.stm32, krzysztof.kozlowski, malattia, hdegoede,
ilpo.jarvinen, markgross, artur.paszkiewicz, jejb,
martin.petersen, sakari.ailus, gregkh, clm, josef, dsterba,
luc.vanoostenryck, rostedt, mhiramat, pmladek, andriy.shevchenko,
linux, senozhatsky, minchan, akpm, dsahern, shuah, keescook, wad,
farbere, David.Laight, arnd, linux-um, linux-kernel, linux-edac,
amd-gfx, dri-devel, linux-hwmon, linux-input, linux-arm-kernel,
linux-sunxi, linux-media, linux-can, netdev, linux-stm32,
platform-driver-x86, linux-scsi, linux-staging, linux-btrfs,
linux-sparse, linux-trace-kernel, linux-mm, linux-kselftest, bpf,
stable
Cc: Christoph Hellwig, Dan Carpenter, Jason A. Donenfeld, Jens Axboe,
Lorenzo Stoakes, Mateusz Guzik, Matthew Wilcox, Pedro Falcato
In-Reply-To: <20250922103241.16213-1-farbere@amazon.com>
From: David Laight <David.Laight@ACULAB.COM>
[ Upstream commit 495bba17cdf95e9703af1b8ef773c55ef0dfe703 ]
Always pass a 'type' through to __clamp_once(), pass '__auto_type' from
clamp() itself.
The expansion of __types_ok3() is reasonable so it isn't worth the added
complexity of avoiding it when a fixed type is used for all three values.
Link: https://lkml.kernel.org/r/8f69f4deac014f558bab186444bac2e8@AcuMS.aculab.com
Signed-off-by: David Laight <david.laight@aculab.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Arnd Bergmann <arnd@kernel.org>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Dan Carpenter <dan.carpenter@linaro.org>
Cc: Jason A. Donenfeld <Jason@zx2c4.com>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Cc: Mateusz Guzik <mjguzik@gmail.com>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Pedro Falcato <pedro.falcato@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Eliav Farber <farbere@amazon.com>
---
include/linux/minmax.h | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/include/linux/minmax.h b/include/linux/minmax.h
index 75fb7a6ad4c6..2bbdd5b5e07e 100644
--- a/include/linux/minmax.h
+++ b/include/linux/minmax.h
@@ -183,29 +183,29 @@
#define __clamp(val, lo, hi) \
((val) >= (hi) ? (hi) : ((val) <= (lo) ? (lo) : (val)))
-#define __clamp_once(val, lo, hi, uval, ulo, uhi) ({ \
- __auto_type uval = (val); \
- __auto_type ulo = (lo); \
- __auto_type uhi = (hi); \
+#define __clamp_once(type, val, lo, hi, uval, ulo, uhi) ({ \
+ type uval = (val); \
+ type ulo = (lo); \
+ type uhi = (hi); \
BUILD_BUG_ON_MSG(statically_true(ulo > uhi), \
"clamp() low limit " #lo " greater than high limit " #hi); \
BUILD_BUG_ON_MSG(!__types_ok3(uval, ulo, uhi), \
"clamp("#val", "#lo", "#hi") signedness error"); \
__clamp(uval, ulo, uhi); })
-#define __careful_clamp(val, lo, hi) \
- __clamp_once(val, lo, hi, __UNIQUE_ID(v_), __UNIQUE_ID(l_), __UNIQUE_ID(h_))
+#define __careful_clamp(type, val, lo, hi) \
+ __clamp_once(type, val, lo, hi, __UNIQUE_ID(v_), __UNIQUE_ID(l_), __UNIQUE_ID(h_))
/**
- * clamp - return a value clamped to a given range with strict typechecking
+ * clamp - return a value clamped to a given range with typechecking
* @val: current value
* @lo: lowest allowable value
* @hi: highest allowable value
*
- * This macro does strict typechecking of @lo/@hi to make sure they are of the
- * same type as @val. See the unnecessary pointer comparisons.
+ * This macro checks @val/@lo/@hi to make sure they have compatible
+ * signedness.
*/
-#define clamp(val, lo, hi) __careful_clamp(val, lo, hi)
+#define clamp(val, lo, hi) __careful_clamp(__auto_type, val, lo, hi)
/**
* clamp_t - return a value clamped to a given range using a given type
@@ -217,7 +217,7 @@
* This macro does no typechecking and uses temporary variables of type
* @type to make all the comparisons.
*/
-#define clamp_t(type, val, lo, hi) __careful_clamp((type)(val), (type)(lo), (type)(hi))
+#define clamp_t(type, val, lo, hi) __careful_clamp(type, val, lo, hi)
/**
* clamp_val - return a value clamped to a given range using val's type
@@ -230,7 +230,7 @@
* type and @lo and @hi are literals that will otherwise be assigned a signed
* integer type.
*/
-#define clamp_val(val, lo, hi) clamp_t(typeof(val), val, lo, hi)
+#define clamp_val(val, lo, hi) __careful_clamp(typeof(val), val, lo, hi)
/*
* Do not check the array parameter using __must_be_array().
--
2.47.3
^ permalink raw reply related
* [PATCH 15/15 6.6.y] minmax.h: remove some #defines that are only expanded once
From: Eliav Farber @ 2025-09-22 10:32 UTC (permalink / raw)
To: richard, anton.ivanov, johannes, dave.hansen, luto, peterz, tglx,
mingo, bp, x86, hpa, tony.luck, qiuxu.zhuo, james.morse, mchehab,
rric, harry.wentland, sunpeng.li, Rodrigo.Siqueira,
alexander.deucher, christian.koenig, Xinhui.Pan, airlied, daniel,
evan.quan, maarten.lankhorst, mripard, tzimmermann, jdelvare,
linux, linus.walleij, dmitry.torokhov, wens, jernej.skrabec,
samuel, agk, snitzer, dm-devel, mailhol.vincent, wg, mkl, davem,
edumazet, kuba, pabeni, alexandre.torgue, joabreu,
mcoquelin.stm32, krzysztof.kozlowski, malattia, hdegoede,
ilpo.jarvinen, markgross, artur.paszkiewicz, jejb,
martin.petersen, sakari.ailus, gregkh, clm, josef, dsterba,
luc.vanoostenryck, rostedt, mhiramat, pmladek, andriy.shevchenko,
linux, senozhatsky, minchan, akpm, dsahern, shuah, keescook, wad,
farbere, David.Laight, arnd, linux-um, linux-kernel, linux-edac,
amd-gfx, dri-devel, linux-hwmon, linux-input, linux-arm-kernel,
linux-sunxi, linux-media, linux-can, netdev, linux-stm32,
platform-driver-x86, linux-scsi, linux-staging, linux-btrfs,
linux-sparse, linux-trace-kernel, linux-mm, linux-kselftest, bpf,
stable
Cc: Christoph Hellwig, Dan Carpenter, Jason A. Donenfeld, Jens Axboe,
Lorenzo Stoakes, Mateusz Guzik, Matthew Wilcox, Pedro Falcato
In-Reply-To: <20250922103241.16213-1-farbere@amazon.com>
From: David Laight <David.Laight@ACULAB.COM>
[ Upstream commit 2b97aaf74ed534fb838d09867d09a3ca5d795208 ]
The bodies of __signed_type_use() and __unsigned_type_use() are much the
same size as their names - so put the bodies in the only line that expands
them.
Similarly __signed_type() is defined separately for 64bit and then used
exactly once just below.
Change the test for __signed_type from CONFIG_64BIT to one based on gcc
defined macros so that the code is valid if it gets used outside of a
kernel build.
Link: https://lkml.kernel.org/r/9386d1ebb8974fbabbed2635160c3975@AcuMS.aculab.com
Signed-off-by: David Laight <david.laight@aculab.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Arnd Bergmann <arnd@kernel.org>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Dan Carpenter <dan.carpenter@linaro.org>
Cc: Jason A. Donenfeld <Jason@zx2c4.com>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Cc: Mateusz Guzik <mjguzik@gmail.com>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Pedro Falcato <pedro.falcato@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Eliav Farber <farbere@amazon.com>
---
include/linux/minmax.h | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/include/linux/minmax.h b/include/linux/minmax.h
index 2bbdd5b5e07e..eaaf5c008e4d 100644
--- a/include/linux/minmax.h
+++ b/include/linux/minmax.h
@@ -46,10 +46,8 @@
* comparison, and these expressions only need to be careful to not cause
* warnings for pointer use.
*/
-#define __signed_type_use(ux) (2 + __is_nonneg(ux))
-#define __unsigned_type_use(ux) (1 + 2 * (sizeof(ux) < 4))
#define __sign_use(ux) (is_signed_type(typeof(ux)) ? \
- __signed_type_use(ux) : __unsigned_type_use(ux))
+ (2 + __is_nonneg(ux)) : (1 + 2 * (sizeof(ux) < 4)))
/*
* Check whether a signed value is always non-negative.
@@ -57,7 +55,7 @@
* A cast is needed to avoid any warnings from values that aren't signed
* integer types (in which case the result doesn't matter).
*
- * On 64-bit any integer or pointer type can safely be cast to 'long'.
+ * On 64-bit any integer or pointer type can safely be cast to 'long long'.
* But on 32-bit we need to avoid warnings about casting pointers to integers
* of different sizes without truncating 64-bit values so 'long' or 'long long'
* must be used depending on the size of the value.
@@ -66,12 +64,12 @@
* them, but we do not use s128 types in the kernel (we do use 'u128',
* but they are handled by the !is_signed_type() case).
*/
-#ifdef CONFIG_64BIT
- #define __signed_type(ux) long
+#if __SIZEOF_POINTER__ == __SIZEOF_LONG_LONG__
+#define __is_nonneg(ux) statically_true((long long)(ux) >= 0)
#else
- #define __signed_type(ux) typeof(__builtin_choose_expr(sizeof(ux) > 4, 1LL, 1L))
+#define __is_nonneg(ux) statically_true( \
+ (typeof(__builtin_choose_expr(sizeof(ux) > 4, 1LL, 1L)))(ux) >= 0)
#endif
-#define __is_nonneg(ux) statically_true((__signed_type(ux))(ux) >= 0)
#define __types_ok(ux, uy) \
(__sign_use(ux) & __sign_use(uy))
--
2.47.3
^ permalink raw reply related
* RE: [PATCH 00/27 5.10.y] Backport minmax.h updates from v6.17-rc6
From: Farber, Eliav @ 2025-09-22 10:45 UTC (permalink / raw)
To: Greg KH
Cc: linux@armlinux.org.uk, jdike@addtoit.com, richard@nod.at,
anton.ivanov@cambridgegreys.com, dave.hansen@linux.intel.com,
luto@kernel.org, peterz@infradead.org, tglx@linutronix.de,
mingo@redhat.com, bp@alien8.de, x86@kernel.org, hpa@zytor.com,
tony.luck@intel.com, qiuxu.zhuo@intel.com, mchehab@kernel.org,
james.morse@arm.com, rric@kernel.org, harry.wentland@amd.com,
sunpeng.li@amd.com, alexander.deucher@amd.com,
christian.koenig@amd.com, airlied@linux.ie, daniel@ffwll.ch,
evan.quan@amd.com, james.qian.wang@arm.com, liviu.dudau@arm.com,
mihail.atanassov@arm.com, brian.starkey@arm.com,
maarten.lankhorst@linux.intel.com, mripard@kernel.org,
tzimmermann@suse.de, robdclark@gmail.com, sean@poorly.run,
jdelvare@suse.com, linux@roeck-us.net, fery@cypress.com,
dmitry.torokhov@gmail.com, agk@redhat.com, snitzer@redhat.com,
dm-devel@redhat.com, rajur@chelsio.com, davem@davemloft.net,
kuba@kernel.org, peppe.cavallaro@st.com, alexandre.torgue@st.com,
joabreu@synopsys.com, mcoquelin.stm32@gmail.com,
malattia@linux.it, hdegoede@redhat.com, mgross@linux.intel.com,
intel-linux-scu@intel.com, artur.paszkiewicz@intel.com,
jejb@linux.ibm.com, martin.petersen@oracle.com,
sakari.ailus@linux.intel.com, clm@fb.com, josef@toxicpanda.com,
dsterba@suse.com, jack@suse.com, tytso@mit.edu,
adilger.kernel@dilger.ca, dushistov@mail.ru,
luc.vanoostenryck@gmail.com, rostedt@goodmis.org,
pmladek@suse.com, sergey.senozhatsky@gmail.com,
andriy.shevchenko@linux.intel.com, linux@rasmusvillemoes.dk,
minchan@kernel.org, ngupta@vflare.org, akpm@linux-foundation.org,
kuznet@ms2.inr.ac.ru, yoshfuji@linux-ipv6.org,
pablo@netfilter.org, kadlec@netfilter.org, fw@strlen.de,
jmaloy@redhat.com, ying.xue@windriver.com, willy@infradead.org,
sashal@kernel.org, ruanjinjie@huawei.com, David.Laight@aculab.com,
herve.codina@bootlin.com, Jason@zx2c4.com, bvanassche@acm.org,
keescook@chromium.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-um@lists.infradead.org,
linux-edac@vger.kernel.org, amd-gfx@lists.freedesktop.org,
dri-devel@lists.freedesktop.org, linux-arm-msm@vger.kernel.org,
freedreno@lists.freedesktop.org, linux-hwmon@vger.kernel.org,
linux-input@vger.kernel.org, linux-media@vger.kernel.org,
netdev@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com,
platform-driver-x86@vger.kernel.org, linux-scsi@vger.kernel.org,
linux-staging@lists.linux.dev, linux-btrfs@vger.kernel.org,
linux-ext4@vger.kernel.org, linux-sparse@vger.kernel.org,
linux-mm@kvack.org, netfilter-devel@vger.kernel.org,
coreteam@netfilter.org, tipc-discussion@lists.sourceforge.net,
stable@vger.kernel.org, Chocron, Jonathan
In-Reply-To: <2025092203-untreated-sloppily-23b5@gregkh>
> On Sun, Sep 21, 2025 at 09:37:02PM +0000, Farber, Eliav wrote:
> > > On Fri, Sep 19, 2025 at 10:17:00AM +0000, Eliav Farber wrote:
> > > > This series includes a total of 27 patches, to align minmax.h of
> > > > v5.15.y with v6.17-rc6.
> > > >
> > > > The set consists of 24 commits that directly update minmax.h:
> > > > 1) 92d23c6e9415 ("overflow, tracing: Define the is_signed_type() macro
> > > > once")
> > >
> > > But this isn't in 5.15.y, so how is this syncing things up?
> > >
> > > I'm all for this, but I got confused here, at the first commit :)
> >
> > It's a typo.
> > It should be 5.10.y and not 5.15.y.
> >
> > > Some of these are also only in newer kernels, which, as you know, is
> > > generally a bad thing (i.e. I can't take patches only for older
> > > kernels.)
> > >
> > > I want these changes, as they are great, but can you perhaps provide
> > > patch series for newer kernels first so that I can then take these?
> >
> > So you'd first like first to align 6.16 with 6.17, then 6.15 with
> > 6.16, then 6.12 with 6.15, then 6.6 with 6.12, and so on until we
> > eventually align 5.10 and even 5.4?
>
> Yes please!
Stable 6.16.8 didn't require any changs.
I pulled the changes for 6.12.48:
https://lore.kernel.org/stable/20250922103123.14538-1-farbere@amazon.com/T/#t
and 6.6.107:
https://lore.kernel.org/stable/20250922103241.16213-1-farbere@amazon.com/T/#t
Once approved, I'll continue with other longterm branches.
---
Regards, Eliav
^ permalink raw reply
* Re: [PATCH 04/27 5.10.y] minmax: add in_range() macro
From: Liviu Dudau @ 2025-09-22 13:10 UTC (permalink / raw)
To: Eliav Farber
Cc: linux, jdike, richard, anton.ivanov, dave.hansen, luto, peterz,
tglx, mingo, bp, x86, hpa, tony.luck, qiuxu.zhuo, mchehab,
james.morse, rric, harry.wentland, sunpeng.li, alexander.deucher,
christian.koenig, airlied, daniel, evan.quan, james.qian.wang,
mihail.atanassov, brian.starkey, maarten.lankhorst, mripard,
tzimmermann, robdclark, sean, jdelvare, linux, fery,
dmitry.torokhov, agk, snitzer, dm-devel, rajur, davem, kuba,
peppe.cavallaro, alexandre.torgue, joabreu, mcoquelin.stm32,
malattia, hdegoede, mgross, intel-linux-scu, artur.paszkiewicz,
jejb, martin.petersen, sakari.ailus, gregkh, clm, josef, dsterba,
jack, tytso, adilger.kernel, dushistov, luc.vanoostenryck,
rostedt, pmladek, sergey.senozhatsky, andriy.shevchenko, linux,
minchan, ngupta, akpm, kuznet, yoshfuji, pablo, kadlec, fw,
jmaloy, ying.xue, willy, sashal, ruanjinjie, David.Laight,
herve.codina, Jason, bvanassche, keescook, linux-arm-kernel,
linux-kernel, linux-um, linux-edac, amd-gfx, dri-devel,
linux-arm-msm, freedreno, linux-hwmon, linux-input, linux-media,
netdev, linux-stm32, platform-driver-x86, linux-scsi,
linux-staging, linux-btrfs, linux-ext4, linux-sparse, linux-mm,
netfilter-devel, coreteam, tipc-discussion, stable, jonnyc
In-Reply-To: <20250919101727.16152-5-farbere@amazon.com>
On Fri, Sep 19, 2025 at 10:17:04AM +0000, Eliav Farber wrote:
> From: "Matthew Wilcox (Oracle)" <willy@infradead.org>
>
> [ Upstream commit f9bff0e31881d03badf191d3b0005839391f5f2b ]
>
> Patch series "New page table range API", v6.
>
> This patchset changes the API used by the MM to set up page table entries.
> The four APIs are:
>
> set_ptes(mm, addr, ptep, pte, nr)
> update_mmu_cache_range(vma, addr, ptep, nr)
> flush_dcache_folio(folio)
> flush_icache_pages(vma, page, nr)
>
> flush_dcache_folio() isn't technically new, but no architecture
> implemented it, so I've done that for them. The old APIs remain around
> but are mostly implemented by calling the new interfaces.
>
> The new APIs are based around setting up N page table entries at once.
> The N entries belong to the same PMD, the same folio and the same VMA, so
> ptep++ is a legitimate operation, and locking is taken care of for you.
> Some architectures can do a better job of it than just a loop, but I have
> hesitated to make too deep a change to architectures I don't understand
> well.
>
> One thing I have changed in every architecture is that PG_arch_1 is now a
> per-folio bit instead of a per-page bit when used for dcache clean/dirty
> tracking. This was something that would have to happen eventually, and it
> makes sense to do it now rather than iterate over every page involved in a
> cache flush and figure out if it needs to happen.
>
> The point of all this is better performance, and Fengwei Yin has measured
> improvement on x86. I suspect you'll see improvement on your architecture
> too. Try the new will-it-scale test mentioned here:
> https://lore.kernel.org/linux-mm/20230206140639.538867-5-fengwei.yin@intel.com/
> You'll need to run it on an XFS filesystem and have
> CONFIG_TRANSPARENT_HUGEPAGE set.
>
> This patchset is the basis for much of the anonymous large folio work
> being done by Ryan, so it's received quite a lot of testing over the last
> few months.
>
> This patch (of 38):
>
> Determine if a value lies within a range more efficiently (subtraction +
> comparison vs two comparisons and an AND). It also has useful (under some
> circumstances) behaviour if the range exceeds the maximum value of the
> type. Convert all the conflicting definitions of in_range() within the
> kernel; some can use the generic definition while others need their own
> definition.
>
> Link: https://lkml.kernel.org/r/20230802151406.3735276-1-willy@infradead.org
> Link: https://lkml.kernel.org/r/20230802151406.3735276-2-willy@infradead.org
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Eliav Farber <farbere@amazon.com>
> ---
> arch/arm/mm/pageattr.c | 6 ++---
> .../drm/arm/display/include/malidp_utils.h | 2 +-
> .../display/komeda/komeda_pipeline_state.c | 24 ++++++++---------
For the malidp and komeda changes:
Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
Best regards,
Liviu
> drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 6 -----
> .../net/ethernet/chelsio/cxgb3/cxgb3_main.c | 18 ++++++-------
> fs/btrfs/misc.h | 2 --
> fs/ext2/balloc.c | 2 --
> fs/ext4/ext4.h | 2 --
> fs/ufs/util.h | 6 -----
> include/linux/minmax.h | 27 +++++++++++++++++++
> lib/logic_pio.c | 3 ---
> net/netfilter/nf_nat_core.c | 6 ++---
> net/tipc/core.h | 2 +-
> net/tipc/link.c | 10 +++----
> 14 files changed, 61 insertions(+), 55 deletions(-)
>
> diff --git a/arch/arm/mm/pageattr.c b/arch/arm/mm/pageattr.c
> index 9790ae3a8c68..3b3bfa825fad 100644
> --- a/arch/arm/mm/pageattr.c
> +++ b/arch/arm/mm/pageattr.c
> @@ -25,7 +25,7 @@ static int change_page_range(pte_t *ptep, unsigned long addr, void *data)
> return 0;
> }
>
> -static bool in_range(unsigned long start, unsigned long size,
> +static bool range_in_range(unsigned long start, unsigned long size,
> unsigned long range_start, unsigned long range_end)
> {
> return start >= range_start && start < range_end &&
> @@ -46,8 +46,8 @@ static int change_memory_common(unsigned long addr, int numpages,
> if (!size)
> return 0;
>
> - if (!in_range(start, size, MODULES_VADDR, MODULES_END) &&
> - !in_range(start, size, VMALLOC_START, VMALLOC_END))
> + if (!range_in_range(start, size, MODULES_VADDR, MODULES_END) &&
> + !range_in_range(start, size, VMALLOC_START, VMALLOC_END))
> return -EINVAL;
>
> data.set_mask = set_mask;
> diff --git a/drivers/gpu/drm/arm/display/include/malidp_utils.h b/drivers/gpu/drm/arm/display/include/malidp_utils.h
> index 49a1d7f3539c..9f83baac6ed8 100644
> --- a/drivers/gpu/drm/arm/display/include/malidp_utils.h
> +++ b/drivers/gpu/drm/arm/display/include/malidp_utils.h
> @@ -35,7 +35,7 @@ static inline void set_range(struct malidp_range *rg, u32 start, u32 end)
> rg->end = end;
> }
>
> -static inline bool in_range(struct malidp_range *rg, u32 v)
> +static inline bool malidp_in_range(struct malidp_range *rg, u32 v)
> {
> return (v >= rg->start) && (v <= rg->end);
> }
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> index 7cc891c091f8..3e414d2fbdda 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> @@ -305,12 +305,12 @@ komeda_layer_check_cfg(struct komeda_layer *layer,
> if (komeda_fb_check_src_coords(kfb, src_x, src_y, src_w, src_h))
> return -EINVAL;
>
> - if (!in_range(&layer->hsize_in, src_w)) {
> + if (!malidp_in_range(&layer->hsize_in, src_w)) {
> DRM_DEBUG_ATOMIC("invalidate src_w %d.\n", src_w);
> return -EINVAL;
> }
>
> - if (!in_range(&layer->vsize_in, src_h)) {
> + if (!malidp_in_range(&layer->vsize_in, src_h)) {
> DRM_DEBUG_ATOMIC("invalidate src_h %d.\n", src_h);
> return -EINVAL;
> }
> @@ -452,14 +452,14 @@ komeda_scaler_check_cfg(struct komeda_scaler *scaler,
> hsize_out = dflow->out_w;
> vsize_out = dflow->out_h;
>
> - if (!in_range(&scaler->hsize, hsize_in) ||
> - !in_range(&scaler->hsize, hsize_out)) {
> + if (!malidp_in_range(&scaler->hsize, hsize_in) ||
> + !malidp_in_range(&scaler->hsize, hsize_out)) {
> DRM_DEBUG_ATOMIC("Invalid horizontal sizes");
> return -EINVAL;
> }
>
> - if (!in_range(&scaler->vsize, vsize_in) ||
> - !in_range(&scaler->vsize, vsize_out)) {
> + if (!malidp_in_range(&scaler->vsize, vsize_in) ||
> + !malidp_in_range(&scaler->vsize, vsize_out)) {
> DRM_DEBUG_ATOMIC("Invalid vertical sizes");
> return -EINVAL;
> }
> @@ -574,13 +574,13 @@ komeda_splitter_validate(struct komeda_splitter *splitter,
> return -EINVAL;
> }
>
> - if (!in_range(&splitter->hsize, dflow->in_w)) {
> + if (!malidp_in_range(&splitter->hsize, dflow->in_w)) {
> DRM_DEBUG_ATOMIC("split in_w:%d is out of the acceptable range.\n",
> dflow->in_w);
> return -EINVAL;
> }
>
> - if (!in_range(&splitter->vsize, dflow->in_h)) {
> + if (!malidp_in_range(&splitter->vsize, dflow->in_h)) {
> DRM_DEBUG_ATOMIC("split in_h: %d exceeds the acceptable range.\n",
> dflow->in_h);
> return -EINVAL;
> @@ -624,13 +624,13 @@ komeda_merger_validate(struct komeda_merger *merger,
> return -EINVAL;
> }
>
> - if (!in_range(&merger->hsize_merged, output->out_w)) {
> + if (!malidp_in_range(&merger->hsize_merged, output->out_w)) {
> DRM_DEBUG_ATOMIC("merged_w: %d is out of the accepted range.\n",
> output->out_w);
> return -EINVAL;
> }
>
> - if (!in_range(&merger->vsize_merged, output->out_h)) {
> + if (!malidp_in_range(&merger->vsize_merged, output->out_h)) {
> DRM_DEBUG_ATOMIC("merged_h: %d is out of the accepted range.\n",
> output->out_h);
> return -EINVAL;
> @@ -866,8 +866,8 @@ void komeda_complete_data_flow_cfg(struct komeda_layer *layer,
> * input/output range.
> */
> if (dflow->en_scaling && scaler)
> - dflow->en_split = !in_range(&scaler->hsize, dflow->in_w) ||
> - !in_range(&scaler->hsize, dflow->out_w);
> + dflow->en_split = !malidp_in_range(&scaler->hsize, dflow->in_w) ||
> + !malidp_in_range(&scaler->hsize, dflow->out_w);
> }
>
> static bool merger_is_available(struct komeda_pipeline *pipe,
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> index 655938df4531..f11da95566da 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> @@ -657,12 +657,6 @@ struct block_header {
> u32 data[];
> };
>
> -/* this should be a general kernel helper */
> -static int in_range(u32 addr, u32 start, u32 size)
> -{
> - return addr >= start && addr < start + size;
> -}
> -
> static bool fw_block_mem(struct a6xx_gmu_bo *bo, const struct block_header *blk)
> {
> if (!in_range(blk->addr, bo->iova, bo->size))
> diff --git a/drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c b/drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c
> index 8a167eea288c..10790a370f22 100644
> --- a/drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c
> +++ b/drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c
> @@ -2131,7 +2131,7 @@ static const struct ethtool_ops cxgb_ethtool_ops = {
> .set_link_ksettings = set_link_ksettings,
> };
>
> -static int in_range(int val, int lo, int hi)
> +static int cxgb_in_range(int val, int lo, int hi)
> {
> return val < 0 || (val <= hi && val >= lo);
> }
> @@ -2162,19 +2162,19 @@ static int cxgb_extension_ioctl(struct net_device *dev, void __user *useraddr)
> return -EINVAL;
> if (t.qset_idx >= SGE_QSETS)
> return -EINVAL;
> - if (!in_range(t.intr_lat, 0, M_NEWTIMER) ||
> - !in_range(t.cong_thres, 0, 255) ||
> - !in_range(t.txq_size[0], MIN_TXQ_ENTRIES,
> + if (!cxgb_in_range(t.intr_lat, 0, M_NEWTIMER) ||
> + !cxgb_in_range(t.cong_thres, 0, 255) ||
> + !cxgb_in_range(t.txq_size[0], MIN_TXQ_ENTRIES,
> MAX_TXQ_ENTRIES) ||
> - !in_range(t.txq_size[1], MIN_TXQ_ENTRIES,
> + !cxgb_in_range(t.txq_size[1], MIN_TXQ_ENTRIES,
> MAX_TXQ_ENTRIES) ||
> - !in_range(t.txq_size[2], MIN_CTRL_TXQ_ENTRIES,
> + !cxgb_in_range(t.txq_size[2], MIN_CTRL_TXQ_ENTRIES,
> MAX_CTRL_TXQ_ENTRIES) ||
> - !in_range(t.fl_size[0], MIN_FL_ENTRIES,
> + !cxgb_in_range(t.fl_size[0], MIN_FL_ENTRIES,
> MAX_RX_BUFFERS) ||
> - !in_range(t.fl_size[1], MIN_FL_ENTRIES,
> + !cxgb_in_range(t.fl_size[1], MIN_FL_ENTRIES,
> MAX_RX_JUMBO_BUFFERS) ||
> - !in_range(t.rspq_size, MIN_RSPQ_ENTRIES,
> + !cxgb_in_range(t.rspq_size, MIN_RSPQ_ENTRIES,
> MAX_RSPQ_ENTRIES))
> return -EINVAL;
>
> diff --git a/fs/btrfs/misc.h b/fs/btrfs/misc.h
> index 6461ebc3a1c1..40ad75511435 100644
> --- a/fs/btrfs/misc.h
> +++ b/fs/btrfs/misc.h
> @@ -8,8 +8,6 @@
> #include <asm/div64.h>
> #include <linux/rbtree.h>
>
> -#define in_range(b, first, len) ((b) >= (first) && (b) < (first) + (len))
> -
> static inline void cond_wake_up(struct wait_queue_head *wq)
> {
> /*
> diff --git a/fs/ext2/balloc.c b/fs/ext2/balloc.c
> index 9bf086821eb3..1d9380c5523b 100644
> --- a/fs/ext2/balloc.c
> +++ b/fs/ext2/balloc.c
> @@ -36,8 +36,6 @@
> */
>
>
> -#define in_range(b, first, len) ((b) >= (first) && (b) <= (first) + (len) - 1)
> -
> struct ext2_group_desc * ext2_get_group_desc(struct super_block * sb,
> unsigned int block_group,
> struct buffer_head ** bh)
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 1dc1292d8977..4adaf97d7435 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -3659,8 +3659,6 @@ static inline void set_bitmap_uptodate(struct buffer_head *bh)
> set_bit(BH_BITMAP_UPTODATE, &(bh)->b_state);
> }
>
> -#define in_range(b, first, len) ((b) >= (first) && (b) <= (first) + (len) - 1)
> -
> /* For ioend & aio unwritten conversion wait queues */
> #define EXT4_WQ_HASH_SZ 37
> #define ext4_ioend_wq(v) (&ext4__ioend_wq[((unsigned long)(v)) %\
> diff --git a/fs/ufs/util.h b/fs/ufs/util.h
> index 4931bec1a01c..89247193d96d 100644
> --- a/fs/ufs/util.h
> +++ b/fs/ufs/util.h
> @@ -11,12 +11,6 @@
> #include <linux/fs.h>
> #include "swab.h"
>
> -
> -/*
> - * some useful macros
> - */
> -#define in_range(b,first,len) ((b)>=(first)&&(b)<(first)+(len))
> -
> /*
> * functions used for retyping
> */
> diff --git a/include/linux/minmax.h b/include/linux/minmax.h
> index abdeae409dad..7affadcb2a29 100644
> --- a/include/linux/minmax.h
> +++ b/include/linux/minmax.h
> @@ -3,6 +3,7 @@
> #define _LINUX_MINMAX_H
>
> #include <linux/const.h>
> +#include <linux/types.h>
>
> /*
> * min()/max()/clamp() macros must accomplish three things:
> @@ -175,6 +176,32 @@
> */
> #define clamp_val(val, lo, hi) clamp_t(typeof(val), val, lo, hi)
>
> +static inline bool in_range64(u64 val, u64 start, u64 len)
> +{
> + return (val - start) < len;
> +}
> +
> +static inline bool in_range32(u32 val, u32 start, u32 len)
> +{
> + return (val - start) < len;
> +}
> +
> +/**
> + * in_range - Determine if a value lies within a range.
> + * @val: Value to test.
> + * @start: First value in range.
> + * @len: Number of values in range.
> + *
> + * This is more efficient than "if (start <= val && val < (start + len))".
> + * It also gives a different answer if @start + @len overflows the size of
> + * the type by a sufficient amount to encompass @val. Decide for yourself
> + * which behaviour you want, or prove that start + len never overflow.
> + * Do not blindly replace one form with the other.
> + */
> +#define in_range(val, start, len) \
> + ((sizeof(start) | sizeof(len) | sizeof(val)) <= sizeof(u32) ? \
> + in_range32(val, start, len) : in_range64(val, start, len))
> +
> /**
> * swap - swap values of @a and @b
> * @a: first value
> diff --git a/lib/logic_pio.c b/lib/logic_pio.c
> index 07b4b9a1f54b..2ea564a40064 100644
> --- a/lib/logic_pio.c
> +++ b/lib/logic_pio.c
> @@ -20,9 +20,6 @@
> static LIST_HEAD(io_range_list);
> static DEFINE_MUTEX(io_range_mutex);
>
> -/* Consider a kernel general helper for this */
> -#define in_range(b, first, len) ((b) >= (first) && (b) < (first) + (len))
> -
> /**
> * logic_pio_register_range - register logical PIO range for a host
> * @new_range: pointer to the IO range to be registered.
> diff --git a/net/netfilter/nf_nat_core.c b/net/netfilter/nf_nat_core.c
> index b7c3c902290f..96b61f0658c8 100644
> --- a/net/netfilter/nf_nat_core.c
> +++ b/net/netfilter/nf_nat_core.c
> @@ -262,7 +262,7 @@ static bool l4proto_in_range(const struct nf_conntrack_tuple *tuple,
> /* If we source map this tuple so reply looks like reply_tuple, will
> * that meet the constraints of range.
> */
> -static int in_range(const struct nf_conntrack_tuple *tuple,
> +static int nf_in_range(const struct nf_conntrack_tuple *tuple,
> const struct nf_nat_range2 *range)
> {
> /* If we are supposed to map IPs, then we must be in the
> @@ -311,7 +311,7 @@ find_appropriate_src(struct net *net,
> &ct->tuplehash[IP_CT_DIR_REPLY].tuple);
> result->dst = tuple->dst;
>
> - if (in_range(result, range))
> + if (nf_in_range(result, range))
> return 1;
> }
> }
> @@ -543,7 +543,7 @@ get_unique_tuple(struct nf_conntrack_tuple *tuple,
> if (maniptype == NF_NAT_MANIP_SRC &&
> !(range->flags & NF_NAT_RANGE_PROTO_RANDOM_ALL)) {
> /* try the original tuple first */
> - if (in_range(orig_tuple, range)) {
> + if (nf_in_range(orig_tuple, range)) {
> if (!nf_nat_used_tuple(orig_tuple, ct)) {
> *tuple = *orig_tuple;
> return;
> diff --git a/net/tipc/core.h b/net/tipc/core.h
> index 73a26b0b9ca1..7c86fa4bb967 100644
> --- a/net/tipc/core.h
> +++ b/net/tipc/core.h
> @@ -199,7 +199,7 @@ static inline int less(u16 left, u16 right)
> return less_eq(left, right) && (mod(right) != mod(left));
> }
>
> -static inline int in_range(u16 val, u16 min, u16 max)
> +static inline int tipc_in_range(u16 val, u16 min, u16 max)
> {
> return !less(val, min) && !more(val, max);
> }
> diff --git a/net/tipc/link.c b/net/tipc/link.c
> index 336d1bb2cf6a..ca96bdb77190 100644
> --- a/net/tipc/link.c
> +++ b/net/tipc/link.c
> @@ -1588,7 +1588,7 @@ static int tipc_link_advance_transmq(struct tipc_link *l, struct tipc_link *r,
> last_ga->bgack_cnt);
> }
> /* Check against the last Gap ACK block */
> - if (in_range(seqno, start, end))
> + if (tipc_in_range(seqno, start, end))
> continue;
> /* Update/release the packet peer is acking */
> bc_has_acked = true;
> @@ -2216,12 +2216,12 @@ static int tipc_link_proto_rcv(struct tipc_link *l, struct sk_buff *skb,
> strncpy(if_name, data, TIPC_MAX_IF_NAME);
>
> /* Update own tolerance if peer indicates a non-zero value */
> - if (in_range(peers_tol, TIPC_MIN_LINK_TOL, TIPC_MAX_LINK_TOL)) {
> + if (tipc_in_range(peers_tol, TIPC_MIN_LINK_TOL, TIPC_MAX_LINK_TOL)) {
> l->tolerance = peers_tol;
> l->bc_rcvlink->tolerance = peers_tol;
> }
> /* Update own priority if peer's priority is higher */
> - if (in_range(peers_prio, l->priority + 1, TIPC_MAX_LINK_PRI))
> + if (tipc_in_range(peers_prio, l->priority + 1, TIPC_MAX_LINK_PRI))
> l->priority = peers_prio;
>
> /* If peer is going down we want full re-establish cycle */
> @@ -2264,13 +2264,13 @@ static int tipc_link_proto_rcv(struct tipc_link *l, struct sk_buff *skb,
> l->rcv_nxt_state = msg_seqno(hdr) + 1;
>
> /* Update own tolerance if peer indicates a non-zero value */
> - if (in_range(peers_tol, TIPC_MIN_LINK_TOL, TIPC_MAX_LINK_TOL)) {
> + if (tipc_in_range(peers_tol, TIPC_MIN_LINK_TOL, TIPC_MAX_LINK_TOL)) {
> l->tolerance = peers_tol;
> l->bc_rcvlink->tolerance = peers_tol;
> }
> /* Update own prio if peer indicates a different value */
> if ((peers_prio != l->priority) &&
> - in_range(peers_prio, 1, TIPC_MAX_LINK_PRI)) {
> + tipc_in_range(peers_prio, 1, TIPC_MAX_LINK_PRI)) {
> l->priority = peers_prio;
> rc = tipc_link_fsm_evt(l, LINK_FAILURE_EVT);
> }
> --
> 2.47.3
>
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
^ permalink raw reply
* Re: Touchpad multitouch leaves ghost fingers
From: Benjamin Tissoires @ 2025-09-22 13:32 UTC (permalink / raw)
To: Yinon Burgansky; +Cc: jikos, linux-input, linux-kernel
In-Reply-To: <CAEU-x4=Zs22b5LX_rsBVMu4BxvDfO+CZkkuWuVNaxv8mqvh9Gw@mail.gmail.com>
Hi,
On Sep 21 2025, Yinon Burgansky wrote:
> When using the touchpad with multi-finger gestures, ghost fingers sometimes
> remain.
> This is reproducible with `hid-replay hid-recorded.txt`: after the
> recording, two ghost fingers remain on the touchpad,
> so a subsequent single-finger tap is interpreted as a triple tap.
> Tapping with three or four fingers resets the device state.
>
> I compared `hid-recorded.txt` to `evtest.txt` for the same events and it
> appears the problem occurs when the device sends a single HID report that
> changes the contact count from 3 -> 1,
> while the kernel's evdev stream removes only one contact (3 -> 2) instead
> of clearing the two removed contacts.
> The following are the exact events where the issue first appeared:
>
> hid recorder:
>
> ```
> # ReportID: 3 / Confidence: 1 | Tip Switch: 1 | Contact Id: 0 | # | X: 792
> | Y: 378
> # | Confidence: 1 | Tip Switch: 1 | Contact Id: 1 | # | X: 564 | Y: 403
> # | Confidence: 1 | Tip Switch: 1 | Contact Id: 2 | # | X: 383 | Y: 542
> # | Confidence: 0 | Tip Switch: 0 | Contact Id: 0 | # | X: 0 | Y: 0
> # | Confidence: 0 | Tip Switch: 0 | Contact Id: 0 | # | X: 0 | Y: 0 | Scan
> Time: 43407 | Contact Count: 3 | Button: 0 | #
> E: 000085.948315 30 03 03 18 03 7a 01 07 34 02 93 01 0b 7f 01 1e 02 00 00
> 00 00 00 00 00 00 00 00 8f a9 03 00
> # ReportID: 3 / Confidence: 1 | Tip Switch: 0 | Contact Id: 1 | # | X: 564
> | Y: 406
> # | Confidence: 0 | Tip Switch: 0 | Contact Id: 0 | # | X: 0 | Y: 0
> # | Confidence: 0 | Tip Switch: 0 | Contact Id: 0 | # | X: 0 | Y: 0
> # | Confidence: 0 | Tip Switch: 0 | Contact Id: 0 | # | X: 0 | Y: 0
> # | Confidence: 0 | Tip Switch: 0 | Contact Id: 0 | # | X: 0 | Y: 0 | Scan
> Time: 43615 | Contact Count: 1 | Button: 0 | #
> E: 000085.960958 30 03 05 34 02 96 01 00 00 00 00 00 00 00 00 00 00 00 00
> 00 00 00 00 00 00 00 00 5f aa 01 00
> ```
Actually, this doesn't seem to be 3->1 but 3 ->0:
- first report has all 3 touches with "Tip Switch: 1" -> 3 touches
- second report has only one report of a touch with "Tip Switch: 0", so
the kernel thinks only Contact Id 1 has been released.
What we are missing here is a common defect in some touchpad: "not seen
means up". And so we need the multitouch class
`MT_CLS_WIN_8_FORCE_MULTI_INPUT_NSMU`.
And following the libinput report, it seems you already found this
yourself :)
Unfortunatelly, because your touchpad is an integrated one using I2C, we
can not dynamically assign this quirk at boot so testing will require
you to recompile the hid-multitouch module or inserting a HID-BPF
module.
I would lean toward providing a small HID-BPF program while the kernel
is being fixed as this will be a much quicker way of fixing it for you
(but the kernel will still need to be fixed).
How does that sound?
Cheers,
Benjamin
>
> evtest:
>
> ```
> Event: time 1758384424.367216, -------------- SYN_REPORT ------------
> Event: time 1758384424.380922, type 3 (EV_ABS), code 47 (ABS_MT_SLOT),
> value 0
> Event: time 1758384424.380922, type 3 (EV_ABS), code 54
> (ABS_MT_POSITION_Y), value 378
> Event: time 1758384424.380922, type 3 (EV_ABS), code 47 (ABS_MT_SLOT),
> value 1
> Event: time 1758384424.380922, type 3 (EV_ABS), code 53
> (ABS_MT_POSITION_X), value 564
> Event: time 1758384424.380922, type 3 (EV_ABS), code 54
> (ABS_MT_POSITION_Y), value 403
> Event: time 1758384424.380922, type 3 (EV_ABS), code 47 (ABS_MT_SLOT),
> value 2
> Event: time 1758384424.380922, type 3 (EV_ABS), code 53
> (ABS_MT_POSITION_X), value 383
> Event: time 1758384424.380922, type 3 (EV_ABS), code 54
> (ABS_MT_POSITION_Y), value 542
> Event: time 1758384424.380922, type 3 (EV_ABS), code 1 (ABS_Y), value 378
> Event: time 1758384424.380922, type 4 (EV_MSC), code 5 (MSC_TIMESTAMP),
> value 547800
> Event: time 1758384424.380922, -------------- SYN_REPORT ------------
> Event: time 1758384424.393487, type 3 (EV_ABS), code 47 (ABS_MT_SLOT),
> value 1
> Event: time 1758384424.393487, type 3 (EV_ABS), code 57
> (ABS_MT_TRACKING_ID), value -1
> Event: time 1758384424.393487, type 1 (EV_KEY), code 333
> (BTN_TOOL_DOUBLETAP), value 1
> Event: time 1758384424.393487, type 1 (EV_KEY), code 334
> (BTN_TOOL_TRIPLETAP), value 0
> Event: time 1758384424.393487, type 4 (EV_MSC), code 5 (MSC_TIMESTAMP),
> value 568600
> ```
>
> This is an old issue (at least a year old) that I still encounter
> occasionally. I originally reported it downstream:
> https://gitlab.freedesktop.org/libinput/libinput/-/issues/1194
>
> ```
> uname -a
> Linux fedora 6.16.7-200.fc42.x86_64 #1 SMP PREEMPT_DYNAMIC Thu Sep 11
> 17:46:54 UTC 2025 x86_64 GNU/Linux
> ```
>
> Please let me know if you need anything else,
> Thank you!
^ permalink raw reply
* Re: [PATCH v5 3/6] dt-bindings: touchscreen: add touchscreen-glitch-threshold-ns property
From: Rob Herring @ 2025-09-22 15:07 UTC (permalink / raw)
To: Dario Binacchi
Cc: linux-kernel, Frank Li, linux-amarula, Conor Dooley, Conor Dooley,
Dmitry Torokhov, Javier Carrasco, Jeff LaBundy,
Krzysztof Kozlowski, devicetree, linux-input
In-Reply-To: <CABGWkvr8X5a0ezeu6HDCMfjh+xbg-bQq4cLwzRD2BvoJsvH_BA@mail.gmail.com>
On Sat, Sep 20, 2025 at 11:39:59AM +0200, Dario Binacchi wrote:
> On Fri, Sep 19, 2025 at 10:44 PM Rob Herring <robh@kernel.org> wrote:
> >
> > On Fri, Sep 19, 2025 at 05:12:42PM +0200, Dario Binacchi wrote:
> > > On Fri, Sep 19, 2025 at 4:38 PM Rob Herring <robh@kernel.org> wrote:
> > > >
> > > > On Thu, Sep 18, 2025 at 10:37:37PM +0200, Dario Binacchi wrote:
> > > > > On Thu, Sep 18, 2025 at 10:04 PM Rob Herring <robh@kernel.org> wrote:
> > > > > >
> > > > > > On Thu, Sep 18, 2025 at 05:52:31PM +0200, Dario Binacchi wrote:
> > > > > > > Add support for glitch threshold configuration. A detected signal is valid
> > > > > > > only if it lasts longer than the set threshold; otherwise, it is regarded
> > > > > > > as a glitch.
> > > > > > >
> > > > > > > Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> > > > > > > Acked-by: Conor Dooley <conor.dooley@microchip.com>
> > > > > > >
> > > > > > > ---
> > > > > > >
> > > > > > > Changes in v5:
> > > > > > > - Add Acked-by tag of Conor Dooley
> > > > > > >
> > > > > > > Changes in v2:
> > > > > > > - Added in v2.
> > > > > > >
> > > > > > > .../devicetree/bindings/input/touchscreen/touchscreen.yaml | 4 ++++
> > > > > > > 1 file changed, 4 insertions(+)
> > > > > > >
> > > > > > > diff --git a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
> > > > > > > index 3e3572aa483a..a60b4d08620d 100644
> > > > > > > --- a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
> > > > > > > +++ b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
> > > > > > > @@ -206,6 +206,10 @@ properties:
> > > > > > >
> > > > > > > unevaluatedProperties: false
> > > > > > >
> > > > > > > + touchscreen-glitch-threshold-ns:
> > > > > > > + description: Minimum duration in nanoseconds a signal must remain stable
> > > > > > > + to be considered valid.
> > > > > >
> > > > > > What's wrong with debounce-delay-ms?
> > > > >
> > > > > Do you mean that I should rename touchscreen-glitch-threshold-ns to
> > > > > debounce-delay-ms?
> > > >
> > > > I mean that's the common property we already have, so use it or explain
> > > > why you aren't using it. I suppose the definition is technically a bit
> > > > different if it's purely a s/w delay vs. h/w monitoring of the signal
> > > > state. I don't think it matters if the interpretation by each driver is
> > > > a bit different.
> > > >
> > > > Maybe msec is not enough resolution for you could be another reason?
> > >
> > > Yes, this is the main reason. As specified in the following patch:
> > > v5 4/6 dt-bindings: touchscreen: fsl,imx6ul-tsc: support glitch threshold
> > >
> > > Drivers must convert this value to IPG clock cycles and map
> > > it to one of the four discrete thresholds exposed by the
> > > TSC_DEBUG_MODE2 register:
> > >
> > > 0: 8191 IPG cycles
> > > 1: 4095 IPG cycles
> > > 2: 2047 IPG cycles
> > > 3: 1023 IPG cycles
> > >
> > > In my case, the IPG clock runs at 66 MHz, which corresponds to:
> > >
> > > 124 µs for 0
> > > 62 µs for 1
> > > 31 us for 2
> > > 15 us for 3
> > >
> > > So using milliseconds would not fit my use case. A possible trade-off
> > > could be to use debounce-delay-us. Would that be acceptable?
> >
> > I agree it wouldn't map to what the h/w provides, but is what the h/w
> > provides actually useful? There's plenty of h/w designed that's not
> > useful. 15us is quite short for a glitch. Do you have an actual cases
> > where the different values above are needed?
>
> Considering an IPG clock at 66 MHz, currently at reset the deglitch
> filter is set to 124 µs,
> the driver sets it to 31 µs with a hardcoded value, and in my use case
> I need to set it to 62 µs,
It would be helpful if the commit message explained why. What platform
needs it and what happens without this support added?
> as you can see in the patch:
> https://lore.kernel.org/all/20250918155240.2536852-6-dario.binacchi@amarulasolutions.com/
> and its handling in
> https://lore.kernel.org/all/20250918155240.2536852-7-dario.binacchi@amarulasolutions.com/
>
> Another option could be to use a specific binding for the
> fsl,imx6ul-tsc controller, as I did in the
> earlier versions of the series.
No, add debounce-delay-us to the common binding.
Rob
^ permalink raw reply
* Re: [PATCH v1 1/2] dt-bindings: input: document Samsung A552 touchscreen controller
From: Rob Herring (Arm) @ 2025-09-22 15:38 UTC (permalink / raw)
To: Ivaylo Ivanov
Cc: linux-arm-kernel, Henrik Rydberg, Dmitry Torokhov,
linux-samsung-soc, linux-input, Krzysztof Kozlowski, Conor Dooley,
linux-kernel, devicetree
In-Reply-To: <20250914134458.2624176-2-ivo.ivanov.ivanov1@gmail.com>
On Sun, 14 Sep 2025 16:44:56 +0300, Ivaylo Ivanov wrote:
> Document the Samsung A552 touchscreen controller, present in devices like
> the Samsung Galaxy S7.
>
> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
> ---
> .../input/touchscreen/samsung,s6sa552.yaml | 54 +++++++++++++++++++
> 1 file changed, 54 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/touchscreen/samsung,s6sa552.yaml
>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
^ permalink raw reply
* Re: Touchpad multitouch leaves ghost fingers
From: Yinon Burgansky @ 2025-09-22 16:25 UTC (permalink / raw)
To: Benjamin Tissoires; +Cc: jikos, linux-input, linux-kernel
In-Reply-To: <CAEU-x4kL45DAddmNahjR2C97+43jchpmXep++LbeP8cXLEWN-w@mail.gmail.com>
Hi Benjamin,
Thank you for the quick and detailed response. The solution sounds
great! It will take me some time to learn how to code, build, and
compile HID-BPF on my machine and figure it out. If you can provide me
with additional guidance to speed up the process, I would greatly
appreciate it!
If it is a common issue, as you suggested, having public instructions
to refer to would also be very helpful for others running into the
same issue (e.g. the other 2 users from the libinput issue).
Thanks,
Yinon
On Mon, Sep 22, 2025 at 7:11 PM Yinon Burgansky
<yinonburgansky@gmail.com> wrote:
>
> Hi Benjamin,
>
> Thank you for the quick and detailed response. The solution sounds great! It will take me some time to learn how to code, build, and compile HID-BPF on my machine and figure it out. If you can provide me with additional guidance to speed up the process, I would greatly appreciate it!
>
> If it is a common issue, as you suggested, having instructions to refer to would also be very helpful for others running into the same issue (e.g. the other 2 users from the libinput issue).
>
> Thanks,
> Yinon
>
>
> On Mon, Sep 22, 2025 at 4:32 PM Benjamin Tissoires <bentiss@kernel.org> wrote:
>>
>> Hi,
>>
>> On Sep 21 2025, Yinon Burgansky wrote:
>> > When using the touchpad with multi-finger gestures, ghost fingers sometimes
>> > remain.
>> > This is reproducible with `hid-replay hid-recorded.txt`: after the
>> > recording, two ghost fingers remain on the touchpad,
>> > so a subsequent single-finger tap is interpreted as a triple tap.
>> > Tapping with three or four fingers resets the device state.
>> >
>> > I compared `hid-recorded.txt` to `evtest.txt` for the same events and it
>> > appears the problem occurs when the device sends a single HID report that
>> > changes the contact count from 3 -> 1,
>> > while the kernel's evdev stream removes only one contact (3 -> 2) instead
>> > of clearing the two removed contacts.
>> > The following are the exact events where the issue first appeared:
>> >
>> > hid recorder:
>> >
>> > ```
>> > # ReportID: 3 / Confidence: 1 | Tip Switch: 1 | Contact Id: 0 | # | X: 792
>> > | Y: 378
>> > # | Confidence: 1 | Tip Switch: 1 | Contact Id: 1 | # | X: 564 | Y: 403
>> > # | Confidence: 1 | Tip Switch: 1 | Contact Id: 2 | # | X: 383 | Y: 542
>> > # | Confidence: 0 | Tip Switch: 0 | Contact Id: 0 | # | X: 0 | Y: 0
>> > # | Confidence: 0 | Tip Switch: 0 | Contact Id: 0 | # | X: 0 | Y: 0 | Scan
>> > Time: 43407 | Contact Count: 3 | Button: 0 | #
>> > E: 000085.948315 30 03 03 18 03 7a 01 07 34 02 93 01 0b 7f 01 1e 02 00 00
>> > 00 00 00 00 00 00 00 00 8f a9 03 00
>> > # ReportID: 3 / Confidence: 1 | Tip Switch: 0 | Contact Id: 1 | # | X: 564
>> > | Y: 406
>> > # | Confidence: 0 | Tip Switch: 0 | Contact Id: 0 | # | X: 0 | Y: 0
>> > # | Confidence: 0 | Tip Switch: 0 | Contact Id: 0 | # | X: 0 | Y: 0
>> > # | Confidence: 0 | Tip Switch: 0 | Contact Id: 0 | # | X: 0 | Y: 0
>> > # | Confidence: 0 | Tip Switch: 0 | Contact Id: 0 | # | X: 0 | Y: 0 | Scan
>> > Time: 43615 | Contact Count: 1 | Button: 0 | #
>> > E: 000085.960958 30 03 05 34 02 96 01 00 00 00 00 00 00 00 00 00 00 00 00
>> > 00 00 00 00 00 00 00 00 5f aa 01 00
>> > ```
>>
>> Actually, this doesn't seem to be 3->1 but 3 ->0:
>> - first report has all 3 touches with "Tip Switch: 1" -> 3 touches
>> - second report has only one report of a touch with "Tip Switch: 0", so
>> the kernel thinks only Contact Id 1 has been released.
>>
>> What we are missing here is a common defect in some touchpad: "not seen
>> means up". And so we need the multitouch class
>> `MT_CLS_WIN_8_FORCE_MULTI_INPUT_NSMU`.
>>
>> And following the libinput report, it seems you already found this
>> yourself :)
>>
>> Unfortunatelly, because your touchpad is an integrated one using I2C, we
>> can not dynamically assign this quirk at boot so testing will require
>> you to recompile the hid-multitouch module or inserting a HID-BPF
>> module.
>>
>> I would lean toward providing a small HID-BPF program while the kernel
>> is being fixed as this will be a much quicker way of fixing it for you
>> (but the kernel will still need to be fixed).
>>
>> How does that sound?
>>
>> Cheers,
>> Benjamin
>>
>> >
>> > evtest:
>> >
>> > ```
>> > Event: time 1758384424.367216, -------------- SYN_REPORT ------------
>> > Event: time 1758384424.380922, type 3 (EV_ABS), code 47 (ABS_MT_SLOT),
>> > value 0
>> > Event: time 1758384424.380922, type 3 (EV_ABS), code 54
>> > (ABS_MT_POSITION_Y), value 378
>> > Event: time 1758384424.380922, type 3 (EV_ABS), code 47 (ABS_MT_SLOT),
>> > value 1
>> > Event: time 1758384424.380922, type 3 (EV_ABS), code 53
>> > (ABS_MT_POSITION_X), value 564
>> > Event: time 1758384424.380922, type 3 (EV_ABS), code 54
>> > (ABS_MT_POSITION_Y), value 403
>> > Event: time 1758384424.380922, type 3 (EV_ABS), code 47 (ABS_MT_SLOT),
>> > value 2
>> > Event: time 1758384424.380922, type 3 (EV_ABS), code 53
>> > (ABS_MT_POSITION_X), value 383
>> > Event: time 1758384424.380922, type 3 (EV_ABS), code 54
>> > (ABS_MT_POSITION_Y), value 542
>> > Event: time 1758384424.380922, type 3 (EV_ABS), code 1 (ABS_Y), value 378
>> > Event: time 1758384424.380922, type 4 (EV_MSC), code 5 (MSC_TIMESTAMP),
>> > value 547800
>> > Event: time 1758384424.380922, -------------- SYN_REPORT ------------
>> > Event: time 1758384424.393487, type 3 (EV_ABS), code 47 (ABS_MT_SLOT),
>> > value 1
>> > Event: time 1758384424.393487, type 3 (EV_ABS), code 57
>> > (ABS_MT_TRACKING_ID), value -1
>> > Event: time 1758384424.393487, type 1 (EV_KEY), code 333
>> > (BTN_TOOL_DOUBLETAP), value 1
>> > Event: time 1758384424.393487, type 1 (EV_KEY), code 334
>> > (BTN_TOOL_TRIPLETAP), value 0
>> > Event: time 1758384424.393487, type 4 (EV_MSC), code 5 (MSC_TIMESTAMP),
>> > value 568600
>> > ```
>> >
>> > This is an old issue (at least a year old) that I still encounter
>> > occasionally. I originally reported it downstream:
>> > https://gitlab.freedesktop.org/libinput/libinput/-/issues/1194
>> >
>> > ```
>> > uname -a
>> > Linux fedora 6.16.7-200.fc42.x86_64 #1 SMP PREEMPT_DYNAMIC Thu Sep 11
>> > 17:46:54 UTC 2025 x86_64 GNU/Linux
>> > ```
>> >
>> > Please let me know if you need anything else,
>> > Thank you!
^ permalink raw reply
* Re: Touchpad multitouch leaves ghost fingers
From: Benjamin Tissoires @ 2025-09-22 16:51 UTC (permalink / raw)
To: Yinon Burgansky; +Cc: jikos, linux-input, linux-kernel
In-Reply-To: <CAEU-x4nv3XnXchevtwN5mkVcxqnpgBobhavxZc7BjS7EgYG8Ng@mail.gmail.com>
On Sep 22 2025, Yinon Burgansky wrote:
> Hi Benjamin,
>
> Thank you for the quick and detailed response. The solution sounds
> great! It will take me some time to learn how to code, build, and
> compile HID-BPF on my machine and figure it out. If you can provide me
> with additional guidance to speed up the process, I would greatly
> appreciate it!
Well, I was meaning that I would provide a HID-BPF MR ready to install
for you. But if you want to play with it, feel free to do so :)
For a jump start on HID-BPF:
https://libevdev.pages.freedesktop.org/udev-hid-bpf/tutorial.html
The advantage of HID-BPF is that we can drop the file in the filesystem
and then forget about it.
>
> If it is a common issue, as you suggested, having public instructions
> to refer to would also be very helpful for others running into the
> same issue (e.g. the other 2 users from the libinput issue).
Yeah, once we get the MR out, we need to update the libinput bug.
My plan is to have the HID-BPF detect whether the quirk has been applied
or not and would gracefully remove itself once the kernel is fixed.
Cheers,
Benjamin
PS: please try to refrain from top-posting your reply. The usage is to
inline your reply or reply below, and there are docs that explain why.
You might get angry answers from people on the LKML if you top post :/
>
> Thanks,
> Yinon
>
> On Mon, Sep 22, 2025 at 7:11 PM Yinon Burgansky
> <yinonburgansky@gmail.com> wrote:
> >
> > Hi Benjamin,
> >
> > Thank you for the quick and detailed response. The solution sounds great! It will take me some time to learn how to code, build, and compile HID-BPF on my machine and figure it out. If you can provide me with additional guidance to speed up the process, I would greatly appreciate it!
> >
> > If it is a common issue, as you suggested, having instructions to refer to would also be very helpful for others running into the same issue (e.g. the other 2 users from the libinput issue).
> >
> > Thanks,
> > Yinon
> >
> >
> > On Mon, Sep 22, 2025 at 4:32 PM Benjamin Tissoires <bentiss@kernel.org> wrote:
> >>
> >> Hi,
> >>
> >> On Sep 21 2025, Yinon Burgansky wrote:
> >> > When using the touchpad with multi-finger gestures, ghost fingers sometimes
> >> > remain.
> >> > This is reproducible with `hid-replay hid-recorded.txt`: after the
> >> > recording, two ghost fingers remain on the touchpad,
> >> > so a subsequent single-finger tap is interpreted as a triple tap.
> >> > Tapping with three or four fingers resets the device state.
> >> >
> >> > I compared `hid-recorded.txt` to `evtest.txt` for the same events and it
> >> > appears the problem occurs when the device sends a single HID report that
> >> > changes the contact count from 3 -> 1,
> >> > while the kernel's evdev stream removes only one contact (3 -> 2) instead
> >> > of clearing the two removed contacts.
> >> > The following are the exact events where the issue first appeared:
> >> >
> >> > hid recorder:
> >> >
> >> > ```
> >> > # ReportID: 3 / Confidence: 1 | Tip Switch: 1 | Contact Id: 0 | # | X: 792
> >> > | Y: 378
> >> > # | Confidence: 1 | Tip Switch: 1 | Contact Id: 1 | # | X: 564 | Y: 403
> >> > # | Confidence: 1 | Tip Switch: 1 | Contact Id: 2 | # | X: 383 | Y: 542
> >> > # | Confidence: 0 | Tip Switch: 0 | Contact Id: 0 | # | X: 0 | Y: 0
> >> > # | Confidence: 0 | Tip Switch: 0 | Contact Id: 0 | # | X: 0 | Y: 0 | Scan
> >> > Time: 43407 | Contact Count: 3 | Button: 0 | #
> >> > E: 000085.948315 30 03 03 18 03 7a 01 07 34 02 93 01 0b 7f 01 1e 02 00 00
> >> > 00 00 00 00 00 00 00 00 8f a9 03 00
> >> > # ReportID: 3 / Confidence: 1 | Tip Switch: 0 | Contact Id: 1 | # | X: 564
> >> > | Y: 406
> >> > # | Confidence: 0 | Tip Switch: 0 | Contact Id: 0 | # | X: 0 | Y: 0
> >> > # | Confidence: 0 | Tip Switch: 0 | Contact Id: 0 | # | X: 0 | Y: 0
> >> > # | Confidence: 0 | Tip Switch: 0 | Contact Id: 0 | # | X: 0 | Y: 0
> >> > # | Confidence: 0 | Tip Switch: 0 | Contact Id: 0 | # | X: 0 | Y: 0 | Scan
> >> > Time: 43615 | Contact Count: 1 | Button: 0 | #
> >> > E: 000085.960958 30 03 05 34 02 96 01 00 00 00 00 00 00 00 00 00 00 00 00
> >> > 00 00 00 00 00 00 00 00 5f aa 01 00
> >> > ```
> >>
> >> Actually, this doesn't seem to be 3->1 but 3 ->0:
> >> - first report has all 3 touches with "Tip Switch: 1" -> 3 touches
> >> - second report has only one report of a touch with "Tip Switch: 0", so
> >> the kernel thinks only Contact Id 1 has been released.
> >>
> >> What we are missing here is a common defect in some touchpad: "not seen
> >> means up". And so we need the multitouch class
> >> `MT_CLS_WIN_8_FORCE_MULTI_INPUT_NSMU`.
> >>
> >> And following the libinput report, it seems you already found this
> >> yourself :)
> >>
> >> Unfortunatelly, because your touchpad is an integrated one using I2C, we
> >> can not dynamically assign this quirk at boot so testing will require
> >> you to recompile the hid-multitouch module or inserting a HID-BPF
> >> module.
> >>
> >> I would lean toward providing a small HID-BPF program while the kernel
> >> is being fixed as this will be a much quicker way of fixing it for you
> >> (but the kernel will still need to be fixed).
> >>
> >> How does that sound?
> >>
> >> Cheers,
> >> Benjamin
> >>
> >> >
> >> > evtest:
> >> >
> >> > ```
> >> > Event: time 1758384424.367216, -------------- SYN_REPORT ------------
> >> > Event: time 1758384424.380922, type 3 (EV_ABS), code 47 (ABS_MT_SLOT),
> >> > value 0
> >> > Event: time 1758384424.380922, type 3 (EV_ABS), code 54
> >> > (ABS_MT_POSITION_Y), value 378
> >> > Event: time 1758384424.380922, type 3 (EV_ABS), code 47 (ABS_MT_SLOT),
> >> > value 1
> >> > Event: time 1758384424.380922, type 3 (EV_ABS), code 53
> >> > (ABS_MT_POSITION_X), value 564
> >> > Event: time 1758384424.380922, type 3 (EV_ABS), code 54
> >> > (ABS_MT_POSITION_Y), value 403
> >> > Event: time 1758384424.380922, type 3 (EV_ABS), code 47 (ABS_MT_SLOT),
> >> > value 2
> >> > Event: time 1758384424.380922, type 3 (EV_ABS), code 53
> >> > (ABS_MT_POSITION_X), value 383
> >> > Event: time 1758384424.380922, type 3 (EV_ABS), code 54
> >> > (ABS_MT_POSITION_Y), value 542
> >> > Event: time 1758384424.380922, type 3 (EV_ABS), code 1 (ABS_Y), value 378
> >> > Event: time 1758384424.380922, type 4 (EV_MSC), code 5 (MSC_TIMESTAMP),
> >> > value 547800
> >> > Event: time 1758384424.380922, -------------- SYN_REPORT ------------
> >> > Event: time 1758384424.393487, type 3 (EV_ABS), code 47 (ABS_MT_SLOT),
> >> > value 1
> >> > Event: time 1758384424.393487, type 3 (EV_ABS), code 57
> >> > (ABS_MT_TRACKING_ID), value -1
> >> > Event: time 1758384424.393487, type 1 (EV_KEY), code 333
> >> > (BTN_TOOL_DOUBLETAP), value 1
> >> > Event: time 1758384424.393487, type 1 (EV_KEY), code 334
> >> > (BTN_TOOL_TRIPLETAP), value 0
> >> > Event: time 1758384424.393487, type 4 (EV_MSC), code 5 (MSC_TIMESTAMP),
> >> > value 568600
> >> > ```
> >> >
> >> > This is an old issue (at least a year old) that I still encounter
> >> > occasionally. I originally reported it downstream:
> >> > https://gitlab.freedesktop.org/libinput/libinput/-/issues/1194
> >> >
> >> > ```
> >> > uname -a
> >> > Linux fedora 6.16.7-200.fc42.x86_64 #1 SMP PREEMPT_DYNAMIC Thu Sep 11
> >> > 17:46:54 UTC 2025 x86_64 GNU/Linux
> >> > ```
> >> >
> >> > Please let me know if you need anything else,
> >> > Thank you!
^ permalink raw reply
* Re: [PATCH RFC 1/3] dt-bindings: input: document Goodix GTX8 Touchscreen ICs
From: Rob Herring (Arm) @ 2025-09-22 17:35 UTC (permalink / raw)
To: Jens Reidel
Cc: Hans de Goede, Henrik Rydberg, devicetree, Neil Armstrong,
phone-devel, Krzysztof Kozlowski, linux-input, linux-kernel,
~postmarketos/upstreaming, linux, Conor Dooley, Dmitry Torokhov
In-Reply-To: <20250918-gtx8-v1-1-cba879c84775@mainlining.org>
On Thu, 18 Sep 2025 16:02:48 +0200, Jens Reidel wrote:
> Document the Goodix GT9886 and GT9896 which are part of the GTX8 series
> of Touchscreen controller ICs from Goodix.
>
> Signed-off-by: Jens Reidel <adrian@mainlining.org>
> ---
> .../bindings/input/touchscreen/goodix,gt9886.yaml | 71 ++++++++++++++++++++++
> 1 file changed, 71 insertions(+)
>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
^ permalink raw reply
* Re: [regression] 1a8953f4f774 ("HID: Add IGNORE quirk for SMARTLINKTECHNOLOGY") causes issue with ID 4c4a:4155 Jieli Technology USB Composite Device
From: Staffan Melin @ 2025-09-22 18:33 UTC (permalink / raw)
To: zhangheng
Cc: Terry Junge, Salvatore Bonaccorso, Jiri Kosina,
Benjamin Tissoires, linux-input, linux-kernel, regressions,
stable, 1114557
In-Reply-To: <01ce8d55-6054-4efa-bed5-ce4c6c6bc0e6@kylinos.cn>
Thank you,
I can confirm that this patch fixes the touchscreen issue on my GPD DUO.
Tested-by: staffan.melin@oscillator.se
Thank you for your work!
Staffan
On 2025-09-22 11:21, zhangheng wrote:
> Please help test this patch, I will push it to the kernel community.
> Currently, the microphone device is functioning normally
^ permalink raw reply
* Re: [PATCH v1 1/2] dt-bindings: input: Add ST-Microelectronics FTS2BA61Y touchscreen
From: Rob Herring (Arm) @ 2025-09-22 20:08 UTC (permalink / raw)
To: Eric Gonçalves
Cc: Dmitry Torokhov, Ivaylo Ivanov, Krzysztof Kozlowski, devicetree,
linux-kernel, linux-input, Conor Dooley, Henrik Rydberg
In-Reply-To: <20250920014450.37787-2-ghatto404@gmail.com>
On Sat, 20 Sep 2025 01:44:49 +0000, Eric Gonçalves wrote:
> Add the bindings for ST-Microelectronics FTS2BA61Y capacitive touchscreen.
>
> Signed-off-by: Eric Gonçalves <ghatto404@gmail.com>
> ---
> .../input/touchscreen/st,fts2ba61y.yaml | 52 +++++++++++++++++++
> 1 file changed, 52 insertions(+)
> create mode 100755 Documentation/devicetree/bindings/input/touchscreen/st,fts2ba61y.yaml
>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
^ permalink raw reply
* Re: Touchpad multitouch leaves ghost fingers
From: Yinon Burgansky @ 2025-09-22 20:32 UTC (permalink / raw)
To: Benjamin Tissoires; +Cc: jikos, linux-input, linux-kernel
In-Reply-To: <c3plpgl2zsx4do2odwdeowodkkdnfqpexlwqg5a5mckyibxlge@qai35f5yeswy>
On Mon, Sep 22, 2025 at 7:51 PM Benjamin Tissoires <bentiss@kernel.org> wrote:
> Well, I was meaning that I would provide a HID-BPF MR ready to install
> for you.
That would be awesome! Thank you very much!
> But if you want to play with it, feel free to do so :)
> For a jump start on HID-BPF:
> https://libevdev.pages.freedesktop.org/udev-hid-bpf/tutorial.html
Thank you for the reference. I’m feeling quite fatigued from the
debugging process, as it took me a considerable amount of time to
identify the issue.
It was surprisingly difficult to determine what exactly was wrong with
the touchpad and to uncover the root cause; perhaps I didn’t search
for the right terms.
However, I’m glad I finally took the time to debug it, submit the bug
report, and find out what was wrong. Thank you!
> The advantage of HID-BPF is that we can drop the file in the filesystem
> and then forget about it.
> My plan is to have the HID-BPF detect whether the quirk has been applied
> or not and would gracefully remove itself once the kernel is fixed.
That's cool, nice design.
> PS: please try to refrain from top-posting your reply. The usage is to
> inline your reply or reply below,
Sorry about that, hopefully I did it right this time.
Thanks,
Yinon
^ permalink raw reply
* Re: [PATCH v6 1/3] dt-bindings: touchscreen: convert eeti bindings to json schema
From: Rob Herring (Arm) @ 2025-09-22 20:52 UTC (permalink / raw)
To: Dario Binacchi
Cc: devicetree, Dmitry Torokhov, Conor Dooley, linux-kernel,
Krzysztof Kozlowski, sebastian.reichel, linux-input,
linux-amarula
In-Reply-To: <20250921173353.2641438-1-dario.binacchi@amarulasolutions.com>
On Sun, 21 Sep 2025 19:33:42 +0200, Dario Binacchi wrote:
> Convert EETI touchscreen controller device tree binding to json-schema.
>
> Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
>
> ---
>
> Changes in v6:
> - Add deprected to the compatible string and attn-gpios
> property
> - Put const 0x2a i2c address for reg property only in case
> of not eeti,exc3000-i2c.
> - Put false the attn-gpios property in case of not
> eeti,exc3000-i2c..
> - Drop example for eeti,exc3000-i2c.
>
> Changes in v5:
> - Move bindings into eeti,exc3000.yaml
> - Remove eeti.yaml
>
> Changes in v2:
> - Added in v2
>
> .../input/touchscreen/eeti,exc3000.yaml | 33 +++++++++++++++----
> .../bindings/input/touchscreen/eeti.txt | 30 -----------------
> 2 files changed, 27 insertions(+), 36 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/input/touchscreen/eeti.txt
>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v6 2/3] dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Add touchscreen child node
From: Rob Herring @ 2025-09-22 20:53 UTC (permalink / raw)
To: Dario Binacchi
Cc: linux-kernel, linux-amarula, sebastian.reichel,
Broadcom internal kernel review list, Conor Dooley,
Dmitry Torokhov, Eric Anholt, Florian Fainelli,
Krzysztof Kozlowski, Ray Jui, Scott Branden, Stefan Wahren,
devicetree, linux-arm-kernel, linux-input, linux-rpi-kernel
In-Reply-To: <20250921173353.2641438-2-dario.binacchi@amarulasolutions.com>
On Sun, Sep 21, 2025 at 07:33:43PM +0200, Dario Binacchi wrote:
> Convert Raspberry Pi firmware 7" touchscreen controller device tree
> binding to json-schema.
>
> Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
>
> ---
>
> (no changes since v5)
>
> Changes in v5:
> - Move bindings into raspberrypi,bcm2835-firmware.yaml
> - Remove raspberrypi,firmware-ts.yaml
> - Update the commit message
>
> Changes in v3:
> - Drop firmware-rpi node and use only touchscreen node to fix warnings
> you can see in
> https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250913092707.1005616-3-dario.binacchi@amarulasolutions.com/
>
> Changes in v2:
> - Added in v2
>
> .../arm/bcm/raspberrypi,bcm2835-firmware.yaml | 27 +++++++++++++++++++
> .../touchscreen/raspberrypi,firmware-ts.txt | 26 ------------------
> 2 files changed, 27 insertions(+), 26 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/input/touchscreen/raspberrypi,firmware-ts.txt
>
> diff --git a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
> index 1f84407a73e4..044544c17e8e 100644
> --- a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
> +++ b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
> @@ -103,6 +103,27 @@ properties:
> - compatible
> - "#pwm-cells"
>
> + touchscreen:
> + type: object
> + additionalProperties: false
Needs a $ref to touchscreen.yaml
> +
> + properties:
> + compatible:
> + const: raspberrypi,firmware-ts
> +
> + firmware:
> + deprecated: true
> + description: Phandle to RPi's firmware device node.
> +
> + touchscreen-size-x: true
> + touchscreen-size-y: true
> + touchscreen-inverted-x: true
> + touchscreen-inverted-y: true
> + touchscreen-swapped-x-y: true
> +
> + required:
> + - compatible
> +
> required:
> - compatible
> - mboxes
> @@ -135,5 +156,11 @@ examples:
> compatible = "raspberrypi,firmware-poe-pwm";
> #pwm-cells = <2>;
> };
> +
> + ts: touchscreen {
> + compatible = "raspberrypi,firmware-ts";
> + touchscreen-size-x = <800>;
> + touchscreen-size-y = <480>;
> + };
> };
> ...
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/raspberrypi,firmware-ts.txt b/Documentation/devicetree/bindings/input/touchscreen/raspberrypi,firmware-ts.txt
> deleted file mode 100644
> index 2a1af240ccc3..000000000000
> --- a/Documentation/devicetree/bindings/input/touchscreen/raspberrypi,firmware-ts.txt
> +++ /dev/null
> @@ -1,26 +0,0 @@
> -Raspberry Pi firmware based 7" touchscreen
> -=====================================
> -
> -Required properties:
> - - compatible: "raspberrypi,firmware-ts"
> -
> -Optional properties:
> - - firmware: Reference to RPi's firmware device node
> - - touchscreen-size-x: See touchscreen.txt
> - - touchscreen-size-y: See touchscreen.txt
> - - touchscreen-inverted-x: See touchscreen.txt
> - - touchscreen-inverted-y: See touchscreen.txt
> - - touchscreen-swapped-x-y: See touchscreen.txt
> -
> -Example:
> -
> -firmware: firmware-rpi {
> - compatible = "raspberrypi,bcm2835-firmware";
> - mboxes = <&mailbox>;
> -
> - ts: touchscreen {
> - compatible = "raspberrypi,firmware-ts";
> - touchscreen-size-x = <800>;
> - touchscreen-size-y = <480>;
> - };
> -};
> --
> 2.43.0
>
> base-commit: f975f08c2e899ae2484407d7bba6bb7f8b6d9d40
> branch: drop-touchscreen.txt
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox