Re: [Xen-devel] [PATCH] x86: don't pass negative time to gtime_to_gtsc() (try 2)

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 10.06.13 at 15:53, George Dunlap <george.dunlap@xxxxxxxxxxxxx> wrote:
> On 10/06/13 14:48, Jan Beulich wrote:
>>>>> On 10.06.13 at 15:44, George Dunlap <George.Dunlap@xxxxxxxxxxxxx> wrote:
>>> On Mon, Jun 10, 2013 at 1:15 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>> This mostly reverts commit eb60be3d ("x86: don't pass negative time to
>>>> gtime_to_gtsc()") and instead corrects __update_vcpu_system_time()'s
>>>> handling of this_cpu(cpu_time).stime_local_stamp dating back before the
>>>> start of a HVM guest (which would otherwise lead to a negative value
>>>> getting passed to gtime_to_gtsc(), causing scale_delta() to produce
>>>> meaningless output).
>>>>
>>>> Flushing the value to zero was wrong, and printing a message for
>>>> something that can validly happen wasn't very useful either.
>>> Has this actually caused problems, or is this just a theoretical fix?
>> The commit this undoes was done because of a crash observed
>> on the test infrastructure. Recently, the log message that got
>> added there was found in another test infrastructure log, getting
>> me to (hopefully) understand what the underlying issue is, leading
>> to the fix here.
> 
> The thing is this:  That line "tsc_stamp = -gtime_to_tsc(-stime)" looks 
> risky; it's the kind of logic that is easy to get wrong (like the "^val" 
> instead of "&~val" thing).
> 
> The code as it is has been tested from April until now and nobody has 
> complained.  With this fix, we're starting over from scratch on our 
> "testing clock".
> 
> So unless this is known to cause an actual problem (other than just 
> unnecessary console messages), I'd be inclined to say it needs to wait 
> until 4.3.1.

While I don't see as much of a risk here as you do, I'm fine with
holding this back if you prefer so.

Jan


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