Re: Kernel broken on processors without performance counters

[Borislav Petkov <bp@alien8.de>]

On Thu, Jul 23, 2015 at 12:42:15PM +0200, Peter Zijlstra wrote:
> > static_likely_init_true_branch(struct static_likely_init_true_key *key)
> > static_likely_init_false_branch(struct static_likely_init_false_key *key)
> > static_unlikely_init_false_branch(struct static_unlikely_init_false_key *key)
> > static_unlikely_init_true_branch(struct static_unlikely_init_true_key *key)
> 
> I'm sorely tempted to go quote cypress hill here...

Yah, those are at least too long and nuts.

> And I realize part of the problem is that we're wanting to use jump
> labels before we can patch them. But surely we can do better.
> 
> extern bool ____wrong_branch_error(void);
> 
> struct static_key_true;
> struct static_key_false;
> 
> #define static_branch_likely(x)							\
> ({										\
> 	bool branch;								\
> 	if (__builtin_types_compatible_p(typeof(x), struct static_key_true))	\
> 		branch = !arch_static_branch(&(x)->key);			\
> 	else if (__builtin_types_compatible_p(typeof(x), struct static_key_false)) \
> 		branch = !arch_static_branch_jump(&(x)->key);			\
> 	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);				\
> 	else if (__builtin_types_compatible_p(typeof(x), struct static_key_false)) \
> 		branch = arch_static_branch_jump(&(x)->key);			\
> 	else									\
> 		branch = ____wrong_branch_error();				\
> 	branch;									\
> })
> 
> Can't we make something like that work?
> 
> So the immediate problem appears to be the 4 different key inits, which don't
> seem very supportive of this separation:
> 
> +#define STATIC_KEY_LIKEY_INIT_TRUE ((struct static_unlikely_init_true_key)        \

LIKELY

> +    { .key.enabled = ATOMIC_INIT(1),                \
> +      .key.entries = (void *)JUMP_LABEL_TYPE_TRUE_BRANCH })
> 
> +#define STATIC_KEY_LIKEY_INIT_FALSE ((struct static_unlikely_init_false_key)    \

Yuck, those struct names are still too long IMO.

> +    { .key.enabled = ATOMIC_INIT(0),                \
> +      .key.entries = (void *)JUMP_LABEL_TYPE_TRUE_BRANCH })
> 
> +#define STATIC_KEY_UNLIKELY_INIT_TRUE ((struct static_unlikely_init_true_key)    \
> +    { .key.enabled = ATOMIC_INIT(1),                \
> +      .key.entries = (void *)JUMP_LABEL_TYPE_FALSE_BRANCH })
> 
> +#define STATIC_KEY_UNLIKELY_INIT_FALSE ((struct static_unlikely_init_false_key)    \
> +    { .key.enabled = ATOMIC_INIT(0),                \
> +      .key.entries = (void *)JUMP_LABEL_TYPE_FALSE_BRANCH })
> 
> 
> But I think we can fix that by using a second __jump_table section, then
> we can augment the LABEL_TYPE_{TRUE,FALSE} thing with the section we
> find the jump_entry in.
> 
> Then we can do:
> 
> #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, }

Let's abbreviate that "STATIC_KEY" thing too:

SK_TRUE_INIT
SK_FALSE_INIT
...

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--