From: "Jan Beulich" <JBeulich@suse.com>
To: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: xen-devel <xen-devel@lists.xenproject.org>,
Dario Faggioli <dario.faggioli@citrix.com>,
Joao Martins <joao.m.martins@oracle.com>
Subject: Re: [PATCH 5/8] x86/time: correctly honor late clearing of TSC related feature flags
Date: Wed, 31 Aug 2016 08:03:25 -0600 [thread overview]
Message-ID: <57C6FFCD020000780010A84B@prv-mh.provo.novell.com> (raw)
In-Reply-To: <9a85173c-57d0-2dca-2dc0-34b5f22fcc73@citrix.com>
>>> On 31.08.16 at 15:42, <andrew.cooper3@citrix.com> wrote:
> On 03/08/16 10:43, Jan Beulich wrote:
>>>>> On 02.08.16 at 21:08, <andrew.cooper3@citrix.com> wrote:
>>> On 04/07/16 16:53, Jan Beulich wrote:
>>>>>>> On 04.07.16 at 17:39, <andrew.cooper3@citrix.com> wrote:
>>>>> On 20/06/16 16:20, Jan Beulich wrote:
>>>>>>>>> On 20.06.16 at 16:32, <andrew.cooper3@citrix.com> wrote:
>>>>>>> On 15/06/16 11:28, Jan Beulich wrote:
>>>>>>>> --- a/xen/arch/x86/time.c
>>>>>>>> +++ b/xen/arch/x86/time.c
>>>>>>>> @@ -1358,6 +1358,24 @@ static void time_calibration(void *unuse
>>>>>>>> &r, 1);
>>>>>>>> }
>>>>>>>>
>>>>>>>> +void __init clear_tsc_cap(unsigned int feature)
>>>>>>>> +{
>>>>>>>> + void (*rendezvous_fn)(void *) = time_calibration_std_rendezvous;
>>>>>>> This should read time_calibration_rendezvous_fn rather than assuming
>>>>>>> time_calibration_std_rendezvous.
>>>>>>>
>>>>>>> Otherwise, there is a risk that it could be reset back to
>>>>>>> time_calibration_std_rendezvous.
>>>>>> But that's the purpose: We may need to switch back.
>>>>> Under what circumstances could we ever move from re-syncing back to not
>>>>> re-syncing?
>>>> verify_tsc_reliability() may result in X86_FEATURE_TSC_RELIABLE
>>>> getting cleared. That's an initcall, which means it runs after
>>>> init_xen_time(), and hence after the rendezvous function got
>>>> established initially.
>>> Right, but that isn't important.
>>>
>>> There will never be a case where, once TSC_RELIABLE is cleared, it is
>>> safe to revert back to std_rendezvous, even if TSC_RELIABLE is
>>> subsequently re-set.
>> You've got this backwards: TSC_RELIABLE may get _cleared_ late.
>
> Quite - I haven't got this backwards.
>
>> Nothing can ever set it late, due to the use of setup_clear_cpu_cap().
>> Reverting back to time_calibration_std_rendezvous() would only be
>> possible if CONSTANT_TSC got cleared late, ...
>
> time_calibration_rendezvous_fn defaults to
> time_calibration_std_rendezvous(), i.e. defaults to the assumption that
> the TSCs are invariant.
>
> We then later call clear_caps(TSC_RELIABLE), and the default changes to
> time_calibration_tsc_rendezvous().
>
> We then later call clear_tsc_cap(CONSTANT_TSC), or indeed that
> CONSTANT_TSC was never set in the first place, and the default switches
> back to time_calibration_std_rendezvous() because of the aformentioned bug.
>
> Once the switch to time_calibration_tsc_rendezvous() is made, it is
> never safe to switch back.
You still don't explain why - I don't see what's wrong with doing
so namely when there wasn't a whole lot of skew gained yet. Plus
I don't see why running with tsc_rendezvous is fine when one of
the two pre-conditions for switching to it isn't met, but we find
out only after having brought up APs.
Jan
> Therefore, your function must read time_calibration_rendezvous_fn and
> not assume time_calibration_std_rendezvous().
>
> ~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
next prev parent reply other threads:[~2016-08-31 14:03 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-15 9:50 [PATCH 0/8] x86/time: improve cross-CPU clock monotonicity (and more) Jan Beulich
2016-06-15 10:26 ` [PATCH 1/8] " Jan Beulich
2016-06-15 10:32 ` Jan Beulich
2016-06-15 22:51 ` Joao Martins
2016-06-16 8:27 ` Jan Beulich
2016-06-16 20:27 ` Joao Martins
2016-06-17 7:32 ` Jan Beulich
2016-06-21 12:05 ` Joao Martins
2016-06-21 12:28 ` Jan Beulich
2016-06-21 13:57 ` Joao Martins
2016-08-02 19:30 ` Andrew Cooper
2016-06-15 10:26 ` [PATCH 2/8] x86: also generate assembler usable equates for synthesized features Jan Beulich
2016-06-20 12:50 ` Andrew Cooper
2016-06-15 10:27 ` [PATCH 3/8] x86/time: introduce and use rdtsc_ordered() Jan Beulich
2016-06-20 12:59 ` Andrew Cooper
2016-06-20 13:06 ` Jan Beulich
2016-06-20 13:07 ` Andrew Cooper
2016-07-11 11:39 ` Dario Faggioli
2016-06-15 10:28 ` [PATCH 4/8] x86/time: calibrate TSC against platform timer Jan Beulich
2016-06-20 14:20 ` Andrew Cooper
2016-06-20 15:19 ` Jan Beulich
2016-08-02 19:25 ` Andrew Cooper
2016-08-03 9:32 ` Jan Beulich
2016-06-15 10:28 ` [PATCH 5/8] x86/time: correctly honor late clearing of TSC related feature flags Jan Beulich
2016-06-20 14:32 ` Andrew Cooper
2016-06-20 15:20 ` Jan Beulich
2016-07-04 15:39 ` Andrew Cooper
2016-07-04 15:53 ` Jan Beulich
2016-08-02 19:08 ` Andrew Cooper
2016-08-03 9:43 ` Jan Beulich
2016-08-31 13:42 ` Andrew Cooper
2016-08-31 14:03 ` Jan Beulich [this message]
2016-06-15 10:29 ` [PATCH 6/8] x86/time: support 32-bit wide ACPI PM timer Jan Beulich
2016-07-04 15:40 ` Andrew Cooper
2016-06-15 10:30 ` [PATCH 7/8] x86/time: fold recurring code Jan Beulich
2016-07-04 15:43 ` Andrew Cooper
2016-06-15 10:30 ` [PATCH 8/8] x86/time: group time stamps into a structure Jan Beulich
2016-07-04 15:57 ` Andrew Cooper
2016-07-01 7:44 ` Ping: [PATCH 0/8] x86/time: improve cross-CPU clock monotonicity (and more) Jan Beulich
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=57C6FFCD020000780010A84B@prv-mh.provo.novell.com \
--to=jbeulich@suse.com \
--cc=andrew.cooper3@citrix.com \
--cc=dario.faggioli@citrix.com \
--cc=joao.m.martins@oracle.com \
--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).