lkml.org 
[lkml]   [2011]   [Mar]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/6] x86-64: Optimize vread_tsc's barriers

* Andy Lutomirski <luto@MIT.EDU> wrote:

> @@ -767,18 +767,38 @@ static cycle_t read_tsc(struct clocksource *cs)
> static cycle_t __vsyscall_fn vread_tsc(void)
> {
> cycle_t ret;
> -
> - /*
> - * Surround the RDTSC by barriers, to make sure it's not
> - * speculated to outside the seqlock critical section and
> - * does not cause time warps:
> + u64 zero, last;
> +
> + /* rdtsc is unordered, and we want it to be ordered like
> + * a load with respect to other CPUs (and we don't want
> + * it to execute absurdly early wrt code on this CPU.)
> + * rdtsc_barrier() is a barrier that provides this ordering
> + * with respect to *earlier* loads. (Which barrier to use
> + * depends on the CPU.)
> */
> rdtsc_barrier();
> - ret = (cycle_t)vget_cycles();
> - rdtsc_barrier();
>
> - return ret >= __vsyscall_gtod_data.clock.cycle_last ?
> - ret : __vsyscall_gtod_data.clock.cycle_last;
> + asm volatile ("rdtsc\n\t"
> + "shl $0x20,%%rdx\n\t"
> + "or %%rdx,%%rax\n\t"
> + "shl $0x20,%%rdx"
> + : "=a" (ret), "=d" (zero) : : "cc");
> +
> + /* zero == 0, but as far as the processor is concerned, zero
> + * depends on the output of rdtsc. So we can use it as a
> + * load barrier by loading something that depends on it.
> + * x86-64 keeps all loads in order wrt each other, so this
> + * ensures that rdtsc is ordered wrt all later loads.
> + */
> +
> + /* This doesn't multiply 'zero' by anything, which *should*
> + * generate nicer code, except that gcc cleverly embeds the
> + * dereference into the cmp and the cmovae. Oh, well.
> + */
> + last = *( (cycle_t *)
> + ((char *)&__vsyscall_gtod_data.clock.cycle_last + zero) );
> +
> + return ret >= last ? ret : last;

Looks like GCC hurts performance here more than it helps. Have you considered
putting the whole function into assembly in a .S file? How maintainable does it
look like?

Thanks,

Ingo


\
 
 \ /
  Last update: 2011-03-29 08:21    [from the cache]
©2003-2011 Jasper Spaans