diff mbox

[v4,3/5] x86/time: refactor read_platform_stime()

Message ID 1473874670-4986-4-git-send-email-joao.m.martins@oracle.com (mailing list archive)
State New, archived
Headers show

Commit Message

Joao Martins Sept. 14, 2016, 5:37 p.m. UTC
To allow the caller to fetch the last read from the clocksource which
was used to calculate system_time. This is a prerequisite for a
subsequent patch that will use this last read.

Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>

Changes since v3:
 - Add mention of this being a prerequisite to a later patch.
---
 xen/arch/x86/time.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

Comments

Jan Beulich Sept. 19, 2016, 10:15 a.m. UTC | #1
>>> On 14.09.16 at 19:37, <joao.m.martins@oracle.com> wrote:
> To allow the caller to fetch the last read from the clocksource which
> was used to calculate system_time. This is a prerequisite for a
> subsequent patch that will use this last read.
> 
> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>

Acked-by: Jan Beulich <jbeulich@suse.com>
with one further minor request:

> --- a/xen/arch/x86/time.c
> +++ b/xen/arch/x86/time.c
> @@ -581,18 +581,22 @@ static void plt_overflow(void *unused)
>      set_timer(&plt_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;
> +    u64 plt_counter, count;
>      s_time_t stime;
>  
>      ASSERT(!local_irq_is_enabled());
>  
>      spin_lock(&platform_timer_lock);
> -    count = plt_stamp64 + ((plt_src.read_counter() - plt_stamp) & plt_mask);
> +    plt_counter = plt_src.read_counter();
> +    count = plt_stamp64 + ((plt_counter - plt_stamp) & plt_mask);
>      stime = __read_platform_stime(count);
>      spin_unlock(&platform_timer_lock);
>  
> +    if ( stamp )
> +        *stamp = plt_counter;

Considering that all current callers pass in NULL and you mean to
add only one (iirc) which doesn't, please add unlikely() here.

Jan
Joao Martins Sept. 19, 2016, 4:11 p.m. UTC | #2
On 09/19/2016 11:15 AM, Jan Beulich wrote:
>>>> On 14.09.16 at 19:37, <joao.m.martins@oracle.com> wrote:
>> To allow the caller to fetch the last read from the clocksource which
>> was used to calculate system_time. This is a prerequisite for a
>> subsequent patch that will use this last read.
>>
>> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
> 
> Acked-by: Jan Beulich <jbeulich@suse.com>
> with one further minor request:
> 
>> --- a/xen/arch/x86/time.c
>> +++ b/xen/arch/x86/time.c
>> @@ -581,18 +581,22 @@ static void plt_overflow(void *unused)
>>      set_timer(&plt_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;
>> +    u64 plt_counter, count;
>>      s_time_t stime;
>>  
>>      ASSERT(!local_irq_is_enabled());
>>  
>>      spin_lock(&platform_timer_lock);
>> -    count = plt_stamp64 + ((plt_src.read_counter() - plt_stamp) & plt_mask);
>> +    plt_counter = plt_src.read_counter();
>> +    count = plt_stamp64 + ((plt_counter - plt_stamp) & plt_mask);
>>      stime = __read_platform_stime(count);
>>      spin_unlock(&platform_timer_lock);
>>  
>> +    if ( stamp )
>> +        *stamp = plt_counter;
> 
> Considering that all current callers pass in NULL and you mean to
> add only one (iirc) which doesn't, please add unlikely() here.
OK, I will add it, Thank you!

Joao
diff mbox

Patch

diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
index e5001d5..af9e31f 100644
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -581,18 +581,22 @@  static void plt_overflow(void *unused)
     set_timer(&plt_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;
+    u64 plt_counter, count;
     s_time_t stime;
 
     ASSERT(!local_irq_is_enabled());
 
     spin_lock(&platform_timer_lock);
-    count = plt_stamp64 + ((plt_src.read_counter() - plt_stamp) & plt_mask);
+    plt_counter = plt_src.read_counter();
+    count = plt_stamp64 + ((plt_counter - plt_stamp) & plt_mask);
     stime = __read_platform_stime(count);
     spin_unlock(&platform_timer_lock);
 
+    if ( stamp )
+        *stamp = plt_counter;
+
     return stime;
 }
 
@@ -726,7 +730,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)));
 }
 
 /***************************************************************************
@@ -1045,7 +1049,7 @@  int cpu_frequency_change(u64 freq)
 
     local_irq_disable();
     /* Platform time /first/, as we may be delayed by platform_timer_lock. */
-    t->stamp.master_stime = read_platform_stime();
+    t->stamp.master_stime = read_platform_stime(NULL);
     curr_tsc = rdtsc_ordered();
     /* TSC-extrapolated time may be bogus after frequency change. */
     /*t->stamp.local_stime = get_s_time_fixed(curr_tsc);*/
@@ -1350,7 +1354,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_ordered();
             }
             atomic_inc(&r->semaphore);
@@ -1390,7 +1394,7 @@  static void time_calibration_std_rendezvous(void *_r)
     {
         while ( atomic_read(&r->semaphore) != (total_cpus - 1) )
             cpu_relax();
-        r->master_stime = read_platform_stime();
+        r->master_stime = read_platform_stime(NULL);
         smp_wmb(); /* write r->master_stime /then/ signal */
         atomic_inc(&r->semaphore);
     }
@@ -1429,7 +1433,7 @@  void time_latch_stamps(void)
     unsigned long flags;
 
     local_irq_save(flags);
-    ap_bringup_ref.master_stime = read_platform_stime();
+    ap_bringup_ref.master_stime = read_platform_stime(NULL);
     ap_bringup_ref.local_tsc = rdtsc_ordered();
     local_irq_restore(flags);
 
@@ -1447,7 +1451,7 @@  void init_percpu_time(void)
     t->tsc_scale = per_cpu(cpu_time, 0).tsc_scale;
 
     local_irq_save(flags);
-    now = read_platform_stime();
+    now = read_platform_stime(NULL);
     tsc = rdtsc_ordered();
     local_irq_restore(flags);