* [PATCH v2 20/27 5.10.y] minmax: fix up min3() and max3() too
From: Eliav Farber @ 2025-10-17 9:05 UTC (permalink / raw)
To: gregkh, stable, 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, liviu.dudau, 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, clm,
josef, dsterba, xiang, chao, 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, farbere, sashal, ruanjinjie, David.Laight, herve.codina,
Jason, keescook, kbusch, nathan, bvanassche, ndesaulniers,
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-erofs, linux-ext4, linux-sparse,
linux-mm, netfilter-devel, coreteam, tipc-discussion
Cc: Linus Torvalds, David Laight, Arnd Bergmann
In-Reply-To: <20251017090519.46992-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 v2 19/27 5.10.y] minmax: improve macro expansion and type checking
From: Eliav Farber @ 2025-10-17 9:05 UTC (permalink / raw)
To: gregkh, stable, 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, liviu.dudau, 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, clm,
josef, dsterba, xiang, chao, 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, farbere, sashal, ruanjinjie, David.Laight, herve.codina,
Jason, keescook, kbusch, nathan, bvanassche, ndesaulniers,
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-erofs, linux-ext4, linux-sparse,
linux-mm, netfilter-devel, coreteam, tipc-discussion
Cc: Linus Torvalds, Arnd Bergmann, David Laight, Lorenzo Stoakes
In-Reply-To: <20251017090519.46992-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 004a030d5ad2..28b21e372751 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -251,6 +251,15 @@ static inline void *offset_to_ptr(const int *off)
*/
#define is_signed_type(type) (((type)(-1)) < (__force type)1)
+/*
+ * 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 v2 18/27 5.10.y] minmax: simplify min()/max()/clamp() implementation
From: Eliav Farber @ 2025-10-17 9:05 UTC (permalink / raw)
To: gregkh, stable, 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, liviu.dudau, 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, clm,
josef, dsterba, xiang, chao, 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, farbere, sashal, ruanjinjie, David.Laight, herve.codina,
Jason, keescook, kbusch, nathan, bvanassche, ndesaulniers,
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-erofs, linux-ext4, linux-sparse,
linux-mm, netfilter-devel, coreteam, tipc-discussion
Cc: Linus Torvalds, David Laight, Lorenzo Stoakes
In-Reply-To: <20251017090519.46992-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 v2 17/27 5.10.y] minmax: don't use max() in situations that want a C constant expression
From: Eliav Farber @ 2025-10-17 9:05 UTC (permalink / raw)
To: gregkh, stable, 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, liviu.dudau, 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, clm,
josef, dsterba, xiang, chao, 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, farbere, sashal, ruanjinjie, David.Laight, herve.codina,
Jason, keescook, kbusch, nathan, bvanassche, ndesaulniers,
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-erofs, linux-ext4, linux-sparse,
linux-mm, netfilter-devel, coreteam, tipc-discussion
Cc: Linus Torvalds, David Laight, Lorenzo Stoakes
In-Reply-To: <20251017090519.46992-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/input/touchscreen/cyttsp4_core.c | 2 +-
drivers/md/dm-integrity.c | 4 ++--
fs/btrfs/tree-checker.c | 2 +-
lib/vsprintf.c | 2 +-
4 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/input/touchscreen/cyttsp4_core.c b/drivers/input/touchscreen/cyttsp4_core.c
index 02a73d9a4def..c10140c9aafa 100644
--- a/drivers/input/touchscreen/cyttsp4_core.c
+++ b/drivers/input/touchscreen/cyttsp4_core.c
@@ -857,7 +857,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/md/dm-integrity.c b/drivers/md/dm-integrity.c
index 7fa3bf74747d..917ba18be77f 100644
--- a/drivers/md/dm-integrity.c
+++ b/drivers/md/dm-integrity.c
@@ -1600,7 +1600,7 @@ static void integrity_metadata(struct work_struct *w)
struct bio *bio = dm_bio_from_per_bio_data(dio, sizeof(struct dm_integrity_io));
char *checksums;
unsigned extra_space = unlikely(digest_size > ic->tag_size) ? digest_size - ic->tag_size : 0;
- char checksums_onstack[max((size_t)HASH_MAX_DIGESTSIZE, MAX_TAG_SIZE)];
+ char checksums_onstack[MAX(HASH_MAX_DIGESTSIZE, MAX_TAG_SIZE)];
sector_t sector;
unsigned sectors_to_process;
@@ -1882,7 +1882,7 @@ static bool __journal_read_write(struct dm_integrity_io *dio, struct bio *bio,
} while (++s < ic->sectors_per_block);
#ifdef INTERNAL_VERIFY
if (ic->internal_hash) {
- char checksums_onstack[max((size_t)HASH_MAX_DIGESTSIZE, MAX_TAG_SIZE)];
+ char checksums_onstack[MAX(HASH_MAX_DIGESTSIZE, MAX_TAG_SIZE)];
integrity_sector_checksum(ic, logical_sector, mem + bv.bv_offset, checksums_onstack);
if (unlikely(memcmp(checksums_onstack, journal_entry_tag(ic, je), ic->tag_size))) {
diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
index c28bb37688c6..fd4768c5e439 100644
--- a/fs/btrfs/tree-checker.c
+++ b/fs/btrfs/tree-checker.c
@@ -587,7 +587,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 b08b8ee1bbc0..90372391ce90 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1078,7 +1078,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 v2 16/27 5.10.y] minmax: make generic MIN() and MAX() macros available everywhere
From: Eliav Farber @ 2025-10-17 9:05 UTC (permalink / raw)
To: gregkh, stable, 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, liviu.dudau, 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, clm,
josef, dsterba, xiang, chao, 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, farbere, sashal, ruanjinjie, David.Laight, herve.codina,
Jason, keescook, kbusch, nathan, bvanassche, ndesaulniers,
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-erofs, linux-ext4, linux-sparse,
linux-mm, netfilter-devel, coreteam, tipc-discussion
Cc: Linus Torvalds, David Laight, Lorenzo Stoakes
In-Reply-To: <20251017090519.46992-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 ++
lib/zstd/zstd_internal.h | 2 --
mm/zsmalloc.c | 1 -
18 files changed, 37 insertions(+), 39 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 b93c33ac8e60..5adba76c3f4d 100644
--- a/drivers/edac/skx_common.h
+++ b/drivers/edac/skx_common.h
@@ -36,7 +36,6 @@
#define I10NM_NUM_CHANNELS 2
#define I10NM_NUM_DIMMS 2
-#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 8f50a038396c..96b03a342f38 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 468efa5ac8fc..3ce87e5f90f7 100644
--- a/drivers/gpu/drm/radeon/evergreen_cs.c
+++ b/drivers/gpu/drm/radeon/evergreen_cs.c
@@ -32,8 +32,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 b4c0f01f52c4..1e0678eb0077 100644
--- a/drivers/hwmon/adt7475.c
+++ b/drivers/hwmon/adt7475.c
@@ -23,23 +23,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 2a569eea4ee8..9a9ca5a8918e 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 4ac8cb262559..b9a58a424933 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 f070e4eb74f4..a66a2ee3a9ed 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 9d7cc62ace2e..8c7594720ef3 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 1a4f2f424996..91a3f4006ae6 100644
--- a/kernel/trace/preemptirq_delay_test.c
+++ b/kernel/trace/preemptirq_delay_test.c
@@ -31,8 +31,6 @@ MODULE_PARM_DESC(burst_size, "The size of a burst (default 1)");
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 b4cf08a5c267..b12f99d4c45c 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 1cf409ef8d04..5b9c7a1bfaf4 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/lib/zstd/zstd_internal.h b/lib/zstd/zstd_internal.h
index dac753397f86..927ed4e8c11c 100644
--- a/lib/zstd/zstd_internal.h
+++ b/lib/zstd/zstd_internal.h
@@ -36,8 +36,6 @@
/*-*************************************
* shared macros
***************************************/
-#define MIN(a, b) ((a) < (b) ? (a) : (b))
-#define MAX(a, b) ((a) > (b) ? (a) : (b))
#define CHECK_F(f) \
{ \
size_t const errcod = f; \
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index f5f80981ac98..bd66c28afb5c 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -126,7 +126,6 @@
#define ISOLATED_BITS 3
#define MAGIC_VAL_BITS 8
-#define MAX(a, b) ((a) >= (b) ? (a) : (b))
/* ZS_MIN_ALLOC_SIZE must be multiple of ZS_ALIGN */
#define ZS_MIN_ALLOC_SIZE \
MAX(32, (ZS_MAX_PAGES_PER_ZSPAGE << PAGE_SHIFT >> OBJ_INDEX_BITS))
--
2.47.3
^ permalink raw reply related
* [PATCH v2 15/27 5.10.y] minmax: simplify and clarify min_t()/max_t() implementation
From: Eliav Farber @ 2025-10-17 9:05 UTC (permalink / raw)
To: gregkh, stable, 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, liviu.dudau, 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, clm,
josef, dsterba, xiang, chao, 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, farbere, sashal, ruanjinjie, David.Laight, herve.codina,
Jason, keescook, kbusch, nathan, bvanassche, ndesaulniers,
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-erofs, linux-ext4, linux-sparse,
linux-mm, netfilter-devel, coreteam, tipc-discussion
Cc: Linus Torvalds, David Laight, Lorenzo Stoakes
In-Reply-To: <20251017090519.46992-1-farbere@amazon.com>
From: Linus Torvalds <torvalds@linux-foundation.org>
[ Upstream commit 017fa3e89187848fd056af757769c9e66ac3e93d ]
This simplifies the min_t() and max_t() macros by no longer making them
work in the context of a C constant expression.
That means that you can no longer use them for static initializers or
for array sizes in type definitions, but there were only a couple of
such uses, and all of them were converted (famous last words) to use
MIN_T/MAX_T instead.
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 | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/include/linux/minmax.h b/include/linux/minmax.h
index a7ef65f78933..9c2848abc804 100644
--- a/include/linux/minmax.h
+++ b/include/linux/minmax.h
@@ -45,17 +45,20 @@
#define __cmp(op, x, y) ((x) __cmp_op_##op (y) ? (x) : (y))
-#define __cmp_once(op, x, y, unique_x, unique_y) ({ \
- typeof(x) unique_x = (x); \
- typeof(y) unique_y = (y); \
+#define __cmp_once_unique(op, type, x, y, ux, uy) \
+ ({ type ux = (x); type uy = (y); __cmp(op, ux, uy); })
+
+#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), \
#op "(" #x ", " #y ") signedness error, fix types or consider u" #op "() before " #op "_t()"); \
- __cmp(op, unique_x, unique_y); })
+ __cmp_once(op, __auto_type, x, y); })
#define __careful_cmp(op, x, y) \
__builtin_choose_expr(__is_constexpr((x) - (y)), \
- __cmp(op, x, y), \
- __cmp_once(op, x, y, __UNIQUE_ID(__x), __UNIQUE_ID(__y)))
+ __cmp(op, x, y), __careful_cmp_once(op, x, y))
#define __clamp(val, lo, hi) \
((val) >= (hi) ? (hi) : ((val) <= (lo) ? (lo) : (val)))
@@ -158,7 +161,7 @@
* @x: first value
* @y: second value
*/
-#define min_t(type, x, y) __careful_cmp(min, (type)(x), (type)(y))
+#define min_t(type, x, y) __cmp_once(min, type, x, y)
/**
* max_t - return maximum of two values, using the specified type
@@ -166,7 +169,7 @@
* @x: first value
* @y: second value
*/
-#define max_t(type, x, y) __careful_cmp(max, (type)(x), (type)(y))
+#define max_t(type, x, y) __cmp_once(max, type, x, y)
/*
* Do not check the array parameter using __must_be_array().
--
2.47.3
^ permalink raw reply related
* [PATCH v2 14/27 5.10.y] minmax: add a few more MIN_T/MAX_T users
From: Eliav Farber @ 2025-10-17 9:05 UTC (permalink / raw)
To: gregkh, stable, 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, liviu.dudau, 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, clm,
josef, dsterba, xiang, chao, 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, farbere, sashal, ruanjinjie, David.Laight, herve.codina,
Jason, keescook, kbusch, nathan, bvanassche, ndesaulniers,
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-erofs, linux-ext4, linux-sparse,
linux-mm, netfilter-devel, coreteam, tipc-discussion
Cc: Linus Torvalds, David Laight, Lorenzo Stoakes
In-Reply-To: <20251017090519.46992-1-farbere@amazon.com>
From: Linus Torvalds <torvalds@linux-foundation.org>
[ Upstream commit 4477b39c32fdc03363affef4b11d48391e6dc9ff ]
Commit 3a7e02c040b1 ("minmax: avoid overly complicated constant
expressions in VM code") added the simpler MIN_T/MAX_T macros in order
to avoid some excessive expansion from the rather complicated regular
min/max macros.
The complexity of those macros stems from two issues:
(a) trying to use them in situations that require a C constant
expression (in static initializers and for array sizes)
(b) the type sanity checking
and MIN_T/MAX_T avoids both of these issues.
Now, in the whole (long) discussion about all this, it was pointed out
that the whole type sanity checking is entirely unnecessary for
min_t/max_t which get a fixed type that the comparison is done in.
But that still leaves min_t/max_t unnecessarily complicated due to
worries about the C constant expression case.
However, it turns out that there really aren't very many cases that use
min_t/max_t for this, and we can just force-convert those.
This does exactly that.
Which in turn will then allow for much simpler implementations of
min_t()/max_t(). All the usual "macros in all upper case will evaluate
the arguments multiple times" rules apply.
We should do all the same things for the regular min/max() vs MIN/MAX()
cases, but that has the added complexity of various drivers defining
their own local versions of MIN/MAX, so that needs another level of
fixes first.
Link: https://lore.kernel.org/all/b47fad1d0cf8449886ad148f8c013dae@AcuMS.aculab.com/
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/x86/mm/pgtable.c | 2 +-
drivers/edac/sb_edac.c | 4 ++--
drivers/gpu/drm/drm_color_mgmt.c | 2 +-
drivers/md/dm-integrity.c | 2 +-
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
fs/erofs/zdata.h | 2 +-
net/ipv4/proc.c | 2 +-
net/ipv6/proc.c | 2 +-
8 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c
index 204b25ee26f0..27e8e3d6be48 100644
--- a/arch/x86/mm/pgtable.c
+++ b/arch/x86/mm/pgtable.c
@@ -107,7 +107,7 @@ static inline void pgd_list_del(pgd_t *pgd)
#define UNSHARED_PTRS_PER_PGD \
(SHARED_KERNEL_PMD ? KERNEL_PGD_BOUNDARY : PTRS_PER_PGD)
#define MAX_UNSHARED_PTRS_PER_PGD \
- max_t(size_t, KERNEL_PGD_BOUNDARY, PTRS_PER_PGD)
+ MAX_T(size_t, KERNEL_PGD_BOUNDARY, PTRS_PER_PGD)
static void pgd_set_mm(pgd_t *pgd, struct mm_struct *mm)
diff --git a/drivers/edac/sb_edac.c b/drivers/edac/sb_edac.c
index 1522d4aa2ca6..714020e7405a 100644
--- a/drivers/edac/sb_edac.c
+++ b/drivers/edac/sb_edac.c
@@ -109,8 +109,8 @@ static const u32 knl_interleave_list[] = {
0x104, 0x10c, 0x114, 0x11c, /* 20-23 */
};
#define MAX_INTERLEAVE \
- (max_t(unsigned int, ARRAY_SIZE(sbridge_interleave_list), \
- max_t(unsigned int, ARRAY_SIZE(ibridge_interleave_list), \
+ (MAX_T(unsigned int, ARRAY_SIZE(sbridge_interleave_list), \
+ MAX_T(unsigned int, ARRAY_SIZE(ibridge_interleave_list), \
ARRAY_SIZE(knl_interleave_list))))
struct interleave_pkg {
diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
index 138ff34b31db..4bc671484c05 100644
--- a/drivers/gpu/drm/drm_color_mgmt.c
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -421,7 +421,7 @@ int drm_plane_create_color_properties(struct drm_plane *plane,
{
struct drm_device *dev = plane->dev;
struct drm_property *prop;
- struct drm_prop_enum_list enum_list[max_t(int, DRM_COLOR_ENCODING_MAX,
+ struct drm_prop_enum_list enum_list[MAX_T(int, DRM_COLOR_ENCODING_MAX,
DRM_COLOR_RANGE_MAX)];
int i, len;
diff --git a/drivers/md/dm-integrity.c b/drivers/md/dm-integrity.c
index 07a7b4e51f0e..7fa3bf74747d 100644
--- a/drivers/md/dm-integrity.c
+++ b/drivers/md/dm-integrity.c
@@ -2431,7 +2431,7 @@ static void do_journal_write(struct dm_integrity_c *ic, unsigned write_start,
unlikely(from_replay) &&
#endif
ic->internal_hash) {
- char test_tag[max_t(size_t, HASH_MAX_DIGESTSIZE, MAX_TAG_SIZE)];
+ char test_tag[MAX_T(size_t, HASH_MAX_DIGESTSIZE, MAX_TAG_SIZE)];
integrity_sector_checksum(ic, sec + ((l - j) << ic->sb->log2_sectors_per_block),
(char *)access_journal_data(ic, i, l), test_tag);
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index b8581a711514..e6fa2782d28f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2267,7 +2267,7 @@ static void stmmac_dma_interrupt(struct stmmac_priv *priv)
u32 channels_to_check = tx_channel_count > rx_channel_count ?
tx_channel_count : rx_channel_count;
u32 chan;
- int status[max_t(u32, MTL_MAX_TX_QUEUES, MTL_MAX_RX_QUEUES)];
+ int status[MAX_T(u32, MTL_MAX_TX_QUEUES, MTL_MAX_RX_QUEUES)];
/* Make sure we never check beyond our status buffer. */
if (WARN_ON_ONCE(channels_to_check > ARRAY_SIZE(status)))
diff --git a/fs/erofs/zdata.h b/fs/erofs/zdata.h
index 68c9b29fc0ca..d10df3f6c700 100644
--- a/fs/erofs/zdata.h
+++ b/fs/erofs/zdata.h
@@ -182,7 +182,7 @@ static inline void z_erofs_onlinepage_endio(struct page *page)
}
#define Z_EROFS_VMAP_ONSTACK_PAGES \
- min_t(unsigned int, THREAD_SIZE / 8 / sizeof(struct page *), 96U)
+ MIN_T(unsigned int, THREAD_SIZE / 8 / sizeof(struct page *), 96U)
#define Z_EROFS_VMAP_GLOBAL_PAGES 2048
#endif
diff --git a/net/ipv4/proc.c b/net/ipv4/proc.c
index 80d13d8f982d..94fbba052b49 100644
--- a/net/ipv4/proc.c
+++ b/net/ipv4/proc.c
@@ -43,7 +43,7 @@
#include <net/sock.h>
#include <net/raw.h>
-#define TCPUDP_MIB_MAX max_t(u32, UDP_MIB_MAX, TCP_MIB_MAX)
+#define TCPUDP_MIB_MAX MAX_T(u32, UDP_MIB_MAX, TCP_MIB_MAX)
/*
* Report socket allocation statistics [mea@utu.fi]
diff --git a/net/ipv6/proc.c b/net/ipv6/proc.c
index bbff3e02e302..929981a8fe98 100644
--- a/net/ipv6/proc.c
+++ b/net/ipv6/proc.c
@@ -27,7 +27,7 @@
#include <net/ipv6.h>
#define MAX4(a, b, c, d) \
- max_t(u32, max_t(u32, a, b), max_t(u32, c, d))
+ MAX_T(u32, MAX_T(u32, a, b), MAX_T(u32, c, d))
#define SNMP_MIB_MAX MAX4(UDP_MIB_MAX, TCP_MIB_MAX, \
IPSTATS_MIB_MAX, ICMP_MIB_MAX)
--
2.47.3
^ permalink raw reply related
* [PATCH v2 12/27 5.10.y] minmax: relax check to allow comparison between unsigned arguments and signed constants
From: Eliav Farber @ 2025-10-17 9:05 UTC (permalink / raw)
To: gregkh, stable, 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, liviu.dudau, 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, clm,
josef, dsterba, xiang, chao, 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, farbere, sashal, ruanjinjie, David.Laight, herve.codina,
Jason, keescook, kbusch, nathan, bvanassche, ndesaulniers,
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-erofs, linux-ext4, linux-sparse,
linux-mm, netfilter-devel, coreteam, tipc-discussion
Cc: Christoph Hellwig, Linus Torvalds
In-Reply-To: <20251017090519.46992-1-farbere@amazon.com>
From: David Laight <David.Laight@ACULAB.COM>
[ Upstream commit 867046cc7027703f60a46339ffde91a1970f2901 ]
Allow (for example) min(unsigned_var, 20).
The opposite min(signed_var, 20u) is still errored.
Since a comparison between signed and unsigned never makes the unsigned
value negative it is only necessary to adjust the __types_ok() test.
Link: https://lkml.kernel.org/r/633b64e2f39e46bb8234809c5595b8c7@AcuMS.aculab.com
Signed-off-by: David Laight <david.laight@aculab.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Jason A. Donenfeld <Jason@zx2c4.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Matthew Wilcox (Oracle) <willy@infradead.org>
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, 17 insertions(+), 7 deletions(-)
diff --git a/include/linux/minmax.h b/include/linux/minmax.h
index 842c1db62ffe..2ec559284a9f 100644
--- a/include/linux/minmax.h
+++ b/include/linux/minmax.h
@@ -10,13 +10,18 @@
/*
* min()/max()/clamp() macros must accomplish three things:
*
- * - avoid multiple evaluations of the arguments (so side-effects like
+ * - Avoid multiple evaluations of the arguments (so side-effects like
* "x++" happen only once) when non-constant.
- * - perform signed v unsigned type-checking (to generate compile
- * errors instead of nasty runtime surprises).
- * - retain result as a constant expressions when called with only
+ * - 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
+ * compared against signed or unsigned arguments.
+ * - Unsigned arguments can be compared against non-negative signed constants.
+ * - Comparison of a signed argument against an unsigned constant fails
+ * even if the constant is below __INT_MAX__ and could be cast to int.
*/
#define __typecheck(x, y) \
(!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
@@ -26,9 +31,14 @@
__builtin_choose_expr(__is_constexpr(is_signed_type(typeof(x))), \
is_signed_type(typeof(x)), 0)
-#define __types_ok(x, y) \
- (__is_signed(x) == __is_signed(y) || \
- __is_signed((x) + 0) == __is_signed((y) + 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) \
+ (__is_signed(x) == __is_signed(y) || \
+ __is_signed((x) + 0) == __is_signed((y) + 0) || \
+ __is_noneg_int(x) || __is_noneg_int(y))
#define __cmp_op_min <
#define __cmp_op_max >
--
2.47.3
^ permalink raw reply related
* [PATCH v2 13/27 5.10.y] minmax: avoid overly complicated constant expressions in VM code
From: Eliav Farber @ 2025-10-17 9:05 UTC (permalink / raw)
To: gregkh, stable, 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, liviu.dudau, 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, clm,
josef, dsterba, xiang, chao, 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, farbere, sashal, ruanjinjie, David.Laight, herve.codina,
Jason, keescook, kbusch, nathan, bvanassche, ndesaulniers,
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-erofs, linux-ext4, linux-sparse,
linux-mm, netfilter-devel, coreteam, tipc-discussion
Cc: Linus Torvalds, Lorenzo Stoakes, David Laight
In-Reply-To: <20251017090519.46992-1-farbere@amazon.com>
From: Linus Torvalds <torvalds@linux-foundation.org>
[ Upstream commit 3a7e02c040b130b5545e4b115aada7bacd80a2b6 ]
The minmax infrastructure is overkill for simple constants, and can
cause huge expansions because those simple constants are then used by
other things.
For example, 'pageblock_order' is a core VM constant, but because it was
implemented using 'min_t()' and all the type-checking that involves, it
actually expanded to something like 2.5kB of preprocessor noise.
And when that simple constant was then used inside other expansions:
#define pageblock_nr_pages (1UL << pageblock_order)
#define pageblock_start_pfn(pfn) ALIGN_DOWN((pfn), pageblock_nr_pages)
and we then use that inside a 'max()' macro:
case ISOLATE_SUCCESS:
update_cached = false;
last_migrated_pfn = max(cc->zone->zone_start_pfn,
pageblock_start_pfn(cc->migrate_pfn - 1));
the end result was that one statement expanding to 253kB in size.
There are probably other cases of this, but this one case certainly
stood out.
I've added 'MIN_T()' and 'MAX_T()' macros for this kind of "core simple
constant with specific type" use. These macros skip the type checking,
and as such need to be very sparingly used only for obvious cases that
have active issues like this.
Reported-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Link: https://lore.kernel.org/all/36aa2cad-1db1-4abf-8dd2-fb20484aabc3@lucifer.local/
Cc: David Laight <David.Laight@aculab.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Eliav Farber <farbere@amazon.com>
---
include/linux/minmax.h | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/include/linux/minmax.h b/include/linux/minmax.h
index 2ec559284a9f..a7ef65f78933 100644
--- a/include/linux/minmax.h
+++ b/include/linux/minmax.h
@@ -270,4 +270,11 @@ static inline bool in_range32(u32 val, u32 start, u32 len)
#define swap(a, b) \
do { typeof(a) __tmp = (a); (a) = (b); (b) = __tmp; } while (0)
+/*
+ * Use these carefully: no type checking, and uses the arguments
+ * multiple times. Use for obvious constants only.
+ */
+#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 v2 11/27 5.10.y] minmax: allow comparisons of 'int' against 'unsigned char/short'
From: Eliav Farber @ 2025-10-17 9:05 UTC (permalink / raw)
To: gregkh, stable, 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, liviu.dudau, 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, clm,
josef, dsterba, xiang, chao, 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, farbere, sashal, ruanjinjie, David.Laight, herve.codina,
Jason, keescook, kbusch, nathan, bvanassche, ndesaulniers,
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-erofs, linux-ext4, linux-sparse,
linux-mm, netfilter-devel, coreteam, tipc-discussion
Cc: Christoph Hellwig, Linus Torvalds
In-Reply-To: <20251017090519.46992-1-farbere@amazon.com>
From: David Laight <David.Laight@ACULAB.COM>
[ Upstream commit 4ead534fba42fc4fd41163297528d2aa731cd121 ]
Since 'unsigned char/short' get promoted to 'signed int' it is safe to
compare them against an 'int' value.
Link: https://lkml.kernel.org/r/8732ef5f809c47c28a7be47c938b28d4@AcuMS.aculab.com
Signed-off-by: David Laight <david.laight@aculab.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Jason A. Donenfeld <Jason@zx2c4.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Matthew Wilcox (Oracle) <willy@infradead.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Eliav Farber <farbere@amazon.com>
---
include/linux/minmax.h | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/include/linux/minmax.h b/include/linux/minmax.h
index c0e738eacefa..842c1db62ffe 100644
--- a/include/linux/minmax.h
+++ b/include/linux/minmax.h
@@ -26,8 +26,9 @@
__builtin_choose_expr(__is_constexpr(is_signed_type(typeof(x))), \
is_signed_type(typeof(x)), 0)
-#define __types_ok(x, y) \
- (__is_signed(x) == __is_signed(y))
+#define __types_ok(x, y) \
+ (__is_signed(x) == __is_signed(y) || \
+ __is_signed((x) + 0) == __is_signed((y) + 0))
#define __cmp_op_min <
#define __cmp_op_max >
--
2.47.3
^ permalink raw reply related
* [PATCH v2 10/27 5.10.y] minmax: fix indentation of __cmp_once() and __clamp_once()
From: Eliav Farber @ 2025-10-17 9:05 UTC (permalink / raw)
To: gregkh, stable, 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, liviu.dudau, 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, clm,
josef, dsterba, xiang, chao, 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, farbere, sashal, ruanjinjie, David.Laight, herve.codina,
Jason, keescook, kbusch, nathan, bvanassche, ndesaulniers,
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-erofs, linux-ext4, linux-sparse,
linux-mm, netfilter-devel, coreteam, tipc-discussion
Cc: Christoph Hellwig, Linus Torvalds
In-Reply-To: <20251017090519.46992-1-farbere@amazon.com>
From: David Laight <David.Laight@ACULAB.COM>
[ Upstream commit f4b84b2ff851f01d0fac619eadef47eb41648534 ]
Remove the extra indentation and align continuation markers.
Link: https://lkml.kernel.org/r/bed41317a05c498ea0209eafbcab45a5@AcuMS.aculab.com
Signed-off-by: David Laight <david.laight@aculab.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Jason A. Donenfeld <Jason@zx2c4.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Matthew Wilcox (Oracle) <willy@infradead.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Eliav Farber <farbere@amazon.com>
---
include/linux/minmax.h | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/include/linux/minmax.h b/include/linux/minmax.h
index 8718fd71a793..c0e738eacefa 100644
--- a/include/linux/minmax.h
+++ b/include/linux/minmax.h
@@ -35,11 +35,11 @@
#define __cmp(op, x, y) ((x) __cmp_op_##op (y) ? (x) : (y))
#define __cmp_once(op, x, y, unique_x, unique_y) ({ \
- typeof(x) unique_x = (x); \
- typeof(y) unique_y = (y); \
- static_assert(__types_ok(x, y), \
- #op "(" #x ", " #y ") signedness error, fix types or consider u" #op "() before " #op "_t()"); \
- __cmp(op, unique_x, unique_y); })
+ typeof(x) unique_x = (x); \
+ typeof(y) unique_y = (y); \
+ static_assert(__types_ok(x, y), \
+ #op "(" #x ", " #y ") signedness error, fix types or consider u" #op "() before " #op "_t()"); \
+ __cmp(op, unique_x, unique_y); })
#define __careful_cmp(op, x, y) \
__builtin_choose_expr(__is_constexpr((x) - (y)), \
@@ -49,16 +49,16 @@
#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); \
- 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 __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); \
+ 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)), \
--
2.47.3
^ permalink raw reply related
* [PATCH v2 09/27 5.10.y] minmax: allow min()/max()/clamp() if the arguments have the same signedness.
From: Eliav Farber @ 2025-10-17 9:05 UTC (permalink / raw)
To: gregkh, stable, 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, liviu.dudau, 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, clm,
josef, dsterba, xiang, chao, 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, farbere, sashal, ruanjinjie, David.Laight, herve.codina,
Jason, keescook, kbusch, nathan, bvanassche, ndesaulniers,
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-erofs, linux-ext4, linux-sparse,
linux-mm, netfilter-devel, coreteam, tipc-discussion
Cc: Christoph Hellwig, Linus Torvalds
In-Reply-To: <20251017090519.46992-1-farbere@amazon.com>
From: David Laight <David.Laight@ACULAB.COM>
[ Upstream commit d03eba99f5bf7cbc6e2fdde3b6fa36954ad58e09 ]
The type-check in min()/max() is there to stop unexpected results if a
negative value gets converted to a large unsigned value. However it also
rejects 'unsigned int' v 'unsigned long' compares which are common and
never problematc.
Replace the 'same type' check with a 'same signedness' check.
The new test isn't itself a compile time error, so use static_assert() to
report the error and give a meaningful error message.
Due to the way builtin_choose_expr() works detecting the error in the
'non-constant' side (where static_assert() can be used) also detects
errors when the arguments are constant.
Link: https://lkml.kernel.org/r/fe7e6c542e094bfca655abcd323c1c98@AcuMS.aculab.com
Signed-off-by: David Laight <david.laight@aculab.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Jason A. Donenfeld <Jason@zx2c4.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Matthew Wilcox (Oracle) <willy@infradead.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Eliav Farber <farbere@amazon.com>
---
include/linux/minmax.h | 60 ++++++++++++++++++++++--------------------
1 file changed, 32 insertions(+), 28 deletions(-)
diff --git a/include/linux/minmax.h b/include/linux/minmax.h
index 2a197f54fe05..8718fd71a793 100644
--- a/include/linux/minmax.h
+++ b/include/linux/minmax.h
@@ -12,9 +12,8 @@
*
* - avoid multiple evaluations of the arguments (so side-effects like
* "x++" happen only once) when non-constant.
- * - perform strict type-checking (to generate warnings instead of
- * nasty runtime surprises). See the "unnecessary" pointer comparison
- * in __typecheck().
+ * - perform signed v unsigned type-checking (to generate compile
+ * errors instead of nasty runtime surprises).
* - retain result as a constant expressions when called with only
* constant expressions (to avoid tripping VLA warnings in stack
* allocation usage).
@@ -22,23 +21,30 @@
#define __typecheck(x, y) \
(!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
-#define __no_side_effects(x, y) \
- (__is_constexpr(x) && __is_constexpr(y))
+/* 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)
-#define __safe_cmp(x, y) \
- (__typecheck(x, y) && __no_side_effects(x, y))
+#define __types_ok(x, y) \
+ (__is_signed(x) == __is_signed(y))
-#define __cmp(x, y, op) ((x) op (y) ? (x) : (y))
+#define __cmp_op_min <
+#define __cmp_op_max >
-#define __cmp_once(x, y, unique_x, unique_y, op) ({ \
+#define __cmp(op, x, y) ((x) __cmp_op_##op (y) ? (x) : (y))
+
+#define __cmp_once(op, x, y, unique_x, unique_y) ({ \
typeof(x) unique_x = (x); \
typeof(y) unique_y = (y); \
- __cmp(unique_x, unique_y, op); })
+ static_assert(__types_ok(x, y), \
+ #op "(" #x ", " #y ") signedness error, fix types or consider u" #op "() before " #op "_t()"); \
+ __cmp(op, unique_x, unique_y); })
-#define __careful_cmp(x, y, op) \
- __builtin_choose_expr(__safe_cmp(x, y), \
- __cmp(x, y, op), \
- __cmp_once(x, y, __UNIQUE_ID(__x), __UNIQUE_ID(__y), op))
+#define __careful_cmp(op, x, y) \
+ __builtin_choose_expr(__is_constexpr((x) - (y)), \
+ __cmp(op, x, y), \
+ __cmp_once(op, x, y, __UNIQUE_ID(__x), __UNIQUE_ID(__y)))
#define __clamp(val, lo, hi) \
((val) >= (hi) ? (hi) : ((val) <= (lo) ? (lo) : (val)))
@@ -47,17 +53,15 @@
typeof(val) unique_val = (val); \
typeof(lo) unique_lo = (lo); \
typeof(hi) unique_hi = (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 __clamp_input_check(lo, hi) \
- (BUILD_BUG_ON_ZERO(__builtin_choose_expr( \
- __is_constexpr((lo) > (hi)), (lo) > (hi), false)))
-
#define __careful_clamp(val, lo, hi) ({ \
- __clamp_input_check(lo, hi) + \
- __builtin_choose_expr(__typecheck(val, lo) && __typecheck(val, hi) && \
- __typecheck(hi, lo) && __is_constexpr(val) && \
- __is_constexpr(lo) && __is_constexpr(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))); })
@@ -67,14 +71,14 @@
* @x: first value
* @y: second value
*/
-#define min(x, y) __careful_cmp(x, y, <)
+#define min(x, y) __careful_cmp(min, x, y)
/**
* max - return maximum of two values of the same or compatible types
* @x: first value
* @y: second value
*/
-#define max(x, y) __careful_cmp(x, y, >)
+#define max(x, y) __careful_cmp(max, x, y)
/**
* umin - return minimum of two non-negative values
@@ -83,7 +87,7 @@
* @y: second value
*/
#define umin(x, y) \
- __careful_cmp((x) + 0u + 0ul + 0ull, (y) + 0u + 0ul + 0ull, <)
+ __careful_cmp(min, (x) + 0u + 0ul + 0ull, (y) + 0u + 0ul + 0ull)
/**
* umax - return maximum of two non-negative values
@@ -91,7 +95,7 @@
* @y: second value
*/
#define umax(x, y) \
- __careful_cmp((x) + 0u + 0ul + 0ull, (y) + 0u + 0ul + 0ull, >)
+ __careful_cmp(max, (x) + 0u + 0ul + 0ull, (y) + 0u + 0ul + 0ull)
/**
* min3 - return minimum of three values
@@ -143,7 +147,7 @@
* @x: first value
* @y: second value
*/
-#define min_t(type, x, y) __careful_cmp((type)(x), (type)(y), <)
+#define min_t(type, x, y) __careful_cmp(min, (type)(x), (type)(y))
/**
* max_t - return maximum of two values, using the specified type
@@ -151,7 +155,7 @@
* @x: first value
* @y: second value
*/
-#define max_t(type, x, y) __careful_cmp((type)(x), (type)(y), >)
+#define max_t(type, x, y) __careful_cmp(max, (type)(x), (type)(y))
/*
* Do not check the array parameter using __must_be_array().
--
2.47.3
^ permalink raw reply related
* [PATCH v2 08/27 5.10.y] minmax: fix header inclusions
From: Eliav Farber @ 2025-10-17 9:05 UTC (permalink / raw)
To: gregkh, stable, 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, liviu.dudau, 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, clm,
josef, dsterba, xiang, chao, 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, farbere, sashal, ruanjinjie, David.Laight, herve.codina,
Jason, keescook, kbusch, nathan, bvanassche, ndesaulniers,
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-erofs, linux-ext4, linux-sparse,
linux-mm, netfilter-devel, coreteam, tipc-discussion
In-Reply-To: <20251017090519.46992-1-farbere@amazon.com>
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
[ Upstream commit f6e9d38f8eb00ac8b52e6d15f6aa9bcecacb081b ]
BUILD_BUG_ON*() macros are defined in build_bug.h. Include it. Replace
compiler_types.h by compiler.h, which provides the former, to have a
definition of the __UNIQUE_ID().
Link: https://lkml.kernel.org/r/20230912092355.79280-1-andriy.shevchenko@linux.intel.com
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Herve Codina <herve.codina@bootlin.com>
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
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, 2 insertions(+), 1 deletion(-)
diff --git a/include/linux/minmax.h b/include/linux/minmax.h
index c813c1187510..2a197f54fe05 100644
--- a/include/linux/minmax.h
+++ b/include/linux/minmax.h
@@ -2,7 +2,8 @@
#ifndef _LINUX_MINMAX_H
#define _LINUX_MINMAX_H
-#include <linux/compiler_types.h>
+#include <linux/build_bug.h>
+#include <linux/compiler.h>
#include <linux/const.h>
#include <linux/types.h>
--
2.47.3
^ permalink raw reply related
* [PATCH v2 07/27 5.10.y] minmax: deduplicate __unconst_integer_typeof()
From: Eliav Farber @ 2025-10-17 9:04 UTC (permalink / raw)
To: gregkh, stable, 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, liviu.dudau, 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, clm,
josef, dsterba, xiang, chao, 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, farbere, sashal, ruanjinjie, David.Laight, herve.codina,
Jason, keescook, kbusch, nathan, bvanassche, ndesaulniers,
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-erofs, linux-ext4, linux-sparse,
linux-mm, netfilter-devel, coreteam, tipc-discussion
In-Reply-To: <20251017090519.46992-1-farbere@amazon.com>
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
[ Upstream commit 5e57418a2031cd5e1863efdf3d7447a16a368172 ]
It appears that compiler_types.h already have an implementation of the
__unconst_integer_typeof() called __unqual_scalar_typeof(). Use it
instead of the copy.
Link: https://lkml.kernel.org/r/20230911154913.4176033-1-andriy.shevchenko@linux.intel.com
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Herve Codina <herve.codina@bootlin.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Eliav Farber <farbere@amazon.com>
---
include/linux/minmax.h | 26 +++-----------------------
1 file changed, 3 insertions(+), 23 deletions(-)
diff --git a/include/linux/minmax.h b/include/linux/minmax.h
index 0e89c78810f6..c813c1187510 100644
--- a/include/linux/minmax.h
+++ b/include/linux/minmax.h
@@ -2,6 +2,7 @@
#ifndef _LINUX_MINMAX_H
#define _LINUX_MINMAX_H
+#include <linux/compiler_types.h>
#include <linux/const.h>
#include <linux/types.h>
@@ -151,27 +152,6 @@
*/
#define max_t(type, x, y) __careful_cmp((type)(x), (type)(y), >)
-/*
- * Remove a const qualifier from integer types
- * _Generic(foo, type-name: association, ..., default: association) performs a
- * comparison against the foo type (not the qualified type).
- * Do not use the const keyword in the type-name as it will not match the
- * unqualified type of foo.
- */
-#define __unconst_integer_type_cases(type) \
- unsigned type: (unsigned type)0, \
- signed type: (signed type)0
-
-#define __unconst_integer_typeof(x) typeof( \
- _Generic((x), \
- char: (char)0, \
- __unconst_integer_type_cases(char), \
- __unconst_integer_type_cases(short), \
- __unconst_integer_type_cases(int), \
- __unconst_integer_type_cases(long), \
- __unconst_integer_type_cases(long long), \
- default: (x)))
-
/*
* Do not check the array parameter using __must_be_array().
* In the following legit use-case where the "array" passed is a simple pointer,
@@ -186,13 +166,13 @@
* 'int *buff' and 'int buff[N]' types.
*
* The array can be an array of const items.
- * typeof() keeps the const qualifier. Use __unconst_integer_typeof() in order
+ * typeof() keeps the const qualifier. Use __unqual_scalar_typeof() in order
* to discard the const qualifier for the __element variable.
*/
#define __minmax_array(op, array, len) ({ \
typeof(&(array)[0]) __array = (array); \
typeof(len) __len = (len); \
- __unconst_integer_typeof(__array[0]) __element = __array[--__len]; \
+ __unqual_scalar_typeof(__array[0]) __element = __array[--__len];\
while (__len--) \
__element = op(__element, __array[__len]); \
__element; })
--
2.47.3
^ permalink raw reply related
* [PATCH v2 06/27 5.10.y] minmax: Introduce {min,max}_array()
From: Eliav Farber @ 2025-10-17 9:04 UTC (permalink / raw)
To: gregkh, stable, 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, liviu.dudau, 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, clm,
josef, dsterba, xiang, chao, 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, farbere, sashal, ruanjinjie, David.Laight, herve.codina,
Jason, keescook, kbusch, nathan, bvanassche, ndesaulniers,
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-erofs, linux-ext4, linux-sparse,
linux-mm, netfilter-devel, coreteam, tipc-discussion
Cc: Andy Shevchenko, Christophe Leroy
In-Reply-To: <20251017090519.46992-1-farbere@amazon.com>
From: Herve Codina <herve.codina@bootlin.com>
[ Upstream commit c952c748c7a983a8bda9112984e6f2c1f6e441a5 ]
Introduce min_array() (resp max_array()) in order to get the
minimal (resp maximum) of values present in an array.
Signed-off-by: Herve Codina <herve.codina@bootlin.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Link: https://lore.kernel.org/r/20230623085830.749991-8-herve.codina@bootlin.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Eliav Farber <farbere@amazon.com>
---
include/linux/minmax.h | 64 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 64 insertions(+)
diff --git a/include/linux/minmax.h b/include/linux/minmax.h
index 7affadcb2a29..0e89c78810f6 100644
--- a/include/linux/minmax.h
+++ b/include/linux/minmax.h
@@ -151,6 +151,70 @@
*/
#define max_t(type, x, y) __careful_cmp((type)(x), (type)(y), >)
+/*
+ * Remove a const qualifier from integer types
+ * _Generic(foo, type-name: association, ..., default: association) performs a
+ * comparison against the foo type (not the qualified type).
+ * Do not use the const keyword in the type-name as it will not match the
+ * unqualified type of foo.
+ */
+#define __unconst_integer_type_cases(type) \
+ unsigned type: (unsigned type)0, \
+ signed type: (signed type)0
+
+#define __unconst_integer_typeof(x) typeof( \
+ _Generic((x), \
+ char: (char)0, \
+ __unconst_integer_type_cases(char), \
+ __unconst_integer_type_cases(short), \
+ __unconst_integer_type_cases(int), \
+ __unconst_integer_type_cases(long), \
+ __unconst_integer_type_cases(long long), \
+ default: (x)))
+
+/*
+ * Do not check the array parameter using __must_be_array().
+ * In the following legit use-case where the "array" passed is a simple pointer,
+ * __must_be_array() will return a failure.
+ * --- 8< ---
+ * int *buff
+ * ...
+ * min = min_array(buff, nb_items);
+ * --- 8< ---
+ *
+ * The first typeof(&(array)[0]) is needed in order to support arrays of both
+ * 'int *buff' and 'int buff[N]' types.
+ *
+ * The array can be an array of const items.
+ * typeof() keeps the const qualifier. Use __unconst_integer_typeof() in order
+ * to discard the const qualifier for the __element variable.
+ */
+#define __minmax_array(op, array, len) ({ \
+ typeof(&(array)[0]) __array = (array); \
+ typeof(len) __len = (len); \
+ __unconst_integer_typeof(__array[0]) __element = __array[--__len]; \
+ while (__len--) \
+ __element = op(__element, __array[__len]); \
+ __element; })
+
+/**
+ * min_array - return minimum of values present in an array
+ * @array: array
+ * @len: array length
+ *
+ * Note that @len must not be zero (empty array).
+ */
+#define min_array(array, len) __minmax_array(min, array, len)
+
+/**
+ * max_array - return maximum of values present in an array
+ * @array: array
+ * @len: array length
+ *
+ * Note that @len must not be zero (empty array).
+ */
+#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
--
2.47.3
^ permalink raw reply related
* [PATCH v2 05/27 5.10.y] minmax: add in_range() macro
From: Eliav Farber @ 2025-10-17 9:04 UTC (permalink / raw)
To: gregkh, stable, 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, liviu.dudau, 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, clm,
josef, dsterba, xiang, chao, 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, farbere, sashal, ruanjinjie, David.Laight, herve.codina,
Jason, keescook, kbusch, nathan, bvanassche, ndesaulniers,
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-erofs, linux-ext4, linux-sparse,
linux-mm, netfilter-devel, coreteam, tipc-discussion
In-Reply-To: <20251017090519.46992-1-farbere@amazon.com>
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 ++++++++---------
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
^ permalink raw reply related
* [PATCH v2 04/27 5.10.y] minmax: clamp more efficiently by avoiding extra comparison
From: Eliav Farber @ 2025-10-17 9:04 UTC (permalink / raw)
To: gregkh, stable, 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, liviu.dudau, 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, clm,
josef, dsterba, xiang, chao, 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, farbere, sashal, ruanjinjie, David.Laight, herve.codina,
Jason, keescook, kbusch, nathan, bvanassche, ndesaulniers,
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-erofs, linux-ext4, linux-sparse,
linux-mm, netfilter-devel, coreteam, tipc-discussion
In-Reply-To: <20251017090519.46992-1-farbere@amazon.com>
From: "Jason A. Donenfeld" <Jason@zx2c4.com>
[ Upstream commit 2122e2a4efc2cd139474079e11939b6e07adfacd ]
Currently the clamp algorithm does:
if (val > hi)
val = hi;
if (val < lo)
val = lo;
But since hi > lo by definition, this can be made more efficient with:
if (val > hi)
val = hi;
else if (val < lo)
val = lo;
So fix up the clamp and clamp_t functions to do this, adding the same
argument checking as for min and min_t.
For simple cases, code generation on x86_64 and aarch64 stay about the
same:
before:
cmp edi, edx
mov eax, esi
cmova edi, edx
cmp edi, esi
cmovnb eax, edi
ret
after:
cmp edi, esi
mov eax, edx
cmovnb esi, edi
cmp edi, edx
cmovb eax, esi
ret
before:
cmp w0, w2
csel w8, w0, w2, lo
cmp w8, w1
csel w0, w8, w1, hi
ret
after:
cmp w0, w1
csel w8, w0, w1, hi
cmp w0, w2
csel w0, w8, w2, lo
ret
On MIPS64, however, code generation improves, by removing arithmetic in
the second branch:
before:
sltu $3,$6,$4
bne $3,$0,.L2
move $2,$6
move $2,$4
.L2:
sltu $3,$2,$5
bnel $3,$0,.L7
move $2,$5
.L7:
jr $31
nop
after:
sltu $3,$4,$6
beq $3,$0,.L13
move $2,$6
sltu $3,$4,$5
bne $3,$0,.L12
move $2,$4
.L13:
jr $31
nop
.L12:
jr $31
move $2,$5
For more complex cases with surrounding code, the effects are a bit
more complicated. For example, consider this simplified version of
timestamp_truncate() from fs/inode.c on x86_64:
struct timespec64 timestamp_truncate(struct timespec64 t, struct inode *inode)
{
struct super_block *sb = inode->i_sb;
unsigned int gran = sb->s_time_gran;
t.tv_sec = clamp(t.tv_sec, sb->s_time_min, sb->s_time_max);
if (t.tv_sec == sb->s_time_max || t.tv_sec == sb->s_time_min)
t.tv_nsec = 0;
return t;
}
before:
mov r8, rdx
mov rdx, rsi
mov rcx, QWORD PTR [r8]
mov rax, QWORD PTR [rcx+8]
mov rcx, QWORD PTR [rcx+16]
cmp rax, rdi
mov r8, rcx
cmovge rdi, rax
cmp rdi, rcx
cmovle r8, rdi
cmp rax, r8
je .L4
cmp rdi, rcx
jge .L4
mov rax, r8
ret
.L4:
xor edx, edx
mov rax, r8
ret
after:
mov rax, QWORD PTR [rdx]
mov rdx, QWORD PTR [rax+8]
mov rax, QWORD PTR [rax+16]
cmp rax, rdi
jg .L6
mov r8, rax
xor edx, edx
.L2:
mov rax, r8
ret
.L6:
cmp rdx, rdi
mov r8, rdi
cmovge r8, rdx
cmp rax, r8
je .L4
xor eax, eax
cmp rdx, rdi
cmovl rax, rsi
mov rdx, rax
mov rax, r8
ret
.L4:
xor edx, edx
jmp .L2
In this case, we actually gain a branch, unfortunately, because the
compiler's replacement axioms no longer as cleanly apply.
So all and all, this change is a bit of a mixed bag.
Link: https://lkml.kernel.org/r/20220926133435.1333846-2-Jason@zx2c4.com
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Kees Cook <keescook@chromium.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Eliav Farber <farbere@amazon.com>
---
include/linux/minmax.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/minmax.h b/include/linux/minmax.h
index 8b092c66c5aa..abdeae409dad 100644
--- a/include/linux/minmax.h
+++ b/include/linux/minmax.h
@@ -38,7 +38,7 @@
__cmp_once(x, y, __UNIQUE_ID(__x), __UNIQUE_ID(__y), op))
#define __clamp(val, lo, hi) \
- __cmp(__cmp(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); \
--
2.47.3
^ permalink raw reply related
* [PATCH v2 03/27 5.10.y] minmax: sanity check constant bounds when clamping
From: Eliav Farber @ 2025-10-17 9:04 UTC (permalink / raw)
To: gregkh, stable, 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, liviu.dudau, 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, clm,
josef, dsterba, xiang, chao, 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, farbere, sashal, ruanjinjie, David.Laight, herve.codina,
Jason, keescook, kbusch, nathan, bvanassche, ndesaulniers,
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-erofs, linux-ext4, linux-sparse,
linux-mm, netfilter-devel, coreteam, tipc-discussion
In-Reply-To: <20251017090519.46992-1-farbere@amazon.com>
From: "Jason A. Donenfeld" <Jason@zx2c4.com>
[ Upstream commit 5efcecd9a3b18078d3398b359a84c83f549e22cf ]
The clamp family of functions only makes sense if hi>=lo. If hi and lo
are compile-time constants, then raise a build error. Doing so has
already caught buggy code. This also introduces the infrastructure to
improve the clamping function in subsequent commits.
[akpm@linux-foundation.org: coding-style cleanups]
[akpm@linux-foundation.org: s@&&\@&& \@]
Link: https://lkml.kernel.org/r/20220926133435.1333846-1-Jason@zx2c4.com
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Kees Cook <keescook@chromium.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Eliav Farber <farbere@amazon.com>
---
include/linux/minmax.h | 26 ++++++++++++++++++++++++--
1 file changed, 24 insertions(+), 2 deletions(-)
diff --git a/include/linux/minmax.h b/include/linux/minmax.h
index 1aea34b8f19b..8b092c66c5aa 100644
--- a/include/linux/minmax.h
+++ b/include/linux/minmax.h
@@ -37,6 +37,28 @@
__cmp(x, y, op), \
__cmp_once(x, y, __UNIQUE_ID(__x), __UNIQUE_ID(__y), op))
+#define __clamp(val, lo, hi) \
+ __cmp(__cmp(val, lo, >), hi, <)
+
+#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); \
+ __clamp(unique_val, unique_lo, unique_hi); })
+
+#define __clamp_input_check(lo, hi) \
+ (BUILD_BUG_ON_ZERO(__builtin_choose_expr( \
+ __is_constexpr((lo) > (hi)), (lo) > (hi), false)))
+
+#define __careful_clamp(val, lo, hi) ({ \
+ __clamp_input_check(lo, hi) + \
+ __builtin_choose_expr(__typecheck(val, lo) && __typecheck(val, hi) && \
+ __typecheck(hi, lo) && __is_constexpr(val) && \
+ __is_constexpr(lo) && __is_constexpr(hi), \
+ __clamp(val, lo, hi), \
+ __clamp_once(val, lo, hi, __UNIQUE_ID(__val), \
+ __UNIQUE_ID(__lo), __UNIQUE_ID(__hi))); })
+
/**
* min - return minimum of two values of the same or compatible types
* @x: first value
@@ -103,7 +125,7 @@
* This macro does strict typechecking of @lo/@hi to make sure they are of the
* same type as @val. See the unnecessary pointer comparisons.
*/
-#define clamp(val, lo, hi) min((typeof(val))max(val, lo), hi)
+#define clamp(val, lo, hi) __careful_clamp(val, lo, hi)
/*
* ..and if you can't take the strict
@@ -138,7 +160,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) min_t(type, max_t(type, val, lo), hi)
+#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
--
2.47.3
^ permalink raw reply related
* [PATCH v2 02/27 5.10.y] btrfs: remove duplicated in_range() macro
From: Eliav Farber @ 2025-10-17 9:04 UTC (permalink / raw)
To: gregkh, stable, 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, liviu.dudau, 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, clm,
josef, dsterba, xiang, chao, 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, farbere, sashal, ruanjinjie, David.Laight, herve.codina,
Jason, keescook, kbusch, nathan, bvanassche, ndesaulniers,
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-erofs, linux-ext4, linux-sparse,
linux-mm, netfilter-devel, coreteam, tipc-discussion
Cc: Johannes Thumshirn
In-Reply-To: <20251017090519.46992-1-farbere@amazon.com>
From: Johannes Thumshirn <johannes.thumshirn@wdc.com>
[ Upstream commit cea628008fc8c6c9c7b53902f6659e040f33c790 ]
The in_range() macro is defined twice in btrfs' source, once in ctree.h
and once in misc.h.
Remove the definition in ctree.h and include misc.h in the files depending
on it.
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Signed-off-by: Eliav Farber <farbere@amazon.com>
---
fs/btrfs/ctree.h | 2 --
fs/btrfs/extent_io.c | 1 +
fs/btrfs/file-item.c | 1 +
fs/btrfs/raid56.c | 1 +
4 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index d9d6a57acafe..a9926fb10c49 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3597,8 +3597,6 @@ static inline int btrfs_defrag_cancelled(struct btrfs_fs_info *fs_info)
return signal_pending(current);
}
-#define in_range(b, first, len) ((b) >= (first) && (b) < (first) + (len))
-
/* Sanity test specific functions */
#ifdef CONFIG_BTRFS_FS_RUN_SANITY_TESTS
void btrfs_test_destroy_inode(struct inode *inode);
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 8498994ef5c6..489d370ddd60 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -13,6 +13,7 @@
#include <linux/pagevec.h>
#include <linux/prefetch.h>
#include <linux/cleancache.h>
+#include "misc.h"
#include "extent_io.h"
#include "extent-io-tree.h"
#include "extent_map.h"
diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
index cbea4f572155..6e46da3ee433 100644
--- a/fs/btrfs/file-item.c
+++ b/fs/btrfs/file-item.c
@@ -9,6 +9,7 @@
#include <linux/highmem.h>
#include <linux/sched/mm.h>
#include <crypto/hash.h>
+#include "misc.h"
#include "ctree.h"
#include "disk-io.h"
#include "transaction.h"
diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c
index 9678d7fa4dcc..ed3e40a4a3cb 100644
--- a/fs/btrfs/raid56.c
+++ b/fs/btrfs/raid56.c
@@ -13,6 +13,7 @@
#include <linux/list_sort.h>
#include <linux/raid/xor.h>
#include <linux/mm.h>
+#include "misc.h"
#include "ctree.h"
#include "disk-io.h"
#include "volumes.h"
--
2.47.3
^ permalink raw reply related
* [PATCH v2 01/27 5.10.y] overflow, tracing: Define the is_signed_type() macro once
From: Eliav Farber @ 2025-10-17 9:04 UTC (permalink / raw)
To: gregkh, stable, 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, liviu.dudau, 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, clm,
josef, dsterba, xiang, chao, 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, farbere, sashal, ruanjinjie, David.Laight, herve.codina,
Jason, keescook, kbusch, nathan, bvanassche, ndesaulniers,
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-erofs, linux-ext4, linux-sparse,
linux-mm, netfilter-devel, coreteam, tipc-discussion
Cc: Arnd Bergmann, Dan Williams, Eric Dumazet, Isabella Basso,
Josh Poimboeuf, Masami Hiramatsu, Sander Vanheule,
Vlastimil Babka, Yury Norov
In-Reply-To: <20251017090519.46992-1-farbere@amazon.com>
From: Bart Van Assche <bvanassche@acm.org>
[ Upstream commit 92d23c6e94157739b997cacce151586a0d07bb8a ]
There are two definitions of the is_signed_type() macro: one in
<linux/overflow.h> and a second definition in <linux/trace_events.h>.
As suggested by Linus Torvalds, move the definition of the
is_signed_type() macro into the <linux/compiler.h> header file. Change
the definition of the is_signed_type() macro to make sure that it does
not trigger any sparse warnings with future versions of sparse for
bitwise types. See also:
https://lore.kernel.org/all/CAHk-=whjH6p+qzwUdx5SOVVHjS3WvzJQr6mDUwhEyTf6pJWzaQ@mail.gmail.com/
https://lore.kernel.org/all/CAHk-=wjQGnVfb4jehFR0XyZikdQvCZouE96xR_nnf5kqaM5qqQ@mail.gmail.com/
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Isabella Basso <isabbasso@riseup.net>
Cc: "Jason A. Donenfeld" <Jason@zx2c4.com>
Cc: Josh Poimboeuf <jpoimboe@kernel.org>
Cc: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Nathan Chancellor <nathan@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: Sander Vanheule <sander@svanheule.net>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Yury Norov <yury.norov@gmail.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
Link: https://lore.kernel.org/r/20220826162116.1050972-3-bvanassche@acm.org
Signed-off-by: Eliav Farber <farbere@amazon.com>
---
include/linux/compiler.h | 6 ++++++
include/linux/overflow.h | 1 -
include/linux/trace_events.h | 2 --
3 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index bbd74420fa21..004a030d5ad2 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -245,6 +245,12 @@ static inline void *offset_to_ptr(const int *off)
/* &a[0] degrades to a pointer: a different type from an array */
#define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
+/*
+ * Whether 'type' is a signed type or an unsigned type. Supports scalar types,
+ * bool and also pointer types.
+ */
+#define is_signed_type(type) (((type)(-1)) < (__force type)1)
+
/*
* 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/overflow.h b/include/linux/overflow.h
index 73bc67ec2136..e6bf14f462e9 100644
--- a/include/linux/overflow.h
+++ b/include/linux/overflow.h
@@ -29,7 +29,6 @@
* https://mail-index.netbsd.org/tech-misc/2007/02/05/0000.html -
* credit to Christian Biere.
*/
-#define is_signed_type(type) (((type)(-1)) < (type)1)
#define __type_half_max(type) ((type)1 << (8*sizeof(type) - 1 - is_signed_type(type)))
#define type_max(T) ((T)((__type_half_max(T) - 1) + __type_half_max(T)))
#define type_min(T) ((T)((T)-type_max(T)-(T)1))
diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index 5af2acb9fb7d..0c8c3cf36f96 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -700,8 +700,6 @@ extern int trace_add_event_call(struct trace_event_call *call);
extern int trace_remove_event_call(struct trace_event_call *call);
extern int trace_event_get_offsets(struct trace_event_call *call);
-#define is_signed_type(type) (((type)(-1)) < (type)1)
-
int ftrace_set_clr_event(struct trace_array *tr, char *buf, int set);
int trace_set_clr_event(const char *system, const char *event, int set);
int trace_array_set_clr_event(struct trace_array *tr, const char *system,
--
2.47.3
^ permalink raw reply related
* [PATCH v2 00/27 5.10.y] Backport minmax.h updates from v6.17-rc7
From: Eliav Farber @ 2025-10-17 9:04 UTC (permalink / raw)
To: gregkh, stable, 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, liviu.dudau, 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, clm,
josef, dsterba, xiang, chao, 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, farbere, sashal, ruanjinjie, David.Laight, herve.codina,
Jason, keescook, kbusch, nathan, bvanassche, ndesaulniers,
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-erofs, linux-ext4, linux-sparse,
linux-mm, netfilter-devel, coreteam, tipc-discussion
This series backports 27 patches to update minmax.h in the 5.10.y
branch, aligning it with v6.17-rc7.
The ultimate goal is to synchronize all long-term branches so that they
include the full set of minmax.h changes.
- 6.12.y has already been backported; the changes are included in
v6.12.49.
- 6.6.y has already been backported; the changes are included in
v6.6.109.
- 6.1.y has already been backported; the changes are currently in the
6.1-stable tree.
- 5.15.y has already been backported; the changes are currently in the
5.15-stable tree.
The key motivation is to bring in commit d03eba99f5bf ("minmax: allow
min()/max()/clamp() if the arguments have the same signedness"), which
is missing in kernel 5.10.y.
In mainline, this change enables min()/max()/clamp() to accept mixed
argument types, provided both have the same signedness. Without it,
backported patches that use these forms may trigger compiler warnings,
which escalate to build failures when -Werror is enabled.
The first two patches in this series were added to prevent build
failures caused by changes introduced later in minmax.h.
- Commit 92d23c6e9415 ("overflow, tracing: Define the is_signed_type()
macro once") is needed for commit 75ca38c1960f ("minmax: allow
min()/max()/clamp()").
- Commit cea628008fc8 ("btrfs: remove duplicated in_range() macro") is
needed for commit f9bff0e31881 ("minmax: add in_range() macro").
The changes were tested using `make allyesconfig` and
`make allmodconfig` for arm64, arm, x86_64 and i386 architectures.
Changes in v2:
The series was updated after initially backporting and approving the
newer long-term branches.
Andy Shevchenko (2):
minmax: deduplicate __unconst_integer_typeof()
minmax: fix header inclusions
Bart Van Assche (1):
overflow, tracing: Define the is_signed_type() macro once
David Laight (11):
minmax: allow min()/max()/clamp() if the arguments have the same
signedness.
minmax: fix indentation of __cmp_once() and __clamp_once()
minmax: allow comparisons of 'int' against 'unsigned char/short'
minmax: relax check to allow comparison between unsigned arguments and
signed constants
minmax.h: add whitespace around operators and after commas
minmax.h: update some comments
minmax.h: reduce the #define expansion of min(), max() and clamp()
minmax.h: use BUILD_BUG_ON_MSG() for the lo < hi test in clamp()
minmax.h: move all the clamp() definitions after the min/max() ones
minmax.h: simplify the variants of clamp()
minmax.h: remove some #defines that are only expanded once
Herve Codina (1):
minmax: Introduce {min,max}_array()
Jason A. Donenfeld (2):
minmax: sanity check constant bounds when clamping
minmax: clamp more efficiently by avoiding extra comparison
Johannes Thumshirn (1):
btrfs: remove duplicated in_range() macro
Linus Torvalds (8):
minmax: avoid overly complicated constant expressions in VM code
minmax: add a few more MIN_T/MAX_T users
minmax: simplify and clarify min_t()/max_t() implementation
minmax: make generic MIN() and MAX() macros available everywhere
minmax: don't use max() in situations that want a C constant
expression
minmax: simplify min()/max()/clamp() implementation
minmax: improve macro expansion and type checking
minmax: fix up min3() and max3() too
Matthew Wilcox (Oracle) (1):
minmax: add in_range() macro
arch/arm/mm/pageattr.c | 6 +-
arch/um/drivers/mconsole_user.c | 2 +
arch/x86/mm/pgtable.c | 2 +-
drivers/edac/sb_edac.c | 4 +-
drivers/edac/skx_common.h | 1 -
.../drm/amd/display/modules/hdcp/hdcp_ddc.c | 2 +
.../drm/amd/pm/powerplay/hwmgr/ppevvmath.h | 14 +-
.../drm/arm/display/include/malidp_utils.h | 2 +-
.../display/komeda/komeda_pipeline_state.c | 24 +-
drivers/gpu/drm/drm_color_mgmt.c | 2 +-
drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 6 -
drivers/gpu/drm/radeon/evergreen_cs.c | 2 +
drivers/hwmon/adt7475.c | 24 +-
drivers/input/touchscreen/cyttsp4_core.c | 2 +-
drivers/md/dm-integrity.c | 6 +-
drivers/media/dvb-frontends/stv0367_priv.h | 3 +
.../net/ethernet/chelsio/cxgb3/cxgb3_main.c | 18 +-
.../net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
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 -
fs/btrfs/ctree.h | 2 -
fs/btrfs/extent_io.c | 1 +
fs/btrfs/file-item.c | 1 +
fs/btrfs/misc.h | 2 -
fs/btrfs/raid56.c | 1 +
fs/btrfs/tree-checker.c | 2 +-
fs/erofs/zdata.h | 2 +-
fs/ext2/balloc.c | 2 -
fs/ext4/ext4.h | 2 -
fs/ufs/util.h | 6 -
include/linux/compiler.h | 15 +
include/linux/minmax.h | 267 ++++++++++++++----
include/linux/overflow.h | 1 -
include/linux/trace_events.h | 2 -
kernel/trace/preemptirq_delay_test.c | 2 -
lib/btree.c | 1 -
lib/decompress_unlzma.c | 2 +
lib/logic_pio.c | 3 -
lib/vsprintf.c | 2 +-
lib/zstd/zstd_internal.h | 2 -
mm/zsmalloc.c | 1 -
net/ipv4/proc.c | 2 +-
net/ipv6/proc.c | 2 +-
net/netfilter/nf_nat_core.c | 6 +-
net/tipc/core.h | 2 +-
net/tipc/link.c | 10 +-
49 files changed, 312 insertions(+), 169 deletions(-)
--
2.47.3
^ permalink raw reply
* Re: [PATCH v6 0/7] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling
From: Antheas Kapenekakis @ 2025-10-17 7:54 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: Denis Benato, platform-driver-x86, linux-input, LKML, Jiri Kosina,
Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede
In-Reply-To: <CAGwozwH3VnTsx8p5N6S1yp4Z9mFfPUdZ4frrnPAveLH2a00K6g@mail.gmail.com>
On Thu, 16 Oct 2025 at 18:16, Antheas Kapenekakis <lkml@antheas.dev> wrote:
>
> On Thu, 16 Oct 2025 at 17:09, Ilpo Järvinen
> <ilpo.jarvinen@linux.intel.com> wrote:
> >
> > On Thu, 16 Oct 2025, Antheas Kapenekakis wrote:
> > > On Thu, 16 Oct 2025 at 13:57, Denis Benato <benato.denis96@gmail.com> wrote:
> > > > On 10/13/25 22:15, Antheas Kapenekakis wrote:
> > > > > This is a two part series which does the following:
> > > > > - Clean-up init sequence
> > > > > - Unify backlight handling to happen under asus-wmi so that all Aura
> > > > > devices have synced brightness controls and the backlight button works
> > > > > properly when it is on a USB laptop keyboard instead of one w/ WMI.
> > > > >
> > > > > For more context, see cover letter of V1. Since V5, I removed some patches
> > > > > to make this easier to merge.
> > > > >
> > > > > All comments with these patches had been addressed since V4.
> > > > I have loaded this patchset for users of asus-linux project to try out.
> > > >
> > > > One of them opened a bug report about a kernel bug that happens
> > > > consistently when closing the lid of his laptop [1].
> > > >
> > > > He also sent another piece of kernel log, but didn't specify anything more
> > > > about this [2].
> > > >
> > > > [1] https://pastebin.com/akZx1w10
> > > > [2] https://pastebin.com/sKdczPgf
> > >
> > > Can you provide a link to the bug report? [2] seems unrelated.
> > >
> > > As for [1], it looks like a trace that stems from a sysfs write to
> > > brightness stemming from userspace that follows the same chain it
> > > would on a stock kernel and times out. Is it present on a stock
> > > kernel?
> > >
> > > Ilpo should know more about this, could the spinlock be interfering?
> >
> > [1] certainly seems to do schedule() from do_kbd_led_set() so it's not
> > possible to use spinlock there.
> >
> > So we're back to what requires the spinlock? And what the spinlock
> > protects?
>
> For that invocation, since it is coming from the cdev device owned by
> asus_wmi, it protects asus_ref.listeners under do_kbd_led_set.
> asus_wmi is protected by the fact it is owned by that device. Spinlock
> is not required in this invocation due to not being an IRQ.
>
> Under asus_hid_event (second to last patch), which is called from an
> IRQ, a spinlock is required for protecting both listeners and the
> asus_ref.asus, and I suspect that scheduling from an IRQ is not
> allowed either. Is that correct?
So it is a bit tricky here. When the IRQ fires, it needs to know
whether asus-wmi will handle the keyboard brightness event so that it
falls back to emitting it.
If we want it to know for sure, it needs to access asus_wmi, so it
needs a spinlock or an IRQ friendly lock. This way, currently,
asus_hid_event will return -EBUSY if there is no led device so the
event propagates through hid.
If we say that it is good enough to know that it was compiled with
IS_REACHABLE(CONFIG_ASUS_WMI), ie the actual implementation of
asus_hid_event in asus-wmi will never return an error, then,
asus_hid_event can schedule a task to fire the event without a lock,
and that task can use a normal locking primitive.
If the task needs to be assigned to a device or have a handle,
asus_hid_listener can be provided to asus_hid_event, so that it is
owned by the calling device.
What would the appropriate locking primitive be in this case?
> Antheas
> >
> > Not related to this particular email in this thread, if the users are
> > testing something with different kernels, it's also important to make sure
> > that the lockdep configs are enabled in both. As it could be that in one
> > kernel lockdep is not enabled and thus it won't do the splat.
> >
> > --
> > i.
> >
> >
> > > My testing on devices that have WMI led controls is a bit limited
> > > unfortunately. However, most of our asus users have been happy with
> > > this series for around half a year now.
> > >
> > > > > ---
> > > > > V5: https://lore.kernel.org/all/20250325184601.10990-1-lkml@antheas.dev/
> > > > > V4: https://lore.kernel.org/lkml/20250324210151.6042-1-lkml@antheas.dev/
> > > > > V3: https://lore.kernel.org/lkml/20250322102804.418000-1-lkml@antheas.dev/
> > > > > V2: https://lore.kernel.org/all/20250320220924.5023-1-lkml@antheas.dev/
> > > > > V1: https://lore.kernel.org/all/20250319191320.10092-1-lkml@antheas.dev/
> > > > >
> > > > > Changes since V5:
> > > > > - It's been a long time
> > > > > - Remove addition of RGB as that had some comments I need to work on
> > > > > - Remove folio patch (already merged)
> > > > > - Remove legacy fix patch 11 from V4. There is a small chance that
> > > > > without this patch, some old NKEY keyboards might not respond to
> > > > > RGB commands according to Luke, but the kernel driver does not do
> > > > > RGB currently. The 0x5d init is done by Armoury crate software in
> > > > > Windows. If an issue is found, we can re-add it or just remove patches
> > > > > 1/2 before merging. However, init could use the cleanup.
> > > > >
> > > > > Changes since V4:
> > > > > - Fix KConfig (reported by kernel robot)
> > > > > - Fix Ilpo's nits, if I missed anything lmk
> > > > >
> > > > > Changes since V3:
> > > > > - Add initializer for 0x5d for old NKEY keyboards until it is verified
> > > > > that it is not needed for their media keys to function.
> > > > > - Cover init in asus-wmi with spinlock as per Hans
> > > > > - If asus-wmi registers WMI handler with brightness, init the brightness
> > > > > in USB Asus keyboards, per Hans.
> > > > > - Change hid handler name to asus-UNIQ:rgb:peripheral to match led class
> > > > > - Fix oops when unregistering asus-wmi by moving unregister outside of
> > > > > the spin lock (but after the asus reference is set to null)
> > > > >
> > > > > Changes since V2:
> > > > > - Check lazy init succeds in asus-wmi before setting register variable
> > > > > - make explicit check in asus_hid_register_listener for listener existing
> > > > > to avoid re-init
> > > > > - rename asus_brt to asus_hid in most places and harmonize everything
> > > > > - switch to a spinlock instead of a mutex to avoid kernel ooops
> > > > > - fixup hid device quirks to avoid multiple RGB devices while still exposing
> > > > > all input vendor devices. This includes moving rgb init to probe
> > > > > instead of the input_configured callbacks.
> > > > > - Remove fan key (during retest it appears to be 0xae that is already
> > > > > supported by hid-asus)
> > > > > - Never unregister asus::kbd_backlight while asus-wmi is active, as that
> > > > > - removes fds from userspace and breaks backlight functionality. All
> > > > > - current mainline drivers do not support backlight hotplugging, so most
> > > > > userspace software (e.g., KDE, UPower) is built with that assumption.
> > > > > For the Ally, since it disconnects its controller during sleep, this
> > > > > caused the backlight slider to not work in KDE.
> > > > >
> > > > > Changes since V1:
> > > > > - Add basic RGB support on hid-asus, (Z13/Ally) tested in KDE/Z13
> > > > > - Fix ifdef else having an invalid signature (reported by kernel robot)
> > > > > - Restore input arguments to init and keyboard function so they can
> > > > > be re-used for RGB controls.
> > > > > - Remove Z13 delay (it did not work to fix the touchpad) and replace it
> > > > > with a HID_GROUP_GENERIC quirk to allow hid-multitouch to load. Squash
> > > > > keyboard rename into it.
> > > > > - Unregister brightness listener before removing work queue to avoid
> > > > > a race condition causing corruption
> > > > > - Remove spurious mutex unlock in asus_brt_event
> > > > > - Place mutex lock in kbd_led_set after LED_UNREGISTERING check to avoid
> > > > > relocking the mutex and causing a deadlock when unregistering leds
> > > > > - Add extra check during unregistering to avoid calling unregister when
> > > > > no led device is registered.
> > > > > - Temporarily HID_QUIRK_INPUT_PER_APP from the ROG endpoint as it causes
> > > > > the driver to create 4 RGB handlers per device. I also suspect some
> > > > > extra events sneak through (KDE had the @@@@@@).
> > > > >
> > > > > Antheas Kapenekakis (7):
> > > > > HID: asus: refactor init sequence per spec
> > > > > HID: asus: prevent binding to all HID devices on ROG
> > > > > platform/x86: asus-wmi: Add support for multiple kbd RGB handlers
> > > > > HID: asus: listen to the asus-wmi brightness device instead of
> > > > > creating one
> > > > > platform/x86: asus-wmi: remove unused keyboard backlight quirk
> > > > > platform/x86: asus-wmi: add keyboard brightness event handler
> > > > > HID: asus: add support for the asus-wmi brightness handler
> > > > >
> > > > > drivers/hid/hid-asus.c | 235 +++++++++++----------
> > > > > drivers/platform/x86/asus-wmi.c | 157 ++++++++++++--
> > > > > include/linux/platform_data/x86/asus-wmi.h | 69 +++---
> > > > > 3 files changed, 291 insertions(+), 170 deletions(-)
> > > > >
> > > > >
> > > > > base-commit: 3a8660878839faadb4f1a6dd72c3179c1df56787
> > > >
> > >
> >
> >
^ permalink raw reply
* Re: [PATCH 01/32] Input: cyttsp5 - Use %pe format specifier
From: Dmitry Torokhov @ 2025-10-17 5:18 UTC (permalink / raw)
To: Ricardo Ribalda
Cc: Linus Walleij, Mauro Carvalho Chehab, Hans Verkuil, Sakari Ailus,
Krzysztof Hałasa, Tomi Valkeinen, Leon Luo, Kieran Bingham,
Jacopo Mondi, Kieran Bingham, Laurent Pinchart,
Niklas Söderlund, Julien Massot, Jacopo Mondi, Daniel Scally,
Dave Stevenson, Benjamin Mugnier, Sylvain Petinot, Yong Zhi,
Bingbu Cao, Tianshu Qiu, Tiffany Lin, Andrew-CT Chen, Yunfei Dong,
Matthias Brugger, AngeloGioacchino Del Regno, Rui Miguel Silva,
Laurent Pinchart, Martin Kepplinger, Purism Kernel Team,
Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Dafna Hirschfeld, Heiko Stuebner, Sylwester Nawrocki,
Krzysztof Kozlowski, Alim Akhtar, Yemike Abhilash Chandra,
Greg Kroah-Hartman, linux-input, linux-kernel, linux-media,
linux-arm-kernel, linux-mediatek, imx, linux-renesas-soc,
linux-rockchip, linux-samsung-soc, linux-staging
In-Reply-To: <20251013-ptr_err-v1-1-2c5efbd82952@chromium.org>
On Mon, Oct 13, 2025 at 02:14:41PM +0000, Ricardo Ribalda wrote:
> The %pe format specifier is designed to print error pointers. It prints
> a symbolic error name (eg. -EINVAL) and it makes the code simpler by
> omitting PTR_ERR()
>
> This patch fixes this cocci report:
> ./cyttsp5.c:927:3-10: WARNING: Consider using %pe to print PTR_ERR()
>
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* [PATCH v2 6/6] HID: intel-ish-hid: ipc: Separate hibernate callbacks in dev_pm_ops
From: Zhang Lixu @ 2025-10-17 2:22 UTC (permalink / raw)
To: linux-input, srinivas.pandruvada, jikos, benjamin.tissoires; +Cc: lixu.zhang
In-Reply-To: <20251017022218.1292451-1-lixu.zhang@intel.com>
The same suspend and resume callbacks are used for both suspend-to-RAM/idle
and hibernation. These callbacks invoke pm_suspend_via_firmware() and
pm_resume_via_firmware(), respectively. In the .freeze() of hibernation,
pm_suspend_via_firmware() returns false, causing the driver to put ISH into
D0i3. However, during the .thaw(), pm_resume_via_firmware() returns true,
leading the driver to treat ISH as resuming from D3 instead of D0i3. The
asymmetric behavior between .freeze() and .thaw() during hibernation can
cause the client connection states on the firmware side and the driver side
to become inconsistent.
To address the inconsistent client connection states issue, separate
hibernate-related callbacks (freeze, thaw) in dev_pm_ops. Since ISH does
not need to save any firmware-related state when entering hibernation, it
is sufficient to call pci_save_state() in .freeze() to prevent the PCI bus
from changing the ISH power state. No actions are required in .thaw().
Signed-off-by: Zhang Lixu <lixu.zhang@intel.com>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
drivers/hid/intel-ish-hid/ipc/pci-ish.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/intel-ish-hid/ipc/pci-ish.c b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
index e4499c83c62e..1612e8cb23f0 100644
--- a/drivers/hid/intel-ish-hid/ipc/pci-ish.c
+++ b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
@@ -397,7 +397,20 @@ static int __maybe_unused ish_resume(struct device *device)
return 0;
}
-static SIMPLE_DEV_PM_OPS(ish_pm_ops, ish_suspend, ish_resume);
+static int __maybe_unused ish_freeze(struct device *device)
+{
+ struct pci_dev *pdev = to_pci_dev(device);
+
+ return pci_save_state(pdev);
+}
+
+static const struct dev_pm_ops __maybe_unused ish_pm_ops = {
+ .suspend = pm_sleep_ptr(ish_suspend),
+ .resume = pm_sleep_ptr(ish_resume),
+ .freeze = pm_sleep_ptr(ish_freeze),
+ .restore = pm_sleep_ptr(ish_resume),
+ .poweroff = pm_sleep_ptr(ish_suspend),
+};
static ssize_t base_version_show(struct device *cdev,
struct device_attribute *attr, char *buf)
--
2.43.0
^ permalink raw reply related
* [PATCH v2 5/6] HID: intel-ish-hid: Use IPC RESET instead of void message in ish_wakeup()
From: Zhang Lixu @ 2025-10-17 2:22 UTC (permalink / raw)
To: linux-input, srinivas.pandruvada, jikos, benjamin.tissoires; +Cc: lixu.zhang
In-Reply-To: <20251017022218.1292451-1-lixu.zhang@intel.com>
On ISH power-up, the bootloader enters sleep after preparing to load the
main firmware, waiting for the driver to be ready. When the driver is
ready, it sends a void message to wake up the bootloader and load the main
firmware. The main firmware then sends MNG_RESET_NOTIFY to the driver for
handshake.
This void message-based IPC handshake only works if the main firmware has
not been loaded. During hibernation resume, if the restore kernel has the
ISH driver, the driver wakes up the bootloader to load the main firmware
and perform IPC handshake. However, when switching to the image kernel,
since the main firmware is already loaded, sending a void message in the
.restore() callback does not trigger IPC handshake.
By sending MNG_RESET_NOTIFY (IPC RESET message) in ish_wakeup() instead of
a void message, we can explicitly wake up the bootloader and perform IPC
handshake, regardless of the firmware state. Additionally, since
ish_ipc_reset() already waits for recvd_hw_ready, the redundant wait for
recvd_hw_ready in ish_hw_start() is removed.
The timeout for waiting for HW ready is set to 10 seconds, matching the
original timeout value used in ish_wakeup(), to ensure reliable wakeup on
hardware that requires more time, such as the Lenovo ThinkPad X1 Titanium
Gen 1.
Signed-off-by: Zhang Lixu <lixu.zhang@intel.com>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
drivers/hid/intel-ish-hid/ipc/ipc.c | 39 ++++++++++++-----------------
1 file changed, 16 insertions(+), 23 deletions(-)
diff --git a/drivers/hid/intel-ish-hid/ipc/ipc.c b/drivers/hid/intel-ish-hid/ipc/ipc.c
index 01a139e17cb5..59355e4a61f8 100644
--- a/drivers/hid/intel-ish-hid/ipc/ipc.c
+++ b/drivers/hid/intel-ish-hid/ipc/ipc.c
@@ -728,22 +728,28 @@ int ish_disable_dma(struct ishtp_device *dev)
* ish_wakeup() - wakeup ishfw from waiting-for-host state
* @dev: ishtp device pointer
*
- * Set the dma enable bit and send a void message to FW,
+ * Set the dma enable bit and send a IPC RESET message to FW,
* it wil wakeup FW from waiting-for-host state.
+ *
+ * Return: 0 for success else error code.
*/
-static void ish_wakeup(struct ishtp_device *dev)
+static int ish_wakeup(struct ishtp_device *dev)
{
+ int ret;
+
/* Set dma enable bit */
ish_reg_write(dev, IPC_REG_ISH_RMP2, IPC_RMP2_DMA_ENABLED);
/*
- * Send 0 IPC message so that ISH FW wakes up if it was already
+ * Send IPC RESET message so that ISH FW wakes up if it was already
* asleep.
*/
- ish_reg_write(dev, IPC_REG_HOST2ISH_DRBL, IPC_DRBL_BUSY_BIT);
+ ret = ish_ipc_reset(dev);
/* Flush writes to doorbell and REMAP2 */
ish_reg_read(dev, IPC_REG_ISH_HOST_FWSTS);
+
+ return ret;
}
/**
@@ -792,11 +798,11 @@ static int _ish_hw_reset(struct ishtp_device *dev)
pci_write_config_word(pdev, pdev->pm_cap + PCI_PM_CTRL, csr);
/* Now we can enable ISH DMA operation and wakeup ISHFW */
- ish_wakeup(dev);
-
- return 0;
+ return ish_wakeup(dev);
}
+#define RECVD_HW_READY_TIMEOUT (10 * HZ)
+
/**
* _ish_ipc_reset() - IPC reset
* @dev: ishtp device pointer
@@ -831,7 +837,8 @@ static int _ish_ipc_reset(struct ishtp_device *dev)
}
wait_event_interruptible_timeout(dev->wait_hw_ready,
- dev->recvd_hw_ready, 2 * HZ);
+ dev->recvd_hw_ready,
+ RECVD_HW_READY_TIMEOUT);
if (!dev->recvd_hw_ready) {
dev_err(dev->devc, "Timed out waiting for HW ready\n");
rv = -ENODEV;
@@ -855,21 +862,7 @@ int ish_hw_start(struct ishtp_device *dev)
set_host_ready(dev);
/* After that we can enable ISH DMA operation and wakeup ISHFW */
- ish_wakeup(dev);
-
- /* wait for FW-initiated reset flow */
- if (!dev->recvd_hw_ready)
- wait_event_interruptible_timeout(dev->wait_hw_ready,
- dev->recvd_hw_ready,
- 10 * HZ);
-
- if (!dev->recvd_hw_ready) {
- dev_err(dev->devc,
- "[ishtp-ish]: Timed out waiting for FW-initiated reset\n");
- return -ENODEV;
- }
-
- return 0;
+ return ish_wakeup(dev);
}
/**
--
2.43.0
^ permalink raw reply related
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