linux-sparse.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/7 5.10.y] Cherry pick of minmax.h commits from 5.15.y
@ 2025-09-16 21:22 Eliav Farber
  2025-09-16 21:22 ` [PATCH 1/7 5.10.y] tracing: Define the is_signed_type() macro once Eliav Farber
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Eliav Farber @ 2025-09-16 21:22 UTC (permalink / raw)
  To: luc.vanoostenryck, rostedt, mingo, akpm, gregkh, sj, David.Laight,
	Jason, andriy.shevchenko, bvanassche, keescook, linux-sparse,
	linux-kernel
  Cc: jonnyc, farbere, stable

This series backports seven commits from v5.15.y that update minmax.h
and related code:

 - ed6e37e30826 ("tracing: Define the is_signed_type() macro once")
 - 998f03984e25 ("minmax: sanity check constant bounds when clamping")
 - d470787b25e6 ("minmax: clamp more efficiently by avoiding extra
   comparison")
 - 1c2ee5bc9f11 ("minmax: fix header inclusions")
 - d53b5d862acd ("minmax: allow min()/max()/clamp() if the arguments
   have the same signedness.")
 - 7ed91c5560df ("minmax: allow comparisons of 'int' against 'unsigned
   char/short'")
 - 22f7794ef5a3 ("minmax: relax check to allow comparison between
   unsigned arguments and signed constants")

The main motivation is commit d53b5d862acd, which removes the strict
type check in min()/max() when both arguments have the same signedness.
Without this, kernel 5.10 builds can emit warnings that become build
failures when -Werror is used.

Additionally, commit ed6e37e30826 from tracing is required as a
dependency; without it, compilation fails.

Andy Shevchenko (1):
  minmax: fix header inclusions

Bart Van Assche (1):
  tracing: Define the is_signed_type() macro once

David Laight (3):
  minmax: allow min()/max()/clamp() if the arguments have the same
    signedness.
  minmax: allow comparisons of 'int' against 'unsigned char/short'
  minmax: relax check to allow comparison between unsigned arguments and
    signed constants

Jason A. Donenfeld (2):
  minmax: sanity check constant bounds when clamping
  minmax: clamp more efficiently by avoiding extra comparison

 include/linux/compiler.h     |  6 +++
 include/linux/minmax.h       | 89 ++++++++++++++++++++++++++----------
 include/linux/overflow.h     |  1 -
 include/linux/trace_events.h |  2 -
 4 files changed, 70 insertions(+), 28 deletions(-)

-- 
2.47.3


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/7 5.10.y] tracing: Define the is_signed_type() macro once
  2025-09-16 21:22 [PATCH 0/7 5.10.y] Cherry pick of minmax.h commits from 5.15.y Eliav Farber
@ 2025-09-16 21:22 ` Eliav Farber
  2025-09-17  8:40   ` Greg KH
  2025-09-16 21:22 ` [PATCH 2/7 5.10.y] minmax: sanity check constant bounds when clamping Eliav Farber
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Eliav Farber @ 2025-09-16 21:22 UTC (permalink / raw)
  To: luc.vanoostenryck, rostedt, mingo, akpm, gregkh, sj, David.Laight,
	Jason, andriy.shevchenko, bvanassche, keescook, linux-sparse,
	linux-kernel
  Cc: jonnyc, farbere, stable, Rasmus Villemoes

From: Bart Van Assche <bvanassche@acm.org>

commit 92d23c6e94157739b997cacce151586a0d07bb8a upstream.

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, 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.

Link: https://lore.kernel.org/all/CAHk-=whjH6p+qzwUdx5SOVVHjS3WvzJQr6mDUwhEyTf6pJWzaQ@mail.gmail.com/
Link: https://lore.kernel.org/all/CAHk-=wjQGnVfb4jehFR0XyZikdQvCZouE96xR_nnf5kqaM5qqQ@mail.gmail.com/
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: Steven Rostedt <rostedt@goodmis.org>
Acked-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
(cherry picked from commit a49a64b5bf195381c09202c524f0f84b5f3e816f)
Signed-off-by: SeongJae Park <sj@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.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 35af574d006f..66dd311ad8eb 100644
--- a/include/linux/overflow.h
+++ b/include/linux/overflow.h
@@ -32,7 +32,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	[flat|nested] 10+ messages in thread

* [PATCH 2/7 5.10.y] minmax: sanity check constant bounds when clamping
  2025-09-16 21:22 [PATCH 0/7 5.10.y] Cherry pick of minmax.h commits from 5.15.y Eliav Farber
  2025-09-16 21:22 ` [PATCH 1/7 5.10.y] tracing: Define the is_signed_type() macro once Eliav Farber
@ 2025-09-16 21:22 ` Eliav Farber
  2025-09-16 21:22 ` [PATCH 3/7 5.10.y] minmax: clamp more efficiently by avoiding extra comparison Eliav Farber
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Eliav Farber @ 2025-09-16 21:22 UTC (permalink / raw)
  To: luc.vanoostenryck, rostedt, mingo, akpm, gregkh, sj, David.Laight,
	Jason, andriy.shevchenko, bvanassche, keescook, linux-sparse,
	linux-kernel
  Cc: jonnyc, farbere, stable

From: "Jason A. Donenfeld" <Jason@zx2c4.com>

commit 5efcecd9a3b18078d3398b359a84c83f549e22cf upstream.

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>
(cherry picked from commit 5efcecd9a3b18078d3398b359a84c83f549e22cf)
Signed-off-by: SeongJae Park <sj@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.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	[flat|nested] 10+ messages in thread

* [PATCH 3/7 5.10.y] minmax: clamp more efficiently by avoiding extra comparison
  2025-09-16 21:22 [PATCH 0/7 5.10.y] Cherry pick of minmax.h commits from 5.15.y Eliav Farber
  2025-09-16 21:22 ` [PATCH 1/7 5.10.y] tracing: Define the is_signed_type() macro once Eliav Farber
  2025-09-16 21:22 ` [PATCH 2/7 5.10.y] minmax: sanity check constant bounds when clamping Eliav Farber
@ 2025-09-16 21:22 ` Eliav Farber
  2025-09-16 21:22 ` [PATCH 4/7 5.10.y] minmax: fix header inclusions Eliav Farber
  2025-09-18 21:01 ` [PATCH 0/7 5.10.y] Cherry pick of minmax.h commits from 5.15.y David Laight
  4 siblings, 0 replies; 10+ messages in thread
From: Eliav Farber @ 2025-09-16 21:22 UTC (permalink / raw)
  To: luc.vanoostenryck, rostedt, mingo, akpm, gregkh, sj, David.Laight,
	Jason, andriy.shevchenko, bvanassche, keescook, linux-sparse,
	linux-kernel
  Cc: jonnyc, farbere, stable

From: "Jason A. Donenfeld" <Jason@zx2c4.com>

commit 2122e2a4efc2cd139474079e11939b6e07adfacd upstream.

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>
(cherry picked from commit 2122e2a4efc2cd139474079e11939b6e07adfacd)
Signed-off-by: SeongJae Park <sj@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.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	[flat|nested] 10+ messages in thread

* [PATCH 4/7 5.10.y] minmax: fix header inclusions
  2025-09-16 21:22 [PATCH 0/7 5.10.y] Cherry pick of minmax.h commits from 5.15.y Eliav Farber
                   ` (2 preceding siblings ...)
  2025-09-16 21:22 ` [PATCH 3/7 5.10.y] minmax: clamp more efficiently by avoiding extra comparison Eliav Farber
@ 2025-09-16 21:22 ` Eliav Farber
  2025-09-18 21:01 ` [PATCH 0/7 5.10.y] Cherry pick of minmax.h commits from 5.15.y David Laight
  4 siblings, 0 replies; 10+ messages in thread
From: Eliav Farber @ 2025-09-16 21:22 UTC (permalink / raw)
  To: luc.vanoostenryck, rostedt, mingo, akpm, gregkh, sj, David.Laight,
	Jason, andriy.shevchenko, bvanassche, keescook, linux-sparse,
	linux-kernel
  Cc: jonnyc, farbere, stable, Herve Codina, Rasmus Villemoes

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

commit f6e9d38f8eb00ac8b52e6d15f6aa9bcecacb081b upstream.

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>
(cherry picked from commit f6e9d38f8eb00ac8b52e6d15f6aa9bcecacb081b)
Signed-off-by: SeongJae Park <sj@kernel.org>
[Fix a conflict due to absence of compiler_types.h include]
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Eliav Farber <farbere@amazon.com>
---
 include/linux/minmax.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/minmax.h b/include/linux/minmax.h
index abdeae409dad..e8e9642809e0 100644
--- a/include/linux/minmax.h
+++ b/include/linux/minmax.h
@@ -2,6 +2,8 @@
 #ifndef _LINUX_MINMAX_H
 #define _LINUX_MINMAX_H
 
+#include <linux/build_bug.h>
+#include <linux/compiler.h>
 #include <linux/const.h>
 
 /*
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/7 5.10.y] tracing: Define the is_signed_type() macro once
  2025-09-16 21:22 ` [PATCH 1/7 5.10.y] tracing: Define the is_signed_type() macro once Eliav Farber
@ 2025-09-17  8:40   ` Greg KH
  2025-09-17 10:37     ` Farber, Eliav
  0 siblings, 1 reply; 10+ messages in thread
From: Greg KH @ 2025-09-17  8:40 UTC (permalink / raw)
  To: Eliav Farber
  Cc: luc.vanoostenryck, rostedt, mingo, akpm, sj, David.Laight, Jason,
	andriy.shevchenko, bvanassche, keescook, linux-sparse,
	linux-kernel, jonnyc, stable, Rasmus Villemoes

On Tue, Sep 16, 2025 at 09:22:53PM +0000, Eliav Farber wrote:
> From: Bart Van Assche <bvanassche@acm.org>
> 
> commit 92d23c6e94157739b997cacce151586a0d07bb8a upstream.

This is only in 6.1, and not other trees, why is it needed here?

> 
> 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, 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.
> 
> Link: https://lore.kernel.org/all/CAHk-=whjH6p+qzwUdx5SOVVHjS3WvzJQr6mDUwhEyTf6pJWzaQ@mail.gmail.com/
> Link: https://lore.kernel.org/all/CAHk-=wjQGnVfb4jehFR0XyZikdQvCZouE96xR_nnf5kqaM5qqQ@mail.gmail.com/
> Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Acked-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> (cherry picked from commit a49a64b5bf195381c09202c524f0f84b5f3e816f)

This is not a valid git id in the tree at all.

greg k-h

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH 1/7 5.10.y] tracing: Define the is_signed_type() macro once
  2025-09-17  8:40   ` Greg KH
@ 2025-09-17 10:37     ` Farber, Eliav
  2025-09-21 17:30       ` Greg KH
  0 siblings, 1 reply; 10+ messages in thread
From: Farber, Eliav @ 2025-09-17 10:37 UTC (permalink / raw)
  To: Greg KH
  Cc: luc.vanoostenryck@gmail.com, rostedt@goodmis.org,
	mingo@redhat.com, akpm@linux-foundation.org, sj@kernel.org,
	David.Laight@aculab.com, Jason@zx2c4.com,
	andriy.shevchenko@linux.intel.com, bvanassche@acm.org,
	keescook@chromium.org, linux-sparse@vger.kernel.org,
	linux-kernel@vger.kernel.org, Chocron, Jonathan,
	stable@vger.kernel.org, Rasmus Villemoes, Farber, Eliav

> On Tue, Sep 16, 2025 at 09:22:53PM +0000, Eliav Farber wrote:
> > From: Bart Van Assche <bvanassche@acm.org>
> >
> > commit 92d23c6e94157739b997cacce151586a0d07bb8a upstream.
>
> This is only in 6.1, and not other trees, why is it needed here?

It exists also in 5.15:
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/include/linux/overflow.h?h=v5.15.193&id=ed6e37e30826b12572636c6bbfe6319233690c90

Without this change, I get many compilation errors when backporting
commit d53b5d862acd ("minmax: allow min()/max()/clamp() if the
arguments have the same signedness.")

  CALL    scripts/atomic/check-atomics.sh
  CC      arch/arm64/kernel/asm-offsets.s
In file included from ./include/linux/bits.h:22,
                 from ./include/linux/ioport.h:15,
                 from ./include/linux/acpi.h:12,
                 from ./include/acpi/apei.h:9,
                 from ./include/acpi/ghes.h:5,
                 from ./include/linux/arm_sdei.h:8,
                 from arch/arm64/kernel/asm-offsets.c:10:
./include/linux/nodemask.h: In function '__first_node':
./include/linux/minmax.h:30:39: error: implicit declaration of function 'is_signed_type' [-Werror=implicit-function-declaration]
   30 |  __builtin_choose_expr(__is_constexpr(is_signed_type(typeof(x))), \
      |                                       ^~~~~~~~~~~~~~
./include/linux/build_bug.h:78:56: note: in definition of macro '__static_assert'
   78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
      |                                                        ^~~~
./include/linux/minmax.h:50:3: note: in expansion of macro 'static_assert'
   50 |   static_assert(__types_ok(x, y),  \
      |   ^~~~~~~~~~~~~
./include/linux/minmax.h:30:24: note: in expansion of macro '__is_constexpr'
   30 |  __builtin_choose_expr(__is_constexpr(is_signed_type(typeof(x))), \
      |                        ^~~~~~~~~~~~~~
./include/linux/minmax.h:38:3: note: in expansion of macro '__is_signed'
   38 |  (__is_signed(x) == __is_signed(y) ||   \
      |   ^~~~~~~~~~~
./include/linux/minmax.h:50:17: note: in expansion of macro '__types_ok'
   50 |   static_assert(__types_ok(x, y),  \
      |                 ^~~~~~~~~~
./include/linux/minmax.h:57:3: note: in expansion of macro '__cmp_once'
   57 |   __cmp_once(op, x, y, __UNIQUE_ID(__x), __UNIQUE_ID(__y)))
      |   ^~~~~~~~~~
./include/linux/minmax.h:160:27: note: in expansion of macro '__careful_cmp'
  160 | #define min_t(type, x, y) __careful_cmp(min, (type)(x), (type)(y))
      |                           ^~~~~~~~~~~~~
./include/linux/nodemask.h:265:9: note: in expansion of macro 'min_t'
  265 |  return min_t(unsigned int, MAX_NUMNODES, find_first_bit(srcp->bits, MAX_NUMNODES));
      |         ^~~~~
./include/linux/minmax.h:30:54: error: expected expression before 'typeof'
   30 |  __builtin_choose_expr(__is_constexpr(is_signed_type(typeof(x))), \
      |                                                      ^~~~~~
./include/linux/build_bug.h:78:56: note: in definition of macro '__static_assert'
...


> > Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> (cherry 
> > picked from commit a49a64b5bf195381c09202c524f0f84b5f3e816f)
>
> This is not a valid git id in the tree at all.

I will fix the mismatch here and above, but please notice that this
hash appears in the link I shared.

---
Regards, Eliav

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/7 5.10.y] Cherry pick of minmax.h commits from 5.15.y
  2025-09-16 21:22 [PATCH 0/7 5.10.y] Cherry pick of minmax.h commits from 5.15.y Eliav Farber
                   ` (3 preceding siblings ...)
  2025-09-16 21:22 ` [PATCH 4/7 5.10.y] minmax: fix header inclusions Eliav Farber
@ 2025-09-18 21:01 ` David Laight
  2025-09-19 10:41   ` Farber, Eliav
  4 siblings, 1 reply; 10+ messages in thread
From: David Laight @ 2025-09-18 21:01 UTC (permalink / raw)
  To: Eliav Farber
  Cc: luc.vanoostenryck, rostedt, mingo, akpm, gregkh, sj, David.Laight,
	Jason, andriy.shevchenko, bvanassche, keescook, linux-sparse,
	linux-kernel, jonnyc, stable

On Tue, 16 Sep 2025 21:22:52 +0000
Eliav Farber <farbere@amazon.com> wrote:

> This series backports seven commits from v5.15.y that update minmax.h
> and related code:
> 
>  - ed6e37e30826 ("tracing: Define the is_signed_type() macro once")
>  - 998f03984e25 ("minmax: sanity check constant bounds when clamping")
>  - d470787b25e6 ("minmax: clamp more efficiently by avoiding extra
>    comparison")
>  - 1c2ee5bc9f11 ("minmax: fix header inclusions")
>  - d53b5d862acd ("minmax: allow min()/max()/clamp() if the arguments
>    have the same signedness.")
>  - 7ed91c5560df ("minmax: allow comparisons of 'int' against 'unsigned
>    char/short'")
>  - 22f7794ef5a3 ("minmax: relax check to allow comparison between
>    unsigned arguments and signed constants")

I think you need to pick up the later changes (from Linus) as well.
Without them nested min() and max() can generate very long lines from
the pre-processor (tens of megabytes) that cause very slow and/or
failing compilations on 32bit and other memory-limited systems.

There are a few other changes needed at the same time.
The current min() and max() can't be used in a few places because
they aren't 'constant enough' with constant arguments.

	David


> 
> The main motivation is commit d53b5d862acd, which removes the strict
> type check in min()/max() when both arguments have the same signedness.
> Without this, kernel 5.10 builds can emit warnings that become build
> failures when -Werror is used.
> 
> Additionally, commit ed6e37e30826 from tracing is required as a
> dependency; without it, compilation fails.
> 
> Andy Shevchenko (1):
>   minmax: fix header inclusions
> 
> Bart Van Assche (1):
>   tracing: Define the is_signed_type() macro once
> 
> David Laight (3):
>   minmax: allow min()/max()/clamp() if the arguments have the same
>     signedness.
>   minmax: allow comparisons of 'int' against 'unsigned char/short'
>   minmax: relax check to allow comparison between unsigned arguments and
>     signed constants
> 
> Jason A. Donenfeld (2):
>   minmax: sanity check constant bounds when clamping
>   minmax: clamp more efficiently by avoiding extra comparison
> 
>  include/linux/compiler.h     |  6 +++
>  include/linux/minmax.h       | 89 ++++++++++++++++++++++++++----------
>  include/linux/overflow.h     |  1 -
>  include/linux/trace_events.h |  2 -
>  4 files changed, 70 insertions(+), 28 deletions(-)
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH 0/7 5.10.y] Cherry pick of minmax.h commits from 5.15.y
  2025-09-18 21:01 ` [PATCH 0/7 5.10.y] Cherry pick of minmax.h commits from 5.15.y David Laight
@ 2025-09-19 10:41   ` Farber, Eliav
  0 siblings, 0 replies; 10+ messages in thread
From: Farber, Eliav @ 2025-09-19 10:41 UTC (permalink / raw)
  To: David Laight
  Cc: luc.vanoostenryck@gmail.com, rostedt@goodmis.org,
	mingo@redhat.com, akpm@linux-foundation.org,
	gregkh@linuxfoundation.org, sj@kernel.org,
	David.Laight@ACULAB.COM, Jason@zx2c4.com,
	andriy.shevchenko@linux.intel.com, bvanassche@acm.org,
	keescook@chromium.org, linux-sparse@vger.kernel.org,
	linux-kernel@vger.kernel.org, Chocron, Jonathan,
	stable@vger.kernel.org

> On Tue, 16 Sep 2025 21:22:52 +0000
> Eliav Farber <farbere@amazon.com> wrote:
>
> > This series backports seven commits from v5.15.y that update minmax.h 
> > and related code:
> >
> >  - ed6e37e30826 ("tracing: Define the is_signed_type() macro once")
> >  - 998f03984e25 ("minmax: sanity check constant bounds when clamping")
> >  - d470787b25e6 ("minmax: clamp more efficiently by avoiding extra
> >    comparison")
> >  - 1c2ee5bc9f11 ("minmax: fix header inclusions")
> >  - d53b5d862acd ("minmax: allow min()/max()/clamp() if the arguments
> >    have the same signedness.")
> >  - 7ed91c5560df ("minmax: allow comparisons of 'int' against 'unsigned
> >    char/short'")
> >  - 22f7794ef5a3 ("minmax: relax check to allow comparison between
> >    unsigned arguments and signed constants")
>
> I think you need to pick up the later changes (from Linus) as well.
> Without them nested min() and max() can generate very long lines from the pre-processor (tens of megabytes) that cause very slow and/or failing compilations on 32bit and other memory-limited systems.
>
> There are a few other changes needed at the same time.
> The current min() and max() can't be used in a few places because they aren't 'constant enough' with constant arguments.

I aligned minmax.h to include all changes in v6.17-rc6.
https://lore.kernel.org/stable/20250919101727.16152-1-farbere@amazon.com/T/#t

---
Regards, Eliav


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/7 5.10.y] tracing: Define the is_signed_type() macro once
  2025-09-17 10:37     ` Farber, Eliav
@ 2025-09-21 17:30       ` Greg KH
  0 siblings, 0 replies; 10+ messages in thread
From: Greg KH @ 2025-09-21 17:30 UTC (permalink / raw)
  To: Farber, Eliav
  Cc: luc.vanoostenryck@gmail.com, rostedt@goodmis.org,
	mingo@redhat.com, akpm@linux-foundation.org, sj@kernel.org,
	David.Laight@aculab.com, Jason@zx2c4.com,
	andriy.shevchenko@linux.intel.com, bvanassche@acm.org,
	keescook@chromium.org, linux-sparse@vger.kernel.org,
	linux-kernel@vger.kernel.org, Chocron, Jonathan,
	stable@vger.kernel.org, Rasmus Villemoes

On Wed, Sep 17, 2025 at 10:37:31AM +0000, Farber, Eliav wrote:
> > On Tue, Sep 16, 2025 at 09:22:53PM +0000, Eliav Farber wrote:
> > > From: Bart Van Assche <bvanassche@acm.org>
> > >
> > > commit 92d23c6e94157739b997cacce151586a0d07bb8a upstream.
> >
> > This is only in 6.1, and not other trees, why is it needed here?
> 
> It exists also in 5.15:
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/include/linux/overflow.h?h=v5.15.193&id=ed6e37e30826b12572636c6bbfe6319233690c90

What?  Ugh, duplicate commit ids.  What a mess :(

Fair enough, I can take this, and I want to, but as this really causes a
problem with our scripts, perhaps use the git id that is references in
the other kernel versions as well so that things don't look totally
wrong?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2025-09-21 17:30 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-16 21:22 [PATCH 0/7 5.10.y] Cherry pick of minmax.h commits from 5.15.y Eliav Farber
2025-09-16 21:22 ` [PATCH 1/7 5.10.y] tracing: Define the is_signed_type() macro once Eliav Farber
2025-09-17  8:40   ` Greg KH
2025-09-17 10:37     ` Farber, Eliav
2025-09-21 17:30       ` Greg KH
2025-09-16 21:22 ` [PATCH 2/7 5.10.y] minmax: sanity check constant bounds when clamping Eliav Farber
2025-09-16 21:22 ` [PATCH 3/7 5.10.y] minmax: clamp more efficiently by avoiding extra comparison Eliav Farber
2025-09-16 21:22 ` [PATCH 4/7 5.10.y] minmax: fix header inclusions Eliav Farber
2025-09-18 21:01 ` [PATCH 0/7 5.10.y] Cherry pick of minmax.h commits from 5.15.y David Laight
2025-09-19 10:41   ` Farber, Eliav

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).