Re: Kernel broken on processors without performance counters

[Thomas Gleixner <tglx@linutronix.de>]

On Tue, 21 Jul 2015, Peter Zijlstra wrote:

> On Fri, Jul 10, 2015 at 04:13:59PM +0200, 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?

It might be very well our universe, you just need access to the proper
drugs to feel comfortable with it.

> > > 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?

> > -#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);
> > +}

The functions above are not part of the interface which should be used
in code, right?

To avoid further confusion, can we please move all the existing mess
and the new helpers into jump_label_internals.h and just put the new
interfaces in jump_label.h?

> > +/* -------------------------------------------------------------------------- */
> > +
> > +/*
> > + * 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)

Inlines please

> > +/*
> > + * 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)

Ditto

All in all much more understandable than the existing horror.

Thanks,

	tglx