Re: [patch] x86: convert rdtscll() to use __native_read_tsc

[Jeremy Fitzhardinge <jeremy@goop.org>]

Ingo Molnar wrote:
> The reason for the __native_read_tsc() / native_read_tsc() distinction is 
> and obscure problem with paravirt function pointers. Such constructs:
>
>   ./xen/enlighten.c:	.read_tsc = native_read_tsc,
>
> do not always work fine with all versions of gcc, if native_read_tsc() is 
> a simple static inline (as it should be) - the build would fail with 
> certain gcc flags.

I don't think that's true.  We rely on taking function pointers of 
static inlines pretty extensively; native_read_tsc is hardly unique in 
this respect.  I don't remember seeing any problems of the sort you 
describe.  (I can well believe this may have been a problem at some 
point, but not during the pv-ops development timeframe.)

> Perhaps the real fix is to do this rename as well:
>
>     native_read_tsc => native_read_tsc_paravirt
>   __native_read_tsc => native_read_tsc
>
> as this makes the native_read_tsc_paravirt() a pure technical variant, to 
> be used in paravirt_ops function pointer assignments. People would thus 
> just use the obvious native_read_tsc() inline function most of the time 
> and could forget about native_read_tsc_paravirt().
>
> Jeremy?
>   

I'm trying to remember the real reason for 
__native_read_tsc/native_read_tsc.  At least part of it is that 
__native_read_tsc is used in a vdso, and so *must* be inlined to avoid a 
bogus call from user to kernel space.  But I don't know why you wouldn't 
want to inline native_read_tsc everywhere.  I have a feeling it may be a 
relic from unification - possibly because x86-64 was late to the 
clocksource party - but I don't remember anything specific.

I think we can probably make do with a single native_read_tsc, so long 
as its always inlined.

    J