Re: [Xen-devel] [PATCH 3/8] x86/time: introduce and use rdtsc_ordered()

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 20/06/16 14:06, Jan Beulich wrote:
>>>> On 20.06.16 at 14:59, <andrew.cooper3@xxxxxxxxxx> wrote:
>> On 15/06/16 11:27, Jan Beulich wrote:
>>> Matching Linux commit 03b9730b76 ("x86/asm/tsc: Add rdtsc_ordered() and
>>> use it in trivial call sites") and earlier ones it builds upon, let's
>>> make sure timing loops don't have their rdtsc()-s re-ordered, as that
>>> would harm precision of the result (values were observed to be several
>>> hundred clocks off without this adjustment).
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>> Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> I have these two additional hunks for v2:
>
> @@ -1124,16 +1124,13 @@ static void local_time_calibration(void)
>   */
>  static void check_tsc_warp(unsigned long tsc_khz, unsigned long *max_warp)
>  {
> -#define rdtsc_barrier() mb()
>      static DEFINE_SPINLOCK(sync_lock);
>      static cycles_t last_tsc;
>  
>      cycles_t start, now, prev, end;
>      int i;
>  
> -    rdtsc_barrier();
> -    start = get_cycles();
> -    rdtsc_barrier();
> +    start = rdtsc_ordered();
>  
>      /* The measurement runs for 20 msecs: */
>      end = start + tsc_khz * 20ULL;
> @@ -1148,9 +1145,7 @@ static void check_tsc_warp(unsigned long
>           */
>          spin_lock(&sync_lock);
>          prev = last_tsc;
> -        rdtsc_barrier();
> -        now = get_cycles();
> -        rdtsc_barrier();
> +        now = rdtsc_ordered();
>          last_tsc = now;
>          spin_unlock(&sync_lock);
>  
>
> May I consider those covered as well?

Yes.

I need to dust off my series removing most of the misuse of mb() in the
Xen codebase now that 4.8 is open.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel