On Mon, 29 Jul 2024 at 16:21, Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxx> wrote:
>
> Attached is the patch I have in my tree right now - it complains about
> a 'bcachefs' comparison between an 'u16' and a 's64', because I also
> removed the 'implicit integer promotion is ok' logic, because I think
> it's wrong.
>
> I don't think a min(u16,s64) is a valid minimum, for exactly the same
> reason a min(u32,s64) is not valid.
Oh, and I noticed that it screws up the 32-bit case, and that does
need a workaround for that.
So here's a better version. The patch contains one possible fix to
bcachefs for the type confusion there, but I'll wait for Kent to
respond on that.
Linus
fs/bcachefs/alloc_background.h | 2 +-
include/linux/compiler.h | 9 ++++++
include/linux/minmax.h | 66 +++++++++++++++++++++++++++++++-----------
3 files changed, 59 insertions(+), 18 deletions(-)
diff --git a/fs/bcachefs/alloc_background.h b/fs/bcachefs/alloc_background.h
index 8d2b62c9588e..b61b92bf7ba9 100644
--- a/fs/bcachefs/alloc_background.h
+++ b/fs/bcachefs/alloc_background.h
@@ -87,7 +87,7 @@ static inline s64 bch2_bucket_sectors_total(struct bch_alloc_v4 a)
return a.stripe_sectors + a.dirty_sectors + a.cached_sectors;
}
-static inline s64 bch2_bucket_sectors_dirty(struct bch_alloc_v4 a)
+static inline u64 bch2_bucket_sectors_dirty(struct bch_alloc_v4 a)
{
return a.stripe_sectors + a.dirty_sectors;
}
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 2594553bb30b..2df665fa2964 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -296,6 +296,15 @@ static inline void *offset_to_ptr(const int *off)
#define is_signed_type(type) (((type)(-1)) < (__force type)1)
#define is_unsigned_type(type) (!is_signed_type(type))
+/*
+ * Useful shorthand for "is this condition known at compile-time?"
+ *
+ * Note that the condition may involve non-constant values,
+ * but the compiler may know enough about the details of the
+ * values to determine that the condition is statically true.
+ */
+#define statically_true(x) (__builtin_constant_p(x) && (x))
+
/*
* This is needed in functions which generate the stack canary, see
* arch/x86/kernel/smpboot.c::start_secondary() for an example.
diff --git a/include/linux/minmax.h b/include/linux/minmax.h
index e3e4353df983..af53ebe3d2b8 100644
--- a/include/linux/minmax.h
+++ b/include/linux/minmax.h
@@ -26,19 +26,52 @@
#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, non-negative integer expressions
+ * are ok for both.
+ *
+ * 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
+ * evaluate it with __builtin_constant_p() etc)
+ */
+#define __sign_use(x,ux) \
+ (is_signed_type(typeof(ux))?2+__is_nonneg(x,ux):1)
-/* 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)
+/*
+ * 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)>32,1LL,1L))
+#endif
+#define __is_nonneg(x,ux) statically_true((__signed_type(ux))(x)>=0)
-#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_ok(x,y,ux,uy) \
+ (__sign_use(x,ux) & __sign_use(y,uy))
+
+#define __types_ok3(x,y,z,ux,uy,uz) \
+ (__sign_use(x,ux) & __sign_use(y,uy) & __sign_use(z,uz))
#define __cmp_op_min <
#define __cmp_op_max >
@@ -53,8 +86,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) \
@@ -67,11 +100,10 @@
__auto_type uval = (val); \
__auto_type ulo = (lo); \
__auto_type uhi = (hi); \
- static_assert(__builtin_choose_expr(__is_constexpr((lo) > (hi)), \
- (lo) <= (hi), true), \
+ BUILD_BUG_ON_MSG(statically_true(ulo > uhi), \
"clamp() low limit " #lo " greater than high limit " #hi); \
- 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) \
