From mboxrd@z Thu Jan 1 00:00:00 1970 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752424AbeAQINR (ORCPT + 1 other); Wed, 17 Jan 2018 03:13:17 -0500 Received: from bombadil.infradead.org ([65.50.211.133]:36055 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750969AbeAQINP (ORCPT ); Wed, 17 Jan 2018 03:13:15 -0500 Date: Wed, 17 Jan 2018 09:13:09 +0100 From: Peter Zijlstra To: Josh Poimboeuf Cc: David Woodhouse , linux-kernel@vger.kernel.org, Dave Hansen , Ashok Raj , Thomas Gleixner , Tim Chen , Andy Lutomirski , Linus Torvalds , Greg KH , Andrea Arcangeli , Andi Kleen , Arjan Van De Ven , Dan Williams , Paolo Bonzini , Jun Nakajima , Asit Mallick Subject: Re: [PATCH v2 11/10] objtool: Even more complex static block checks Message-ID: <20180117081309.GJ2228@hirez.programming.kicks-ass.net> References: <20180116142825.376986833@infradead.org> <20180116194917.GH2228@hirez.programming.kicks-ass.net> <20180117031232.y7t25srlfsym5len@treble> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180117031232.y7t25srlfsym5len@treble> User-Agent: Mutt/1.9.2 (2017-12-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: On Tue, Jan 16, 2018 at 09:12:32PM -0600, Josh Poimboeuf wrote: > On Tue, Jan 16, 2018 at 08:49:17PM +0100, Peter Zijlstra wrote: > > Subject: objtool: Even more complex static block checks > > From: Peter Zijlstra > > Date: Tue Jan 16 20:17:01 CET 2018 > > > > I've observed GCC transform: > > > > f() > > { > > if (!static_branch_unlikely()) > > return; > > > > static_assert(); > > A; > > } > > > > g() > > { > > f(); > > } > > > > Into: > > > > f() > > { > > static_assert(); > > A; > > } > > > > g() > > { > > if (static_branch_unlikely()) > > f(); > > } > > > > Which results in the assertion landing at f+0. The transformation is > > valid and useful; it avoids a pointless CALL+RET sequence, so we'll > > have to teach objtool how to deal with this. > > > > Do this by marking all CALL destinations with static_call when called > > from a static_block and non_static_call when called outside a > > static_block. This allows us to identify functions called exclusively > > from a static_block and start them with a static_block. > > Ew... where'd you place the assertion to trigger this? Its the patch I pastebin'ed you earlier, also see below. > It's late and my brain has already clocked out, so I'll need to revisit > this tomorrow. But now I'm wondering if my basic block idea would be a > better way to solve this. I would think basic-blocks are inside functions, and this patch goes across functions, something you'd still need even if you had basic blocks. Also, basic blocks are non-trivial because they can overlap. I've implemented something like that before for perf, see commit: 70fbe0574558 ("perf annotate: Add branch stack / basic block") We could probably lift that code fairly easily. --- Subject: jump_label: Add static assertion to every static_branch From: Peter Zijlstra Date: Tue Jan 16 15:27:36 CET 2018 for testing.. not sure we wants this in general Signed-off-by: Peter Zijlstra (Intel) --- arch/x86/include/asm/jump_label.h | 1 + include/linux/jump_label.h | 8 ++++++-- 2 files changed, 7 insertions(+), 2 deletions(-) --- a/arch/x86/include/asm/jump_label.h +++ b/arch/x86/include/asm/jump_label.h @@ -76,6 +76,7 @@ static __always_inline bool arch_static_ * * Also works with static_cpu_has(). */ +#define arch_static_assert arch_static_assert static __always_inline void arch_static_assert(void) { asm volatile ("1:\n\t" --- a/include/linux/jump_label.h +++ b/include/linux/jump_label.h @@ -323,6 +323,10 @@ extern bool ____wrong_branch_error(void) #ifdef HAVE_JUMP_LABEL +#ifndef arch_static_assert +#define arch_static_assert (void) +#endif + /* * Combine the right initial value (type) with the right branch order * to generate the desired result. @@ -388,7 +392,7 @@ extern bool ____wrong_branch_error(void) branch = !arch_static_branch_jump(&(x)->key, true); \ else \ branch = ____wrong_branch_error(); \ - branch; \ + branch && (arch_static_assert(), true); \ }) #define static_branch_unlikely(x) \ @@ -400,7 +404,7 @@ extern bool ____wrong_branch_error(void) branch = arch_static_branch(&(x)->key, false); \ else \ branch = ____wrong_branch_error(); \ - branch; \ + branch && (arch_static_assert(), true); \ }) #else /* !HAVE_JUMP_LABEL */ > Josh