Re: [Xen-devel] [PATCH 4/5] x86/time: refactor read_platform_stime()

[Andrew Cooper]

On 21/03/16 11:45, Joao Martins wrote:
>
> All fixed, though I found one change missing in this series, specifically on
> time_calibration_std_rendezvous. Otherwise this commit would break 
> compilation.
> See chunk below for the change I am adding:
>
> @@ -1377,7 +1380,7 @@ static void time_calibration_std_rendezvous(void *_r)
>  {
>  while ( atomic_read(>semaphore) != (total_cpus - 1) )
>  cpu_relax();
> -r->master_stime = read_platform_stime();
> +r->master_stime = read_platform_stime(NULL);
>  mb(); /* write r->master_stime /then/ signal */
>  atomic_inc(>semaphore);
>  }
>
> Having this fixed, could I still keep your Reviewed-by?

Yes - that's fine.

~Andrew

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

Re: [Xen-devel] [PATCH 4/5] x86/time: refactor read_platform_stime()

[Joao Martins]

On 03/18/2016 08:34 PM, Andrew Cooper wrote:
> On 17/03/16 16:12, Joao Martins wrote:
>> To fetch the last read from the clocksource which was used to
>> calculate system_time. In the case of clocksource=tsc we will
>> use it to set tsc_timestamp.
>>
>> Signed-off-by: Joao Martins 
> 
> Again, just minor style issues.
> 
> Reviewed-by Andrew Cooper 
> 
All fixed, though I found one change missing in this series, specifically on
time_calibration_std_rendezvous. Otherwise this commit would break compilation.
See chunk below for the change I am adding:

@@ -1377,7 +1380,7 @@ static void time_calibration_std_rendezvous(void *_r)
 {
 while ( atomic_read(>semaphore) != (total_cpus - 1) )
 cpu_relax();
-r->master_stime = read_platform_stime();
+r->master_stime = read_platform_stime(NULL);
 mb(); /* write r->master_stime /then/ signal */
 atomic_inc(>semaphore);
 }

Having this fixed, could I still keep your Reviewed-by?

>> ---
>> Cc: Keir Fraser 
>> Cc: Jan Beulich 
>> Cc: Andrew Cooper 
>> ---
>>  xen/arch/x86/time.c | 16 ++--
>>  1 file changed, 10 insertions(+), 6 deletions(-)
>>
>> diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
>> index 5af8902..89c35d0 100644
>> --- a/xen/arch/x86/time.c
>> +++ b/xen/arch/x86/time.c
>> @@ -508,6 +508,7 @@ static s_time_t stime_platform_stamp; /* System time at 
>> below platform time */
>>  static u64 platform_timer_stamp;  /* Platform time at above system time 
>> */
>>  static u64 plt_stamp64;  /* 64-bit platform counter stamp   
>> */
>>  static u64 plt_stamp;/* hardware-width platform counter stamp   
>> */
>> +static u64 plt_stamp_counter;/* last read since read_counter */
> 
> This can have its scope reduced to within read_platform_stime()
> 
>>  static struct timer plt_overflow_timer;
>>  
>>  static s_time_t __read_platform_stime(u64 platform_time)
>> @@ -566,7 +567,7 @@ static void plt_overflow(void *unused)
>>  set_timer(_overflow_timer, NOW() + plt_overflow_period);
>>  }
>>  
>> -static s_time_t read_platform_stime(void)
>> +static s_time_t read_platform_stime(u64 *stamp)
>>  {
>>  u64 count;
>>  s_time_t stime;
>> @@ -574,8 +575,11 @@ static s_time_t read_platform_stime(void)
>>  ASSERT(!local_irq_is_enabled());
>>  
>>  spin_lock(_timer_lock);
>> -count = plt_stamp64 + ((plt_src.read_counter() - plt_stamp) & plt_mask);
>> +plt_stamp_counter = plt_src.read_counter();
>> +count = plt_stamp64 + ((plt_stamp_counter - plt_stamp) & plt_mask);
>>  stime = __read_platform_stime(count);
>> +if (stamp)
> 
> Spaces.
> 
> ~Andrew
> 

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

[Xen-devel] [PATCH 4/5] x86/time: refactor read_platform_stime()

[Joao Martins]

To fetch the last read from the clocksource which was used to
calculate system_time. In the case of clocksource=tsc we will
use it to set tsc_timestamp.

Signed-off-by: Joao Martins 
---
Cc: Keir Fraser 
Cc: Jan Beulich 
Cc: Andrew Cooper 
---
 xen/arch/x86/time.c | 16 ++--
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
index 5af8902..89c35d0 100644
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -508,6 +508,7 @@ static s_time_t stime_platform_stamp; /* System time at 
below platform time */
 static u64 platform_timer_stamp;  /* Platform time at above system time */
 static u64 plt_stamp64;  /* 64-bit platform counter stamp   */
 static u64 plt_stamp;/* hardware-width platform counter stamp   */
+static u64 plt_stamp_counter;/* last read since read_counter */
 static struct timer plt_overflow_timer;
 
 static s_time_t __read_platform_stime(u64 platform_time)
@@ -566,7 +567,7 @@ static void plt_overflow(void *unused)
 set_timer(_overflow_timer, NOW() + plt_overflow_period);
 }
 
-static s_time_t read_platform_stime(void)
+static s_time_t read_platform_stime(u64 *stamp)
 {
 u64 count;
 s_time_t stime;
@@ -574,8 +575,11 @@ static s_time_t read_platform_stime(void)
 ASSERT(!local_irq_is_enabled());
 
 spin_lock(_timer_lock);
-count = plt_stamp64 + ((plt_src.read_counter() - plt_stamp) & plt_mask);
+plt_stamp_counter = plt_src.read_counter();
+count = plt_stamp64 + ((plt_stamp_counter - plt_stamp) & plt_mask);
 stime = __read_platform_stime(count);
+if (stamp)
+*stamp = plt_stamp_counter;
 spin_unlock(_timer_lock);
 
 return stime;
@@ -693,7 +697,7 @@ void cstate_restore_tsc(void)
 if ( boot_cpu_has(X86_FEATURE_NONSTOP_TSC) )
 return;
 
-write_tsc(stime2tsc(read_platform_stime()));
+write_tsc(stime2tsc(read_platform_stime(NULL)));
 }
 
 /***
@@ -1012,7 +1016,7 @@ int cpu_frequency_change(u64 freq)
 
 local_irq_disable();
 /* Platform time /first/, as we may be delayed by platform_timer_lock. */
-t->stime_master_stamp = read_platform_stime();
+t->stime_master_stamp = read_platform_stime(NULL);
 /* TSC-extrapolated time may be bogus after frequency change. */
 /*t->stime_local_stamp = get_s_time();*/
 t->stime_local_stamp = t->stime_master_stamp;
@@ -1330,7 +1334,7 @@ static void time_calibration_tsc_rendezvous(void *_r)
 
 if ( r->master_stime == 0 )
 {
-r->master_stime = read_platform_stime();
+r->master_stime = read_platform_stime(NULL);
 r->master_tsc_stamp = rdtsc();
 }
 atomic_inc(>semaphore);
@@ -1422,7 +1426,7 @@ void init_percpu_time(void)
 
 local_irq_save(flags);
 t->local_tsc_stamp = rdtsc();
-now = read_platform_stime();
+now = read_platform_stime(NULL);
 local_irq_restore(flags);
 
 t->stime_master_stamp = now;
-- 
2.1.4


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

Re: [Xen-devel] [PATCH 4/5] x86/time: refactor read_platform_stime()

[Andrew Cooper]

On 17/03/16 16:12, Joao Martins wrote:
> To fetch the last read from the clocksource which was used to
> calculate system_time. In the case of clocksource=tsc we will
> use it to set tsc_timestamp.
>
> Signed-off-by: Joao Martins 

Again, just minor style issues.

Reviewed-by Andrew Cooper 

> ---
> Cc: Keir Fraser 
> Cc: Jan Beulich 
> Cc: Andrew Cooper 
> ---
>  xen/arch/x86/time.c | 16 ++--
>  1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
> index 5af8902..89c35d0 100644
> --- a/xen/arch/x86/time.c
> +++ b/xen/arch/x86/time.c
> @@ -508,6 +508,7 @@ static s_time_t stime_platform_stamp; /* System time at 
> below platform time */
>  static u64 platform_timer_stamp;  /* Platform time at above system time 
> */
>  static u64 plt_stamp64;  /* 64-bit platform counter stamp   
> */
>  static u64 plt_stamp;/* hardware-width platform counter stamp   
> */
> +static u64 plt_stamp_counter;/* last read since read_counter */

This can have its scope reduced to within read_platform_stime()

>  static struct timer plt_overflow_timer;
>  
>  static s_time_t __read_platform_stime(u64 platform_time)
> @@ -566,7 +567,7 @@ static void plt_overflow(void *unused)
>  set_timer(_overflow_timer, NOW() + plt_overflow_period);
>  }
>  
> -static s_time_t read_platform_stime(void)
> +static s_time_t read_platform_stime(u64 *stamp)
>  {
>  u64 count;
>  s_time_t stime;
> @@ -574,8 +575,11 @@ static s_time_t read_platform_stime(void)
>  ASSERT(!local_irq_is_enabled());
>  
>  spin_lock(_timer_lock);
> -count = plt_stamp64 + ((plt_src.read_counter() - plt_stamp) & plt_mask);
> +plt_stamp_counter = plt_src.read_counter();
> +count = plt_stamp64 + ((plt_stamp_counter - plt_stamp) & plt_mask);
>  stime = __read_platform_stime(count);
> +if (stamp)

Spaces.

~Andrew

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