Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753965AbbGWTOV (ORCPT ); Thu, 23 Jul 2015 15:14:21 -0400 Received: from mail-qk0-f174.google.com ([209.85.220.174]:36265 "EHLO mail-qk0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753295AbbGWTOR (ORCPT ); Thu, 23 Jul 2015 15:14:17 -0400 Message-ID: <55B13D05.9050304@gmail.com> Date: Thu, 23 Jul 2015 15:14:13 -0400 From: Jason Baron User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.8.0 MIME-Version: 1.0 To: Peter Zijlstra CC: Borislav Petkov , Andy Lutomirski , Thomas Gleixner , Mikulas Patocka , Paul Mackerras , Arnaldo Carvalho de Melo , Kees Cook , Andrea Arcangeli , Vince Weaver , "hillf.zj" , Valdis Kletnieks , "linux-kernel@vger.kernel.org" , Steven Rostedt Subject: Re: Kernel broken on processors without performance counters References: <20150721154959.GS19282@twins.programming.kicks-ass.net> <20150721161215.GU19282@twins.programming.kicks-ass.net> <20150721181553.GA3378@nazgul.tnic> <55AE9471.1000601@gmail.com> <20150722042403.GA6345@nazgul.tnic> <55AFCDA4.5010406@gmail.com> <20150723104215.GH25159@twins.programming.kicks-ass.net> <55B0F808.2060302@gmail.com> <20150723143308.GD19282@twins.programming.kicks-ass.net> <20150723144930.GK18673@twins.programming.kicks-ass.net> In-Reply-To: <20150723144930.GK18673@twins.programming.kicks-ass.net> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 17315 Lines: 520 On 07/23/2015 10:49 AM, Peter Zijlstra wrote: > On Thu, Jul 23, 2015 at 04:33:08PM +0200, Peter Zijlstra wrote: >> Lemme finish this and I'll post it. > Compile tested on x86_64 only.. > > Please have a look, I think you said I got some of the logic wrong, I've > not double checked that. > > I'll go write comments and double check things. > > --- > arch/arm/include/asm/jump_label.h | 22 +++++++- > arch/arm64/include/asm/jump_label.h | 22 +++++++- > arch/mips/include/asm/jump_label.h | 23 +++++++- > arch/powerpc/include/asm/jump_label.h | 23 +++++++- > arch/s390/include/asm/jump_label.h | 23 +++++++- > arch/sparc/include/asm/jump_label.h | 38 ++++++++++--- > arch/x86/include/asm/jump_label.h | 24 +++++++- > include/linux/jump_label.h | 101 +++++++++++++++++++++++++++++++++- > kernel/jump_label.c | 89 +++++++++++++++--------------- > 9 files changed, 297 insertions(+), 68 deletions(-) > > diff --git a/arch/arm/include/asm/jump_label.h b/arch/arm/include/asm/jump_label.h > index 5f337dc5c108..6c9789b33497 100644 > --- a/arch/arm/include/asm/jump_label.h > +++ b/arch/arm/include/asm/jump_label.h > @@ -13,14 +13,32 @@ > #define JUMP_LABEL_NOP "nop" > #endif > > -static __always_inline bool arch_static_branch(struct static_key *key) > +static __always_inline bool arch_static_branch(struct static_key *key, bool inv) > { > + unsigned long kval = (unsigned long)key + inv; > + > asm_volatile_goto("1:\n\t" > JUMP_LABEL_NOP "\n\t" > ".pushsection __jump_table, \"aw\"\n\t" > ".word 1b, %l[l_yes], %c0\n\t" > ".popsection\n\t" > - : : "i" (key) : : l_yes); > + : : "i" (kval) : : l_yes); > + > + return false; > +l_yes: > + return true; > +} > + > +static __always_inline bool arch_static_branch_jump(struct static_key *key, bool inv) > +{ > + unsigned long kval = (unsigned long)key + inv; > + > + asm_volatile_goto("1:\n\t" > + "b %l[l_yes]\n\t" > + ".pushsection __jump_table, \"aw\"\n\t" > + ".word 1b, %l[l_yes], %c0\n\t" > + ".popsection\n\t" > + : : "i" (kval) : : l_yes); > > return false; > l_yes: > diff --git a/arch/arm64/include/asm/jump_label.h b/arch/arm64/include/asm/jump_label.h > index c0e5165c2f76..e5cda5d75c62 100644 > --- a/arch/arm64/include/asm/jump_label.h > +++ b/arch/arm64/include/asm/jump_label.h > @@ -26,14 +26,32 @@ > > #define JUMP_LABEL_NOP_SIZE AARCH64_INSN_SIZE > > -static __always_inline bool arch_static_branch(struct static_key *key) > +static __always_inline bool arch_static_branch(struct static_key *key, bool inv) > { > + unsigned long kval = (unsigned long)key + inv; > + > asm goto("1: nop\n\t" > ".pushsection __jump_table, \"aw\"\n\t" > ".align 3\n\t" > ".quad 1b, %l[l_yes], %c0\n\t" > ".popsection\n\t" > - : : "i"(key) : : l_yes); > + : : "i"(kval) : : l_yes); > + > + return false; > +l_yes: > + return true; > +} > + > +static __always_inline bool arch_static_branch_jump(struct static_key *key, bool inv) > +{ > + unsigned long kval = (unsigned long)key + inv; > + > + asm goto("1: b %l[l_yes]\n\t" > + ".pushsection __jump_table, \"aw\"\n\t" > + ".align 3\n\t" > + ".quad 1b, %l[l_yes], %c0\n\t" > + ".popsection\n\t" > + : : "i"(kval) : : l_yes); > > return false; > l_yes: > diff --git a/arch/mips/include/asm/jump_label.h b/arch/mips/include/asm/jump_label.h > index 608aa57799c8..d9fca6f52a93 100644 > --- a/arch/mips/include/asm/jump_label.h > +++ b/arch/mips/include/asm/jump_label.h > @@ -26,14 +26,33 @@ > #define NOP_INSN "nop" > #endif > > -static __always_inline bool arch_static_branch(struct static_key *key) > +static __always_inline bool arch_static_branch(struct static_key *key, bool inv) > { > + unsigned long kval = (unsigned long)key + inv; > + > asm_volatile_goto("1:\t" NOP_INSN "\n\t" > "nop\n\t" > ".pushsection __jump_table, \"aw\"\n\t" > WORD_INSN " 1b, %l[l_yes], %0\n\t" > ".popsection\n\t" > - : : "i" (key) : : l_yes); > + : : "i" (kval) : : l_yes); > + > + return false; > +l_yes: > + return true; > +} > + > +static __always_inline bool arch_static_branch_jump(struct static_key *key, bool inv) > +{ > + unsigned long kval = (unsigned long)key + inv; > + > + asm_volatile_goto("1:\tj %l[l_yes]\n\t" > + "nop\n\t" > + ".pushsection __jump_table, \"aw\"\n\t" > + WORD_INSN " 1b, %l[l_yes], %0\n\t" > + ".popsection\n\t" > + : : "i" (kval) : : l_yes); > + > return false; > l_yes: > return true; > diff --git a/arch/powerpc/include/asm/jump_label.h b/arch/powerpc/include/asm/jump_label.h > index efbf9a322a23..c7cffedb1801 100644 > --- a/arch/powerpc/include/asm/jump_label.h > +++ b/arch/powerpc/include/asm/jump_label.h > @@ -18,14 +18,33 @@ > #define JUMP_ENTRY_TYPE stringify_in_c(FTR_ENTRY_LONG) > #define JUMP_LABEL_NOP_SIZE 4 > > -static __always_inline bool arch_static_branch(struct static_key *key) > +static __always_inline bool arch_static_branch(struct static_key *key, bool inv) > { > + unsigned long kval = (unsigned long)key + inv; > + > asm_volatile_goto("1:\n\t" > "nop\n\t" > ".pushsection __jump_table, \"aw\"\n\t" > JUMP_ENTRY_TYPE "1b, %l[l_yes], %c0\n\t" > ".popsection \n\t" > - : : "i" (key) : : l_yes); > + : : "i" (kval) : : l_yes); > + > + return false; > +l_yes: > + return true; > +} > + > +static __always_inline bool arch_static_branch_jump(struct static_key *key, bool inv) > +{ > + unsigned long kval = (unsigned long)key + inv; > + > + asm_volatile_goto("1:\n\t" > + "b %l[l_yes]\n\t" > + ".pushsection __jump_table, \"aw\"\n\t" > + JUMP_ENTRY_TYPE "1b, %l[l_yes], %c0\n\t" > + ".popsection \n\t" > + : : "i" (kval) : : l_yes); > + > return false; > l_yes: > return true; > diff --git a/arch/s390/include/asm/jump_label.h b/arch/s390/include/asm/jump_label.h > index 69972b7957ee..4734b09b9fce 100644 > --- a/arch/s390/include/asm/jump_label.h > +++ b/arch/s390/include/asm/jump_label.h > @@ -12,14 +12,33 @@ > * We use a brcl 0,2 instruction for jump labels at compile time so it > * can be easily distinguished from a hotpatch generated instruction. > */ > -static __always_inline bool arch_static_branch(struct static_key *key) > +static __always_inline bool arch_static_branch(struct static_key *key, bool inv) > { > + unsigned long kval = (unsigned long)key + inv; > + > asm_volatile_goto("0: brcl 0,"__stringify(JUMP_LABEL_NOP_OFFSET)"\n" > ".pushsection __jump_table, \"aw\"\n" > ".balign 8\n" > ".quad 0b, %l[label], %0\n" > ".popsection\n" > - : : "X" (key) : : label); > + : : "X" (kval) : : label); > + > + return false; > +label: > + return true; > +} > + > +static __always_inline bool arch_static_branch_jump(struct static_key *key, bool inv) > +{ > + unsigned long kval = (unsigned long)key + inv; > + > + asm_volatile_goto("0: j %l[l_yes]\n" > + ".pushsection __jump_table, \"aw\"\n" > + ".balign 8\n" > + ".quad 0b, %l[label], %0\n" > + ".popsection\n" > + : : "X" (kval) : : label); > + > return false; > label: > return true; > diff --git a/arch/sparc/include/asm/jump_label.h b/arch/sparc/include/asm/jump_label.h > index cc9b04a2b11b..764f3c21da21 100644 > --- a/arch/sparc/include/asm/jump_label.h > +++ b/arch/sparc/include/asm/jump_label.h > @@ -7,16 +7,36 @@ > > #define JUMP_LABEL_NOP_SIZE 4 > > -static __always_inline bool arch_static_branch(struct static_key *key) > +static __always_inline bool arch_static_branch(struct static_key *key, bool inv) > { > - asm_volatile_goto("1:\n\t" > - "nop\n\t" > - "nop\n\t" > - ".pushsection __jump_table, \"aw\"\n\t" > - ".align 4\n\t" > - ".word 1b, %l[l_yes], %c0\n\t" > - ".popsection \n\t" > - : : "i" (key) : : l_yes); > + unsigned long kval = (unsigned long)key + inv; > + > + asm_volatile_goto("1:\n\t" > + "nop\n\t" > + "nop\n\t" > + ".pushsection __jump_table, \"aw\"\n\t" > + ".align 4\n\t" > + ".word 1b, %l[l_yes], %c0\n\t" > + ".popsection \n\t" > + : : "i" (kval) : : l_yes); > + > + return false; > +l_yes: > + return true; > +} > + > +static __always_inline bool arch_static_branch_jump(struct static_key *key, bool inv) > +{ > + unsigned long kval = (unsigned long)key + inv; > + > + asm_volatile_goto("1:\n\t" > + "b %l[l_yes]\n\t" > + ".pushsection __jump_table, \"aw\"\n\t" > + ".align 4\n\t" > + ".word 1b, %l[l_yes], %c0\n\t" > + ".popsection \n\t" > + : : "i" (kval) : : l_yes); > + > return false; > l_yes: > return true; > diff --git a/arch/x86/include/asm/jump_label.h b/arch/x86/include/asm/jump_label.h > index a4c1cf7e93f8..43531baaee38 100644 > --- a/arch/x86/include/asm/jump_label.h > +++ b/arch/x86/include/asm/jump_label.h > @@ -16,15 +16,35 @@ > # define STATIC_KEY_INIT_NOP GENERIC_NOP5_ATOMIC > #endif > > -static __always_inline bool arch_static_branch(struct static_key *key) > +static __always_inline bool arch_static_branch(struct static_key *key, bool inv) > { > + unsigned long kval = (unsigned long)key + inv; > + > asm_volatile_goto("1:" > ".byte " __stringify(STATIC_KEY_INIT_NOP) "\n\t" > ".pushsection __jump_table, \"aw\" \n\t" > _ASM_ALIGN "\n\t" > _ASM_PTR "1b, %l[l_yes], %c0 \n\t" > ".popsection \n\t" > - : : "i" (key) : : l_yes); > + : : "i" (kval) : : l_yes); > + > + return false; > +l_yes: > + return true; > +} > + > +static __always_inline bool arch_static_branch_jump(struct static_key *key, bool inv) > +{ > + unsigned long kval = (unsigned long)key + inv; > + > + asm_volatile_goto("1:" > + "jmp %l[l_yes]\n\t" > + ".pushsection __jump_table, \"aw\" \n\t" > + _ASM_ALIGN "\n\t" > + _ASM_PTR "1b, %l[l_yes], %c0 \n\t" > + ".popsection \n\t" > + : : "i" (kval) : : l_yes); > + > return false; > l_yes: > return true; > diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h > index f4de473f226b..4c97fed7acea 100644 > --- a/include/linux/jump_label.h > +++ b/include/linux/jump_label.h > @@ -122,7 +122,7 @@ static inline bool jump_label_get_branch_default(struct static_key *key) > > static __always_inline bool static_key_false(struct static_key *key) > { > - return arch_static_branch(key); > + return arch_static_branch(key, false); > } > > static __always_inline bool static_key_true(struct static_key *key) > @@ -213,6 +213,105 @@ static inline bool static_key_enabled(struct static_key *key) > return static_key_count(key) > 0; > } > > +static inline void static_key_enable(struct static_key *key) > +{ > + int count = static_key_count(key); > + > + WARN_ON_ONCE(count < 0 || count > 1); > + > + if (!count) > + static_key_slow_inc(key); > +} > + > +static inline void static_key_disable(struct static_key *key) > +{ > + int count = static_key_count(key); > + > + WARN_ON_ONCE(count < 0 || count > 1); > + > + if (count) > + static_key_slow_dec(key); > +} > + > + > +/* -------------------------------------------------------------------------- */ > + > +/* > + * Two type wrappers around static_key, such that we can use compile time > + * type differentiation to emit the right code. > + * > + * All the below code is macros in order to play type games. > + */ > + > +struct static_key_true { > + struct static_key key; > +}; > + > +struct static_key_false { > + struct static_key key; > +}; > + > +#define STATIC_KEY_TRUE_INIT (struct static_key_true) { .key = STATIC_KEY_INIT_TRUE, } > +#define STATIC_KEY_FALSE_INIT (struct static_key_false){ .key = STATIC_KEY_INIT_FALSE, } > + > +#define DEFINE_STATIC_KEY_TRUE(name) \ > + struct static_key_true name = STATIC_KEY_TRUE_INIT > + > +#define DEFINE_STATIC_KEY_FALSE(name) \ > + struct static_key_false name = STATIC_KEY_FALSE_INIT > + > +#ifdef HAVE_JUMP_LABEL > + > +/* > + * Combine the right initial value (type) with the right branch order and section > + * to generate the desired result. > + */ > + > +#define static_branch_likely(x) \ > +({ \ > + bool branch; \ > + if (__builtin_types_compatible_p(typeof(x), struct static_key_true)) \ > + branch = !arch_static_branch(&(x)->key, false); \ > + else if (__builtin_types_compatible_p(typeof(x), struct static_key_false)) \ > + branch = !arch_static_branch_jump(&(x)->key, true); \ > + else \ > + branch = ____wrong_branch_error(); \ > + branch; \ > +}) > + > +#define static_branch_unlikely(x) \ > +({ \ > + bool branch; \ > + if (__builtin_types_compatible_p(typeof(x), struct static_key_true)) \ > + branch = arch_static_branch(&(x)->key, true); \ > + else if (__builtin_types_compatible_p(typeof(x), struct static_key_false)) \ > + branch = arch_static_branch_jump(&(x)->key, false); \ > + else \ > + branch = ____wrong_branch_error(); \ > + branch; \ > +}) > + > +#else /* !HAVE_JUMP_LABEL */ > + > +#define static_branch_likely(x) static_key_true(&(x)->key) > +#define static_branch_unlikely(x) static_key_false(&(x)->key) > + > +#endif /* HAVE_JUMP_LABEL */ > + > +/* > + * Advanced usage; refcount, branch is enabled when: count != 0 > + */ > + > +#define static_branch_inc(x) static_key_slow_inc(&(x)->key) > +#define static_branch_dec(x) static_key_slow_dec(&(x)->key) > + > +/* > + * Normal usage; boolean enable/disable. > + */ > + > +#define static_branch_enable(x) static_key_enable(&(x)->key) > +#define static_branch_disable(x) static_key_disable(&(x)->key) > + > #endif /* _LINUX_JUMP_LABEL_H */ > > #endif /* __ASSEMBLY__ */ > diff --git a/kernel/jump_label.c b/kernel/jump_label.c > index 52ebaca1b9fc..8d0cb621c5bc 100644 > --- a/kernel/jump_label.c > +++ b/kernel/jump_label.c > @@ -54,7 +54,7 @@ jump_label_sort_entries(struct jump_entry *start, struct jump_entry *stop) > sort(start, size, sizeof(struct jump_entry), jump_label_cmp, NULL); > } > > -static void jump_label_update(struct static_key *key, int enable); > +static void jump_label_update(struct static_key *key); > > void static_key_slow_inc(struct static_key *key) > { > @@ -63,13 +63,8 @@ void static_key_slow_inc(struct static_key *key) > return; > > jump_label_lock(); > - if (atomic_read(&key->enabled) == 0) { > - if (!jump_label_get_branch_default(key)) > - jump_label_update(key, JUMP_LABEL_ENABLE); > - else > - jump_label_update(key, JUMP_LABEL_DISABLE); > - } > - atomic_inc(&key->enabled); > + if (atomic_inc_and_test(&key->enabled)) > + jump_label_update(key); > jump_label_unlock(); > } > EXPORT_SYMBOL_GPL(static_key_slow_inc); > @@ -87,10 +82,7 @@ static void __static_key_slow_dec(struct static_key *key, > atomic_inc(&key->enabled); > schedule_delayed_work(work, rate_limit); > } else { > - if (!jump_label_get_branch_default(key)) > - jump_label_update(key, JUMP_LABEL_DISABLE); > - else > - jump_label_update(key, JUMP_LABEL_ENABLE); > + jump_label_update(key); > } > jump_label_unlock(); > } > @@ -149,7 +141,7 @@ static int __jump_label_text_reserved(struct jump_entry *iter_start, > return 0; > } > > -/* > +/* > * Update code which is definitely not currently executing. > * Architectures which need heavyweight synchronization to modify > * running code can override this to make the non-live update case > @@ -158,37 +150,44 @@ static int __jump_label_text_reserved(struct jump_entry *iter_start, > void __weak __init_or_module arch_jump_label_transform_static(struct jump_entry *entry, > enum jump_label_type type) > { > - arch_jump_label_transform(entry, type); > + arch_jump_label_transform(entry, type); > +} > + > +static struct static_key *static_key_cast(jump_label_t key_addr) > +{ > + return (struct static_key *)(key_addr & ~1UL); > +} > + > +static bool static_key_inv(jump_label_t key_addr) > +{ > + return key_addr & 1UL; > +} > + > +static enum jump_label_type jump_label_type(struct jump_entry *entry) > +{ > + struct static_key *key = static_key_cast(iter->key); > + bool true_branch = jump_label_get_branch_default(key); > + bool state = static_key_enabled(key); > + bool inv = static_key_inv(iter->key); > + > + return (true_branch ^ state) ^ inv; iiuc...this allows both the old style keys to co-exist with the new ones. IE state wouldn't be set for the new ones. And inv shouldn't be set for the old ones. Is that correct? Thanks, -Jason -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/