Re: [PATCH] x86/watchdog: Use real timestamps for watchdog timeout

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 24/05/13 12:42, Jan Beulich wrote:
>>>> On 24.05.13 at 12:33, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> On 24/05/13 11:13, Tim Deegan wrote:
>>> At 10:57 +0100 on 24 May (1369393060), Andrew Cooper wrote:
>>>> On 24/05/13 08:09, Jan Beulich wrote:
>>>>> You can't use NOW() here - while the time updating code is safe
>>>>> against normal interrupts, it's not atomic wrt NMIs.
>>>> But NMIs are latched at the hardware level.  If we get a nested NMI the
>>>> Xen will be toast on the exit path anyway.
>>> The problem is that an NMI can arrive while local_time_calibration() is
>>> writing its results, so calling NOW() in the NMI handler might return
>>> garbage. 
>> Aah - I see.  Sorry - I misunderstood the original point.
>>
>> Yes - that is an issue.
>>
>> Two solutions come to mind.
>>
>> 1) Along with the local_irq_disable()/enable() pairs in
>> local_time_calibration, having an atomic_t indicating "time data update
>> in progress", allowing the NMI handler to decide to bail early.
>>
>> 2) Modify local_time_calibration() to fill in a shadow cpu_time set, and
>> a different atomic_t to indicate which one is consistent.  This would
>> allow the NMI handler to always use one consistent set of timing
>> information.
> What's the advantage of either over using the plain TSC values?
> Accuracy of the expiry, but the accuracy here doesn't matter
> much (as long as its not off by an order of a magnitude), and
> would be achieved by some presumably not very neat code of no
> other (general) use.
>
> And if picking up one of these, then 2 seems the preferable one to
> me.
>
> Jan
>

The TSC scale is recalculated as part of local_time_calibration().  I
couldn't spot any other sensible way of converting a TSC delta to
something approximating seconds.  Have I missed something?

~Andrew