Received: by 2002:a05:6a10:17d3:0:0:0:0 with SMTP id hz19csp3114942pxb; Tue, 13 Apr 2021 19:41:42 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwea9y19tWHU30RanIm6nJ4QLi6EnY75po8cAU83hX3O4nz+SAPypQhAXRadCUFnjmOD0X8 X-Received: by 2002:a05:6402:1853:: with SMTP id v19mr38151539edy.179.1618368102061; Tue, 13 Apr 2021 19:41:42 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1618368102; cv=none; d=google.com; s=arc-20160816; b=c+yvxr71c6bZgroZ8A/k5mlu6GPEbst0LLFYcWJb2gMpa/dTIzYeiBmQEpcjcEs6nD +syL7Ou98dLRylku21U4cPl527D+T2Xv3WzM/7UF0lP48spkn13GWvND90rCXq7oFb3g 2J+F9YJpt3LQ4/7Hc+bvfqbxPg9J70JEVi/rF4xDl0+Uh0bXMjjz8afmGr04mUjzA082 YPnv/wb8dHE0i2ax9rv/4Kb7uY8Emm7V6wjqyeyuO1tdGNz2qJZaLiuXi/GGXB+dvNMl 8mCBO1qwAObveaDBo/2VdQl7uKq/jUfb3P82z6dL7FBjyjA9gOHeug1T7D9rGOkddmMk Pucw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:date:cc:to:subject:from:message-id; bh=cMA3bYMVdYySRsxMLO0veFCvWZ2aeRY95gdHtlmvJf0=; b=sL54lxD7nvBamiYEX8k+FkSHLS8st63lvDx7rdoCVh8avyoSVRVy4S2yuJ7fgsasC4 fIvxaed48CLGVczDuBmstNwif6qqZ2pGV+nX9MS9MqiN40cHqhhakbae9wnPxb+Donfw EbZB5857xKcVW56CiSWCTjsIiz9yyhGLPfpqo30Geb4tbNkPnzSGwvF5wIP8A6O7IpMh byIvkXOaYrfuheMmrlL5ggBWpdozvaWLQBHxlztX/iboU6ooGRPhBIKxOba+wJTWN8fK Hv5CidviuQkxR0DXJ2XERCB/mgS5cI8H9lrtR6CaBoYkJjN+EL/AMKS+1eBjpOXfikSz SlTA== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id bq17si8637378ejb.227.2021.04.13.19.41.18; Tue, 13 Apr 2021 19:41:42 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1346348AbhDMQic (ORCPT + 99 others); Tue, 13 Apr 2021 12:38:32 -0400 Received: from pegase1.c-s.fr ([93.17.236.30]:13999 "EHLO pegase1.c-s.fr" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237196AbhDMQib (ORCPT ); Tue, 13 Apr 2021 12:38:31 -0400 Received: from localhost (mailhub1-int [192.168.12.234]) by localhost (Postfix) with ESMTP id 4FKWW45VSQz9v4hH; Tue, 13 Apr 2021 18:38:08 +0200 (CEST) X-Virus-Scanned: Debian amavisd-new at c-s.fr Received: from pegase1.c-s.fr ([192.168.12.234]) by localhost (pegase1.c-s.fr [192.168.12.234]) (amavisd-new, port 10024) with ESMTP id bqKT4YHHhrjn; Tue, 13 Apr 2021 18:38:08 +0200 (CEST) Received: from messagerie.si.c-s.fr (messagerie.si.c-s.fr [192.168.25.192]) by pegase1.c-s.fr (Postfix) with ESMTP id 4FKWW44hB0z9v4h8; Tue, 13 Apr 2021 18:38:08 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by messagerie.si.c-s.fr (Postfix) with ESMTP id 2F4768B7AA; Tue, 13 Apr 2021 18:38:10 +0200 (CEST) X-Virus-Scanned: amavisd-new at c-s.fr Received: from messagerie.si.c-s.fr ([127.0.0.1]) by localhost (messagerie.si.c-s.fr [127.0.0.1]) (amavisd-new, port 10023) with ESMTP id 64xtLPtuoMsE; Tue, 13 Apr 2021 18:38:10 +0200 (CEST) Received: from po16121vm.idsi0.si.c-s.fr (unknown [192.168.4.90]) by messagerie.si.c-s.fr (Postfix) with ESMTP id C97CF8B75F; Tue, 13 Apr 2021 18:38:09 +0200 (CEST) Received: by po16121vm.idsi0.si.c-s.fr (Postfix, from userid 0) id 9D25067A15; Tue, 13 Apr 2021 16:38:09 +0000 (UTC) Message-Id: From: Christophe Leroy Subject: [PATCH v2 1/2] powerpc/bug: Remove specific powerpc BUG_ON() and WARN_ON() on PPC32 To: Benjamin Herrenschmidt , Paul Mackerras , Michael Ellerman Cc: linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org Date: Tue, 13 Apr 2021 16:38:09 +0000 (UTC) Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org powerpc BUG_ON() and WARN_ON() are based on using twnei instruction. For catching simple conditions like a variable having value 0, this is efficient because it does the test and the trap at the same time. But most conditions used with BUG_ON or WARN_ON are more complex and forces GCC to format the condition into a 0 or 1 value in a register. This will usually require 2 to 3 instructions. The most efficient solution would be to use __builtin_trap() because GCC is able to optimise the use of the different trap instructions based on the requested condition, but this is complex if not impossible for the following reasons: - __builtin_trap() is a non-recoverable instruction, so it can't be used for WARN_ON - Knowing which line of code generated the trap would require the analysis of DWARF information. This is not a feature we have today. As mentioned in commit 8d4fbcfbe0a4 ("Fix WARN_ON() on bitfield ops") the way WARN_ON() is implemented is suboptimal. That commit also mentions an issue with 'long long' condition. It fixed it for WARN_ON() but the same problem still exists today with BUG_ON() on PPC32. It will be fixed by using the generic implementation. By using the generic implementation, gcc will naturally generate a branch to the unconditional trap generated by BUG(). As modern powerpc implement zero-cycle branch, that's even more efficient. And for the functions using WARN_ON() and its return, the test on return from WARN_ON() is now also used for the WARN_ON() itself. On PPC64 we don't want it because we want to be able to use CFAR register to track how we entered the code that trapped. The CFAR register would be clobbered by the branch. A simple test function: unsigned long test9w(unsigned long a, unsigned long b) { if (WARN_ON(!b)) return 0; return a / b; } Before the patch: 0000046c : 46c: 7c 89 00 34 cntlzw r9,r4 470: 55 29 d9 7e rlwinm r9,r9,27,5,31 474: 0f 09 00 00 twnei r9,0 478: 2c 04 00 00 cmpwi r4,0 47c: 41 82 00 0c beq 488 480: 7c 63 23 96 divwu r3,r3,r4 484: 4e 80 00 20 blr 488: 38 60 00 00 li r3,0 48c: 4e 80 00 20 blr After the patch: 00000468 : 468: 2c 04 00 00 cmpwi r4,0 46c: 41 82 00 0c beq 478 470: 7c 63 23 96 divwu r3,r3,r4 474: 4e 80 00 20 blr 478: 0f e0 00 00 twui r0,0 47c: 38 60 00 00 li r3,0 480: 4e 80 00 20 blr So we see before the patch we need 3 instructions on the likely path to handle the WARN_ON(). With the patch the trap goes on the unlikely path. See below the difference at the entry of system_call_exception where we have several BUG_ON(), allthough less impressing. With the patch: 00000000 : 0: 81 6a 00 84 lwz r11,132(r10) 4: 90 6a 00 88 stw r3,136(r10) 8: 71 60 00 02 andi. r0,r11,2 c: 41 82 00 70 beq 7c 10: 71 60 40 00 andi. r0,r11,16384 14: 41 82 00 6c beq 80 18: 71 6b 80 00 andi. r11,r11,32768 1c: 41 82 00 68 beq 84 20: 94 21 ff e0 stwu r1,-32(r1) 24: 93 e1 00 1c stw r31,28(r1) 28: 7d 8c 42 e6 mftb r12 ... 7c: 0f e0 00 00 twui r0,0 80: 0f e0 00 00 twui r0,0 84: 0f e0 00 00 twui r0,0 Without the patch: 00000000 : 0: 94 21 ff e0 stwu r1,-32(r1) 4: 93 e1 00 1c stw r31,28(r1) 8: 90 6a 00 88 stw r3,136(r10) c: 81 6a 00 84 lwz r11,132(r10) 10: 69 60 00 02 xori r0,r11,2 14: 54 00 ff fe rlwinm r0,r0,31,31,31 18: 0f 00 00 00 twnei r0,0 1c: 69 60 40 00 xori r0,r11,16384 20: 54 00 97 fe rlwinm r0,r0,18,31,31 24: 0f 00 00 00 twnei r0,0 28: 69 6b 80 00 xori r11,r11,32768 2c: 55 6b 8f fe rlwinm r11,r11,17,31,31 30: 0f 0b 00 00 twnei r11,0 34: 7d 8c 42 e6 mftb r12 Signed-off-by: Christophe Leroy --- arch/powerpc/include/asm/bug.h | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/arch/powerpc/include/asm/bug.h b/arch/powerpc/include/asm/bug.h index d1635ffbb179..101dea4eec8d 100644 --- a/arch/powerpc/include/asm/bug.h +++ b/arch/powerpc/include/asm/bug.h @@ -68,7 +68,11 @@ BUG_ENTRY("twi 31, 0, 0", 0); \ unreachable(); \ } while (0) +#define HAVE_ARCH_BUG + +#define __WARN_FLAGS(flags) BUG_ENTRY("twi 31, 0, 0", BUGFLAG_WARNING | (flags)) +#ifdef CONFIG_PPC64 #define BUG_ON(x) do { \ if (__builtin_constant_p(x)) { \ if (x) \ @@ -78,8 +82,6 @@ } \ } while (0) -#define __WARN_FLAGS(flags) BUG_ENTRY("twi 31, 0, 0", BUGFLAG_WARNING | (flags)) - #define WARN_ON(x) ({ \ int __ret_warn_on = !!(x); \ if (__builtin_constant_p(__ret_warn_on)) { \ @@ -93,9 +95,10 @@ unlikely(__ret_warn_on); \ }) -#define HAVE_ARCH_BUG #define HAVE_ARCH_BUG_ON #define HAVE_ARCH_WARN_ON +#endif + #endif /* __ASSEMBLY __ */ #else #ifdef __ASSEMBLY__ -- 2.25.0