Messages in this thread | | | Date | Tue, 29 Mar 2011 08:18:25 +0200 | From | Ingo Molnar <> | Subject | Re: [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
| |