Re: [Xen-devel] [PATCH 8/8] x86/time: group time stamps into a structure

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 15/06/16 11:30, Jan Beulich wrote:
> If that had been done from the beginning, mistakes like the one
> corrected in commit b64438c7c1 ("x86/time: use correct (local) time
> stamp in constant-TSC calibration fast path") would likely never have
> happened.
>
> Also add a few "const" to make more obvious when things aren't expected
> to change.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

With one further suggestion.

> @@ -1037,40 +1029,34 @@ static void local_time_calibration(void)
>      {
>          /* Atomically read cpu_calibration struct and write cpu_time struct. 
> */
>          local_irq_disable();
> -        t->local_tsc_stamp    = c->local_tsc_stamp;
> -        t->stime_local_stamp  = c->stime_local_stamp;
> -        t->stime_master_stamp = c->stime_master_stamp;
> +        t->stamp = *c;
>          local_irq_enable();
>          update_vcpu_system_time(current);
>          goto out;
>      }
>  
> -    prev_tsc          = t->local_tsc_stamp;
> -    prev_local_stime  = t->stime_local_stamp;
> -    prev_master_stime = t->stime_master_stamp;
> +    prev = t->stamp;
>  
>      /* Disabling IRQs ensures we atomically read cpu_calibration struct. */
>      local_irq_disable();
> -    curr_tsc          = c->local_tsc_stamp;
> -    curr_local_stime  = c->stime_local_stamp;
> -    curr_master_stime = c->stime_master_stamp;
> +    curr = *c;
>      local_irq_enable();
>  
>  #if 0

Turning this #if 0 into an "if ( 0 ) /* For debugging. */" would cause
the printk()s to be parsed and discarded, and avoid the risk of printk
parameters bitrotting.

>      printk("PRE%d: tsc=%"PRIu64" stime=%"PRIu64" master=%"PRIu64"\n",
> -           smp_processor_id(), prev_tsc, prev_local_stime, 
> prev_master_stime);
> +           smp_processor_id(), prev.local_tsc, prev.local_stime, 
> prev.master_stime);
>      printk("CUR%d: tsc=%"PRIu64" stime=%"PRIu64" master=%"PRIu64
>             " -> %"PRId64"\n",
> -           smp_processor_id(), curr_tsc, curr_local_stime, curr_master_stime,
> -           curr_master_stime - curr_local_stime);
> +           smp_processor_id(), curr.local_tsc, curr.local_stime, 
> curr.master_stime,
> +           curr.master_stime - curr.local_stime);
>  #endif
>  


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