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.
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
