On 07/10/2015 10:13 AM, Peter Zijlstra wrote: > On Wed, Jul 08, 2015 at 05:36:43PM -0700, Andy Lutomirski wrote: >>>> In what universe is "static_key_false" a reasonable name for a >>>> function that returns true if a static key is true? >> I think the current naming is almost maximally bad. The naming would >> be less critical if it were typesafe, though. > How about something like so on top? It will allow us to slowly migrate > existing and new users over to the hopefully saner interface? > > --- > include/linux/jump_label.h | 67 > +++++++++++++++++++++++++++++++++++++++++++++- > kernel/sched/core.c | 18 ++++++------- > 2 files changed, 74 insertions(+), 11 deletions(-) > > diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h > index f4de473f226b..98ed3c2ec78d 100644 > --- a/include/linux/jump_label.h > +++ b/include/linux/jump_label.h > @@ -213,6 +213,71 @@ static inline bool static_key_enabled(struct static_key > *key) > return static_key_count(key) > 0; > } > > -#endif /* _LINUX_JUMP_LABEL_H */ > +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); > +}
should those be __static_key_enable()/disable() to indicate that we don't that we don't want ppl using these directly. Similarly for other 'internal' functions. > + > +/* > -------------------------------------------------------------------------- */ > + > +/* > + * likely -- default enabled, puts the branch body in-line > + */ > + > +struct static_likely_key { > + struct static_key key; > +}; > + > +#define STATIC_LIKELY_KEY_INIT (struct static_likely_key){ .key = > STATIC_KEY_INIT_TRUE, } > + > +static inline bool static_likely_branch(struct static_likely_key *key) > +{ > + return static_key_true(&key->key); > +} > + > +/* > + * unlikely -- default disabled, puts the branch body out-of-line > + */ > + > +struct static_unlikely_key { > + struct static_key key; > +}; > + > +#define STATIC_UNLIKELY_KEY_INIT (struct static_unlikely_key){ .key = > STATIC_KEY_INIT_FALSE, } > + > +static inline bool static_unlikely_branch(struct static_unlikely_key *key) > +{ > + return static_key_false(&key->key); > +} > + > +/* > + * Advanced usage; refcount, branch is enabled when: count != 0 > + */ > + > +#define static_branch_inc(_k) static_key_slow_inc(&(_k)->key) > +#define static_branch_dec(_k) static_key_slow_dec(&(_k)->key) > + I think of these as operations on the 'static_key' so I still like 'static_key_inc()/dec()' (removing the 'slow' makes them different still). > +/* > + * Normal usage; boolean enable/disable. > + */ > + > +#define static_branch_enable(_k) static_key_enable(&(_k)->key) > +#define static_branch_disable(_k) static_key_disable(&(_k)->key) > Same here maybe: static_key_set_true()/false()? Thanks, -Jason -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/