From d63bcd350e1a3ba6196dadb26cb2f36f0ba1e182 Mon Sep 17 00:00:00 2001 From: Dave Hansen Date: Fri, 18 Oct 2024 11:07:47 -0700 Subject: [PATCH] x86/percpu: Avoid comparing unsigned types to -1 clang warns when comparing an unsinged type to -1 since the comparison is always false. This can be quickly reproduced by setting CONFIG_WERROR=y and running: make W=1 CC=clang-14 net/ipv4/tcp_output.o net/ipv4/tcp_output.c:187:3: error: result of comparison of constant -1 with expression of type 'u8' (aka 'unsigned char') is always false [-Werror,-Wtautological-constant-out-of-range-compare] 187 | NET_ADD_STATS(sock_net(sk), LINUX_MIB_TCPACKCOMPRESSED, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 188 | tp->compressed_ack); | ~~~~~~~~~~~~~~~~~~~ ... arch/x86/include/asm/percpu.h:238:31: note: expanded from macro 'percpu_add_op' 238 | ((val) == 1 || (val) == -1)) ? \ | ~~~~~ ^ ~~ Fix this by avoiding a comparison of an uncast -1 to 'val'. Doing this in addition to the existing 'pao_ID__' calculation would make it even more unreadable. Remove 'pao_ID__' and replace it with the three components of its calculation. This preserves some unintuitive but useful behavior. For instance, gcc sees: percpu_add_op(..., var, (u8)-1); and can transform that into a "dec". Clang, on the other hand, sees the 'u8' type and assumes that "(val) == -1" is false, which was the root of the warning. This is useful gcc behavior because: #define this_cpu_sub(pcp, val) this_cpu_add(pcp, -(typeof(pcp))(val)) so any code that does: this_cpu_sub(A, 1) where 'A' is an unsigned type generates a "dec". Clang, on the other hand generates a less-efficient "add". Reported-by: Andy Shevchenko Signed-off-by: Dave Hansen --- arch/x86/include/asm/percpu.h | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/arch/x86/include/asm/percpu.h b/arch/x86/include/asm/percpu.h index c55a79d5feae..57d9759c692e 100644 --- a/arch/x86/include/asm/percpu.h +++ b/arch/x86/include/asm/percpu.h @@ -234,18 +234,19 @@ do { \ */ #define percpu_add_op(size, qual, var, val) \ do { \ - const int pao_ID__ = (__builtin_constant_p(val) && \ - ((val) == 1 || (val) == -1)) ? \ - (int)(val) : 0; \ + const int pao_const__ = __builtin_constant_p(val); \ + const int pao_inc__ = (val) == 1; \ + const int pao_dec__ = (typeof(var))(val) == \ + (typeof(var))-1; \ \ if (0) { \ typeof(var) pao_tmp__; \ pao_tmp__ = (val); \ (void)pao_tmp__; \ } \ - if (pao_ID__ == 1) \ + if (pao_const__ && pao_inc__) \ percpu_unary_op(size, qual, "inc", var); \ - else if (pao_ID__ == -1) \ + else if (pao_const__ && pao_dec__) \ percpu_unary_op(size, qual, "dec", var); \ else \ percpu_binary_op(size, qual, "add", var, val); \ -- 2.34.1