[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



>>> On 05.04.16 at 12:55, <joao.m.martins@xxxxxxxxxx> wrote:
> On 04/05/2016 11:09 AM, Jan Beulich wrote:
>>>>> On 29.03.16 at 15:44, <joao.m.martins@xxxxxxxxxx> wrote:
>>> +    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.

Well, I understand all this, but still wonder - is this second use then
correct? See also the comments on the next patch.

Jan


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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.