Re: [PATCH -v2 6/8] jump_label: Add a new static_key interface

[Borislav Petkov <bp@alien8.de>]

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