xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: "Jan Beulich" <JBeulich@suse.com>
To: Joao Martins <joao.m.martins@oracle.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>,
	Dario Faggioli <dario.faggioli@citrix.com>,
	Tim Deegan <tim@xen.org>,
	George Dunlap <george.dunlap@citrix.com>,
	"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>
Subject: Re: BUG: NOW() seems to (sometimes) go backwards!
Date: Thu, 09 Jun 2016 05:31:52 -0600	[thread overview]
Message-ID: <57596FC802000078000F363D@prv-mh.provo.novell.com> (raw)
In-Reply-To: <57596E3002000078000F361D@prv-mh.provo.novell.com>

>>> On 09.06.16 at 13:25, <JBeulich@suse.com> wrote:
>>>> On 09.06.16 at 12:24, <joao.m.martins@oracle.com> wrote:
>>>>>> Yet when the scaling values get set only once at boot, monotonic
>>>>>> (cross-CPU) TSC means monotonic (cross-CPU) returns from NOW().
>>>>>>
>>>>> Yep. And at this point, this is what needs to be verified, I guess...
>>>> I think get_s_time_fixed doesn't guarantee monotonicity across CPUs being it
>>>> different socket or (SMT) siblings. local_tsc_stamp is seeded differently on 
> 
>> 
>>>> each CPU
>>>> i.e. rdtsc() right after doing the platform time read on the calibration 
>>>> rendezvous.
>>>> Plus stime_local_stamp is seeded with values taken from platform timer 
>>>> (HPET, ACPI,
>>>> PIT) on local_time_calibration which means that get_s_time isn't solely 
>>>> based on TSC
>>>> and that there will always be a gap between stime_local_stamp and 
>>>> local_tsc_stamp.
>>>> TSC is indeed monotonic on a TSC invariant box, but the delta that is 
>>>> computed
>>>> differs from cpu to cpu according to when time calibration happens on each 
>>>> CPU - thus
>>>> not guaranteeing the desired monotonicity property. Having stime_local_stamp 
> 
>> 
>>>> be based
>>>> on the same timestamp that of the local_tsc_stamp plus having a single
>>>> local_tsc_stamp as reference would address this behaviour - see also below.
>>> 
>>> The quality of get_s_time_fixed() output indeed heavily depends on
>>> t->local_tsc_stamp and t->stime_local_stamp being a properly
>>> matched pair. Yet in local_time_calibration()'s constant-TSC case,
>>> they're a direct copy from the respective cpu_calibration fields.
>>> The
>>> main issue I could see here is that on SMT siblings the hardware
>>> switching between the two may introduce arbitrary delays between
>>> them. And with CPU frequency changes, the latency between the
>>> rdtsc() and the execution of get_s_time() finishing could also be
>>> pretty variable. I wonder whether c->local_tsc_stamp wouldn't
>>> better be written with the TSC value used by get_s_time() (or,
>>> which should amount to the same effect, whether we shouldn't
>>> simply call get_s_time_fixed() here with the just sampled TSC value).
>> Indeed, but notice that in this copy for the constant-TSC case:
>> t->stime_local_stamp is written with c->stime_master_stamp - ending
>> up the former being discarded. So even changing that pair to correctly
>> match it wouldn't change the result. At point of which I wonder if copying
>> rendezvous c->stime_master_stamp to t->stime_local_stamp on
>> local_time_calibration is correct?
> 
> Yeah, I stumbled across that too, and I am in the process of
> trying whether this in fact was just a copy-and-paste mistake
> many years ago.

And indeed changing that makes all of the dozens-of-microseconds
backwards moves (compared cross-CPU) go away.

The largest ones left are now around 3.7us, and apparently always
between CPU0 and CPU1 (which I suspect may be due to the special
role CPU0 plays here).

Jan


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

  reply	other threads:[~2016-06-09 11:31 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-07 15:54 BUG: NOW() seems to (sometimes) go backwards! Dario Faggioli
2016-06-08 10:42 ` Jan Beulich
2016-06-08 13:43   ` Dario Faggioli
2016-06-08 14:01     ` Jan Beulich
2016-06-08 20:36     ` Joao Martins
2016-06-09  8:05       ` Jan Beulich
2016-06-09 10:24         ` Joao Martins
2016-06-09 10:43           ` Joao Martins
2016-06-09 11:25           ` Jan Beulich
2016-06-09 11:31             ` Jan Beulich [this message]
2016-06-09 10:19 ` George Dunlap

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=57596FC802000078000F363D@prv-mh.provo.novell.com \
    --to=jbeulich@suse.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=dario.faggioli@citrix.com \
    --cc=george.dunlap@citrix.com \
    --cc=joao.m.martins@oracle.com \
    --cc=tim@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).