Re: [Xen-devel] [PATCH v2 4/6] x86/time: streamline platform time init on plt_init()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 05.04.16 at 17:12, <joao.m.martins@xxxxxxxxxx> wrote:
> On 04/05/2016 12:46 PM, Jan Beulich wrote:
>>>>> On 29.03.16 at 15:44, <joao.m.martins@xxxxxxxxxx> wrote:
>>> @@ -516,17 +519,31 @@ static s_time_t __read_platform_stime(u64 
>>> platform_time)
>>>      return (stime_platform_stamp + scale_delta(diff, &plt_scale));
>>>  }
>>>  
>>> +static void __plt_init(void)
>>> +{
>>> +    u64 count;
>>> +
>>> +    ASSERT(spin_is_locked(&platform_timer_lock));
>>> +    count = plt_src.read_counter();
>>> +    plt_stamp64 += (count - plt_stamp) & plt_mask;
>>> +    plt_stamp = count;
>>> +}
>> 
>> Note that this has nothing to do with "init" - it updates the two time
>> stamps, as is being made clear by ...
>> 
>>>  static void plt_overflow(void *unused)
>>>  {
>>>      int i;
>>> -    u64 count;
>>>      s_time_t now, plt_now, plt_wrap;
>>>  
>>>      spin_lock_irq(&platform_timer_lock);
>>>  
>>> -    count = plt_src.read_counter();
>>> -    plt_stamp64 += (count - plt_stamp) & plt_mask;
>>> -    plt_stamp = count;
>>> +    __plt_init();
>> 
>> ... this use.
>> 
> Would you prefer changing the name to e.g "set_plt_stamp" ?

Or simply plt_update()?

>>> +    {
>>> +        plt_init();
>>> +    }
>>> +    else
>>> +    {
>>> +        plt_overflow_period = scale_delta(
>>> +            1ull << (pts->counter_bits - 1), &plt_scale);
>>> +        init_timer(&plt_overflow_timer, plt_overflow, NULL, 0);
>>> +        plt_overflow(NULL);
>>> +
>>> +        printk("Platform timer overflow period is %lu secs\n",
>>> +               plt_overflow_period/SECONDS(1));
>> 
>> If we want this logged at all, then please at most as XENLOG_INFO.
> OK.
> 
>> Plus - is seconds granularity fine grained enough for all sources, i.e.
>> wouldn't there for typical HPET just be a single digit, not a lot of
>> precision that is?
> Could be, my HPET was around 2 minutes overflow period, but PIT was a single
> digit as you mention. I will change that to MILLISECS(1000) for higher 
> precision

How is MILLISECS(1000) different from SECONDS(1)?

> - or I can remove it entirely if you prefer not logging this info.

Well, there had been times where this information would have been
quite useful in diagnosing problems. That's been a while back, but
knowing we had such issues I can't just say "drop the message",
even if I hope we won't have any similar problems anymore.

Jan


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