Re: [Xen-devel] [PATCH v2 2/6] x86/time: refactor init_platform_time()

[Joao Martins <joao.m.martins@xxxxxxxxxx>]

On 04/05/2016 11:09 AM, Jan Beulich wrote:
>>>> On 29.03.16 at 15:44, <joao.m.martins@xxxxxxxxxx> wrote:
>> --- a/xen/arch/x86/time.c
>> +++ b/xen/arch/x86/time.c
>> @@ -533,6 +533,30 @@ static void resume_platform_timer(void)
>>      plt_stamp = plt_src.read_counter();
>>  }
>>  
>> +static int __init try_platform_timer(struct platform_timesource *pts)
>> +{
>> +    int rc = -1;
> 
> Pointless initializer. In fact ...
> 
>> +    rc = pts->init(pts);
> 
> ... this could be the initializer.
> 
Ah yes. Will fix it.

>> +    if ( rc <= 0 )
>> +        return rc;
>> +
>> +    plt_mask = (u64)~0ull >> (64 - pts->counter_bits);
>> +
>> +    set_time_scale(&plt_scale, pts->frequency);
>> +
>> +    plt_overflow_period = scale_delta(
>> +        1ull << (pts->counter_bits - 1), &plt_scale);
>> +    init_timer(&plt_overflow_timer, plt_overflow, NULL, 0);
>> +    plt_src = *pts;
>> +    plt_overflow(NULL);
>> +
>> +    platform_timer_stamp = plt_stamp64;
>> +    stime_platform_stamp = NOW();
>> +
>> +    return rc;
>> +}
> 
> Moving here all this setting up of static/global data makes me
> wonder how you mean to consistently re-use this function for
> your new purpose.
My purpose is to reuse this initialization part for the case of switching from
from one clocksource to TSC at a later point (which is done in a different place
i.e. verify_tsc_reliability). Though this static/global data part in particular
is done if the clocksource initialization succeeds so I merged that in a single
helper hence the name "try_platform_timer". It also looks cleaner, and we would
leave opt_clocksource checking with plt_timers in init_platform_time.

Joao

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