Date: Tue, 4 Aug 2015 06:21:12 +0200
From: Borislav Petkov <bp@alien8.de>
To: Andy Lutomirski <luto@amacapital.net>
Cc: Steven Rostedt <rostedt@goodmis.org>,
        Peter Zijlstra <peterz@infradead.org>,
        Vlastimil Babka <vbabka@suse.cz>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Ingo Molnar <mingo@kernel.org>, Jason Baron <jasonbaron0@gmail.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Will Deacon <will.deacon@arm.com>, liuj97@gmail.com, rabin@rab.in,
        Ralf Baechle <ralf@linux-mips.org>,
        David Daney <ddaney@caviumnetworks.com>,
        Benjamin Herrenschmidt <benh@kernel.crashing.org>,
        michael@ellerman.id.au, Heiko Carstens <heiko.carstens@de.ibm.com>,
        "David S. Miller" <davem@davemloft.net>
Subject: Re: [PATCH -v2 6/8] jump_label: Add a new static_key interface
Message-ID: <20150804042112.GF31787@nazgul.tnic>
References: <20150728132313.164884020@infradead.org>
 <55B87E7A.2070509@suse.cz>
 <20150729084906.GH19282@twins.programming.kicks-ass.net>
 <20150803150359.0e76b576@gandalf.local.home>
 <20150803191816.GC25159@twins.programming.kicks-ass.net>
 <20150803152810.5a7bcf06@gandalf.local.home>
 <20150803200002.GE25159@twins.programming.kicks-ass.net>
 <20150803175757.2adf3275@gandalf.local.home>
 <20150804033733.GB31787@nazgul.tnic>
 <CALCETrX2EshRg87fBkHQggRj6kmpVF3+BBSTjsgmdw9A-7MFDQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <CALCETrX2EshRg87fBkHQggRj6kmpVF3+BBSTjsgmdw9A-7MFDQ@mail.gmail.com>
User-Agent: Mutt/1.5.23 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1648
Lines: 56

On Mon, Aug 03, 2015 at 09:07:53PM -0700, Andy Lutomirski wrote:
> Except that, with the new interface, static_key_likely is the other
> way around, right?  If the key is true (i.e. enabled), then it doesn't
> branch.
> 
> I think of the key as a boolean thing that happens to work by code
> patching under the hood.  The fancy patching affects the performance
> but doesn't really make it functionally different from a regular
> variable.  How about making it extra explicit:
> 
> static_key_set(&key, value);
> 
> where value is a bool or maybe even an unsigned int?

Let's have an actual example:

+       if (static_branch_likely(&__use_tsc)) {
+               u64 tsc_now = rdtsc();
+
+               /* return the value in ns */
+               return cycles_2_ns(tsc_now);
+       }

Well, I can see how the likely/unlikely things can confuse. They
actually don't have anything to do with where we will branch to but how
the code will be laid out, AFAICT. So I'm reading this as:

	if (use_tsc)) {
		RDTSC;
		return;
	}

and then it is straightforward.

So in this case, the jump will be disabled and we won't branch anywhere.
It actually becomes:

	RDTSC;
	return;

which can't get any more optimal than it is.

Hmm, yeah, I see how that can be confusing... But the asm is finally
fine. Hey, at least one thing...

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