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>,
	Wei Liu <wei.liu2@citrix.com>,
	Dario Faggioli <dario.faggioli@eu.citrix.com>,
	xen-devel <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH] x86/time: use correct (local) time stamp in constant-TSC calibration fast path
Date: Thu, 09 Jun 2016 09:52:24 -0600	[thread overview]
Message-ID: <5759ACD802000078000F3A78@prv-mh.provo.novell.com> (raw)
In-Reply-To: <57598484.1040300@oracle.com>

>>> On 09.06.16 at 17:00, <joao.m.martins@oracle.com> wrote:
> On 06/09/2016 01:57 PM, Jan Beulich wrote:
>>>>> On 09.06.16 at 14:11, <joao.m.martins@oracle.com> wrote:
>> So in effect for the fast path the patch
>> changes the situation from c->stime_local_stamp being effectively
>> unused to c->stime_master_stamp being so. In the former case, if
>> that really hadn't been a typo, deleting the write of that field from
>> time_calibration_std_rendezvous() would have made sense, as
>> get_s_time() certainly is more overhead than the simply memory
>> read and write needed for keeping c->stime_master_stamp up to
>> date (despite not being used).
> I agree, but what I meant previously was more of a concern meaning: CPU 0 is 
> doing an
> expensive read_platform_time (e.g. HPET supposedly microseconds order, plus 
> a
> non-contented lock) to set stime_master_stamp that doesn't get used at all -
> effectively not using the clocksource set initially at boot.

Yeah, there's likely some cleanup potential here, but of course we
need to be pretty careful about not doing something that may be
needed by some code paths but not others. But if you think you
can help the situation without harming maintainability, feel free to
go ahead.

> What if verify_tsc_reliability clears out X86_FEATURE_TSC_RELIABLE when 
> failing
> the warp test? The calibration function is set early on right after 
> interrupts are
> enabled and the time warp check later on when all CPUs are up. So on 
> problematic
> platforms it's possible that std_rendezvous is used with a constant TSC but 
> still
> deemed unreliable. We still keep incrementing deltas at roughly about the 
> same time,
> but in effect with this change the stime_local_stamp would be TSC-only based 
> thus
> leading to warps with an unreliable TSC? And there's also the CPU 
> hotplug/onlining
> case that we once discussed.

I agree that we're likely in trouble in such a case. But for the
moment I'd be glad if we could get the "normal" case work right.

Jan


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

  reply	other threads:[~2016-06-09 15:52 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-09 12:01 Jan Beulich
2016-06-09 12:10 ` Andrew Cooper
2016-06-09 12:11 ` Joao Martins
2016-06-09 12:57   ` Jan Beulich
2016-06-09 15:00     ` Joao Martins
2016-06-09 15:52       ` Jan Beulich [this message]
2016-06-09 18:19         ` Joao Martins
2016-06-10  6:59           ` Jan Beulich
2016-06-10  9:29             ` Jan Beulich
2016-06-10 17:07               ` Joao Martins
2016-06-09 12:12 ` Wei Liu

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=5759ACD802000078000F3A78@prv-mh.provo.novell.com \
    --to=jbeulich@suse.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=dario.faggioli@eu.citrix.com \
    --cc=joao.m.martins@oracle.com \
    --cc=wei.liu2@citrix.com \
    --cc=xen-devel@lists.xenproject.org \
    --subject='Re: [PATCH] x86/time: use correct (local) time stamp in constant-TSC calibration fast path' \
    /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

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).