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

[Joao Martins <joao.m.martins@oracle.com>]

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

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