svn commit: r238755 - head/sys/x86/x86

[Andriy Gapon]

on 25/07/2012 01:10 Jim Harris said the following:
> Author: jimharris
> Date: Tue Jul 24 22:10:11 2012
> New Revision: 238755
> URL: http://svn.freebsd.org/changeset/base/238755
> 
> Log:
>   Add rmb() to tsc_read_##x to enforce serialization of rdtsc captures.
>   
>   Intel Architecture Manual specifies that rdtsc instruction is not serialized,
>   so without this change, TSC synchronization test would periodically fail,
>   resulting in use of HPET timecounter instead of TSC-low.  This caused
>   severe performance degradation (40-50%) when running high IO/s workloads due to
>   HPET MMIO reads and GEOM stat collection.
>   
>   Tests on Xeon E5-2600 (Sandy Bridge) 8C systems were seeing TSC synchronization
>   fail approximately 20% of the time.

Should rather the synchronization test be fixed if it's the culprit?
Or is this change universally good for the real uses of TSC?

>   Sponsored by: Intel
>   Reviewed by: kib
>   MFC after: 3 days
> 
> Modified:
>   head/sys/x86/x86/tsc.c
> 
> Modified: head/sys/x86/x86/tsc.c
> ==============================================================================
> --- head/sys/x86/x86/tsc.c	Tue Jul 24 20:15:41 2012	(r238754)
> +++ head/sys/x86/x86/tsc.c	Tue Jul 24 22:10:11 2012	(r238755)
> @@ -328,6 +328,7 @@ init_TSC(void)
>  
>  #ifdef SMP
>  
> +/* rmb is required here because rdtsc is not a serializing instruction. */
>  #define	TSC_READ(x)			\
>  static void				\
>  tsc_read_##x(void *arg)			\
> @@ -335,6 +336,7 @@ tsc_read_##x(void *arg)			\
>  	uint32_t *tsc = arg;		\
>  	u_int cpu = PCPU_GET(cpuid);	\
>  					\
> +	rmb();				\
>  	tsc[cpu * 3 + x] = rdtsc32();	\
>  }
>  TSC_READ(0)
> 


-- 
Andriy Gapon