Date: Tue, 21 Jul 2015 17:43:27 +0200 (CEST)
From: Thomas Gleixner <tglx@linutronix.de>
To: Peter Zijlstra <peterz@infradead.org>
cc: Andy Lutomirski <luto@amacapital.net>, Jason Baron <jasonbaron0@gmail.com>,
        Mikulas Patocka <mpatocka@redhat.com>,
        Paul Mackerras <paulus@samba.org>,
        Arnaldo Carvalho de Melo <acme@kernel.org>,
        Kees Cook <keescook@chromium.org>,
        Andrea Arcangeli <aarcange@redhat.com>,
        Vince Weaver <vince@deater.net>, "hillf.zj" <hillf.zj@alibaba-inc.com>,
        Valdis Kletnieks <Valdis.Kletnieks@vt.edu>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Borislav Petkov <bp@alien8.de>, Steven Rostedt <rostedt@goodmis.org>
Subject: Re: Kernel broken on processors without performance counters
In-Reply-To: <20150721082107.GE18673@twins.programming.kicks-ass.net>
Message-ID: <alpine.DEB.2.11.1507211726040.18576@nanos>
References: <alpine.LRH.2.02.1507081000540.5217@file01.intranet.prod.int.rdu2.redhat.com> <20150708160750.GQ19282@twins.programming.kicks-ass.net> <CALCETrVqeG=3sKJv=pe9ZfaeRmx9uz+jNmXGmPu-7Hah4NvYDQ@mail.gmail.com> <559D8250.8000707@gmail.com>
 <CALCETrUb8tjDLQHF3q=_F7JRXhC8iEHW9o2xFsUbqsmyEOAiCQ@mail.gmail.com> <20150710141359.GJ19282@twins.programming.kicks-ass.net> <20150721082107.GE18673@twins.programming.kicks-ass.net>
User-Agent: Alpine 2.11 (DEB 23 2013-08-11)
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3143
Lines: 104

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